Fix handling of C99 "extern inline" semantics when dealing with

multiple declarations of the function. Should fix PR3989 and
<rdar://problem/6818429>.

llvm-svn: 69905

7 files changed, 93 insertions, 11 deletions

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index a573c48af33..1d2c39e18a1 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -515,6 +515,7 @@ private:
   // NOTE: VC++ treats enums as signed, avoid using the StorageClass enum
   unsigned SClass : 2;
   bool IsInline : 1;
+  bool C99InlineDefinition : 1;
   bool IsVirtual : 1;
   bool IsPure : 1;
   bool InheritedPrototype : 1;
@@ -531,9 +532,9 @@ protected:
     : ValueDecl(DK, DC, L, N, T), 
       DeclContext(DK),
       ParamInfo(0), Body(), PreviousDeclaration(0),
-      SClass(S), IsInline(isInline), IsVirtual(false), IsPure(false),
-      InheritedPrototype(false), HasPrototype(true), IsDeleted(false), 
-      TypeSpecStartLoc(TSSL) {}
+      SClass(S), IsInline(isInline), C99InlineDefinition(false), 
+      IsVirtual(false), IsPure(false), InheritedPrototype(false), 
+      HasPrototype(true), IsDeleted(false), TypeSpecStartLoc(TSSL) {}
 
   virtual ~FunctionDecl() {}
   virtual void Destroy(ASTContext& C);
@@ -679,6 +680,11 @@ 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; }
+
   /// isOverloadedOperator - Whether this function declaration
   /// represents an C++ overloaded operator, e.g., "operator+".
   bool isOverloadedOperator() const { 
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index c07eac993be..378223e3a71 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -242,7 +242,7 @@ GetLinkageForFunction(const FunctionDecl *FD, const LangOptions &Features) {
   // this is C89 mode, we use to GNU semantics.
   if (FD->hasAttr<GNUInlineAttr>() || (!Features.C99 && !Features.CPlusPlus)) {
     // extern inline in GNU mode is like C99 inline.
-    if (FD->getStorageClass() == FunctionDecl::Extern)
+    if (FD->isC99InlineDefinition())
       return CodeGenModule::GVA_C99Inline;
     // Normal inline is a strong symbol.
     return CodeGenModule::GVA_StrongExternal;
@@ -254,11 +254,10 @@ GetLinkageForFunction(const FunctionDecl *FD, const LangOptions &Features) {
     return CodeGenModule::GVA_CXXInline;
   
   assert(Features.C99 && "Must be in C99 mode if not in C89 or C++ mode");
-  // extern inline in C99 is a strong definition.
-  if (FD->getStorageClass() == FunctionDecl::Extern) 
-    return CodeGenModule::GVA_StrongExternal;
-  
-  return CodeGenModule::GVA_C99Inline;
+  if (FD->isC99InlineDefinition())
+    return CodeGenModule::GVA_C99Inline;
+
+  return CodeGenModule::GVA_StrongExternal;
 }
 
 /// SetFunctionDefinitionAttributes - Set attributes for a global.
diff --git a/clang/lib/Frontend/PCHReader.cpp b/clang/lib/Frontend/PCHReader.cpp
index 2f6206dd094..d225c68392f 100644
--- a/clang/lib/Frontend/PCHReader.cpp
+++ b/clang/lib/Frontend/PCHReader.cpp
@@ -179,6 +179,7 @@ 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->setVirtual(Record[Idx++]);
   FD->setPure(Record[Idx++]);
   FD->setInheritedPrototype(Record[Idx++]);
diff --git a/clang/lib/Frontend/PCHWriter.cpp b/clang/lib/Frontend/PCHWriter.cpp
index 62e129919b1..0c640e7b052 100644
--- a/clang/lib/Frontend/PCHWriter.cpp
+++ b/clang/lib/Frontend/PCHWriter.cpp
@@ -356,6 +356,7 @@ 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->isVirtual());
   Record.push_back(D->isPure());
   Record.push_back(D->inheritedPrototype());
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index c7a45dc9d70..b3592086ddd 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -833,7 +833,21 @@ bool Sema::MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old) {
   // Merge the storage class.
   New->setStorageClass(Old->getStorageClass());
 
-  // FIXME: need to implement inline semantics
+  // 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())
@@ -2177,6 +2191,19 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
         isOutOfScopePreviousDeclaration(PrevDecl, DC, Context)))
     PrevDecl = 0;
 
+  // FIXME: We need to determine whether the GNU inline attribute will
+  // be applied to this function declaration, since it affects
+  // declaration merging. This hack will go away when the FIXME below
+  // is resolved, since we should be putting *all* attributes onto the
+  // declaration now.
+  for (const AttributeList *Attr = D.getDeclSpec().getAttributes();
+       Attr; Attr = Attr->getNext()) {
+    if (Attr->getKind() == AttributeList::AT_gnu_inline) {
+      NewFD->addAttr(::new (Context) GNUInlineAttr());
+      break;
+    }
+  }
+
   // Perform semantic checking on the function declaration.
   bool OverloadableAttrRequired = false; // FIXME: HACK!
   if (CheckFunctionDeclaration(NewFD, PrevDecl, Redeclaration,
@@ -2292,6 +2319,32 @@ bool Sema::CheckFunctionDeclaration(FunctionDecl *NewFD, NamedDecl *&PrevDecl,
       InvalidDecl = true;
   }
 
+  // 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() && 
+      NewFD->getDeclContext()->getLookupContext()->isTranslationUnit()) {
+    bool GNUInline = NewFD->hasAttr<GNUInlineAttr>() || 
+      (PrevDecl && PrevDecl->hasAttr<GNUInlineAttr>());
+    if (GNUInline || (!getLangOptions().CPlusPlus && !getLangOptions().C99)) {
+      // GNU "extern inline" is the same as "inline" in C99.
+      if (NewFD->getStorageClass() == FunctionDecl::Extern)
+        NewFD->setC99InlineDefinition(true);
+    } else if (getLangOptions().C99 && 
+               NewFD->getStorageClass() == FunctionDecl::None)
+      NewFD->setC99InlineDefinition(true);
+  }           
+
   // Check for a previous declaration of this name.
   if (!PrevDecl && NewFD->isExternC(Context)) {
     // Since we did not find anything by this name and we're declaring
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index d0c0ab86e1b..d05b99ac365 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1474,7 +1474,11 @@ static void HandleGNUInlineAttr(Decl *d, const AttributeList &Attr, Sema &S) {
     return;
   }
   
-  d->addAttr(::new (S.Context) GNUInlineAttr());
+  // FIXME: We only do this because of the hack in
+  // Sema::ActOnFunctionDeclarator, which needs to add the
+  // GNUInlineAttr early.
+  if (!d->hasAttr<GNUInlineAttr>())
+    d->addAttr(::new (S.Context) GNUInlineAttr());
 }
 
 static void HandleRegparmAttr(Decl *d, const AttributeList &Attr, Sema &S) {
diff --git a/clang/test/CodeGen/inline.c b/clang/test/CodeGen/inline.c
index eb8ee718d32..6011f42f9cf 100644
--- a/clang/test/CodeGen/inline.c
+++ b/clang/test/CodeGen/inline.c
@@ -7,6 +7,9 @@
 // RUN: not grep unreferenced2 %t &&
 // RUN: grep "define void @gnu_inline()" %t &&
 // RUN: grep "define available_externally void @gnu_ei_inline()" %t &&
+// RUN: grep "define void @test3()" %t &&
+// RUN: grep "define i32 @test1" %t &&
+// RUN: grep "define i32 @test2" %t &&
 
 // RUN: echo "\nC99 tests:" &&
 // RUN: clang %s -emit-llvm -S -o %t -std=c99 &&
@@ -17,6 +20,8 @@
 // RUN: grep "define void @unreferenced2()" %t &&
 // RUN: grep "define void @gnu_inline()" %t &&
 // RUN: grep "define available_externally void @gnu_ei_inline()" %t &&
+// RUN: grep "define i32 @test1" %t &&
+// RUN: grep "define i32 @test2" %t &&
 
 // RUN: echo "\nC++ tests:" &&
 // RUN: clang %s -emit-llvm -S -o %t -std=c++98 &&
@@ -45,3 +50,16 @@ __inline __attribute((__gnu_inline__)) void gnu_inline() {}
 extern inline __attribute__((gnu_inline)) void gnu_ei_inline() {}
 void (*P)() = gnu_ei_inline;
 
+// <rdar://problem/6818429>
+int test1();
+inline int test1() { return 4; }
+inline int test2() { return 5; }
+inline int test2();
+int test2();
+
+void test_test1() { test1(); }
+void test_test2() { test2(); }
+
+// PR3989
+extern inline void test3() __attribute__((gnu_inline));
+inline void test3()  {}