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 ↗

(On Diff #80650)

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 ↗

(On Diff #80650)

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 ↗

(On Diff #80650)

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 ↗

(On Diff #80650)

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 ↗

(On Diff #80650)

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 ↗

(On Diff #80650)

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 ↗

(On Diff #80650)

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 :)

cfe/trunk/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!");

cfe/trunk/include/clang/Sema/DeclSpec.h

struct FunctionTypeInfo : TypeInfoCommon {
 
     /// The location of the right parenthesis in the source.
     unsigned RParenLoc;
 
     /// NumParams - This is the number of formal parameters specified by the
     /// declarator.
     unsigned NumParams;
 
-    /// NumExceptions - This is the number of types in the dynamic-exception-
-    /// decl, if the function has one.
-    unsigned NumExceptions;
+    /// NumExceptionsOrDecls - This is the number of types in the
+    /// dynamic-exception-decl, if the function has one. In C, this is the
+    /// number of declarations in the function prototype.
+    unsigned NumExceptionsOrDecls;
 
     /// \brief The location of the ref-qualifier, if any.
     ///
     /// If this is an invalid location, there is no ref-qualifier.
     unsigned RefQualifierLoc;
 
     /// \brief The location of the const-qualifier, if any.
     ///
union {
 
       /// \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. Does not exist in
+      /// C++.
+      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)
+      switch (getExceptionSpecType()) {
+      default:
+        break;
+      case EST_Dynamic:
         delete[] Exceptions;
-      else if (getExceptionSpecType() == EST_Unparsed)
+        break;
+      case EST_Unparsed:
         delete ExceptionSpecTokens;
+        break;
+      case EST_None:
+        if (NumExceptionsOrDecls != 0)
+          delete[] DeclsInPrototype;
+        break;
+      }
     }
 
     /// 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 {
struct FunctionTypeInfo : TypeInfoCommon {
     /// qualifier.
     bool hasMutableQualifier() const { return getMutableLoc().isValid(); }
 
     /// \brief Get the type of exception specification this function has.
     ExceptionSpecificationType getExceptionSpecType() const {
       return static_cast<ExceptionSpecificationType>(ExceptionSpecType);
     }
 
+    /// \brief Get the number of dynamic exception specifications.
+    unsigned getNumExceptions() const {
+      assert(ExceptionSpecType != EST_None);
+      return NumExceptionsOrDecls;
+    }
+
+    /// \brief Get the non-parameter decls defined within this function
+    /// prototype. Typically these are tag declarations.
+    ArrayRef<NamedDecl *> getDeclsInPrototype() const {
+      assert(ExceptionSpecType == EST_None);
+      return llvm::makeArrayRef(DeclsInPrototype, NumExceptionsOrDecls);
+    }
+
     /// \brief Determine whether this function declarator had a
     /// trailing-return-type.
     bool hasTrailingReturnType() const { return HasTrailingReturnType; }
 
     /// \brief Get the trailing-return-type for this function declarator.
     ParsedType getTrailingReturnType() const { return TrailingReturnType; }
   };
 
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,

cfe/trunk/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,

cfe/trunk/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);
-
   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))
     for (CXXConstructorDecl::init_const_iterator I = C->init_begin(),

cfe/trunk/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();
 

cfe/trunk/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
+  // declaration. They will be moved into the scope of the function. Only do
+  // this in C and not C++, where the decls will continue to live in the
+  // surrounding context.
+  SmallVector<NamedDecl *, 0> DeclsInPrototype;
+  if (getCurScope()->getFlags() & Scope::FunctionDeclarationScope &&
+      !getLangOpts().CPlusPlus) {
+    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,

cfe/trunk/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());

cfe/trunk/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;

cfe/trunk/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.RefQualifierLoc         = RefQualifierLoc.getRawEncoding();
   I.Fun.ConstQualifierLoc       = ConstQualifierLoc.getRawEncoding();
   I.Fun.VolatileQualifierLoc    = VolatileQualifierLoc.getRawEncoding();
   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.NumExceptionsOrDecls    = 0;
   I.Fun.Exceptions              = nullptr;
   I.Fun.NoexceptExpr            = nullptr;
   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");
DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
   }
 
   // Check what exception specification information we should actually store.
   switch (ESpecType) {
   default: break; // By default, save nothing.
   case EST_Dynamic:
     // new[] an exception array if needed
     if (NumExceptions) {
-      I.Fun.NumExceptions = NumExceptions;
+      I.Fun.NumExceptionsOrDecls = NumExceptions;
       I.Fun.Exceptions = new DeclaratorChunk::TypeAndRange[NumExceptions];
       for (unsigned i = 0; i != NumExceptions; ++i) {
         I.Fun.Exceptions[i].Ty = Exceptions[i];
         I.Fun.Exceptions[i].Range = ExceptionRanges[i];
       }
     }
     break;
 
   case EST_ComputedNoexcept:
     I.Fun.NoexceptExpr = NoexceptExpr;
     break;
 
   case EST_Unparsed:
     I.Fun.ExceptionSpecTokens = ExceptionSpecTokens;
     break;
   }
+
+  if (!DeclsInPrototype.empty()) {
+    assert(ESpecType == EST_None && NumExceptions == 0 &&
+           "cannot have exception specifiers and decls in prototype");
+    I.Fun.NumExceptionsOrDecls = DeclsInPrototype.size();
+    // 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!");

cfe/trunk/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();
       }
     }
+
+    if (!getLangOpts().CPlusPlus) {
+      // In C, 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 (NamedDecl *NonParmDecl : FTI.getDeclsInPrototype()) {
+        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);
 

cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp

case DeclaratorChunk::Function:
       for (unsigned i = 0, e = Chunk.Fun.NumParams; i != e; ++i) {
         ParmVarDecl *Param = cast<ParmVarDecl>(Chunk.Fun.Params[i].Param);
         QualType ParamTy = Param->getType();
         assert(!ParamTy.isNull() && "Couldn't parse type?");
         if (ParamTy->containsUnexpandedParameterPack()) return true;
       }
 
       if (Chunk.Fun.getExceptionSpecType() == EST_Dynamic) {
-        for (unsigned i = 0; i != Chunk.Fun.NumExceptions; ++i) {
+        for (unsigned i = 0; i != Chunk.Fun.getNumExceptions(); ++i) {
           if (Chunk.Fun.Exceptions[i]
                   .Ty.get()
                   ->containsUnexpandedParameterPack())
             return true;
         }
       } else if (Chunk.Fun.getExceptionSpecType() == EST_ComputedNoexcept &&
                  Chunk.Fun.NoexceptExpr->containsUnexpandedParameterPack())
         return true;

cfe/trunk/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());
 }
 
case DeclaratorChunk::Function: {
         SmallVector<QualType, 4> Exceptions;
         SmallVector<ParsedType, 2> DynamicExceptions;
         SmallVector<SourceRange, 2> DynamicExceptionRanges;
         Expr *NoexceptExpr = nullptr;
 
         if (FTI.getExceptionSpecType() == EST_Dynamic) {
           // FIXME: It's rather inefficient to have to split into two vectors
           // here.
-          unsigned N = FTI.NumExceptions;
+          unsigned N = FTI.getNumExceptions();
           DynamicExceptions.reserve(N);
           DynamicExceptionRanges.reserve(N);
           for (unsigned I = 0; I != N; ++I) {
             DynamicExceptions.push_back(FTI.Exceptions[I].Ty);
             DynamicExceptionRanges.push_back(FTI.Exceptions[I].Range);
           }
         } else if (FTI.getExceptionSpecType() == EST_ComputedNoexcept) {
           NoexceptExpr = FTI.NoexceptExpr;

cfe/trunk/test/Misc/ast-dump-decl.c

struct testIndirectFieldDecl {
   struct {
     int TestIndirectFieldDecl;
   };
 };
 // CHECK:      IndirectFieldDecl{{.*}} TestIndirectFieldDecl 'int'
 // CHECK-NEXT:   Field{{.*}} ''
 // CHECK-NEXT:   Field{{.*}} 'TestIndirectFieldDecl'
 
+// FIXME: It would be nice to dump the enum and its enumerators.
 int TestFunctionDecl(int x, enum { e } y) {
   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
 
+// FIXME: It would be nice to 'Enum' and 'e'.
+int TestFunctionDecl2(enum Enum { e } x) { return x; }
+// CHECK:      FunctionDecl{{.*}} TestFunctionDecl2 'int (enum {{.*}})'
+// 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();

cfe/trunk/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

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

cfe/trunk/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;

 void f2(struct S6 { int x; } p); // expected-error{{parameter type}}
 
 struct pr19018 {
   short foo6 (enum bar0 {qq} bar3); // expected-error{{cannot be defined in a parameter type}}
 };
 
 void pr19018_1 (enum e19018_1 {qq} x); // expected-error{{cannot be defined in a parameter type}}
 void pr19018_1a (enum e19018_1 {qq} x); // expected-error{{cannot be defined in a parameter type}}
-e19018_1 x2;  // expected-error{{unknown type name 'e19018_1'}}
+e19018_1 x2;
 
 void pr19018_2 (enum {qq} x); // expected-error{{cannot be defined in a parameter type}}
 void pr19018_3 (struct s19018_2 {int qq;} x); // expected-error{{cannot be defined in a parameter type}}
 void pr19018_4 (struct {int qq;} x); // expected-error{{cannot be defined in a parameter type}}
 void pr19018_5 (struct { void qq(); } x); // expected-error{{cannot be defined in a parameter type}}
 void pr19018_5 (struct s19018_2 { void qq(); } x); // expected-error{{cannot be defined in a parameter type}}
 
 struct pr19018a {
   static int xx;
   void func1(enum t19018 {qq} x); // expected-error{{cannot be defined in a parameter type}}
   void func2(enum t19018 {qq} x); // expected-error{{cannot be defined in a parameter type}}
   void func3(enum {qq} x);        // expected-error{{cannot be defined in a parameter type}}
   void func4(struct t19018 {int qq;} x);  // expected-error{{cannot be defined in a parameter type}}
   void func5(struct {int qq;} x); // expected-error{{cannot be defined in a parameter type}}
   void func6(struct { void qq(); } x); // expected-error{{cannot be defined in a parameter type}}
   void func7(struct t19018 { void qq(); } x); // expected-error{{cannot be defined in a parameter type}}
   void func8(struct { int qq() { return xx; }; } x); // expected-error{{cannot be defined in a parameter type}}
   void func9(struct t19018 { int qq() { return xx; }; } x); // expected-error{{cannot be defined in a parameter type}}
 };
 
 struct s19018b {
   void func1 (enum en_2 {qq} x); // expected-error{{cannot be defined in a parameter type}}
-  en_2 x1;  // expected-error{{unknown type name 'en_2'}}
+  en_2 x1;
   void func2 (enum en_3 {qq} x); // expected-error{{cannot be defined in a parameter type}}
-  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'}}
+  enum en_3 x2;
 };
 
 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'}}
+  short bar5 (struct foo4 {} bar2); // expected-error{{'foo4' cannot be defined in a parameter type}} \
+                                    // expected-note{{declared here}}
+
+  long foo5 (float foo6 = foo4);  // expected-error{{'foo4' does not refer to a value}}
 };
+
+// expected-error@+2 {{cannot be defined in a parameter type}}
+// expected-note@+1 {{previous definition is here}}
+void func_with_eh_and_type(struct type_in_eh {} o) throw(int) {}
+struct type_in_eh {}; // expected-error {{redefinition of 'type_in_eh'}}