views:

240

answers:

3

Hi,

I've got a big big code base that includes two main namespaces: the engine and the application.

The engine defines a vector3 class as a typedef of another vector3 class, with equality operators that sit in the engine namespace, not in the vector3 class. I added a class to the application that also had equality operators in the application namespace.

When I tried to compile, unrelated but near-by vector3 comparisons failed because it couldn't find an appropriate equality operator. I suspected I was causing a conflict so moved my equality operators into the class I added, and the compile succeeded.

// engine.h
namespace Engine
{
    class Vector3Impl { ... };
    typedef Vector3Impl Vector3;
    bool operator==(Vector3 const &lhs, Vector3 const &rhs) { ... }
}


// myfile.cpp
#include "engine.h"

namespace application
{
    class MyClass { ... };
    bool operator==(MyClass const &lhs, MyClass const &rhs) { ... }

    void myFunc(...)
    {
        if ( myClassA == myClassB ) { ... } // builds
    }

    void anotherFunc(...)
    {
        Engine::Vector3 a, b;
        ...
        if ( a == b ) { ... } // fails
    }
}

However after thinking about it I can't see why the compile failed. There are no implicit conversions from vector3s to my class or vice-versa, and argument-dependent look-up should be pulling in the equality operator from the engine namespace and matching it.

I've tried reproducing this bug in a sample C++ project but that refuses to break. There must be something in the big big code base that is causing this problem, but I'm not sure where to start looking. Something like the opposite of a rogue "using Engine"? Anyone got any ideas?

A: 
bool operator==(Vector3 const &lhs, Vector3 const &rhs) { ... }

The canonical definition of an equality operator defined on a class should only have one argument, namely the rhs. The lhs is this. Don't know if this would be a solution to your problem though.

This is what I would write :

class Vector3 { bool operator==( const Vector3 & rhs) const { ... } };

QBziZ
I believe tenpn wrote down his initial code - with the operators defined outside the class, in the enclosing namespaces.
xtofl
There is nothing wrong with writing a stand-alone operator== taking two arguments. It has the advantage of allowing implicit conversions for the first argument, if they are so desired.
Gorpik
"The canonical definition of an equality operator" depends on where you're coding it: Inside the class, you're right. Outside, "tenpn" is right, and I prefer his notation: Meyers and Sutter discussed the idea non-members non-friend functions where better than member functions for encapsulation...
paercebal
Oops, you seem to be correct. My answer is correct by itself, but is not so helpful in relation to the OP's code. I have misread his code.@paercebal : that's why I explicitely said "defined on a class"
QBziZ
A: 

I once ran into the same problem with a compiler that didn't have Argument Dependent Lookup (Koenig Lookup - thanks @igor) (VC6 I think). This means that when it sees an operator, it just looks in the enclosing namespaces.

So can you tell us what compiler you use?

Moving to another compiler solved it.

Very inconvenient indeed.

xtofl
+2  A: 

C++ Standard, 3.4.4.2 declares:

For each argument type T in the function call, there is a set of zero or more associated namespaces and a set of zero or more associated classes to be considered. The sets of namespaces and classes is determined entirely by the types of the function arguments (and the namespace of any template template argument). Typedef names and using-declarations used to specify the types do not contribute to this set.

ADL doesn't work with typedef's.

Igor Semenov
It does compile on my VC2003, and on Comeau, though...
xtofl
Indeed it also seems I simplified my example too much. My typedef'd Vector3Impl was actually from another namespace: namespace Other { class Vector3Impl {}; }Adding that subtlety to my test file brought the error up.The solution is to move the equality operator from Engine to Other.
tenpn