views:

369

answers:

5

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.

A: 

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.

John Zwinck
+5  A: 

First, the cause of this issue: C++ Standard 13.3.3.2 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*).

Johannes Schaub - litb
Thanks for the link to the standard and the wrapper for bool was also a helpful hint.
Greg Domjan
I quoted the wrong standard text back then and figured that my answer was nedlessly complex. Adding an oveload for `wchar_t const*` is better, i think. Feel free to give the check-mark to Mark-Ransom now, who basically says the same.
Johannes Schaub - litb
+1  A: 

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?

Federico Ramponi
We don't currently use a string literal except for the empty string. I believe your quite right that it is nothing to do with the string being empty.
Greg Domjan
The string literal does not need to be empty, the compiler will always prefer the cast to bool to the initalization of the string class. By the way, I think I saw warnings in VC++ 2005 relating to "forcing value to bool".
MP24
+5  A: 

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);
Mark Ransom
Not exactly. L"" is a wide character string literal.
ChrisN
+1  A: 

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));
}
zdan