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

[Andrey Bokhanko via cfe-commits]

andreybokhanko marked 3 inline comments as done.
andreybokhanko added a comment.

John,

Thank you for the review!

All your comments but one are fixed. See below for details on the single one I didn't manage to get fixed.

Andrey


================
Comment at: lib/CodeGen/CodeGenModule.h:354
@@ +353,3 @@
+  /// call).
+  llvm::DenseSet<GlobalDecl> ExplicitDefinitions;
+
----------------
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.


http://reviews.llvm.org/D11297