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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to