[PATCH] CodeGen: Emit some functions as weak_odr under -fms-extensions

[Richard Smith]

================
Comment at: lib/AST/Decl.cpp:2569-2570
@@ -2568,1 +2568,4 @@
 
+// \brief The combination of the extern and inline keywords under MSVC forces
+// the function to be required.
+bool FunctionDecl::isMSExternInline() const {
----------------
`///`, not `//`.

================
Comment at: lib/AST/Decl.cpp:2571
@@ +2570,3 @@
+// the function to be required.
+bool FunctionDecl::isMSExternInline() const {
+  const ASTContext &Context = getASTContext();
----------------
It's a little odd for this to check for `extern` but not `inlined`. Maybe at least assert that the function `isInlined` and mention that in the comment?

================
Comment at: lib/AST/Decl.cpp:2588
@@ +2587,3 @@
+      return false;
+  return Redecl->getStorageClass() == SC_Extern;
+}
----------------
Maybe move this check to the start of the function to avoid walking the redecls in the common case that the function isn't `extern`?

================
Comment at: lib/AST/Decl.cpp:2610-2612
@@ -2587,5 +2609,5 @@
 ///
 /// Specifically, this determines if adding the current declaration to the set
 /// of redeclarations of the given functions causes
 /// isInlineDefinitionExternallyVisible to change from false to true.
 bool FunctionDecl::doesDeclarationForceExternallyVisibleDefinition() const {
----------------
The corresponding update to `isInlineDefinitionExternallyVisible` seems to be missing.

================
Comment at: lib/CodeGen/CodeGenModule.cpp:561-562
@@ +560,4 @@
+    // replaced, the function definition cannot be discarded.
+    if (D->getMostRecentDecl()->isMSExternInline())
+      return llvm::Function::WeakODRLinkage;
+
----------------
Is there any risk that we'll emit IR for the function definition before we see the 'extern' declaration? (Will we ever need to patch up the linkage on an existing IR function?)


http://llvm-reviews.chandlerc.com/D3207

BRANCH
  master

ARCANIST PROJECT
  clang