Store decls in prototypes on the declarator instead of in the AST

[rnk]

Summary

This saves two pointers (!) from FunctionDecl that were being used for
some rare and questionable C-only functionality. The
DeclsInPrototypeScope ArrayRef was added in r151712 in order to parse
this kind of C code:

enum e {x, y};
int f(enum {y, x} n) {
 return x; // should return 1, not 0
}

The challenge is that we parse 'int f(enum {y, x} n)' it its own
function prototype scope that gets popped before we build the
FunctionDecl for 'f'. The original change was doing two questionable
things:

1. Saving all tag decls introduced in prototype scope on a TU-global

Sema variable. This is problematic when you have cases like this, where
'x' and 'y' shouldn't be visible in 'f':

void f(void (*fp)(enum { x, y } e)) { /* no x */ }

This patch fixes that, so now 'f' can't see 'x', which is consistent
with GCC.

2. Storing the decls in FunctionDecl in ActOnFunctionDeclarator so that

they could be used in ActOnStartOfFunctionDef. This is just an
inefficient way to move information around. The AST lives forever, but
the list of non-parameter decls in prototype scope is short lived. By
moving this stuff to the Declarator, we get the right (short) lifetime.

As the original change was the author's first major Clang patch, they
can be forgiven.

[thakis]

(Unrelated, but if you're looking at memory: When I was looking at it a while ago, IIRC a surprising amount of memory was taken up by CXXBasePaths objects, and just reordering fields to pack it better made that object several bytes smaller. IIRC I accidentally reverted my local patch for this, but if you're in the mood it's probably not too hard to recreate)

[rnk]

• Remove an assert and test that edge case

[rsmith]

include/clang/Sema/DeclSpec.h

1240

It seems plausible that generated code could have more than 256 such declarations. This class has a pointer, a set of bit-fields, 12 unsigneds, and another pointer, so we could give this a full 32 bits without increasing the size of the class. Alternatively we could share the storage with NumExceptions, since this can only be non-zero in C and that can only be non-zero in C++.

1311

Might make sense to put this into the preceding union, if we care about how big this type is.

test/Misc/ast-dump-decl.c

109–110

Why is this not here any more? Looks like SemaDecl.cpp:8255 should have added it to this DeclContext. It'd be nice for -ast-dump to still dump these declarations somewhere.

[rnk]

include/clang/Sema/DeclSpec.h

1240

I did it as a union with the EH machinery initially, but it changes our behavior on this test case:

void f(struct Foo {} o) {} // error in C++
struct Foo {}; // follow on error for redefining type Foo

We actually have test cases for this at clang/test/SemaCXX/type-definition-in-specifier.cpp. I don't think it's important to preserve that exact behavior, though, so performance may be more important. I just did it this way to avoid premature optimization.

test/Misc/ast-dump-decl.c

109–110

I changed things to dump non-parameter decls in the function now, but we only move NamedDecls into the function DeclContext, so the output is a little different. Think it matters?

[rnk]

• Allow more decls in prototypes, dump fd decls

[rnk]

• Reuse EH specifier storage in C
• Revert dumper change

include/clang/Sema/DeclSpec.h

1240

As discussed offline, I updated the C++ test case and made this stuff not take up any space in C++ mode.

test/Misc/ast-dump-decl.c

109–110

I reverted my change to the dumper. Without more work, it's hard to know what decls came from the function prototype and which come from the function body. Filtering our parameters isn't enough. Think it's OK as is?

[rsmith]

I'd much prefer we take this size reduction now and worry about the minor loss of ast dump fidelity later. Please delete the commented out code though :)

include/clang/AST/Decl.h

public:
   };
 
 private:
   /// ParamInfo - new[]'d array of pointers to VarDecls for the formal
   /// parameters of this function.  This is null if a prototype or if there are
   /// 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.
   // NOTE: VC++ packs bitfields poorly if the types differ.
   unsigned SClass : 2;
   unsigned IsInline : 1;
   unsigned IsInlineSpecified : 1;
   unsigned IsVirtualAsWritten : 1;
public:
   ParmVarDecl *getParamDecl(unsigned i) {
     assert(i < getNumParams() && "Illegal param #");
     return ParamInfo[i];
   }
   void setParams(ArrayRef<ParmVarDecl *> NewParamInfo) {
     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
   /// arguments (in C++).
   unsigned getMinRequiredArguments() const;
 
   QualType getReturnType() const {
     assert(getType()->getAs<FunctionType>() && "Expected a FunctionType!");

include/clang/Sema/DeclSpec.h

struct FunctionTypeInfo : TypeInfoCommon {
     unsigned DeleteParams : 1;
 
     /// HasTrailingReturnType - If this is true, a trailing return type was
     /// specified.
     unsigned HasTrailingReturnType : 1;
 
     /// The location of the left parenthesis in the source.
     unsigned LParenLoc;
 
     /// When isVariadic is true, the location of the ellipsis in the source.
     unsigned EllipsisLoc;
 
     /// The location of the right parenthesis in the source.
     unsigned RParenLoc;
 
     /// NumParams - This is the number of formal parameters specified by the
     /// declarator.
struct FunctionTypeInfo : TypeInfoCommon {
     /// \brief The end location of the exception specification, if any.
     unsigned ExceptionSpecLocEnd;
 
     /// Params - This is a pointer to a new[]'d array of ParamInfo objects that
     /// describe the parameters specified by this function declarator.  null if
     /// there are no parameters specified.
     ParamInfo *Params;
 
+    /// NumDeclsInPrototype - Count of non-parameter declarations that appeared
+    /// in the prototype. These are generally tag types or enumerators.
+    unsigned NumDeclsInPrototype;
+
     union {
       /// \brief Pointer to a new[]'d array of TypeAndRange objects that
       /// contain the types in the function's dynamic exception specification
       /// and their locations, if there is one.
       TypeAndRange *Exceptions;
 
       /// \brief Pointer to the expression in the noexcept-specifier of this
       /// function, if it has one.
       Expr *NoexceptExpr;
   
       /// \brief Pointer to the cached tokens for an exception-specification
       /// that has not yet been parsed.
       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;
 
     /// \brief Reset the parameter list to having zero parameters.
     ///
     /// This is used in various places for error recovery.
     void freeParams() {
       for (unsigned I = 0; I < NumParams; ++I)
         Params[I].DefaultArgTokens.reset();
       if (DeleteParams) {
         delete[] Params;
         DeleteParams = false;
       }
       NumParams = 0;
     }
 
     void destroy() {
       if (DeleteParams)
         delete[] Params;
       if (getExceptionSpecType() == EST_Dynamic)
         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,
     /// like "void foo(a,b,c)".  In a function definition, this will be followed
     /// by the parameter type definitions.
     bool isKNRPrototype() const { return !hasPrototype && NumParams != 0; }
 
     SourceLocation getLParenLoc() const {
static DeclaratorChunk getFunction(bool HasProto,
                                      SourceLocation MutableLoc,
                                      ExceptionSpecificationType ESpecType,
                                      SourceRange ESpecRange,
                                      ParsedType *Exceptions,
                                      SourceRange *ExceptionRanges,
                                      unsigned NumExceptions,
                                      Expr *NoexceptExpr,
                                      CachedTokens *ExceptionSpecTokens,
+                                     ArrayRef<NamedDecl *> DeclsInPrototype,
                                      SourceLocation LocalRangeBegin,
                                      SourceLocation LocalRangeEnd,
                                      Declarator &TheDeclarator,
                                      TypeResult TrailingReturnType =
                                                     TypeResult());
 
   /// \brief Return a DeclaratorChunk for a block.
   static DeclaratorChunk getBlockPointer(unsigned TypeQuals,

include/clang/Sema/Sema.h

public:
   //
 
   struct SkipBodyInfo {
     SkipBodyInfo() : ShouldSkip(false), Previous(nullptr) {}
     bool ShouldSkip;
     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();
 
   bool isSimpleTypeSpecifier(tok::TokenKind Kind) const;
 
   ParsedType getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
                          Scope *S, CXXScopeSpec *SS = nullptr,

lib/AST/ASTDumper.cpp

case EST_Uninstantiated:
       break;
     }
   }
 
   if (const FunctionTemplateSpecializationInfo *FTSI =
           D->getTemplateSpecializationInfo())
     dumpTemplateArgumentList(*FTSI->TemplateArguments);
 
-  for (ArrayRef<NamedDecl *>::iterator
-       I = D->getDeclsInPrototypeScope().begin(),
-       E = D->getDeclsInPrototypeScope().end(); I != E; ++I)
-    dumpDecl(*I);
+  for (const Decl *ChildDecl : D->decls())
+    if (!isa<ParmVarDecl>(ChildDecl))
+      dumpDecl(ChildDecl);
 
   if (!D->param_begin() && D->getNumParams())
     dumpChild([=] { OS << "<<NULL params x " << D->getNumParams() << ">>"; });
   else
     for (const ParmVarDecl *Parameter : D->parameters())
       dumpDecl(Parameter);
 
   if (const CXXConstructorDecl *C = dyn_cast<CXXConstructorDecl>(D))

lib/AST/Decl.cpp

void FunctionDecl::setParams(ASTContext &C,
 
   // Zero params -> null pointer.
   if (!NewParamInfo.empty()) {
     ParamInfo = new (C) ParmVarDecl*[NewParamInfo.size()];
     std::copy(NewParamInfo.begin(), NewParamInfo.end(), ParamInfo);
   }
 }
 
-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
 /// arguments (in C++) or are parameter packs (C++11).
 unsigned FunctionDecl::getMinRequiredArguments() const {
   if (!getASTContext().getLangOpts().CPlusPlus)
     return getNumParams();
 

lib/Parse/ParseDecl.cpp

if (getLangOpts().CPlusPlus) {
         LocalEndLoc = Tok.getLocation();
         SourceRange Range;
         TrailingReturnType = ParseTrailingReturnType(Range);
         EndLoc = Range.getEnd();
       }
     }
   }
 
+  // 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,
                                              LParenLoc,
                                              ParamInfo.data(), ParamInfo.size(),
                                              EllipsisLoc, RParenLoc,
                                              DS.getTypeQualifiers(),
                                              RefQualifierIsLValueRef,
                                              RefQualifierLoc, ConstQualifierLoc,
                                              VolatileQualifierLoc,
                                              RestrictQualifierLoc,
                                              /*MutableLoc=*/SourceLocation(),
                                              ESpecType, ESpecRange,
                                              DynamicExceptions.data(),
                                              DynamicExceptionRanges.data(),
                                              DynamicExceptions.size(),
                                              NoexceptExpr.isUsable() ?
                                                NoexceptExpr.get() : nullptr,
                                              ExceptionSpecTokens,
+                                             DeclsInPrototype,
                                              StartLoc, LocalEndLoc, D,
                                              TrailingReturnType),
                 FnAttrs, EndLoc);
 }
 
 /// ParseRefQualifier - Parses a member function ref-qualifier. Returns
 /// true if a ref-qualifier is found.
 bool Parser::ParseRefQualifier(bool &RefQualifierIsLValueRef,

lib/Parse/ParseExpr.cpp

ParamInfo.AddTypeInfo(DeclaratorChunk::getFunction(/*HasProto=*/true,
                                              /*MutableLoc=*/NoLoc,
                                              EST_None,
                                              /*ESpecRange=*/SourceRange(),
                                              /*Exceptions=*/nullptr,
                                              /*ExceptionRanges=*/nullptr,
                                              /*NumExceptions=*/0,
                                              /*NoexceptExpr=*/nullptr,
                                              /*ExceptionSpecTokens=*/nullptr,
+                                             /*DeclsInPrototype=*/None,
                                              CaretLoc, CaretLoc,
                                              ParamInfo),
                           attrs, CaretLoc);
 
     MaybeParseGNUAttributes(ParamInfo);
 
     // Inform sema that we are starting a block.
     Actions.ActOnBlockArguments(CaretLoc, ParamInfo, getCurScope());

lib/Parse/ParseExprCXX.cpp

D.AddTypeInfo(DeclaratorChunk::getFunction(/*hasProto=*/true,
                                            MutableLoc,
                                            ESpecType, ESpecRange,
                                            DynamicExceptions.data(),
                                            DynamicExceptionRanges.data(),
                                            DynamicExceptions.size(),
                                            NoexceptExpr.isUsable() ?
                                              NoexceptExpr.get() : nullptr,
                                            /*ExceptionSpecTokens*/nullptr,
+                                           /*DeclsInPrototype=*/None,
                                            LParenLoc, FunLocalRangeEnd, D,
                                            TrailingReturnType),
                   Attr, DeclEndLoc);
   } else if (Tok.isOneOf(tok::kw_mutable, tok::arrow, tok::kw___attribute,
                          tok::kw_constexpr) ||
              (Tok.is(tok::l_square) && NextToken().is(tok::l_square))) {
     // It's common to forget that one needs '()' before 'mutable', an attribute
     // specifier, or the result type. Deal with this.
D.AddTypeInfo(DeclaratorChunk::getFunction(/*hasProto=*/true,
                                                MutableLoc,
                                                EST_None,
                                                /*ESpecRange=*/SourceRange(),
                                                /*Exceptions=*/nullptr,
                                                /*ExceptionRanges=*/nullptr,
                                                /*NumExceptions=*/0,
                                                /*NoexceptExpr=*/nullptr,
                                                /*ExceptionSpecTokens=*/nullptr,
+                                               /*DeclsInPrototype=*/None,
                                                DeclLoc, DeclEndLoc, D,
                                                TrailingReturnType),
                   Attr, DeclEndLoc);
   }
 
   // FIXME: Rename BlockScope -> ClosureScope if we decide to continue using
   // it.
   unsigned ScopeFlags = Scope::BlockScope | Scope::FnScope | Scope::DeclScope;

lib/Sema/DeclSpec.cpp

DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
                                              ExceptionSpecificationType
                                                  ESpecType,
                                              SourceRange ESpecRange,
                                              ParsedType *Exceptions,
                                              SourceRange *ExceptionRanges,
                                              unsigned NumExceptions,
                                              Expr *NoexceptExpr,
                                              CachedTokens *ExceptionSpecTokens,
+                                             ArrayRef<NamedDecl*>
+                                                 DeclsInPrototype,
                                              SourceLocation LocalRangeBegin,
                                              SourceLocation LocalRangeEnd,
                                              Declarator &TheDeclarator,
                                              TypeResult TrailingReturnType) {
   assert(!(TypeQuals & DeclSpec::TQ_atomic) &&
          "function cannot have _Atomic qualifier");
 
   DeclaratorChunk I;
DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
   I.Fun.RestrictQualifierLoc    = RestrictQualifierLoc.getRawEncoding();
   I.Fun.MutableLoc              = MutableLoc.getRawEncoding();
   I.Fun.ExceptionSpecType       = ESpecType;
   I.Fun.ExceptionSpecLocBeg     = ESpecRange.getBegin().getRawEncoding();
   I.Fun.ExceptionSpecLocEnd     = ESpecRange.getEnd().getRawEncoding();
   I.Fun.NumExceptions           = 0;
   I.Fun.Exceptions              = nullptr;
   I.Fun.NoexceptExpr            = nullptr;
+  I.Fun.NumDeclsInPrototype     = DeclsInPrototype.size();
   I.Fun.HasTrailingReturnType   = TrailingReturnType.isUsable() ||
                                   TrailingReturnType.isInvalid();
   I.Fun.TrailingReturnType      = TrailingReturnType.get();
 
   assert(I.Fun.TypeQuals == TypeQuals && "bitfield overflow");
   assert(I.Fun.ExceptionSpecType == ESpecType && "bitfield overflow");
 
   // new[] a parameter array if needed.
DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
   case EST_ComputedNoexcept:
     I.Fun.NoexceptExpr = NoexceptExpr;
     break;
 
   case EST_Unparsed:
     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;
 }
 
 void Declarator::setDecompositionBindings(
     SourceLocation LSquareLoc,
     ArrayRef<DecompositionDeclarator::Binding> Bindings,
     SourceLocation RSquareLoc) {
   assert(!hasName() && "declarator given multiple names!");

lib/Sema/SemaDecl.cpp

if (I != ExtnameUndeclaredIdentifiers.end()) {
         Diag(NewFD->getLocation(), diag::warn_redefine_extname_not_applied)
             << /*Variable*/0 << NewFD;
     }
   }
 
   // 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
     // single void argument.
     // We let through "const void" here because Sema::GetTypeForDeclarator
     // already checks for that case.
     if (FTIHasNonVoidParameters(FTI) && FTI.Params[0].Param) {
       for (unsigned i = 0, e = FTI.NumParams; i != e; ++i) {
         ParmVarDecl *Param = cast<ParmVarDecl>(FTI.Params[i].Param);
         assert(Param->getDeclContext() != NewFD && "Was set before ?");
         Param->setDeclContext(NewFD);
         Params.push_back(Param);
 
         if (Param->isInvalidDecl())
           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)
     // parameters for use in the declaration.
     //
     // @code
     // typedef void fn(int);
     // fn f;
Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
   } else {
     assert(R->isFunctionNoProtoType() && NewFD->getNumParams() == 0 &&
            "Should not need args for typedef of non-prototype fn");
   }
 
   // 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(),
                                        Context, 0));
 
   // Functions returning a variably modified type violate C99 6.7.5.2p2
   // because all functions have linkage.
   if (!NewFD->isInvalidDecl() &&
Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
 
   if (FnBodyScope)
     PushDeclContext(FnBodyScope, FD);
 
   // Check the validity of our function parameters
   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);
 
     // If this has an identifier, add it to the scope stack.
     if (Param->getIdentifier() && FnBodyScope) {
       CheckShadow(FnBodyScope, Param);
 
       PushOnScopeChains(Param, FnBodyScope);
     }
   }
 
-  // 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);
 
   // dllimport cannot be applied to non-inline function definitions.
   if (FD->hasAttr<DLLImportAttr>() && !FD->isInlined() &&
       !FD->isTemplateInstantiation()) {
     assert(!FD->hasAttr<DLLExportAttr>());
D.AddTypeInfo(DeclaratorChunk::getFunction(/*HasProto=*/false,
                                              /*MutableLoc=*/NoLoc,
                                              EST_None,
                                              /*ESpecRange=*/SourceRange(),
                                              /*Exceptions=*/nullptr,
                                              /*ExceptionRanges=*/nullptr,
                                              /*NumExceptions=*/0,
                                              /*NoexceptExpr=*/nullptr,
                                              /*ExceptionSpecTokens=*/nullptr,
+                                             /*DeclsInPrototype=*/None,
                                              Loc, Loc, D),
                 DS.getAttributes(),
                 SourceLocation());
   D.SetIdentifier(&II, Loc);
 
   // Insert this function into translation-unit scope.
 
   DeclContext *PrevDC = CurContext;
if (getLangOpts().CPlusPlus) {
       if (TUK == TUK_Definition && !IsTypeSpecifier) {
         Diag(Loc, diag::err_type_defined_in_param_type)
             << Name;
         Invalid = true;
       }
     } else if (!PrevDecl) {
       Diag(Loc, diag::warn_decl_in_param_list) << Context.getTagDeclType(New);
     }
-    DeclsInPrototypeScope.push_back(New);
   }
 
   if (Invalid)
     New->setInvalidDecl();
 
   if (Attr)
     ProcessDeclAttributeList(S, New, Attr);
 

lib/Sema/SemaType.cpp

declarator.AddInnermostTypeInfo(DeclaratorChunk::getFunction(
       /*RestrictQualifierLoc=*/NoLoc,
       /*MutableLoc=*/NoLoc, EST_None,
       /*ESpecRange=*/SourceRange(),
       /*Exceptions=*/nullptr,
       /*ExceptionRanges=*/nullptr,
       /*NumExceptions=*/0,
       /*NoexceptExpr=*/nullptr,
       /*ExceptionSpecTokens=*/nullptr,
+      /*DeclsInPrototype=*/None,
       loc, loc, declarator));
 
   // For consistency, make sure the state still has us as processing
   // the decl spec.
   assert(state.getCurrentChunkIndex() == declarator.getNumTypeObjects() - 1);
   state.setCurrentChunkIndex(declarator.getNumTypeObjects());
 }
 

test/Misc/ast-dump-decl.c

 // CHECK:      IndirectFieldDecl{{.*}} TestIndirectFieldDecl 'int'
 // CHECK-NEXT:   Field{{.*}} ''
 // CHECK-NEXT:   Field{{.*}} 'TestIndirectFieldDecl'
 
 int TestFunctionDecl(int x, enum { e } y) {
   return x;
 }
 // CHECK:      FunctionDecl{{.*}} TestFunctionDecl 'int (int, enum {{.*}})'
-// CHECK-NEXT:   EnumDecl
-// CHECK-NEXT:     EnumConstantDecl{{.*}} e
+// CHECK-NEXT:   EnumConstantDecl{{.*}} e
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
 // CHECK-NEXT:   ParmVarDecl{{.*}} y
 // CHECK-NEXT:   CompoundStmt
 
+int TestFunctionDecl2(enum Enum { e } x) { return x; }
+// CHECK:      FunctionDecl{{.*}} TestFunctionDecl2 'int (enum {{.*}})'
+// CHECK-NEXT:   EnumDecl
+// CHECK-NEXT:     EnumConstantDecl{{.*}} e
+// CHECK-NEXT:   ParmVarDecl{{.*}} x
+// CHECK-NEXT:   CompoundStmt
+
+
 int TestFunctionDeclProto(int x);
 // CHECK:      FunctionDecl{{.*}} TestFunctionDeclProto 'int (int)'
 // CHECK-NEXT:   ParmVarDecl{{.*}} x
 
 extern int TestFunctionDeclSC();
 // CHECK:      FunctionDecl{{.*}} TestFunctionDeclSC 'int ()' extern
 
 inline int TestFunctionDeclInline();

test/PCH/decl-in-prototype.c

+// 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

test/Sema/decl-in-prototype.c

 // 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);
 }
 
 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);
 }
 
 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}}

 }
 
 void pr19018_1 (enum e19018 { qq } x); // expected-warning{{declaration of 'enum e19018' will not be visible outside of this function}}
 enum e19018 qq; //expected-error{{tentative definition has type 'enum e19018' that is never completed}} \
                 //expected-note{{forward declaration of 'enum e19018'}}
 
 // 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);
+}

test/SemaCXX/type-definition-in-specifier.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -verify %s
 
 struct S0;
 struct S1;
 struct S2;
 struct S3;
 struct S4;
 struct S5;
 struct S6;
enum en_3 x2; // expected-error{{ISO C++ forbids forward references to 'enum' types}} \
                 // expected-error{{field has incomplete type 'enum en_3'}} \
                 // expected-note{{forward declaration of 'en_3'}}
 };
 
 struct pr18963 {
   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