4

I've just found a pretty dangerous bug in my code, and I feel like it should have been caught by the compiler. Am I wrong? In essence, a reference member of a class is allowed to be initialized by itself in the constructor. Here's the test code that compiles in Visual Studio 2017 without errors or warnings:

struct foo
{
    foo() : reference(reference) {}

    int& reference;
};

int main()
{
    foo fooOb;
}

UPDATE: I see that there is a similar question from 2009 here. Do other compilers behave the same in 2017, or is it a VS 2017 issue? If they do, it kinda suggests to me that this is legal C++, but I don't see how it could be.

PlinyTheElder
  • 1,454
  • 1
  • 10
  • 15
  • 7
    This is nearly always wrong but there is no rule in C++ that mandates catching such errors at compile time. – n. m. could be an AI Jul 09 '19 at 16:21
  • I've seen the same in gcc – lib Jul 09 '19 at 16:24
  • 1
    C++ has a common concept called "[Undefined Behavior](https://en.wikipedia.org/wiki/Undefined_behavior)", which describes problematic code that the compiler does not have to detect or warn about. Your access of a reference variable before it is initialized is an example of that. – Drew Dormann Jul 09 '19 at 16:27
  • Interesting! Since code like that is always wrong, theoretically would it be safe to introduce an additional rule in the specification that would fail this code? – PlinyTheElder Jul 09 '19 at 16:38
  • 1
    Odds are no one has come up with a wording or an implementation of a rule that prevents self initialization that doesn't either mess up desirable behaviour elsewhere in the language or result in a disproportionate addition of complexity to the compiler. – user4581301 Jul 09 '19 at 16:51
  • Kinda related: https://stackoverflow.com/q/8595061/560648 – Lightness Races in Orbit Jul 09 '19 at 17:27

2 Answers2

3

Do other compilers behave the same in 2017, or is it a VS 2017 issue?

At the time this is written:

  • GCC and Clang both warn, with -Winit-self and -Wuninitialized respectively.

  • Neither MSVC nor ICC warn about it.

If they do, it kinda suggests to me that this is legal C++, but I don't see how it could be.

It is legal in the sense that the compiler is not required to fail compilation, but this is never what you want, since you will trigger UB.

Acorn
  • 24,970
  • 5
  • 40
  • 69
  • 1
    The compiler is never "required to fail compilation" except in response to a `#error` directive. For a "diagnosable error" the compiler is only required to issue a diagnostic. Having done that, it is free to continue to compile the code, with an implementation-specific interpretation of what the code is supposed to do. That's a hook for compiler-specific extensions. – Pete Becker Jul 09 '19 at 16:47
  • @PeteBecker Thanks Pete, feel free to edit the answer to explain it properly! – Acorn Jul 09 '19 at 16:55
  • 1
    @SergeyA — it’s in [intro.compliance]/1. It starts with “The set of diagnosable rules consists of all syntactic and semantic rules in this International Standard except for those rules containing an explicit notation that ‘no diagnostic is required’ or which are described as resulting in ‘undefined behavior.’” Then it says “If a program contains no violations of the rules in this International Standard, a conforming implementation shall, within its resource limits, accept and correctly execute that program.” ... – Pete Becker Jul 09 '19 at 23:43
  • ... and then it says “If a program contains a violation of any diagnosable rule or an occurrence of a construct described in this Standard as ‘conditionally-supported’ when the implementation does not support that construct, a conforming implementation shall issue at least one diagnostic message.” – Pete Becker Jul 09 '19 at 23:45
  • So, a “diagnosable rule” is any rule that isn’t marked “no diagnostic required” and isn’t marked as “undefined behavior”. If there are no rule violations (not restricted to diagnosable rules) a conforming compiler has to compile it. If a program has a violation of a diagnosable rule the compiler is required to issue a diagnostic. – Pete Becker Jul 09 '19 at 23:57
  • @SergeyA -- I've been thinking about posting a self-answered question, but if you'd like to post a question, by all means do do. Then I can delete these comments, which really aren't pertinent to this particular question. – Pete Becker Jul 10 '19 at 13:40
  • @SergeyA -- dang, that's putting on the pressure! – Pete Becker Jul 10 '19 at 13:47
  • @PeteBecker What doesn't kill us, makes us stronger! :P – SergeyA Jul 10 '19 at 13:48
2

MSVC 2017 yields:

warning C26495: Variable 'foo::reference' is uninitialized. Always initialize a member variable (type.6)

You may need to enable code analysis by going to Project Properties -> Code analysis (/analyze)

user7860670
  • 35,849
  • 4
  • 58
  • 84