]> granicus.if.org Git - clang/commitdiff
Reapply r151638 and r151641.
authorJames Molloy <james.molloy@arm.com>
Wed, 29 Feb 2012 10:24:19 +0000 (10:24 +0000)
committerJames Molloy <james.molloy@arm.com>
Wed, 29 Feb 2012 10:24:19 +0000 (10:24 +0000)
The bug that was caught by Apple's internal buildbots was valid and also showed another bug in my implementation.

These are now fixed, with regression tests added to catch them both (not Darwin-specific).

Original log:
====================

Revert r151638 because it causes assertion hit on PCH creation for Cocoa.h

Original log:
---------------------
Correctly track tags and enum members defined in the prototype of a function, and ensure they are properly scoped.

This fixes code such as:

enum e {x, y};
int f(enum {y, x} n) {
 return 0;
}

This finally fixes PR5464 and PR5477.
---------------------

I also reverted r151641 which was enhancement on top of r151638.

====================

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

16 files changed:
include/clang/AST/Decl.h
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Scope.h
include/clang/Sema/Sema.h
lib/AST/Decl.cpp
lib/AST/DumpXML.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/IdentifierResolver.cpp
lib/Sema/Scope.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
test/CodeGen/decl-in-prototype.c [new file with mode: 0644]
test/Misc/warning-flags.c
test/Sema/decl-in-prototype.c [new file with mode: 0644]
test/Sema/fn-ptr-as-fn-prototype.c [new file with mode: 0644]

index c99ad115cfa48ea6c8fe9e7ed7cc3455b59535c4..1338657ff49764da57d954a543aa1473793eb36b 100644 (file)
@@ -1431,6 +1431,11 @@ private:
   /// no formals.
   ParmVarDecl **ParamInfo;
 
+  /// DeclsInPrototypeScope - Array of pointers to NamedDecls for
+  /// decls defined in the function prototype that are not parameters. E.g.
+  /// 'enum Y' in 'void f(enum Y {AA} x) {}'.
+  llvm::ArrayRef<NamedDecl*> DeclsInPrototypeScope;
+
   LazyDeclStmtPtr Body;
 
   // FIXME: This can be packed into the bitfields in Decl.
@@ -1796,6 +1801,11 @@ public:
     setParams(getASTContext(), NewParamInfo);
   }
 
+  const llvm::ArrayRef<NamedDecl*> &getDeclsInPrototypeScope() const {
+    return DeclsInPrototypeScope;
+  }
+  void setDeclsInPrototypeScope(llvm::ArrayRef<NamedDecl *> NewDecls);
+
   /// getMinRequiredArguments - Returns the minimum number of arguments
   /// needed to call this function. This may be fewer than the number of
   /// function parameters, if some of the parameters have default
index 7b45a9a71c5c36c9054a0d2a470697672acac95c..f175b39d709703dfe41731c78d32845c595b08de 100644 (file)
@@ -244,6 +244,7 @@ def VectorConversions : DiagGroup<"vector-conversions">;      // clang specific
 def VexingParse : DiagGroup<"vexing-parse">;
 def VLA : DiagGroup<"vla">;
 def VolatileRegisterVar : DiagGroup<"volatile-register-var">;
+def Visibility : DiagGroup<"visibility">;
 
 // GCC calls -Wdeprecated-writable-strings -Wwrite-strings.
 def GCCWriteStrings : DiagGroup<"write-strings" , [DeprecatedWritableStr]>;
index c347c2e0023e05aa3cd7c0c55bf79917a3118d2d..78d1d8d3d40fa69415dd5de65c1765c54cfe7e1e 100644 (file)
@@ -132,7 +132,11 @@ def warn_unused_variable : Warning<"unused variable %0">,
 def warn_unused_exception_param : Warning<"unused exception parameter %0">,
   InGroup<UnusedExceptionParameter>, DefaultIgnore;
 def warn_decl_in_param_list : Warning<
-  "declaration of %0 will not be visible outside of this function">;
+  "declaration of %0 will not be visible outside of this function">,
+  InGroup<Visibility>;
+def warn_redefinition_in_param_list : Warning<
+  "redefinition of %0 will not be visible outside of this function">,
+  InGroup<Visibility>;
 def warn_empty_parens_are_function_decl : Warning<
   "empty parentheses interpreted as a function declaration">,
   InGroup<VexingParse>;
index 9b3dd9299df3f615adc57b45675ab92249030ce0..e9aa173cb9c783d8ba3247b4262dd20d37ce6389 100644 (file)
@@ -297,6 +297,10 @@ public:
   /// \brief Determine whether this scope is a C++ 'try' block.
   bool isTryScope() const { return getFlags() & Scope::TryScope; }
 
+  /// containedInPrototypeScope - Return true if this or a parent scope
+  /// is a FunctionPrototypeScope.
+  bool containedInPrototypeScope() const;
+
   typedef UsingDirectivesTy::iterator udir_iterator;
   typedef UsingDirectivesTy::const_iterator const_udir_iterator;
 
index 499ee31d4a69efd55788c9e8b357519edc770b15..d7ff5491f82e4c72164305151fc6b89b56bda474 100644 (file)
@@ -902,6 +902,17 @@ public:
   // Symbol table / Decl tracking callbacks: SemaDecl.cpp.
   //
 
+  /// List of decls defined in a function prototype. This contains EnumConstants
+  /// that incorrectly end up in translation unit scope because there is no
+  /// function to pin them on. ActOnFunctionDeclarator reads this list and patches
+  /// them into the FunctionDecl.
+  std::vector<NamedDecl*> DeclsInPrototypeScope;
+  /// Nonzero if we are currently parsing a function declarator. This is a counter
+  /// as opposed to a boolean so we can deal with nested function declarators
+  /// such as:
+  ///     void f(void (*g)(), ...)
+  unsigned InFunctionDeclarator;
+
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = 0);
 
   void DiagnoseUseOfUnimplementedSelectors();
@@ -1048,6 +1059,8 @@ public:
   // Returns true if the variable declaration is a redeclaration
   bool CheckVariableDeclaration(VarDecl *NewVD, LookupResult &Previous);
   void CheckCompleteVariableDeclaration(VarDecl *var);
+  void ActOnStartFunctionDeclarator();
+  void ActOnEndFunctionDeclarator();
   NamedDecl* ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
                                      TypeSourceInfo *TInfo,
                                      LookupResult &Previous,
index 433ce100cfd5270f58a70cfd210d5ac6562e88fc..bd7579996f4a1916c76a054d1df25aa15f84c1fe 100644 (file)
@@ -1847,6 +1847,16 @@ void FunctionDecl::setParams(ASTContext &C,
   }
 }
 
+void FunctionDecl::setDeclsInPrototypeScope(llvm::ArrayRef<NamedDecl *> NewDecls) {
+  assert(DeclsInPrototypeScope.empty() && "Already has prototype decls!");
+
+  if (!NewDecls.empty()) {
+    NamedDecl **A = new (getASTContext()) NamedDecl*[NewDecls.size()];
+    std::copy(NewDecls.begin(), NewDecls.end(), A);
+    DeclsInPrototypeScope = llvm::ArrayRef<NamedDecl*>(A, NewDecls.size());
+  }
+}
+
 /// getMinRequiredArguments - Returns the minimum number of arguments
 /// needed to call this function. This may be fewer than the number of
 /// function parameters, if some of the parameters have default
index b4038ddaf9522b93ec760b7a43af7933e4e1cffe..b180e808ab40525dfe9ab0bedeebdf908b8972f0 100644 (file)
@@ -497,6 +497,10 @@ struct XMLDumper : public XMLDeclVisitor<XMLDumper>,
     for (FunctionDecl::param_iterator
            I = D->param_begin(), E = D->param_end(); I != E; ++I)
       dispatch(*I);
+    for (llvm::ArrayRef<NamedDecl*>::iterator
+           I = D->getDeclsInPrototypeScope().begin(), E = D->getDeclsInPrototypeScope().end();
+         I != E; ++I)
+      dispatch(*I);
     if (D->doesThisDeclarationHaveABody())
       dispatch(D->getBody());
   }
index 4d93eaf85d5c89fccb34e42c52476de04e1ba9bf..2d8271db407841c055f2a02d743633892681f56b 100644 (file)
@@ -4226,6 +4226,8 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
   ExprResult NoexceptExpr;
   ParsedType TrailingReturnType;
   
+  Actions.ActOnStartFunctionDeclarator();
+
   SourceLocation EndLoc;
   if (isFunctionDeclaratorIdentifierList()) {
     if (RequiresArg)
@@ -4308,6 +4310,8 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
                                              EndLoc, D,
                                              TrailingReturnType),
                 attrs, EndLoc);
+
+  Actions.ActOnEndFunctionDeclarator();
 }
 
 /// isFunctionDeclaratorIdentifierList - This parameter list may have an
index b4691617d01e1781816313089080a7bd609a220b..218323134aae664734d93dcc643f22e5f6fa6511 100644 (file)
@@ -113,7 +113,7 @@ bool IdentifierResolver::isDeclInScope(Decl *D, DeclContext *Ctx,
                              bool ExplicitInstantiationOrSpecialization) const {
   Ctx = Ctx->getRedeclContext();
 
-  if (Ctx->isFunctionOrMethod()) {
+  if (Ctx->isFunctionOrMethod() || S->isFunctionPrototypeScope()) {
     // Ignore the scopes associated within transparent declaration contexts.
     while (S->getEntity() &&
            ((DeclContext *)S->getEntity())->isTransparentContext())
index c76f61af6c7c8891485739d202dd90fe0a9c5c67..10f12ce844f8de9c77e41de63876a5f23047bc4a 100644 (file)
@@ -59,3 +59,13 @@ void Scope::Init(Scope *parent, unsigned flags) {
   Entity = 0;
   ErrorTrap.reset();
 }
+
+bool Scope::containedInPrototypeScope() const {
+  const Scope *S = this;
+  while (S) {
+    if (S->isFunctionPrototypeScope())
+      return true;
+    S = S->getParent();
+  }
+  return false;
+}
index cfb4e4c91b9ee57630efbaf18343dcf6b5cbd8da..bbb7af8aefedc1b6458a6213370763b917bc353a 100644 (file)
@@ -94,7 +94,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
     ObjCShouldCallSuperDealloc(false),
     ObjCShouldCallSuperFinalize(false),
     TUKind(TUKind),
-    NumSFINAEErrors(0), SuppressAccessChecking(false), 
+    NumSFINAEErrors(0), InFunctionDeclarator(0), SuppressAccessChecking(false), 
     AccessCheckingSFINAE(false), InNonInstantiationSFINAEContext(false),
     NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1),
     CurrentInstantiationScope(0), TyposCorrected(0),
index f091d858b533398bfc1fcea75edfb15b927d45ef..7a0ae94739e3dc34968b891aed28e8ec9006973b 100644 (file)
@@ -1249,6 +1249,15 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
   }
 }
 
+void Sema::ActOnStartFunctionDeclarator() {
+  ++InFunctionDeclarator;
+}
+
+void Sema::ActOnEndFunctionDeclarator() {
+  assert(InFunctionDeclarator);
+  --InFunctionDeclarator;
+}
+
 /// \brief Look for an Objective-C class in the translation unit.
 ///
 /// \param Id The name of the Objective-C class we're looking for. If
@@ -5241,6 +5250,15 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   // Finally, we know we have the right number of parameters, install them.
   NewFD->setParams(Params);
 
+  // Find all anonymous symbols defined during the declaration of this function
+  // and add to NewFD. This lets us track decls such 'enum Y' in:
+  //
+  //   void f(enum Y {AA} x) {}
+  //
+  // which would otherwise incorrectly end up in the translation unit scope.
+  NewFD->setDeclsInPrototypeScope(DeclsInPrototypeScope);
+  DeclsInPrototypeScope.clear();
+
   // Process the non-inheritable attributes on this declaration.
   ProcessDeclAttributes(S, NewFD, D,
                         /*NonInheritable=*/true, /*Inheritable=*/false);
@@ -7225,6 +7243,43 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D) {
     }
   }
 
+  // If we had any tags defined in the function prototype,
+  // introduce them into the function scope.
+  if (FnBodyScope) {
+    for (llvm::ArrayRef<NamedDecl*>::iterator I = FD->getDeclsInPrototypeScope().begin(),
+           E = FD->getDeclsInPrototypeScope().end(); I != E; ++I) {
+      NamedDecl *D = *I;
+
+      // Some of these decls (like enums) may have been pinned to the translation unit
+      // for lack of a real context earlier. If so, remove from the translation unit
+      // and reattach to the current context.
+      if (D->getLexicalDeclContext() == Context.getTranslationUnitDecl()) {
+        // Is the decl actually in the context?
+        for (DeclContext::decl_iterator DI = Context.getTranslationUnitDecl()->decls_begin(),
+               DE = Context.getTranslationUnitDecl()->decls_end(); DI != DE; ++DI) {
+          if (*DI == D) {  
+            Context.getTranslationUnitDecl()->removeDecl(D);
+            break;
+          }
+        }
+        // Either way, reassign the lexical decl context to our FunctionDecl.
+        D->setLexicalDeclContext(CurContext);
+      }
+
+      // If the decl has a non-null name, make accessible in the current scope.
+      if (!D->getName().empty())
+        PushOnScopeChains(D, FnBodyScope, /*AddToContext=*/false);
+
+      // Similarly, dive into enums and fish their constants out, making them
+      // accessible in this scope.
+      if (EnumDecl *ED = dyn_cast<EnumDecl>(D)) {
+        for (EnumDecl::enumerator_iterator EI = ED->enumerator_begin(),
+               EE = ED->enumerator_end(); EI != EE; ++EI)
+          PushOnScopeChains(*EI, FnBodyScope, /*AddToContext=*/false);
+      }
+    }
+  }
+
   // Checking attributes of current function definition
   // dllimport attribute.
   DLLImportAttr *DA = FD->getAttr<DLLImportAttr>();
@@ -8177,7 +8232,12 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
                   !isa<CXXRecordDecl>(Def) ||
                   cast<CXXRecordDecl>(Def)->getTemplateSpecializationKind() 
                                                == TSK_ExplicitSpecialization) {
-                Diag(NameLoc, diag::err_redefinition) << Name;
+                // A redeclaration in function prototype scope in C isn't
+                // visible elsewhere, so merely issue a warning.
+                if (!getLangOptions().CPlusPlus && S->containedInPrototypeScope())
+                  Diag(NameLoc, diag::warn_redefinition_in_param_list) << Name;
+                else
+                  Diag(NameLoc, diag::err_redefinition) << Name;
                 Diag(Def->getLocation(), diag::note_previous_definition);
                 // If this is a redefinition, recover by making this
                 // struct be anonymous, which will make any later
@@ -8459,6 +8519,12 @@ CreateNewDecl:
         II->isStr("FILE"))
       Context.setFILEDecl(New);
 
+  // If we were in function prototype scope (and not in C++ mode), add this
+  // tag to the list of decls to inject into the function definition scope.
+  if (S->isFunctionPrototypeScope() && !getLangOptions().CPlusPlus &&
+      InFunctionDeclarator && Name)
+    DeclsInPrototypeScope.push_back(New);
+
   OwnedDecl = true;
   return New;
 }
@@ -10142,6 +10208,12 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceLocation LBraceLoc,
 
   Enum->completeDefinition(BestType, BestPromotionType,
                            NumPositiveBits, NumNegativeBits);
+
+  // If we're declaring a function, ensure this decl isn't forgotten about -
+  // it needs to go into the function scope.
+  if (InFunctionDeclarator)
+    DeclsInPrototypeScope.push_back(Enum);
+
 }
 
 Decl *Sema::ActOnFileScopeAsmDecl(Expr *expr,
diff --git a/test/CodeGen/decl-in-prototype.c b/test/CodeGen/decl-in-prototype.c
new file mode 100644 (file)
index 0000000..949793d
--- /dev/null
@@ -0,0 +1,21 @@
+// RUN: %clang -emit-llvm -S -o - %s | FileCheck %s
+
+const int AA = 5;
+
+// CHECK: define i32 @f1
+int f1(enum {AA,BB} E) {
+    // CHECK: ret i32 1
+    return BB;
+}
+
+// CHECK: define i32 @f2
+int f2(enum {AA=7,BB} E) {
+    // CHECK: ret i32 7
+    return AA;
+}
+
+// Check nested function declarators work.
+int f(void (*g)(), enum {AA,BB} h) {
+    // CHECK: ret i32 0
+    return AA;
+}
index 6621f08fcf7c1333f30243a30099875fe005fba4..5c0e80055a362480765314211d9dd62487423da8 100644 (file)
@@ -17,7 +17,7 @@ This test serves two purposes:
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (257):
+CHECK: Warnings without flags (256):
 CHECK-NEXT:   ext_anonymous_struct_union_qualified
 CHECK-NEXT:   ext_binary_literal
 CHECK-NEXT:   ext_cast_fn_obj
@@ -137,7 +137,6 @@ CHECK-NEXT:   warn_conv_to_base_not_used
 CHECK-NEXT:   warn_conv_to_self_not_used
 CHECK-NEXT:   warn_conv_to_void_not_used
 CHECK-NEXT:   warn_cxx0x_right_shift_in_template_arg
-CHECK-NEXT:   warn_decl_in_param_list
 CHECK-NEXT:   warn_delete_array_type
 CHECK-NEXT:   warn_division_by_zero
 CHECK-NEXT:   warn_double_const_requires_fp64
diff --git a/test/Sema/decl-in-prototype.c b/test/Sema/decl-in-prototype.c
new file mode 100644 (file)
index 0000000..05b8e0a
--- /dev/null
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1_only -verify %s
+
+const int AA = 5;
+
+int f1(enum {AA,BB} E) {
+    return BB;
+}
+
+int f2(enum {AA=7,BB} E) {
+    return AA;
+}
+
+struct a {
+};
+
+int f3(struct a { } *); // expected-warning {{will not be visible outside of this function}}
+
+struct A { struct b { int j; } t; }; // expected-note {{previous definition is here}}
+
+int f4(struct A { struct b { int j; } t; } *); // expected-warning {{declaration of 'struct A' will not be visible outside of this function}} expected-warning {{redefinition of 'b' will not be visible outside of this function}}
+
+struct aA {
+    struct ab { // expected-note {{previous definition is here}} expected-note {{previous definition is here}}
+        int j;
+    } b;
+};
+
+int f5(struct aA { struct ab { int j; } b; struct ab { char glorx; } glorx; } *); // expected-warning {{declaration of 'struct aA' will not be visible}} expected-warning {{redefinition of 'ab' will not be visible}} expected-warning {{redefinition of 'ab' will not be visible}}
+
+void f6(struct z {int b;} c) { // expected-warning {{declaration of 'struct z' will not be visible outside of this function}}
+    struct z d;
+    d.b = 4;
+}
diff --git a/test/Sema/fn-ptr-as-fn-prototype.c b/test/Sema/fn-ptr-as-fn-prototype.c
new file mode 100644 (file)
index 0000000..cf95c97
--- /dev/null
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1_only -ast-print %s | FileCheck %s
+
+// This testcase checks the functionality of 
+// Sema::ActOn{Start,End}FunctionDeclarator, specifically checking that
+// ActOnEndFunctionDeclarator is called after the typedef so the enum
+// is in the global scope, not the scope of f().
+
+// CHECK: typedef void (*g)();
+typedef void (*g) ();
+// CHECK: enum {
+enum {
+  k = -1
+};
+// CHECK: void f() {
+void f() {}