[PATCH] D11297: PR17829: Functions declared extern "C" with a name matching a mangled C++ function are allowed

[John McCall via cfe-commits]

rjmccall added inline comments.

================
Comment at: lib/CodeGen/CodeGenModule.h:354
@@ +353,3 @@
+  /// call).
+  llvm::DenseSet<GlobalDecl> ExplicitDefinitions;
+
----------------
andreybokhanko wrote:
> Checking that a GlobalDecl is not in ExplicitDefinitions yet is actually required to avoid printing multiple identical warnings.
> 
> In my example:
> 
> ```
> 1: struct T {
> 2:   ~T() {}
> 3: };
> 4: 
> 5: extern "C" void _ZN1TD1Ev();
> 6: 
> 7: int main() {
> 8:   _ZN1TD1Ev();
> 9:   T t;
> 10: }
> ```
> 
> ~T() is added to the list of deferred decls twice. Judging from this comment in "EmitDeferred" method:
> 
>     // Check to see if we've already emitted this.  This is necessary
>     // for a couple of reasons: first, decls can end up in the
>     // deferred-decls queue multiple times, and second, decls can end
>     // up with definitions in unusual ways (e.g. by an extern inline
>     // function acquiring a strong function redefinition).  Just
>     // ignore these cases.
> 
> this is pretty normal ("decls can end up in the deferred-decls queue multiple times").
> 
> This means that we can call "GetOrCreateLLVMFunction"(..., /*IsForDefinition*/=true) for duplicated decls several times, which is fine in general, *but* will print the "duplicated mangled names" diagnostic multiple times as well -- unless we check that we already printed a warning on duplicated mangled names for given decl.
> 
> As for not emitting diagnostics for different globals -- this won't happen, as we will call "GetOrCreateLLVMFunction" at least once for each global with a definition, and thus, will print a warning for everyone.
> 
> I thought really hard (honestly!) on how to prevent duplicated diagnostics without usage of an additional set, but didn't found any solution. If you have any hints here, they would be much appreciated.
Okay, that's fine.  Can you at least make sure you only add to the set when you emit the warning?

As a minor optimization, you can add it to the set and check whether it was already there in the same operation, so that this would look like:

  if (LookupRepresentativeDecl(...) && DiagnosedConflictingDefinitions.insert(GD).second) {
    ...
  }


http://reviews.llvm.org/D11297