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