Rework the way we determine whether an externally visible symbol is

generated for an inline function definition, taking into account C99
and GNU inline/extern inline semantics. This solution is simpler,
cleaner, and fixes PR4536.


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@81670 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 69 insertions, 80 deletions

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index 444ee4a777..77c085a355 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -580,25 +580,64 @@ unsigned FunctionDecl::getMinRequiredArguments() const {
   return NumRequiredArgs;
 }
 
-bool FunctionDecl::hasActiveGNUInlineAttribute(ASTContext &Context) const {
-  if (!isInline() || !hasAttr<GNUInlineAttr>())
-    return false;
-
-  for (redecl_iterator I = redecls_begin(), E = redecls_end(); I != E; ++I)
-    if (I->isInline() && !I->hasAttr<GNUInlineAttr>())
-      return false;
-
-  return true;
-}
-
-bool FunctionDecl::isExternGNUInline(ASTContext &Context) const {
-  if (!hasActiveGNUInlineAttribute(Context))
+/// \brief For an inline function definition in C, determine whether the 
+/// definition will be externally visible.
+///
+/// Inline function definitions are always available for inlining optimizations.
+/// However, depending on the language dialect, declaration specifiers, and
+/// attributes, the definition of an inline function may or may not be
+/// "externally" visible to other translation units in the program.
+///
+/// In C99, inline definitions are not externally visible by default. However,
+/// if even one of the globa-scope declarations is marked "extern inline", the
+/// inline definition becomes externally visible (C99 6.7.4p6).
+///
+/// In GNU89 mode, or if the gnu_inline attribute is attached to the function
+/// definition, we use the GNU semantics for inline, which are nearly the 
+/// opposite of C99 semantics. In particular, "inline" by itself will create 
+/// an externally visible symbol, but "extern inline" will not create an 
+/// externally visible symbol.
+bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
+  assert(isThisDeclarationADefinition() && "Must have the function definition");
+  assert(isInline() && "Function must be inline");
+  
+  if (!getASTContext().getLangOptions().C99 || hasAttr<GNUInlineAttr>()) {
+    // GNU inline semantics. Based on a number of examples, we came up with the
+    // following heuristic: if the "inline" keyword is present on a
+    // declaration of the function but "extern" is not present on that
+    // declaration, then the symbol is externally visible. Otherwise, the GNU
+    // "extern inline" semantics applies and the symbol is not externally
+    // visible.
+    for (redecl_iterator Redecl = redecls_begin(), RedeclEnd = redecls_end();
+         Redecl != RedeclEnd;
+         ++Redecl) {
+      if (Redecl->isInline() && Redecl->getStorageClass() != Extern)
+        return true;
+    }
+    
+    // GNU "extern inline" semantics; no externally visible symbol.
     return false;
-
-  for (redecl_iterator I = redecls_begin(), E = redecls_end(); I != E; ++I)
-    if (I->getStorageClass() == Extern && I->hasAttr<GNUInlineAttr>())
-      return true;
-
+  }
+  
+  // C99 6.7.4p6:
+  //   [...] If all of the file scope declarations for a function in a 
+  //   translation unit include the inline function specifier without extern, 
+  //   then the definition in that translation unit is an inline definition.
+  for (redecl_iterator Redecl = redecls_begin(), RedeclEnd = redecls_end();
+       Redecl != RedeclEnd;
+       ++Redecl) {
+    // Only consider file-scope declarations in this test.
+    if (!Redecl->getLexicalDeclContext()->isTranslationUnit())
+      continue;
+    
+    if (!Redecl->isInline() || Redecl->getStorageClass() == Extern) 
+      return true; // Not an inline definition
+  }
+  
+  // C99 6.7.4p6:
+  //   An inline definition does not provide an external definition for the 
+  //   function, and does not forbid an external definition in another 
+  //   translation unit.
   return false;
 }
 
diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index 5dfc4bc1b9..05f14682d7 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -269,34 +269,19 @@ GetLinkageForFunction(ASTContext &Context, const FunctionDecl *FD,
   if (!FD->isInline())
     return External;
 
-  // If the inline function explicitly has the GNU inline attribute on it, or if
-  // this is C89 mode, we use to GNU semantics.
-  if (!Features.C99 && !Features.CPlusPlus) {
-    // extern inline in GNU mode is like C99 inline.
-    if (FD->getStorageClass() == FunctionDecl::Extern)
-      return CodeGenModule::GVA_C99Inline;
-    // Normal inline is a strong symbol.
-    return CodeGenModule::GVA_StrongExternal;
-  } else if (FD->hasActiveGNUInlineAttribute(Context)) {
-    // GCC in C99 mode seems to use a different decision-making
-    // process for extern inline, which factors in previous
-    // declarations.
-    if (FD->isExternGNUInline(Context))
-      return CodeGenModule::GVA_C99Inline;
-    // Normal inline is a strong symbol.
-    return External;
-  }
+  if (!Features.CPlusPlus || FD->hasAttr<GNUInlineAttr>()) {
+    // GNU or C99 inline semantics. Determine whether this symbol should be
+    // externally visible.
+    if (FD->isInlineDefinitionExternallyVisible())
+      return External;
 
-  // The definition of inline changes based on the language.  Note that we
-  // have already handled "static inline" above, with the GVA_Internal case.
-  if (Features.CPlusPlus)  // inline and extern inline.
-    return CodeGenModule::GVA_CXXInline;
-
-  assert(Features.C99 && "Must be in C99 mode if not in C89 or C++ mode");
-  if (FD->isC99InlineDefinition())
+    // C99 inline semantics, where the symbol is not externally visible.
     return CodeGenModule::GVA_C99Inline;
+  }
 
-  return CodeGenModule::GVA_StrongExternal;
+  // C++ inline semantics
+  assert(Features.CPlusPlus && "Must be in C++ mode");
+  return CodeGenModule::GVA_CXXInline;
 }
 
 /// SetFunctionDefinitionAttributes - Set attributes for a global.
diff --git a/lib/Frontend/PCHReaderDecl.cpp b/lib/Frontend/PCHReaderDecl.cpp
index 03ac3aad22..c3c05b0d5d 100644
--- a/lib/Frontend/PCHReaderDecl.cpp
+++ b/lib/Frontend/PCHReaderDecl.cpp
@@ -225,7 +225,6 @@ void PCHDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
                    cast_or_null<FunctionDecl>(Reader.GetDecl(Record[Idx++])));
   FD->setStorageClass((FunctionDecl::StorageClass)Record[Idx++]);
   FD->setInline(Record[Idx++]);
-  FD->setC99InlineDefinition(Record[Idx++]);
   FD->setVirtualAsWritten(Record[Idx++]);
   FD->setPure(Record[Idx++]);
   FD->setHasInheritedPrototype(Record[Idx++]);
diff --git a/lib/Frontend/PCHWriterDecl.cpp b/lib/Frontend/PCHWriterDecl.cpp
index 7a15abf0a0..1ea3fc1487 100644
--- a/lib/Frontend/PCHWriterDecl.cpp
+++ b/lib/Frontend/PCHWriterDecl.cpp
@@ -225,7 +225,6 @@ void PCHDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
   Writer.AddDeclRef(D->getPreviousDeclaration(), Record);
   Record.push_back(D->getStorageClass()); // FIXME: stable encoding
   Record.push_back(D->isInline());
-  Record.push_back(D->isC99InlineDefinition());
   Record.push_back(D->isVirtualAsWritten());
   Record.push_back(D->isPure());
   Record.push_back(D->hasInheritedPrototype());
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 40ef66306b..002372aa90 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -917,25 +917,10 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old) {
   MergeAttributes(New, Old, Context);
 
   // Merge the storage class.
-  if (Old->getStorageClass() != FunctionDecl::Extern)
+  if (Old->getStorageClass() != FunctionDecl::Extern &&
+      Old->getStorageClass() != FunctionDecl::None)
     New->setStorageClass(Old->getStorageClass());
 
-  // Merge "inline"
-  if (Old->isInline())
-    New->setInline(true);
-
-  // If this function declaration by itself qualifies as a C99 inline
-  // definition (C99 6.7.4p6), but the previous definition did not,
-  // then the function is not a C99 inline definition.
-  if (New->isC99InlineDefinition() && !Old->isC99InlineDefinition())
-    New->setC99InlineDefinition(false);
-  else if (Old->isC99InlineDefinition() && !New->isC99InlineDefinition()) {
-    // Mark all preceding definitions as not being C99 inline definitions.
-    for (const FunctionDecl *Prev = Old; Prev;
-         Prev = Prev->getPreviousDeclaration())
-      const_cast<FunctionDecl *>(Prev)->setC99InlineDefinition(false);
-  }
-
   // Merge "pure" flag.
   if (Old->isPure())
     New->setPure();
@@ -2859,24 +2844,6 @@ void Sema::CheckFunctionDeclaration(FunctionDecl *NewFD, NamedDecl *&PrevDecl,
       return NewFD->setInvalidDecl();
   }
 
-  // C99 6.7.4p6:
-  //   [... ] For a function with external linkage, the following
-  //   restrictions apply: [...] If all of the file scope declarations
-  //   for a function in a translation unit include the inline
-  //   function specifier without extern, then the definition in that
-  //   translation unit is an inline definition. An inline definition
-  //   does not provide an external definition for the function, and
-  //   does not forbid an external definition in another translation
-  //   unit.
-  //
-  // Here we determine whether this function, in isolation, would be a
-  // C99 inline definition. MergeCompatibleFunctionDecls looks at
-  // previous declarations.
-  if (NewFD->isInline() && getLangOptions().C99 &&
-      NewFD->getStorageClass() == FunctionDecl::None &&
-      NewFD->getDeclContext()->getLookupContext()->isTranslationUnit())
-    NewFD->setC99InlineDefinition(true);
-
   // Check for a previous declaration of this name.
   if (!PrevDecl && NewFD->isExternC()) {
     // Since we did not find anything by this name and we're declaring