================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits