[PATCH] D15686: PR25910: clang allows two var definitions with the same mangled name

[Andrey Bokhanko via cfe-commits]

andreybokhanko added a comment.

In http://reviews.llvm.org/D15686#325266, @rnk wrote:

> I thought we already addressed this issue with @rjmccall and decided that, if the user intentionally declares extern "C" variables with an _Z prefix, then we know they are intentionally attempting to name some C++ global variable.
>
> I'd rather just warn on all extern "C" declarations starting with _Z, and let users keep the pieces when things break.


That doesn't contradicts my patch at all. See the test -- where there is one declaration and one definition:

// We expect no warnings here, as there is only declaration of _ZN2nm3abcE
// global, no definitions.
extern "C" {

  int _ZN2nm3abcE;

}

namespace nm {

  float abc = 2;

}
// CHECK: @_ZN2nm3abcE = global float

nothing bad happens.

But if there are two definitions:

extern "C" {

  int _ZN2nm3abcE = 1; // expected-note {{previous definition is here}}

}

namespace nm {

  float abc = 2; // expected-error {{definition with same mangled name as another definition}}

}

an error is printed, as it should be.

Here is what John wrote himself (http://reviews.llvm.org/D11297#211351):

"I disagree with the conclusions in that thread (and wish you had CC'ed me; I unfortunately do not have time to keep up with cfe-dev). There is roughly zero chance that somebody is declaring an extern "C" symbol with a name that matches an Itanium mangled name with any intent other than to name that entity. We should not be rejecting this code or silently renaming one of the symbols. We should, of course, still diagnose attempts to *define* the same symbol multiple times."

John, may I ask you to take a look at this patch and confirm (or deny :-)) that it follows your understanding of how things should be?

Yours,
Andrey


http://reviews.llvm.org/D15686