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.

llvm-svn: 81670

clang/include/clang/AST/Decl.h

@@ -793,7 +793,6 @@ private:
   // NOTE: VC++ treats enums as signed, avoid using the StorageClass enum
   unsigned SClass : 2;
   bool IsInline : 1;
-  bool C99InlineDefinition : 1;
   bool IsVirtualAsWritten : 1;
   bool IsPure : 1;
   bool HasInheritedPrototype : 1;
@@ -835,7 +834,7 @@ protected:
     : DeclaratorDecl(DK, DC, L, N, T, DInfo),
       DeclContext(DK),
       ParamInfo(0), Body(),
-      SClass(S), IsInline(isInline), C99InlineDefinition(false),
+      SClass(S), IsInline(isInline), 
       IsVirtualAsWritten(false), IsPure(false), HasInheritedPrototype(false),
       HasWrittenPrototype(true), IsDeleted(false), IsTrivial(false),
       IsCopyAssignment(false),
@@ -1019,24 +1018,8 @@ public:
   bool isInline() const { return IsInline; }
   void setInline(bool I) { IsInline = I; }
 
-  /// \brief Whether this function is an "inline definition" as
-  /// defined by C99.
-  bool isC99InlineDefinition() const { return C99InlineDefinition; }
-  void setC99InlineDefinition(bool I) { C99InlineDefinition = I; }
-
-  /// \brief Determines whether this function has a gnu_inline
-  /// attribute that affects its semantics.
-  ///
-  /// The gnu_inline attribute only introduces GNU inline semantics
-  /// when all of the inline declarations of the function are marked
-  /// gnu_inline.
-  bool hasActiveGNUInlineAttribute(ASTContext &Context) const;
-
-  /// \brief Determines whether this function is a GNU "extern
-  /// inline", which is roughly the opposite of a C99 "extern inline"
-  /// function.
-  bool isExternGNUInline(ASTContext &Context) const;
-
+  bool isInlineDefinitionExternallyVisible() const;
+                       
   /// isOverloadedOperator - Whether this function declaration
   /// represents an C++ overloaded operator, e.g., "operator+".
   bool isOverloadedOperator() const {

clang/lib/AST/Decl.cpp

@@ -580,25 +580,64 @@ unsigned FunctionDecl::getMinRequiredArguments() const {
   return NumRequiredArgs;
 }
 
-bool FunctionDecl::hasActiveGNUInlineAttribute(ASTContext &Context) const {
-  if (!isInline() || !hasAttr<GNUInlineAttr>())
+/// \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->isInline() && !I->hasAttr<GNUInlineAttr>())
-      return false;
-
-  return true;
-}
-
-bool FunctionDecl::isExternGNUInline(ASTContext &Context) const {
-  if (!hasActiveGNUInlineAttribute(Context))
-    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;
 }
 

clang/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;
+
+    // C99 inline semantics, where the symbol is not externally visible.
+    return CodeGenModule::GVA_C99Inline;
   }
 
-  // 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())
-    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.

clang/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++]);

clang/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());

clang/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

clang/test/CodeGen/inline.c

@@ -1,5 +1,5 @@
-// RUN: echo "C89 tests:" &&
-// RUN: clang %s -emit-llvm -S -o %t -std=c89 &&
+// RUN: echo "GNU89 tests:" &&
+// RUN: clang %s -emit-llvm -S -o %t -std=gnu89 &&
 // RUN: grep "define available_externally i32 @ei()" %t &&
 // RUN: grep "define i32 @foo()" %t &&
 // RUN: grep "define i32 @bar()" %t &&
@@ -24,9 +24,9 @@
 // RUN: grep "define available_externally void @gnu_ei_inline()" %t &&
 // RUN: grep "define i32 @test1" %t &&
 // RUN: grep "define i32 @test2" %t &&
-// RUN: grep "define available_externally void @test3" %t &&
+// RUN: grep "define void @test3" %t &&
 // RUN: grep "define available_externally i32 @test4" %t &&
-// RUN: grep "define i32 @test5" %t &&
+// RUN: grep "define available_externally i32 @test5" %t &&
 
 // RUN: echo "\nC++ tests:" &&
 // RUN: clang %s -emit-llvm -S -o %t -std=c++98 &&
@@ -67,9 +67,7 @@ void test_test2() { test2(); }
 
 // PR3989
 extern __inline void test3() __attribute__((gnu_inline));
-__inline void test3() {}
-
-void test_test3() { test3(); }
+__inline void __attribute__((gnu_inline)) test3() {}
 
 extern int test4(void);
 extern __inline __attribute__ ((__gnu_inline__)) int test4(void)
@@ -79,7 +77,7 @@ extern __inline __attribute__ ((__gnu_inline__)) int test4(void)
 
 void test_test4() { test4(); }
 
-extern __inline int test5(void);
+extern __inline int test5(void)  __attribute__ ((__gnu_inline__));
 extern __inline int __attribute__ ((__gnu_inline__)) test5(void)
 {
   return 0;

clang/test/CodeGen/inline2.c

@@ -0,0 +1,62 @@
+// RUN: clang-cc -std=gnu89 -triple i386-apple-darwin9 -emit-llvm %s -o - | FileCheck -check-prefix GNU89 %s &&
+// RUN: clang-cc -std=c99 -triple i386-apple-darwin9 -emit-llvm %s -o - | FileCheck -check-prefix C99 %s
+
+// CHECK-GNU89: define i32 @f0()
+// CHECK-C99: define i32 @f0()
+int f0(void);
+int f0(void) { return 0; }
+
+// CHECK-GNU89: define i32 @f1()
+// CHECK-C99: define i32 @f1()
+inline int f1(void);
+int f1(void) { return 0; }
+
+// CHECK-GNU89: define i32 @f2()
+// CHECK-C99: define i32 @f2()
+int f2(void);
+inline int f2(void) { return 0; }
+
+// CHECK-GNU89: define i32 @f3()
+// CHECK-C99: define i32 @f3()
+extern inline int f3(void);
+int f3(void) { return 0; }
+
+// CHECK-GNU89: define i32 @f5()
+// CHECK-C99: define i32 @f5()
+extern inline int f5(void);
+inline int f5(void) { return 0; }
+
+// CHECK-GNU89: define i32 @f6()
+// CHECK-C99: define i32 @f6()
+inline int f6(void);
+extern inline int f6(void) { return 0; }
+
+// CHECK-GNU89: define i32 @f7()
+// CHECK-C99: define i32 @f7()
+extern inline int f7(void);
+extern int f7(void) { return 0; }
+
+// CHECK-GNU89: define i32 @fA()
+inline int fA(void) { return 0; }
+
+// CHECK-GNU89: define available_externally i32 @f4()
+// CHECK-C99: define i32 @f4()
+int f4(void);
+extern inline int f4(void) { return 0; }
+
+// CHECK-GNU89: define available_externally i32 @f8()
+// CHECK-C99: define i32 @f8()
+extern int f8(void);
+extern inline int f8(void) { return 0; }
+
+// CHECK-GNU89: define available_externally i32 @f9()
+// CHECK-C99: define i32 @f9()
+extern inline int f9(void);
+extern inline int f9(void) { return 0; }
+
+// CHECK-C99: define available_externally i32 @fA()
+
+int test_all() { 
+  return f0() + f1() + f2() + f3() + f4() + f5() + f6() + f7() + f8() + f9() 
+    + fA();
+}