Today’s post is about an incident with our compiler and a small little feature that sometimes seems to be underused or simply forgotten: Keyword explicit.

How a Bug in Our Code Could Hide a Bug in the Compiler

A few days ago when I had completed a little coding at work, I recompiled the project and got a very weird compiler error in a very distant part of the code. I would never had guessed that that particular part of the code could be influenced by my changes, but somehow something I did to one of the headers changed everything on the other side of the little code-world I work in. The error was in a very simple function and looked like this:

#include <string>
//more includes...

namespace MyNS {
  bool checkSomeStrings(std::string const& a, std::string const& b) {
    if (a == b) //ERROR
    //...
  }
}

error: ambiguous call to operator ==
could be: operator==(Sion const&, Sion const&)
      or: operator==(Annie const&, Annie const&)

“Wait, what????”. I did not believe the error message. That could not be. I mean, it’s std::string, right? If that line can’t compile, we are in serious trouble! I changed the code just a bit, and recompiled. The error remained.

What we figured out after some time was that we had two problems: The first and really grave problem was that the compiler somehow had forgotten what ADL aka Koenig Lookup was. It simply had no idea that there was an operator== for strings somewhere in namespace std that should be used for the comparison.

The other problem that had hidden the compiler bug for some months was that one of our ancient user defined classes had an non-explicit one-argument constructor taking a std::string. Only when I included a header somewhere up in the include hierarchy that defined another equally ancient class Sion, the problem emerged, because that class hat a similar constructor.

What Had Happened

When there was only Annie and the bug did prevent the compiler from finding operator== for strings, it found a workaround: Non-explicit one-argument constructors can be used for implicit conversions, so the compiler happily converted both strings into Annies and used operator== for Annie to compare those. Problem solved (for the compiler).

Only after I messed around with the headers and the definition of Sion was visible at that point in the code, the compiler got lost: it still did not see operator== for strings. But it now could do two different versions of that “workaround”, either by converting to Annie as before, or by converting to Sion. Thus the ambiguity error.

Fixing Our Bug

The first thing to do was taking the implicit conversions out of the equation, by simply adding explicit to the constructors of both Annie and Sion. I did some other minor refactorings once since I had touched those classes anyways and recompiled, expecting some error about the compiler finding no suitable operator== anymore.

class Annie {
  //...
public:
  explicit Annie(string const&);
};

I was wrong. It compiled cleanly, and when I debugged the function to make sure it had not found yet another “workaround”, I found it used std::operator== for strings, as it should do. I did not understand why the ADL bug would simply disappear, but I did not want to put too much time in more investigation. Our compiler sometimes acts weird like that.

However, one or two days later my colleague Ennox came across the ADL bug again. He had changed something in a distant header file and suddenly the compiler complained about a call to an undefined operator==. After some trying around that line and a few similar others in the same source file now look like this:

if (std::operator==(a,b))
{
  //...
}

Ugh. Not very nice. Not very simple. But what else is there to do if one is stuck with a compiler like that?

Lessons Learned

What I learned from that incident was that not every developer using C++ does know or care about or remember that little feature named explicit. In addition this was the first time I came across a real world example where a bug had occurred because someone did not follow the basic rule about implicit conversions:

By default, don’t allow implicit conversions for user defined types. Only make them possible after careful considerations, otherwise your compiler will surprise you applying them where you least expect it.

Implicit conversions to and from user defined types are possible, if those types provide constructors that can be called with one argument, and/or if they provide conversion operators. The keyword explicit does forbid those implicit conversions for constructors since C++98, and for conversion operators since C++11.

class LB {
public:
  explicit operator double();
};

So, the above rule expressed a bit closer to the coding is:

When writing a conversion operator or a constructor that can be called with one argument, make it explicit by default.