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

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.

Moving these things to the Declarator solves both of these issues.

Reviewers: rsmith

Subscribers: jmolloy, cfe-commits

Differential Revision: https://reviews.llvm.org/D27279

llvm-svn: 289225
This commit is contained in:
Reid Kleckner 2016-12-09 17:14:05 +00:00
parent 1ef90d2f25
commit 078aea9043
16 changed files with 176 additions and 109 deletions

View File

@ -1601,11 +1601,6 @@ private:
/// no formals.
ParmVarDecl **ParamInfo;
/// DeclsInPrototypeScope - Array of pointers to NamedDecls for
/// decls defined in the function prototype that are not parameters. E.g.
/// 'enum Y' in 'void f(enum Y {AA} x) {}'.
ArrayRef<NamedDecl *> DeclsInPrototypeScope;
LazyDeclStmtPtr Body;
// FIXME: This can be packed into the bitfields in DeclContext.
@ -2050,11 +2045,6 @@ public:
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

View File

@ -1248,9 +1248,10 @@ struct DeclaratorChunk {
/// 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.
///
@ -1300,6 +1301,11 @@ struct DeclaratorChunk {
/// \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
@ -1322,10 +1328,20 @@ struct DeclaratorChunk {
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,
@ -1395,6 +1411,19 @@ struct DeclaratorChunk {
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; }
@ -1540,6 +1569,7 @@ struct DeclaratorChunk {
unsigned NumExceptions,
Expr *NoexceptExpr,
CachedTokens *ExceptionSpecTokens,
ArrayRef<NamedDecl *> DeclsInPrototype,
SourceLocation LocalRangeBegin,
SourceLocation LocalRangeEnd,
Declarator &TheDeclarator,

View File

@ -1527,12 +1527,6 @@ public:
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();

View File

@ -1164,11 +1164,6 @@ void ASTDumper::VisitFunctionDecl(const FunctionDecl *D) {
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

View File

@ -2840,28 +2840,6 @@ void FunctionDecl::setParams(ASTContext &C,
}
}
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

View File

@ -5827,6 +5827,21 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
}
}
// 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,
@ -5846,6 +5861,7 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
NoexceptExpr.isUsable() ?
NoexceptExpr.get() : nullptr,
ExceptionSpecTokens,
DeclsInPrototype,
StartLoc, LocalEndLoc, D,
TrailingReturnType),
FnAttrs, EndLoc);

View File

@ -2842,6 +2842,7 @@ ExprResult Parser::ParseBlockLiteralExpression() {
/*NumExceptions=*/0,
/*NoexceptExpr=*/nullptr,
/*ExceptionSpecTokens=*/nullptr,
/*DeclsInPrototype=*/None,
CaretLoc, CaretLoc,
ParamInfo),
attrs, CaretLoc);

View File

@ -1244,6 +1244,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
NoexceptExpr.isUsable() ?
NoexceptExpr.get() : nullptr,
/*ExceptionSpecTokens*/nullptr,
/*DeclsInPrototype=*/None,
LParenLoc, FunLocalRangeEnd, D,
TrailingReturnType),
Attr, DeclEndLoc);
@ -1313,6 +1314,7 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
/*NumExceptions=*/0,
/*NoexceptExpr=*/nullptr,
/*ExceptionSpecTokens=*/nullptr,
/*DeclsInPrototype=*/None,
DeclLoc, DeclEndLoc, D,
TrailingReturnType),
Attr, DeclEndLoc);

View File

@ -173,6 +173,8 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
unsigned NumExceptions,
Expr *NoexceptExpr,
CachedTokens *ExceptionSpecTokens,
ArrayRef<NamedDecl*>
DeclsInPrototype,
SourceLocation LocalRangeBegin,
SourceLocation LocalRangeEnd,
Declarator &TheDeclarator,
@ -204,7 +206,7 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
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() ||
@ -240,7 +242,7 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
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];
@ -257,6 +259,17 @@ DeclaratorChunk DeclaratorChunk::getFunction(bool hasProto,
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;
}

View File

@ -8222,8 +8222,9 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
// 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,19 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
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)
@ -8266,15 +8280,6 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
// Finally, we know we have the right number of parameters, install them.
NewFD->setParams(Params);
// Find all anonymous symbols defined during the declaration of this function
// and add to NewFD. This lets us track decls such 'enum Y' in:
//
// void f(enum Y {AA} x) {}
//
// which would otherwise incorrectly end up in the translation unit scope.
NewFD->setDeclsInPrototypeScope(DeclsInPrototypeScope);
DeclsInPrototypeScope.clear();
if (D.getDeclSpec().isNoreturnSpecified())
NewFD->addAttr(
::new(Context) C11NoReturnAttr(D.getDeclSpec().getNoreturnSpecLoc(),
@ -11632,6 +11637,29 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
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);
@ -11644,39 +11672,6 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
}
}
// If we had any tags defined in the function prototype,
// introduce them into the function scope.
if (FnBodyScope) {
for (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);
@ -12161,6 +12156,7 @@ NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
/*NumExceptions=*/0,
/*NoexceptExpr=*/nullptr,
/*ExceptionSpecTokens=*/nullptr,
/*DeclsInPrototype=*/None,
Loc, Loc, D),
DS.getAttributes(),
SourceLocation());
@ -13430,7 +13426,6 @@ CreateNewDecl:
} else if (!PrevDecl) {
Diag(Loc, diag::warn_decl_in_param_list) << Context.getTagDeclType(New);
}
DeclsInPrototypeScope.push_back(New);
}
if (Invalid)

View File

@ -772,7 +772,7 @@ bool Sema::containsUnexpandedParameterPacks(Declarator &D) {
}
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())

View File

@ -718,6 +718,7 @@ static void maybeSynthesizeBlockSignature(TypeProcessingState &state,
/*NumExceptions=*/0,
/*NoexceptExpr=*/nullptr,
/*ExceptionSpecTokens=*/nullptr,
/*DeclsInPrototype=*/None,
loc, loc, declarator));
// For consistency, make sure the state still has us as processing
@ -4469,7 +4470,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
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) {

View File

@ -102,16 +102,22 @@ struct testIndirectFieldDecl {
// 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

View File

@ -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

View File

@ -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 @@ enum e19018 qq; //expected-error{{tentative definition has type 'enum e19018' th
// 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);
}

View File

@ -1,4 +1,4 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
// RUN: %clang_cc1 -fexceptions -fcxx-exceptions -verify %s
struct S0;
struct S1;
@ -30,7 +30,7 @@ struct pr19018 {
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}}
@ -53,14 +53,19 @@ struct pr19018a {
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'}}