Having recently introduced an overload of a method the application started to fail.
Finally tracking it down, the new method is being called where I did not expect it to be.
We had
setValue( const std::wstring& name, const std::wstring& value );
std::wstring avalue( func() );
setValue( L"string", avalue );
std::wstring bvalue( func2() ? L"true", L"false" );
setValue( L"bool", bvalue );
setValue( L"empty", L"" );
It was changed so that when a bool value is stored we use the same strings (internal data storage of strings)
setValue( const std::wstring& name, const std::wstring& value );
setValue( const std::wstring& name, const bool& value );
std::wstring avalue( func() );
setValue( L"string", avalue );
setValue( L"bool", func2() );
setValue( L"empty", L"" ); << --- this FAILS!?!
The problem with L"" is that it is implicitly casting and previously it was happy to be a std::wstring, but not it prefers to be a bool.
The MSVC compiler does not complain, or raise warning, so I'm worried that even if I "fix" the setValue( L"empty", L"" ); to be
setValue( L"empty", std::wstring() );
somebody else may come later and simply use setValue( L"empty", L"" ); and have to track down this issue again.
We thought to use explicit on the method but it is not a valid keyword for this usage.
Is there some way to get the compiler to complain about this, or otherwise prevent the issue? Otherwise I'm thinking to change the name of the method which takes a bool to ensure it can't make an incorrect guess.
First, the cause of this issue: C++ Standard [over.ics.rank]/2.1
1 defines an order for conversion sequences. It says that a user defined conversion sequence is worse than a standard conversion sequence. What happens in your case is that the string literal undergoes a boolean-conversion (defined at 4.12
. This is a standard conversion). It does not use the user defined conversion to std::wstring
which would be needed if it took the other overload.
I would recommend to simply change the name of one of the overloads or adding an overload that accepts the string literal directly (using parameter type wchar_t const*
).
1)
When comparing the basic forms of implicit conversion sequences (as defined in [over.best.ics]
)
(2.1) a standard conversion sequence is a better conversion sequence than a user-defined conversion sequence or an ellipsis conversion sequence, and
(2.2) a user-defined conversion sequence is a better conversion sequence than an ellipsis conversion sequence.
L"" is a pointer to a wide character string. The compiler believes that the conversion to a bool takes precedence over a conversion to a std::wstring.
To fix the problem, introduce a new setValue:
void setValue(std::wstring const& name, const wchar_t * value);
Since bool is a built-in type, the conversion from wchar_t to bool is preferred. I would say that the simplest solution is to add an overload that takes a wchar_t array and cast explicitly in there:
setValue( const std::wstring& name, const wchar_t s[] )
{
setValue(name, wstring(s));
}
To simplify a bit, the following code
#include <iostream>
using namespace std;
void f(const string &s)
{ cout << "string version called" << endl; }
void f(const bool &b)
{ cout << "bool version called" << endl; }
int main()
{ f("Hello World"); }
prints "bool version called". Are you sure that your code fails only with the empty string?
You could make the new function take some other type than bool--maybe just a proxy for bool--which is not convertible from a literal string. But really I'd just rename the bool-taking function and be done with it.