[PATCH] Defer codegen of dllexport/attribute(used) inline method definitions

[Richard Smith]

LGTM

================
Comment at: lib/CodeGen/ModuleBuilder.cpp:92
@@ -91,3 @@
-      if (!D->isDependentContext() &&
-          (D->hasAttr<UsedAttr>() || D->hasAttr<ConstructorAttr>() ||
-           D->hasAttr<DLLExportAttr>())) {
----------------
Hans Wennborg wrote:
> Richard Smith wrote:
> > An alternative fix would be to check `D->isUsed()` instead of checking for `UsedAttr` here. That'd avoid the locality degradation and memory allocation here.
> > 
> > Or just remove this check and unconditionally call `EmitTopLevelDecl`? That would better match what we do for out-of-line method definitions, and would avoid duplicating this between here and `ASTContext::DeclMustBeEmitted`.
> Checking D->isUsed() certainly sounds simpler :-) I'll try that out.
> 
> We can't unconditionally call EmitTopLevelDecl, because it will call DeclMustBeEmitted, which might calculate the gva linkage, and then we fail this test from SemaCXX/undefined-internal.cpp :(
> 
>       namespace test7 {
>         typedef struct {
>           void bar();
>           void foo() { bar(); }
>         } A;
>       }
> 
> I suspect that test doesn't work well with the "used" attribute either.
Arrgh! That's horrible, but it does justify delaying the emission to end of TU. The existing code is wrong for this case even without checking 'used'. A comment here mentioning this case would be useful for future archaeologists.

http://reviews.llvm.org/D4038