[PATCH] Fix asserts about emitting constexpr methods twice during dllexport explicit instantiation (PR21718)

[Hans Wennborg]

Thanks for the reviews!


================
Comment at: lib/CodeGen/CodeGenModule.cpp:1105-1108
@@ -1104,2 +1104,6 @@
 
-    assert(GV == GetGlobalValue(getMangledName(D)));
+    if (GV)
+      assert(GV == GetGlobalValue(getMangledName(D)));
+    else
+      GV = GetGlobalValue(getMangledName(D));
+
----------------
majnemer wrote:
> Maybe:
>   assert(!GV || GV == GetGlobalValue(getMangledName(D)));
>   if (!GV)
>     GV = GetGlobalValue(getMangledName(D));
Done.

================
Comment at: lib/CodeGen/CodeGenModule.cpp:1389
@@ +1388,3 @@
+    assert(!MayBeEmittedEagerly(Global));
+    addDeferredDeclToEmit(nullptr, GD);
+  } else {
----------------
majnemer wrote:
> You may want to make this a little more clear:
>   addDeferredDeclToEmit(/*GV=*/nullptr, GD);
Done.

================
Comment at: lib/CodeGen/CodeGenModule.h:1198-1199
@@ -1197,4 +1197,4 @@
 
-  /// Determine if the given decl can be emitted lazily; this is only relevant
-  /// for definitions. The given decl must be either a function or var decl.
-  bool MayDeferGeneration(const ValueDecl *D);
+  /// Determine if the given definition must be emitted, or can otherwise be
+  /// emitted lazily.
+  bool MustBeEmitted(const ValueDecl *D);
----------------
rsmith wrote:
> This sounds like "determine whether either (a) the definition must be emitted or (b) the definition can be emitted lazily" whereas I think you mean "determine whether the definition must be emitted; if this returns \c false, the definition can be emitted lazily if it's used".
Thanks! That's much clearer.

http://reviews.llvm.org/D6674

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/