From 22ffb44c7f484fd31ee76997b158de993811e920 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 28 Jan 2016 19:25:00 +0000 Subject: [PATCH] Include RecordDecls from anonymous unions in the AST. For void f() { union { int i; }; } clang used to omit the RecordDecl from the anonymous union from the AST. That's because the code creating it only called PushOnScopeChains(), which adds it to the current DeclContext, which here is the function's DeclContext. But RecursiveASTVisitor doesn't descent into all decls in a FunctionDecl. Instead, for DeclContexts that contain statements, return the RecordDecl so that it can be included in the DeclStmt containing the VarDecl for the union. Interesting bits from the AST before this change: |-FunctionDecl | `-CompoundStmt | |-DeclStmt | | `-VarDecl 0x589cd60 col:3 implicit used 'union (anonymous at test.cc:3:3)' callinit After this change: -FunctionDecl | `-CompoundStmt | |-DeclStmt | | |-CXXRecordDecl 0x4612e48 col:3 union definition | | | |-FieldDecl 0x4612f70 col:15 referenced i 'int' | | `-VarDecl 0x4613010 col:3 implicit used 'union (anonymous at test.cc:3:3)' callinit This is now closer to how anonymous struct and unions are represented as members of structs. It also enabled deleting some one-off code in the template instantiation code. Finally, it fixes a crash with ASTMatchers, see the included test case (this fixes http://crbug.com/580749). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@259079 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Sema/Sema.h | 10 ++++---- lib/Parse/ParseDecl.cpp | 11 +++++++-- lib/Parse/ParseDeclCXX.cpp | 11 ++++++--- lib/Parse/ParseTemplate.cpp | 6 ++++- lib/Parse/Parser.cpp | 8 ++++++- lib/Sema/SemaDecl.cpp | 29 ++++++++++++++++------- lib/Sema/SemaTemplateInstantiateDecl.cpp | 14 ----------- unittests/ASTMatchers/ASTMatchersTest.cpp | 22 +++++++++++++++++ 8 files changed, 77 insertions(+), 34 deletions(-) diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index c55c41f1eb..0b270fe283 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1861,12 +1861,12 @@ public: void ActOnPopScope(SourceLocation Loc, Scope *S); void ActOnTranslationUnitScope(Scope *S); - Decl *ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, - DeclSpec &DS); - Decl *ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, - DeclSpec &DS, + Decl *ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS, + RecordDecl *&AnonRecord); + Decl *ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS, MultiTemplateParamsArg TemplateParams, - bool IsExplicitInstantiation = false); + bool IsExplicitInstantiation, + RecordDecl *&AnonRecord); Decl *BuildAnonymousStructOrUnion(Scope *S, DeclSpec &DS, AccessSpecifier AS, diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 7b7adac2b7..49b593155b 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -1530,9 +1530,14 @@ Parser::ParseSimpleDeclaration(unsigned Context, ProhibitAttributes(Attrs); DeclEnd = Tok.getLocation(); if (RequireSemi) ConsumeToken(); + RecordDecl *AnonRecord = nullptr; Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none, - DS); + DS, AnonRecord); DS.complete(TheDecl); + if (AnonRecord) { + Decl* decls[] = {AnonRecord, TheDecl}; + return Actions.BuildDeclaratorGroup(decls, /*TypeMayContainAuto=*/false); + } return Actions.ConvertDeclToDeclGroup(TheDecl); } @@ -3520,8 +3525,10 @@ void Parser::ParseStructDeclaration( // If there are no declarators, this is a free-standing declaration // specifier. Let the actions module cope with it. if (Tok.is(tok::semi)) { + RecordDecl *AnonRecord = nullptr; Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none, - DS); + DS, AnonRecord); + assert(!AnonRecord && "Did not expect anonymous struct or union here"); DS.complete(TheDecl); return; } diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp index bcef9595cf..814234d96d 100644 --- a/lib/Parse/ParseDeclCXX.cpp +++ b/lib/Parse/ParseDeclCXX.cpp @@ -2400,10 +2400,15 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier AS, if (DS.isFriendSpecified()) ProhibitAttributes(FnAttrs); - Decl *TheDecl = - Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS, DS, TemplateParams); + RecordDecl *AnonRecord = nullptr; + Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec( + getCurScope(), AS, DS, TemplateParams, false, AnonRecord); DS.complete(TheDecl); - return DeclGroupPtrTy::make(DeclGroupRef(TheDecl)); + if (AnonRecord) { + Decl* decls[] = {AnonRecord, TheDecl}; + return Actions.BuildDeclaratorGroup(decls, /*TypeMayContainAuto=*/false); + } + return Actions.ConvertDeclToDeclGroup(TheDecl); } ParsingDeclarator DeclaratorInfo(*this, DS, Declarator::MemberContext); diff --git a/lib/Parse/ParseTemplate.cpp b/lib/Parse/ParseTemplate.cpp index 098ac6836f..fffc40ee4e 100644 --- a/lib/Parse/ParseTemplate.cpp +++ b/lib/Parse/ParseTemplate.cpp @@ -209,11 +209,15 @@ Parser::ParseSingleDeclarationAfterTemplate( if (Tok.is(tok::semi)) { ProhibitAttributes(prefixAttrs); DeclEnd = ConsumeToken(); + RecordDecl *AnonRecord = nullptr; Decl *Decl = Actions.ParsedFreeStandingDeclSpec( getCurScope(), AS, DS, TemplateInfo.TemplateParams ? *TemplateInfo.TemplateParams : MultiTemplateParamsArg(), - TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation); + TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation, + AnonRecord); + assert(!AnonRecord && + "Anonymous unions/structs should not be valid with template"); DS.complete(Decl); return Decl; } diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 5eb83837bc..e33586c57c 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -883,8 +883,14 @@ Parser::ParseDeclOrFunctionDefInternal(ParsedAttributesWithRange &attrs, if (Tok.is(tok::semi)) { ProhibitAttributes(attrs); ConsumeToken(); - Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS, DS); + RecordDecl *AnonRecord = nullptr; + Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none, + DS, AnonRecord); DS.complete(TheDecl); + if (AnonRecord) { + Decl* decls[] = {AnonRecord, TheDecl}; + return Actions.BuildDeclaratorGroup(decls, /*TypeMayContainAuto=*/false); + } return Actions.ConvertDeclToDeclGroup(TheDecl); } diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index f64970473f..cb79948341 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -3601,9 +3601,11 @@ void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) { /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with /// no declarator (e.g. "struct foo;") is parsed. -Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, - DeclSpec &DS) { - return ParsedFreeStandingDeclSpec(S, AS, DS, MultiTemplateParamsArg()); +Decl * +Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS, + RecordDecl *&AnonRecord) { + return ParsedFreeStandingDeclSpec(S, AS, DS, MultiTemplateParamsArg(), false, + AnonRecord); } // The MS ABI changed between VS2013 and VS2015 with regard to numbers used to @@ -3709,10 +3711,11 @@ static unsigned GetDiagnosticTypeSpecifierID(DeclSpec::TST T) { /// ParsedFreeStandingDeclSpec - This method is invoked when a declspec with /// no declarator (e.g. "struct foo;") is parsed. It also accepts template /// parameters to cope with template friend declarations. -Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, - DeclSpec &DS, - MultiTemplateParamsArg TemplateParams, - bool IsExplicitInstantiation) { +Decl * +Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, DeclSpec &DS, + MultiTemplateParamsArg TemplateParams, + bool IsExplicitInstantiation, + RecordDecl *&AnonRecord) { Decl *TagD = nullptr; TagDecl *Tag = nullptr; if (DS.getTypeSpecType() == DeclSpec::TST_class || @@ -3807,9 +3810,19 @@ Decl *Sema::ParsedFreeStandingDeclSpec(Scope *S, AccessSpecifier AS, if (!Record->getDeclName() && Record->isCompleteDefinition() && DS.getStorageClassSpec() != DeclSpec::SCS_typedef) { if (getLangOpts().CPlusPlus || - Record->getDeclContext()->isRecord()) + Record->getDeclContext()->isRecord()) { + // If CurContext is a DeclContext that can contain statements, + // RecursiveASTVisitor won't visit the decls that + // BuildAnonymousStructOrUnion() will put into CurContext. + // Also store them here so that they can be part of the + // DeclStmt that gets created in this case. + // FIXME: Also return the IndirectFieldDecls created by + // BuildAnonymousStructOr union, for the same reason? + if (CurContext->isFunctionOrMethod()) + AnonRecord = Record; return BuildAnonymousStructOrUnion(S, DS, AS, Record, Context.getPrintingPolicy()); + } DeclaresAnything = false; } diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index 7a452af778..9403dd9fba 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -491,13 +491,6 @@ Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D) { Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D, bool InstantiatingVarTemplate) { - // If this is the variable for an anonymous struct or union, - // instantiate the anonymous struct/union type first. - if (const RecordType *RecordTy = D->getType()->getAs()) - if (RecordTy->getDecl()->isAnonymousStructOrUnion()) - if (!VisitCXXRecordDecl(cast(RecordTy->getDecl()))) - return nullptr; - // Do substitution on the type of the declaration TypeSourceInfo *DI = SemaRef.SubstType(D->getTypeSourceInfo(), TemplateArgs, @@ -2673,13 +2666,6 @@ Decl *TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl( const TemplateArgumentListInfo &TemplateArgsInfo, ArrayRef Converted) { - // If this is the variable for an anonymous struct or union, - // instantiate the anonymous struct/union type first. - if (const RecordType *RecordTy = D->getType()->getAs()) - if (RecordTy->getDecl()->isAnonymousStructOrUnion()) - if (!VisitCXXRecordDecl(cast(RecordTy->getDecl()))) - return nullptr; - // Do substitution on the type of the declaration TypeSourceInfo *DI = SemaRef.SubstType(D->getTypeSourceInfo(), TemplateArgs, diff --git a/unittests/ASTMatchers/ASTMatchersTest.cpp b/unittests/ASTMatchers/ASTMatchersTest.cpp index 67d3de9f81..7fd933718f 100644 --- a/unittests/ASTMatchers/ASTMatchersTest.cpp +++ b/unittests/ASTMatchers/ASTMatchersTest.cpp @@ -4240,6 +4240,28 @@ TEST(HasAncestor, ImplicitArrayCopyCtorDeclRefExpr) { declRefExpr(to(decl(hasAncestor(decl())))))); } +TEST(HasAncestor, AnonymousUnionMemberExpr) { + EXPECT_TRUE(matches("int F() {\n" + " union { int i; };\n" + " return i;\n" + "}\n", + memberExpr(member(hasAncestor(decl()))))); + EXPECT_TRUE(matches("void f() {\n" + " struct {\n" + " struct { int a; int b; };\n" + " } s;\n" + " s.a = 4;\n" + "}\n", + memberExpr(member(hasAncestor(decl()))))); + EXPECT_TRUE(matches("void f() {\n" + " struct {\n" + " struct { int a; int b; };\n" + " } s;\n" + " s.a = 4;\n" + "}\n", + declRefExpr(to(decl(hasAncestor(decl())))))); +} + TEST(HasParent, MatchesAllParents) { EXPECT_TRUE(matches( "template struct C { static void f() { 42; } };" -- 2.50.1