Is this code broken or is there a bug in g++?

Question

For some strange reason g++ (versions 4.5.0 and 4.5.2) cannot compile this code:

bool somefunc() {
    return false;
}

class C {
  public:
    static const int a = 0;
    static const int b = 1;
};

class myclass {
  public:
    int check() {
        return somefunc() ? C::a : C::b;
        // if(somefunc()) return C::a; else return C::b;
    }
};

int main() {
    myclass obj;
    obj.check();
    return 0;
}

It gives me this error:

/tmp/ccyvvTUy.o:/home/mati/test.cpp:14: undefined reference to `C::a'
/tmp/ccyvvTUy.o:/home/mati/test.cpp:14: undefined reference to `C::b'
collect2: ld returned 1 exit status

What's strange if I change problematic line to the commented line it compiles fine. Is it something wrong with my code and something I don't understand about C++ or is it just a bug in G++ ?

I just found that this IS a bug in G++ because when I compile it with optimizations enabled it compiles fine so it must be a bug. — matix2267, Jul 11 '11 at 18:22
When you say confirmed, do you mean that there is a bug filed and/or fixed or just that it also occurs for you? — Matt K, Jul 11 '11 at 18:28
Side note: You can actually _fix_ it by using unary `+`. `return somefunc() ? +C::a : +C::b` compiles fine (gcc 4.7). — Vitus, Jul 11 '11 at 22:34
@vitus yes because `+C::a` immediately converts it to rvalue and in this case it is considered not used so you don't have to provide definition outside the class (see: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#712). — matix2267, Jul 12 '11 at 07:55

Konrad Rudolph · Accepted Answer · 2011-07-11T18:47:20.930

18

There is an ongoing debate on whether this code is actually legal or not.

Either way, according to some readings the constants actually do need to be defined before usage, not just declared. That is,

class C {
  public:
    static const int a = 0;
    static const int b = 1;
};

const int C::a;
const int C::b;

Or just use the enum hack that was used to accomodate older compilers (but which may after all be the only legal way):

class C {
  public:
    enum { a = 0, b = 1 };
};

edited Jul 11 '11 at 18:47

answered Jul 11 '11 at 18:27

Konrad Rudolph

530,221
131
937
1,214

2

Despite that GCC should either reject or accept this code both with and without optimisations and it should accept or reject both this version and with uncommented the other line so this is some kind of bug. – matix2267 Jul 11 '11 at 18:30
4

@matix2267 It’s not so easy. If the standard doesn’t require a diagnostic for this code then the behaviour may actually legally differ for different optimisation settings. Furthermore, notice that this is a *link time*, not compile time, error and the compiler will just skip the whole function since you don’t use the result anywhere (and it’s a constant known at compile time). – Konrad Rudolph Jul 11 '11 at 18:33
4

@matix2267, the error is coming from the linker, not the compiler. The optimizer simply optimized away the the output that was tripping up the linker. – Mark Ransom Jul 11 '11 at 18:33
1

Could you provide a link to some samples of this debate? I haven't used C++ very much, and as such my naive perspective is that the requirement of the out-of-class definition seems redundant. In the few times I've programmed with C++, I've only seen that done with methods contained by a class, with the prototype being in the class declaration and the actual definition of the function being outside of it. Or is it simply that you'd normally do the assignment outside of the class? Because if so, then it makes a bit more sense. – JAB Jul 11 '11 at 18:36
1

@JAB The out-of-class definition in general is certainly not redundant, and there’s no controversy over that either: **You need to define static constants outside of the class**, full stop. The only exception might (!) apply to integral constants whose definition you never use – in other words, which you only ever use as rvalues. [Here’s such a discussion](http://stackoverflow.com/questions/257288/is-it-possible-to-write-a-c-template-to-check-for-a-functions-existence/257315#257315). – Konrad Rudolph Jul 11 '11 at 18:45
@Konrad: I was wondering about its redundancy when the assignment is done inside the class definition. Note that bit at the end of my previous comment. – JAB Jul 11 '11 at 18:50
@JAB Well, the redundancy comes from the fact that C++ usually (except for `inline`s distinguishes between declaration and definition of class members (functions and statics). This is certainly confounded here by the fact that integral static constants can be *initialised* inline (yet another thing, distinct from declaration and definition!). So in a way, yes. However, it’s usual to initialise these constants inline and define them outside of the class, as shown in my code. C++ is just verbose that way. – Konrad Rudolph Jul 11 '11 at 18:53
@Konrad: Ah, I see. That's interesting to know. – JAB Jul 11 '11 at 19:02

score 5 · Answer 2 · answered Jul 11 '11 at 18:37

5

See 9.4.2/4:

If a static data member is of const integral or const enumeration type, its declaration in the class definition can specify a constant initializer which shall be an integral constant expression (5.19). In that case, the member can appear in integral constant expressions within its scope. The member shall still be defined in a namespace scope if it is used in the program and the namespace scope definition shall not contain an initializer.

There is some contention (I believe) as to the precise meaning of "used" although my understanding is roughly that the if program requires the address of the variable then it's "used" in this context. It doesn't seem unreasonable in my mind at all that changing the ternary to an if/else or changing optimization level could change the view of the program as seen by g++ thus causing the failure or success. Also note that it doesn't say anything about a required diagnostic if you violate the requirement here.

You can't go wrong by always defining your static members.

answered Jul 11 '11 at 18:37

Mark B

95,107
10
109
188

Does that mean a static data member that is not `const` wouldn't need to be defined in a namespace scope? – JAB Jul 11 '11 at 18:38
@JAB No, this is a special exception stating one case where it *may not always* be required. See 9.4.2/2 "...The definition for a static data member shall appear in a namespace scope enclosing the member’s class definition." – Mark B Jul 11 '11 at 18:39
1

Okay, so it is indeed as I pondered in my comment on Konrad's answer. "The declaration of a static data member in its class definition is not a definition and may be of an incomplete type other than cv-qualified void. The definition for a static data member shall appear in a namespace scope enclosing the member’s class definition." Which makes me wonder why that limited exception for `static const` members exists. Are private static constants used much more often within classes in C++ than public ones? Or am I misunderstanding the "within its scope" part? – JAB Jul 11 '11 at 18:50
@JAB I believe the exception is nominally to allow for not needing a definition for constants that are only used at compile time. In practice it's safer (and always correct) just to always provide the definition. – Mark B Jul 11 '11 at 18:54
I see. That info on the ternary operator from a comment on an answer to the question Konrad linked was interesting, too. – JAB Jul 11 '11 at 19:02
The standard actually never defines what "use" means. :-) That's one part of the problem. The other is that the optimizer knows the values, and can most often optimize away any "use". Is it then still used? – Bo Persson Jul 11 '11 at 19:32

score 0 · Answer 3 · answered Jul 11 '11 at 18:37

0

Try defining it globally without optimisation.

const int C::a = 0;

answered Jul 11 '11 at 18:37

QuentinUK

2,997
21
20

1

It's not about "how to make this work" but "why it doesn't work". I can easily work around this problem by uncommenting 15th line and commenting out 14th line and then linker is happy. Strange thing is that 14th and 15th lines are (I think) equivalent so it shouldn't make any difference but it does. – matix2267 Jul 11 '11 at 18:51
2

@matix: Apparently they aren't equivalent. http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#712 – JAB Jul 11 '11 at 18:57
@JAB Thanks this clarified a lot of things for me. So it looks this code is not correct. I wonder why there are no linker errors with optimizations turned on. Here's another question: why if I change that line to: return 0 ? C::a : C::b; it compiles fine? After all no optimizations means none at all, not even removing of this never-used branch. I'd say the executable code should actually check if 0 equals 0 to see which branch to chose and then it wouldn't be different from this and all those rules about ternary operator using constants would apply, so it shouldn't compile too. – matix2267 Jul 11 '11 at 19:19
"I'd say the executable code should actually check if 0 equals 0." You're using a literal constant there, though. Literals are generally evaluated at compile time, are they not? – JAB Jul 11 '11 at 20:37

score -3 · Answer 4 · answered Jul 11 '11 at 20:08

-3

change the following line return somefunc() ? C::a : C::b;

to return (somefunc() ? C::a : C::b);

It should compile.

answered Jul 11 '11 at 20:08

Raghu Sugavanam

1

Is this code broken or is there a bug in g++?

4 Answers4