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

[David Majnemer]

================
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 {
----------------
Richard Smith wrote:
> `///`, not `//`.
Done.

================
Comment at: lib/AST/Decl.cpp:2571
@@ +2570,3 @@
+// the function to be required.
+bool FunctionDecl::isMSExternInline() const {
+  const ASTContext &Context = getASTContext();
----------------
Richard Smith wrote:
> 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?
Done.

================
Comment at: lib/AST/Decl.cpp:2588
@@ +2587,3 @@
+      return false;
+  return Redecl->getStorageClass() == SC_Extern;
+}
----------------
Richard Smith wrote:
> 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`?
Done.

================
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 {
----------------
Richard Smith wrote:
> The corresponding update to `isInlineDefinitionExternallyVisible` seems to be missing.
We don't want one because it is only called when one of the following happens:
- We aren't in C++ mode and we don't have MSVC compat enabled.
- We have the GNU inline attribute enabled.

When it does return true, it is only used to help determine if the function should be given a `GVALinkage` of `external`.

It seems like we don't want the MS behavior to influence this check.

================
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;
+
----------------
Richard Smith wrote:
> 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?)
An `extern` declaration can trigger code emission only if the function has been previously marked as inline via either the `inline` or `constexpr` keywords.

This makes me believe that the answer to your question is no, I don't think we would need to patch up the linkage. 


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

BRANCH
  master

ARCANIST PROJECT
  clang