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