When an "inline" declaration was followed by a definition not marked

"inline", we weren't giving the definition weak linkage because the
"inline" bit wasn't propagated. This was a longstanding FIXME that,
somehow, hadn't triggered a bug in the wild. Fix this problem by
tracking whether any declaration was marked "inline", and clean up the
semantics of GNU's "extern inline" semantics calculation based on this
change.

Fixes <rdar://problem/8740363>.

llvm-svn: 121373

clang/include/clang/AST/Decl.h

@@ -1183,6 +1183,7 @@ private:
   unsigned SClass : 2;
   unsigned SClassAsWritten : 2;
   bool IsInline : 1;
+  bool IsInlineSpecified : 1;
   bool IsVirtualAsWritten : 1;
   bool IsPure : 1;
   bool HasInheritedPrototype : 1;
@@ -1261,11 +1262,12 @@ private:
 protected:
   FunctionDecl(Kind DK, DeclContext *DC, const DeclarationNameInfo &NameInfo,
                QualType T, TypeSourceInfo *TInfo,
-               StorageClass S, StorageClass SCAsWritten, bool isInline)
+               StorageClass S, StorageClass SCAsWritten, bool isInlineSpecified)
     : DeclaratorDecl(DK, DC, NameInfo.getLoc(), NameInfo.getName(), T, TInfo),
       DeclContext(DK),
       ParamInfo(0), Body(),
-      SClass(S), SClassAsWritten(SCAsWritten), IsInline(isInline),
+      SClass(S), SClassAsWritten(SCAsWritten), 
+      IsInline(isInlineSpecified), IsInlineSpecified(isInlineSpecified),
       IsVirtualAsWritten(false), IsPure(false), HasInheritedPrototype(false),
       HasWrittenPrototype(true), IsDeleted(false), IsTrivial(false),
       HasImplicitReturnZero(false),
@@ -1290,11 +1292,11 @@ public:
                               TypeSourceInfo *TInfo,
                               StorageClass S = SC_None,
                               StorageClass SCAsWritten = SC_None,
-                              bool isInline = false,
+                              bool isInlineSpecified = false,
                               bool hasWrittenPrototype = true) {
     DeclarationNameInfo NameInfo(N, L);
     return FunctionDecl::Create(C, DC, NameInfo, T, TInfo, S, SCAsWritten,
-                                isInline, hasWrittenPrototype);
+                                isInlineSpecified, hasWrittenPrototype);
   }
 
   static FunctionDecl *Create(ASTContext &C, DeclContext *DC,
@@ -1302,7 +1304,7 @@ public:
                               QualType T, TypeSourceInfo *TInfo,
                               StorageClass S = SC_None,
                               StorageClass SCAsWritten = SC_None,
-                              bool isInline = false,
+                              bool isInlineSpecified = false,
                               bool hasWrittenPrototype = true);
 
   DeclarationNameInfo getNameInfo() const {
@@ -1493,10 +1495,13 @@ public:
 
   /// \brief Determine whether the "inline" keyword was specified for this
   /// function.
-  bool isInlineSpecified() const { return IsInline; }
+  bool isInlineSpecified() const { return IsInlineSpecified; }
                        
   /// Set whether the "inline" keyword was specified for this function.
-  void setInlineSpecified(bool I) { IsInline = I; }
+  void setInlineSpecified(bool I) { 
+    IsInlineSpecified = I; 
+    IsInline = I;
+  }
 
   /// \brief Determine whether this function should be inlined, because it is
   /// either marked "inline" or is a member function of a C++ class that

clang/lib/AST/Decl.cpp

@@ -1304,6 +1304,9 @@ FunctionDecl::setPreviousDeclaration(FunctionDecl *PrevDecl) {
     assert((!PrevDecl || PrevFunTmpl) && "Function/function template mismatch");
     FunTmpl->setPreviousDeclaration(PrevFunTmpl);
   }
+  
+  if (PrevDecl->IsInline)
+    IsInline = true;
 }
 
 const FunctionDecl *FunctionDecl::getCanonicalDecl() const {
@@ -1410,14 +1413,7 @@ unsigned FunctionDecl::getMinRequiredArguments() const {
 }
 
 bool FunctionDecl::isInlined() const {
-  // FIXME: This is not enough. Consider:
-  //
-  // inline void f();
-  // void f() { }
-  //
-  // f is inlined, but does not have inline specified.
-  // To fix this we should add an 'inline' flag to FunctionDecl.
-  if (isInlineSpecified())
+  if (IsInline)
     return true;
   
   if (isa<CXXMethodDecl>(this)) {
@@ -1471,20 +1467,22 @@ bool FunctionDecl::isInlineDefinitionExternallyVisible() const {
   ASTContext &Context = getASTContext();
   
   if (!Context.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.
+    // If it's not the case that both 'inline' and 'extern' are
+    // specified on the definition, then this inline definition is
+    // externally visible.
+    if (!(isInlineSpecified() && getStorageClassAsWritten() == SC_Extern))
+      return true;
+    
+    // If any declaration is 'inline' but not 'extern', then this definition
+    // is externally visible.
     for (redecl_iterator Redecl = redecls_begin(), RedeclEnd = redecls_end();
          Redecl != RedeclEnd;
          ++Redecl) {
-      if (Redecl->isInlineSpecified() && Redecl->getStorageClass() != SC_Extern)
+      if (Redecl->isInlineSpecified() && 
+          Redecl->getStorageClassAsWritten() != SC_Extern)
         return true;
-    }
+    }    
     
-    // GNU "extern inline" semantics; no externally visible symbol.
     return false;
   }
   
@@ -2058,9 +2056,10 @@ FunctionDecl *FunctionDecl::Create(ASTContext &C, DeclContext *DC,
                                    const DeclarationNameInfo &NameInfo,
                                    QualType T, TypeSourceInfo *TInfo,
                                    StorageClass S, StorageClass SCAsWritten,
-                                   bool isInline, bool hasWrittenPrototype) {
+                                   bool isInlineSpecified, 
+                                   bool hasWrittenPrototype) {
   FunctionDecl *New = new (C) FunctionDecl(Function, DC, NameInfo, T, TInfo,
-                                           S, SCAsWritten, isInline);
+                                           S, SCAsWritten, isInlineSpecified);
   New->HasWrittenPrototype = hasWrittenPrototype;
   return New;
 }

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -386,7 +386,8 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
 
   FD->SClass = (StorageClass)Record[Idx++];
   FD->setStorageClassAsWritten((StorageClass)Record[Idx++]);
-  FD->setInlineSpecified(Record[Idx++]);
+  FD->IsInline = Record[Idx++];
+  FD->IsInlineSpecified = Record[Idx++];
   FD->setVirtualAsWritten(Record[Idx++]);
   FD->setPure(Record[Idx++]);
   FD->setHasInheritedPrototype(Record[Idx++]);

clang/lib/Serialization/ASTWriterDecl.cpp

@@ -303,6 +303,7 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) {
 
   Record.push_back(D->getStorageClass()); // FIXME: stable encoding
   Record.push_back(D->getStorageClassAsWritten());
+  Record.push_back(D->IsInline);
   Record.push_back(D->isInlineSpecified());
   Record.push_back(D->isVirtualAsWritten());
   Record.push_back(D->isPure());

clang/test/CodeGenCXX/inline-functions.cpp

@@ -21,3 +21,11 @@ void B<char>::f() { }
 
 // CHECK: define void @_Z1fv
 void f() { }
+
+// <rdar://problem/8740363>
+inline void f1(int);
+
+// CHECK: define linkonce_odr void @_Z2f1i
+void f1(int) { }
+
+void test_f1() { f1(17); }