rnk updated this revision to Diff 80650.
rnk added a comment.

- Remove an assert and test that edge case


https://reviews.llvm.org/D27279

Files:
  include/clang/AST/Decl.h
  include/clang/Sema/DeclSpec.h
  include/clang/Sema/Sema.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Sema/DeclSpec.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaType.cpp
  test/Misc/ast-dump-decl.c
  test/PCH/decl-in-prototype.c
  test/Sema/decl-in-prototype.c
  test/SemaCXX/type-definition-in-specifier.cpp

Index: test/SemaCXX/type-definition-in-specifier.cpp
===================================================================
--- test/SemaCXX/type-definition-in-specifier.cpp
+++ test/SemaCXX/type-definition-in-specifier.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -verify %s
 
 struct S0;
 struct S1;
@@ -64,3 +64,6 @@
   short bar5 (struct foo4 {} bar2); // expected-error{{'foo4' cannot be defined in a parameter type}}
   long foo5 (float foo6 = foo4);  // expected-error{{use of undeclared identifier 'foo4'}}
 };
+
+void func_with_eh_and_type(struct type_in_eh {} o) throw(int) {} // expected-error{{cannot be defined in a parameter type}}
+struct type_in_eh {}; // no diagnostics
Index: test/Sema/decl-in-prototype.c
===================================================================
--- test/Sema/decl-in-prototype.c
+++ test/Sema/decl-in-prototype.c
@@ -1,13 +1,19 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
+#define SA(n, c) int arr##n[(c) ? 1 : -1] = {}
+
 const int AA = 5;
 
 int f1(enum {AA,BB} E) { // expected-warning {{will not be visible outside of this function}}
-    return BB;
+  SA(1, AA == 0);
+  SA(2, BB == 1);
+  return BB;
 }
 
 int f2(enum {AA=7,BB} E) { // expected-warning {{will not be visible outside of this function}}
-    return AA;
+  SA(1, AA == 7);
+  SA(2, BB == 8);
+  return AA;
 }
 
 struct a {
@@ -38,3 +44,11 @@
 
 // Only warn once, even if we create two declarations.
 void f(struct q *, struct __attribute__((aligned(4))) q *); // expected-warning {{will not be visible outside}}
+
+// This enum inside the function pointer parameter shouldn't leak into the
+// function.
+enum { BB = 0 };
+void enum_in_fun_in_fun(void (*fp)(enum { AA, BB } e)) { // expected-warning {{will not be visible}}
+  SA(1, AA == 5);
+  SA(2, BB == 0);
+}
Index: test/PCH/decl-in-prototype.c
===================================================================
--- /dev/null
+++ test/PCH/decl-in-prototype.c
@@ -0,0 +1,27 @@
+// Test that we serialize the enum decl in the function prototype somehow.
+// These decls aren't serialized quite the same way as parameters.
+
+// Test this without pch.
+// RUN: %clang_cc1 -include %s -emit-llvm -o - %s | FileCheck %s
+
+// Test with pch.
+// RUN: %clang_cc1 -emit-pch -o %t %s
+// RUN: %clang_cc1 -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define i32 @main()
+// CHECK:   ret i32 1
+
+#ifndef HEADER
+#define HEADER
+
+static inline __attribute__((always_inline)) f(enum { x, y } p) {
+  return y;
+}
+
+#else
+
+int main() {
+  return f(0);
+}
+
+#endif
Index: test/Misc/ast-dump-decl.c
===================================================================
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -106,8 +106,6 @@
   return x;
 }
 // CHECK:      FunctionDecl{{.*}} TestFunctionDecl 'int (int, enum {{.*}})'
-// CHECK-NEXT:   EnumDecl
-// CHECK-NEXT:     EnumConstantDecl{{.*}} e
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
 // CHECK-NEXT:   ParmVarDecl{{.*}} y
 // CHECK-NEXT:   CompoundStmt
Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -718,6 +718,7 @@
       /*NumExceptions=*/0,
       /*NoexceptExpr=*/nullptr,
       /*ExceptionSpecTokens=*/nullptr,
+      /*DeclsInPrototype=*/None,
       loc, loc, declarator));
 
   // For consistency, make sure the state still has us as processing
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -8222,8 +8222,9 @@
   // Copy the parameter declarations from the declarator D to the function
   // declaration NewFD, if they are available.  First scavenge them into Params.
   SmallVector<ParmVarDecl*, 16> Params;
-  if (D.isFunctionDeclarator()) {
-    DeclaratorChunk::FunctionTypeInfo &FTI = D.getFunctionTypeInfo();
+  unsigned FTIIdx;
+  if (D.isFunctionDeclarator(FTIIdx)) {
+    DeclaratorChunk::FunctionTypeInfo &FTI = D.getTypeObject(FTIIdx).Fun;
 
     // Check for C99 6.7.5.3p10 - foo(void) is a non-varargs
     // function that takes no arguments, not a function that takes a
@@ -8241,6 +8242,18 @@
           NewFD->setInvalidDecl();
       }
     }
+
+    // Find all the non-parameter declarations from the prototype and move them
+    // into the new function decl context as well. Typically they will have been
+    // added to the surrounding context of the prototype.
+    for (size_t I = 0; I < FTI.NumDeclsInPrototype; ++I) {
+      NamedDecl *NonParmDecl = FTI.DeclsInPrototype[I];
+      DeclContext *OldDC = NonParmDecl->getDeclContext();
+      if (OldDC->containsDecl(NonParmDecl))
+        OldDC->removeDecl(NonParmDecl);
+      NonParmDecl->setDeclContext(NewFD);
+      NewFD->addDecl(NonParmDecl);
+    }
   } else if (const FunctionProtoType *FT = R->getAs<FunctionProtoType>()) {
     // When we're declaring a function with a typedef, typeof, etc as in the
     // following example, we'll need to synthesize (unnamed)
@@ -8266,15 +8279,6 @@
   // 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();
-
   if (D.getDeclSpec().isNoreturnSpecified())
     NewFD->addAttr(
         ::new(Context) C11NoReturnAttr(D.getDeclSpec().getNoreturnSpecLoc(),
@@ -11623,6 +11627,29 @@
   CheckParmsForFunctionDef(FD->parameters(),
                            /*CheckParameterNames=*/true);
 
+  // Add non-parameter declarations already in the function to the current
+  // scope.
+  if (FnBodyScope) {
+    for (Decl *NPD : FD->decls()) {
+      auto *NonParmDecl = dyn_cast<NamedDecl>(NPD);
+      if (!NonParmDecl)
+        continue;
+      assert(!isa<ParmVarDecl>(NonParmDecl) &&
+             "parameters should not be in newly created FD yet");
+
+      // If the decl has a name, make it accessible in the current scope.
+      if (NonParmDecl->getDeclName())
+        PushOnScopeChains(NonParmDecl, FnBodyScope, /*AddToContext=*/false);
+
+      // Similarly, dive into enums and fish their constants out, making them
+      // accessible in this scope.
+      if (auto *ED = dyn_cast<EnumDecl>(NonParmDecl)) {
+        for (auto *EI : ED->enumerators())
+          PushOnScopeChains(EI, FnBodyScope, /*AddToContext=*/false);
+      }
+    }
+  }
+
   // Introduce our parameters into the function scope
   for (auto Param : FD->parameters()) {
     Param->setOwningFunction(FD);
@@ -11635,39 +11662,6 @@
     }
   }
 
-  // If we had any tags defined in the function prototype,
-  // introduce them into the function scope.
-  if (FnBodyScope) {
-    for (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?
-        if (Context.getTranslationUnitDecl()->containsDecl(D))
-          Context.getTranslationUnitDecl()->removeDecl(D);
-        // 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 (auto *ED = dyn_cast<EnumDecl>(D)) {
-        for (auto *EI : ED->enumerators())
-          PushOnScopeChains(EI, FnBodyScope, /*AddToContext=*/false);
-      }
-    }
-  }
-
   // Ensure that the function's exception specification is instantiated.
   if (const FunctionProtoType *FPT = FD->getType()->getAs<FunctionProtoType>())
     ResolveExceptionSpec(D->getLocation(), FPT);
@@ -12152,6 +12146,7 @@
                                              /*NumExceptions=*/0,
                                              /*NoexceptExpr=*/nullptr,
                                              /*ExceptionSpecTokens=*/nullptr,
+                                             /*DeclsInPrototype=*/None,
                                              Loc, Loc, D),
                 DS.getAttributes(),
                 SourceLocation());
@@ -13421,7 +13416,6 @@
     } else if (!PrevDecl) {
       Diag(Loc, diag::warn_decl_in_param_list) << Context.getTagDeclType(New);
     }
-    DeclsInPrototypeScope.push_back(New);
   }
 
   if (Invalid)
Index: lib/Sema/DeclSpec.cpp
===================================================================
--- lib/Sema/DeclSpec.cpp
+++ lib/Sema/DeclSpec.cpp
@@ -173,6 +173,8 @@
                                              unsigned NumExceptions,
                                              Expr *NoexceptExpr,
                                              CachedTokens *ExceptionSpecTokens,
+                                             ArrayRef<NamedDecl*>
+                                                 DeclsInPrototype,
                                              SourceLocation LocalRangeBegin,
                                              SourceLocation LocalRangeEnd,
                                              Declarator &TheDeclarator,
@@ -188,6 +190,7 @@
   I.Fun.hasPrototype            = hasProto;
   I.Fun.isVariadic              = EllipsisLoc.isValid();
   I.Fun.isAmbiguous             = isAmbiguous;
+  I.Fun.NumDeclsInPrototype     = DeclsInPrototype.size();
   I.Fun.LParenLoc               = LParenLoc.getRawEncoding();
   I.Fun.EllipsisLoc             = EllipsisLoc.getRawEncoding();
   I.Fun.RParenLoc               = RParenLoc.getRawEncoding();
@@ -257,6 +260,14 @@
     I.Fun.ExceptionSpecTokens = ExceptionSpecTokens;
     break;
   }
+
+  if (!DeclsInPrototype.empty()) {
+    // Copy the array of decls into stable heap storage.
+    I.Fun.DeclsInPrototype = new NamedDecl *[DeclsInPrototype.size()];
+    for (size_t J = 0; J < DeclsInPrototype.size(); ++J)
+      I.Fun.DeclsInPrototype[J] = DeclsInPrototype[J];
+  }
+
   return I;
 }
 
Index: lib/Parse/ParseExprCXX.cpp
===================================================================
--- lib/Parse/ParseExprCXX.cpp
+++ lib/Parse/ParseExprCXX.cpp
@@ -1244,6 +1244,7 @@
                                            NoexceptExpr.isUsable() ?
                                              NoexceptExpr.get() : nullptr,
                                            /*ExceptionSpecTokens*/nullptr,
+                                           /*DeclsInPrototype=*/None,
                                            LParenLoc, FunLocalRangeEnd, D,
                                            TrailingReturnType),
                   Attr, DeclEndLoc);
@@ -1313,6 +1314,7 @@
                                                /*NumExceptions=*/0,
                                                /*NoexceptExpr=*/nullptr,
                                                /*ExceptionSpecTokens=*/nullptr,
+                                               /*DeclsInPrototype=*/None,
                                                DeclLoc, DeclEndLoc, D,
                                                TrailingReturnType),
                   Attr, DeclEndLoc);
Index: lib/Parse/ParseExpr.cpp
===================================================================
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -2836,6 +2836,7 @@
                                              /*NumExceptions=*/0,
                                              /*NoexceptExpr=*/nullptr,
                                              /*ExceptionSpecTokens=*/nullptr,
+                                             /*DeclsInPrototype=*/None,
                                              CaretLoc, CaretLoc,
                                              ParamInfo),
                           attrs, CaretLoc);
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -5827,6 +5827,18 @@
     }
   }
 
+  // Collect non-parameter declarations from the prototype if this is a function
+  // definition.
+  SmallVector<NamedDecl *, 0> DeclsInPrototype;
+  if (getCurScope()->getFlags() & Scope::FunctionDeclarationScope) {
+    for (Decl *D : getCurScope()->decls()) {
+      NamedDecl *ND = dyn_cast<NamedDecl>(D);
+      if (!ND || isa<ParmVarDecl>(ND))
+        continue;
+      DeclsInPrototype.push_back(ND);
+    }
+  }
+
   // Remember that we parsed a function type, and remember the attributes.
   D.AddTypeInfo(DeclaratorChunk::getFunction(HasProto,
                                              IsAmbiguous,
@@ -5846,6 +5858,7 @@
                                              NoexceptExpr.isUsable() ?
                                                NoexceptExpr.get() : nullptr,
                                              ExceptionSpecTokens,
+                                             DeclsInPrototype,
                                              StartLoc, LocalEndLoc, D,
                                              TrailingReturnType),
                 FnAttrs, EndLoc);
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2840,28 +2840,6 @@
   }
 }
 
-void FunctionDecl::setDeclsInPrototypeScope(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::makeArrayRef(A, NewDecls.size());
-    // Move declarations introduced in prototype to the function context.
-    for (auto I : NewDecls) {
-      DeclContext *DC = I->getDeclContext();
-      // Forward-declared reference to an enumeration is not added to
-      // declaration scope, so skip declaration that is absent from its
-      // declaration contexts.
-      if (DC->containsDecl(I)) {
-          DC->removeDecl(I);
-          I->setDeclContext(this);
-          addDecl(I);
-      }
-    }
-  }
-}
-
 /// 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: lib/AST/ASTDumper.cpp
===================================================================
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -1164,11 +1164,6 @@
           D->getTemplateSpecializationInfo())
     dumpTemplateArgumentList(*FTSI->TemplateArguments);
 
-  for (ArrayRef<NamedDecl *>::iterator
-       I = D->getDeclsInPrototypeScope().begin(),
-       E = D->getDeclsInPrototypeScope().end(); I != E; ++I)
-    dumpDecl(*I);
-
   if (!D->param_begin() && D->getNumParams())
     dumpChild([=] { OS << "<<NULL params x " << D->getNumParams() << ">>"; });
   else
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1527,12 +1527,6 @@
     NamedDecl *Previous;
   };
 
-  /// 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;
-
   DeclGroupPtrTy ConvertDeclToDeclGroup(Decl *Ptr, Decl *OwnedType = nullptr);
 
   void DiagnoseUseOfUnimplementedSelectors();
Index: include/clang/Sema/DeclSpec.h
===================================================================
--- include/clang/Sema/DeclSpec.h
+++ include/clang/Sema/DeclSpec.h
@@ -1235,6 +1235,10 @@
     /// specified.
     unsigned HasTrailingReturnType : 1;
 
+    /// NumDeclsInPrototype - Count of non-parameter declarations that appeared
+    /// in the prototype. These are generally tag types or enumerators.
+    unsigned NumDeclsInPrototype : 8;
+
     /// The location of the left parenthesis in the source.
     unsigned LParenLoc;
 
@@ -1302,6 +1306,10 @@
       CachedTokens *ExceptionSpecTokens;
     };
 
+    /// Pointer to a new[]'d array of declarations that need to be available
+    /// for lookup inside the function body, if one exists.
+    NamedDecl **DeclsInPrototype;
+
     /// \brief If HasTrailingReturnType is true, this is the trailing return
     /// type specified.
     UnionParsedType TrailingReturnType;
@@ -1326,6 +1334,8 @@
         delete[] Exceptions;
       else if (getExceptionSpecType() == EST_Unparsed)
         delete ExceptionSpecTokens;
+      if (NumDeclsInPrototype != 0)
+        delete[] DeclsInPrototype;
     }
 
     /// isKNRPrototype - Return true if this is a K&R style identifier list,
@@ -1540,6 +1550,7 @@
                                      unsigned NumExceptions,
                                      Expr *NoexceptExpr,
                                      CachedTokens *ExceptionSpecTokens,
+                                     ArrayRef<NamedDecl *> DeclsInPrototype,
                                      SourceLocation LocalRangeBegin,
                                      SourceLocation LocalRangeEnd,
                                      Declarator &TheDeclarator,
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1601,11 +1601,6 @@
   /// 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) {}'.
-  ArrayRef<NamedDecl *> DeclsInPrototypeScope;
-
   LazyDeclStmtPtr Body;
 
   // FIXME: This can be packed into the bitfields in DeclContext.
@@ -2050,11 +2045,6 @@
     setParams(getASTContext(), NewParamInfo);
   }
 
-  ArrayRef<NamedDecl *> getDeclsInPrototypeScope() const {
-    return DeclsInPrototypeScope;
-  }
-  void setDeclsInPrototypeScope(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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to