[cfe-commits] r149963 - in /cfe/trunk: lib/AST/Decl.cpp test/CodeGen/inline.c

Eli Friedman eli.friedman at gmail.com
Mon Feb 6 19:50:19 PST 2012


Author: efriedma
Date: Mon Feb  6 21:50:18 2012
New Revision: 149963

URL: http://llvm.org/viewvc/llvm-project?rev=149963&view=rev
Log:
Make FunctionDecl::doesDeclarationForceExternallyVisibleDefinition use the same logic as FunctionDecl::isInlineDefinitionExternallyVisible to figure out whether to emit a definition.  Based on work by Anton Yartsev.


Modified:
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/test/CodeGen/inline.c

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=149963&r1=149962&r2=149963&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Feb  6 21:50:18 2012
@@ -1883,31 +1883,79 @@
   return false;
 }
 
+static bool RedeclForcesDefC99(const FunctionDecl *Redecl) {
+  // Only consider file-scope declarations in this test.
+  if (!Redecl->getLexicalDeclContext()->isTranslationUnit())
+    return false;
+
+  // Only consider explicit declarations; the presence of a builtin for a
+  // libcall shouldn't affect whether a definition is externally visible.
+  if (Redecl->isImplicit())
+    return false;
+
+  if (!Redecl->isInlineSpecified() || Redecl->getStorageClass() == SC_Extern) 
+    return true; // Not an inline definition
+
+  return false;
+}
+
 /// \brief For a function declaration in C or C++, determine whether this
 /// declaration causes the definition to be externally visible.
 ///
-/// Determines whether this is the first non-inline redeclaration of an inline
-/// function in a language where "inline" does not normally require an
-/// externally visible definition.
+/// 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 {
   assert(!doesThisDeclarationHaveABody() &&
          "Must have a declaration without a body.");
 
   ASTContext &Context = getASTContext();
 
-  // In C99 mode, a function may have an inline definition (causing it to
-  // be deferred) then redeclared later.  As a special case, "extern inline"
-  // is not required to produce an external symbol.
-  if (Context.getLangOptions().GNUInline || !Context.getLangOptions().C99 ||
-      Context.getLangOptions().CPlusPlus)
+  if (Context.getLangOptions().GNUInline || hasAttr<GNUInlineAttr>()) {
+    // With GNU inlining, a declaration with 'inline' but not 'extern', forces
+    // an externally visible definition.
+    //
+    // FIXME: What happens if gnu_inline gets added on after the first
+    // declaration?
+    if (!isInlineSpecified() || getStorageClassAsWritten() == SC_Extern)
+      return false;
+
+    const FunctionDecl *Prev = this;
+    bool FoundBody = false;
+    while ((Prev = Prev->getPreviousDecl())) {
+      FoundBody |= Prev->Body;
+
+      if (Prev->Body) {
+        // If it's not the case that both 'inline' and 'extern' are
+        // specified on the definition, then it is always externally visible.
+        if (!Prev->isInlineSpecified() ||
+            Prev->getStorageClassAsWritten() != SC_Extern)
+          return false;
+      } else if (Prev->isInlineSpecified() && 
+                 Prev->getStorageClassAsWritten() != SC_Extern) {
+        return false;
+      }
+    }
+    return FoundBody;
+  }
+
+  if (Context.getLangOptions().CPlusPlus)
     return false;
-  if (getLinkage() != ExternalLinkage || isInlineSpecified())
+
+  // 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.
+  if (isInlineSpecified() && getStorageClass() != SC_Extern)
     return false;
-  const FunctionDecl *Definition = 0;
-  if (hasBody(Definition))
-    return Definition->isInlined() &&
-           Definition->isInlineDefinitionExternallyVisible();
-  return false;
+  const FunctionDecl *Prev = this;
+  bool FoundBody = false;
+  while ((Prev = Prev->getPreviousDecl())) {
+    FoundBody |= Prev->Body;
+    if (RedeclForcesDefC99(Prev))
+      return false;
+  }
+  return FoundBody;
 }
 
 /// \brief For an inline function definition in C or C++, determine whether the 
@@ -1933,6 +1981,9 @@
   ASTContext &Context = getASTContext();
   
   if (Context.getLangOptions().GNUInline || hasAttr<GNUInlineAttr>()) {
+    // Note: If you change the logic here, please change
+    // doesDeclarationForceExternallyVisibleDefinition as well.
+    //
     // If it's not the case that both 'inline' and 'extern' are
     // specified on the definition, then this inline definition is
     // externally visible.
@@ -1951,7 +2002,7 @@
     
     return false;
   }
-  
+
   // 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, 
@@ -1959,17 +2010,8 @@
   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;
-
-    // Only consider explicit declarations; the presence of a builtin for a
-    // libcall shouldn't affect whether a definition is externally visible.
-    if (Redecl->isImplicit())
-      continue;
-
-    if (!Redecl->isInlineSpecified() || Redecl->getStorageClass() == SC_Extern) 
-      return true; // Not an inline definition
+    if (RedeclForcesDefC99(*Redecl))
+      return true;
   }
   
   // C99 6.7.4p6:

Modified: cfe/trunk/test/CodeGen/inline.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/inline.c?rev=149963&r1=149962&r2=149963&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/inline.c (original)
+++ cfe/trunk/test/CodeGen/inline.c Mon Feb  6 21:50:18 2012
@@ -13,8 +13,14 @@
 // RUN: grep "define available_externally i32 @test4" %t
 // RUN: grep "define available_externally i32 @test5" %t
 // RUN: grep "define i32 @test6" %t
+// RUN: grep "define void @test7" %t
+// RUN: grep "define i.. @strlcpy" %t
+// RUN: not grep test9 %t
+// RUN: grep "define void @testA" %t
+// RUN: grep "define void @testB" %t
+// RUN: grep "define void @testC" %t
 
-// RUN: echo "\nC99 tests:"
+// RUN: echo "C99 tests:"
 // RUN: %clang %s -O1 -emit-llvm -S -o %t -std=gnu99
 // RUN: grep "define i32 @ei()" %t
 // RUN: grep "define available_externally i32 @foo()" %t
@@ -29,9 +35,14 @@
 // RUN: grep "define available_externally i32 @test4" %t
 // RUN: grep "define available_externally i32 @test5" %t
 // RUN: grep "define i32 @test6" %t
+// RUN: grep "define void @test7" %t
 // RUN: grep "define available_externally i.. @strlcpy" %t
+// RUN: grep "define void @test9" %t
+// RUN: grep "define void @testA" %t
+// RUN: grep "define void @testB" %t
+// RUN: grep "define void @testC" %t
 
-// RUN: echo "\nC++ tests:"
+// RUN: echo "C++ tests:"
 // RUN: %clang -x c++ %s -O1 -emit-llvm -S -o %t -std=c++98
 // RUN: grep "define linkonce_odr i32 @_Z2eiv()" %t
 // RUN: grep "define linkonce_odr i32 @_Z3foov()" %t
@@ -102,3 +113,17 @@
 // PR11062; the fact that the function is named strlcpy matters here.
 inline __typeof(sizeof(int)) strlcpy(char *dest, const char *src, __typeof(sizeof(int)) size) { return 3; }
 void test8() { strlcpy(0,0,0); }
+
+// PR10657; the test crashed in C99 mode
+extern inline void test9() { }
+void test9();
+
+inline void testA() {}
+void testA();
+
+void testB();
+inline void testB() {}
+extern void testB();
+
+extern inline void testC() {}
+inline void testC();





More information about the cfe-commits mailing list