views:

383

answers:

3

This piece of code compiles and runs as expected on GCC 3.x and 4.x:

#include <stdio.h>

typedef union buggedUnion
{   
public:
      // 4 var init constructor
     inline buggedUnion(int _i) {
      i = _i;
     }

     friend inline const buggedUnion operator - (int A, const buggedUnion &B) {
      return buggedUnion(A - B.i);
     }

     friend inline const buggedUnion operator - (const buggedUnion &A, const buggedUnion &B) {
      return buggedUnion(A.i - B.i);
     }

     int i;

} buggedUnion;

int main()
{
    buggedUnion first(10);
    buggedUnion second(5);

    buggedUnion result = 10 - (first - second);

    printf("%d\n", result.i); // 0

    return 0;
}

MSVC, however, will not compile that code, complaining:

main.cpp(60) : error C3767: '-': candidate function(s) not accessible
        could be the friend function at 'main.cpp(41)' : '-'  [may be found via argument-dependent lookup]
        or the friend function at       'main.cpp(45)' : '-'  [may be found via argument-dependent lookup]
main.cpp(60) : error C2676: binary '-' : 'buggedUnion' does not define this operator or a conversion to a type acceptable to the predefined operator

Which of the compilers is correct? How can this be resolved? I'm trying to achieve clean code (no outside friend methods) while maintaining portability, flexibility and self-documenting code.

Some notes:

  • This is a test-case to show the problem, the original data-type is much more sophisticated and carefully designed, albeit not working in MSVC (main compiler is GCC, though MSVC compatibility is also desired).
  • Adding 'public:' at the start of the union declaration does not resolve it.
  • Adding 'public:' before each operator does not resolve it
  • Converting the test case to a struct/class does fix it, but this is not desired (Please no flames, I got reasons. Most of them are limitations of the C++ language)
  • Operator method is to be left at global scope (not a member function)

Optimal solution would not rely on moving the declaration outside of the union definition for aestetic reasons (over 24 different combinations of operators and operands), but will be done if there is no other solution.

+1  A: 

It is difficult to say which one is right, since unnamed structs are not allowed by the standard (although they are a common extension), and as such the program is ill-formed.

Edit: It does seem to be a bug in msvc, since the following code, which is perfectly valid, fails to compile.

union buggedUnion
{
    friend buggedUnion operator - (int A, const buggedUnion &B) {
        return B;
    }

    friend buggedUnion operator - (const buggedUnion &A, const buggedUnion &B) {
        return A;
    }

    int i;
};


int main()
{
    buggedUnion first = { 1 };
    buggedUnion second = { 1 };
    buggedUnion result = 3 - (first - second);
}

You can work around this by defining the functions outside the class.

union buggedUnion
{
    int i;
};

buggedUnion operator - (int A, const buggedUnion &B) {
    return B;
}

buggedUnion operator - (const buggedUnion &A, const buggedUnion &B) {
    return A;
}

You can even retain the friend status by declaring the functions inside the class (but still defining them outside), but I doubt you'd ever need that in a union.

Note that I removed the unnecessary typedef and inlines.

avakar
I have updated the test case to prove the anonymous struct has nothing to do with it.
LiraNuna
This is NOT a bug in Visual C++ this is the correct behavior as per spec. Friend functions defined inside a class have no visibility outside the class, and should have none.
James D
Coding the Wheel, friend functions are not members of the class, they are injected to the enclosing scope and as such are most certainly visible.
avakar
I never said they are members of the class. I said that friend functions whose bodies are placed inside the class are no longer visible in the enclosing scope, even though yes, the function itself lives in the enclosing scope. This is a name-lookup issue. The code in the examples above is malformed though lenient compilers will accept it.
James D
So you're saying those functions can only be looked up via ADL? (I'm not disputing that as I'm not certain right now.) Even if that is the case, why shouldn't they be found? By the way, Comeau compiles it fine
avakar
Well, no, I wouldn't bet the farm on it. But I do note that according to the GCC docs "However, in ISO C++ a friend function which is not declared in an enclosing scope can only be found using argument dependent lookup." http://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html But who knows..
James D
It's definitely a bug, since unions participate in ADL (see ISO C++ 3.2.4[basic.lookup.koenig]/2), and so a function or operator call where one or both arguments are of that union type should consider those friend functions during overload resolution. Furthermore, it does find them correctly if you call unqualified `operator-` explicitly, i.e.: `buggedUnion result = operator- (3, operator- (first - second))`.
Pavel Minaev
Yes, they can only be found via ADL - but the code is a classic example of ADL!
Pavel Minaev
A: 

You need to declare those friend function in the enclosing scope, as once you declare them within the class, they're no longer visible in the external scope. So either move the function body out of the class as avakar said, or keep them in the class and add the following line to reintroduce the name into the enclosing scope:

extern const buggedUnion operator-(const buggedUnion& A, const buggedUnion&B);

int main()
{
    ...etc

Hope this helps. Not sure whether it's a bug but it appears (?) to me to be correct behavior, now implemented correctly, which many compilers used to interpret differently. See: --ffriend-injection in http://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html.

James D
So I've looked it up. You're right that by defining the function inside the class, you removed it from normal lookup (it's 11.4/5). But ADL lookup is not effected by this. I still believe that it's MSVC bug, especially since the same code works with `struct`.
avakar
Well it appears I'm outnumbered. Luckily this is the top priority on the MS development stack, so if it is a bug, we can all rest easy knowing it will be fixed in just a couple days... :)
James D
You never know, if the issue's still there in the beta of 10, and someone raises it, they might do something about it. I don't know the internal politics or priorities of the MSVC team, but in general in any company, targetting bug reports at the beta means it's more likely to land on the desk of someone who's actually willing to modify the code base...
Steve Jessop
A: 

The following code compiles correctly in Visual C++ 2008:

union buggedUnion
{
    int i;

    friend buggedUnion operator - (int A, const buggedUnion &B);
    friend buggedUnion operator - (const buggedUnion &A, const buggedUnion &B);
};

buggedUnion operator - (int A, const buggedUnion &B)
{
    return B;
}

buggedUnion operator - (const buggedUnion &A, const buggedUnion &B)
{
    return A;
}

While the MSDN documentation states that writing the definition of the friend function inside a class actually puts the function in file scope, this appears to not work for unions. Since it works for class and struct, I suspect this may be a bug. Since the implementation I have given above should work on GCC, I think it can be considered a suitable workaround and the bug will likely not be fixed in MSVC.

Zooba