Index: lib/AST/DeclBase.cpp =================================================================== --- lib/AST/DeclBase.cpp +++ lib/AST/DeclBase.cpp @@ -1350,6 +1350,32 @@ (D->NextInContextAndBits.getPointer() || D == LastDecl)); } +/// shouldBeHidden - Determine whether a declaration which was declared +/// within its semantic context should be invisible to qualified name lookup. +static bool shouldBeHidden(NamedDecl *D) { + // Skip unnamed declarations. + if (!D->getDeclName()) + return true; + + // Skip entities that can't be found by name lookup into a particular + // context. + if ((D->getIdentifierNamespace() == 0 && !isa(D)) || + D->isTemplateParameter()) + return true; + + // Skip template specializations. + // FIXME: This feels like a hack. Should DeclarationName support + // template-ids, or is there a better way to keep specializations + // from being visible? + if (isa(D)) + return true; + if (auto *FD = dyn_cast(D)) + if (FD->isFunctionTemplateSpecialization()) + return true; + + return false; +} + void DeclContext::removeDecl(Decl *D) { assert(D->getLexicalDeclContext() == this && "decl being removed from non-lexical context"); @@ -1372,7 +1398,7 @@ } } } - + // Mark that D is no longer in the decl chain. D->NextInContextAndBits.setPointer(nullptr); @@ -1380,8 +1406,14 @@ if (isa(D)) { auto *ND = cast(D); + // Do not try to remove the declaration if that is invisible to qualified + // lookup. E.g. template specializations are skipped. + if (shouldBeHidden(ND)) + return; + // Remove only decls that have a name - if (!ND->getDeclName()) return; + if (!ND->getDeclName()) + return; auto *DC = D->getDeclContext(); do { @@ -1438,32 +1470,6 @@ makeDeclVisibleInContextWithFlags(ND, true, true); } -/// shouldBeHidden - Determine whether a declaration which was declared -/// within its semantic context should be invisible to qualified name lookup. -static bool shouldBeHidden(NamedDecl *D) { - // Skip unnamed declarations. - if (!D->getDeclName()) - return true; - - // Skip entities that can't be found by name lookup into a particular - // context. - if ((D->getIdentifierNamespace() == 0 && !isa(D)) || - D->isTemplateParameter()) - return true; - - // Skip template specializations. - // FIXME: This feels like a hack. Should DeclarationName support - // template-ids, or is there a better way to keep specializations - // from being visible? - if (isa(D)) - return true; - if (auto *FD = dyn_cast(D)) - if (FD->isFunctionTemplateSpecialization()) - return true; - - return false; -} - /// buildLookup - Build the lookup data structure with all of the /// declarations in this DeclContext (and any other contexts linked /// to it or transparent contexts nested within it) and return it. Index: unittests/AST/ASTImporterTest.cpp =================================================================== --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1797,5 +1797,38 @@ compoundStmt(has(callExpr(has(unresolvedMemberExpr()))))))))); } +struct DeclContextTest : ASTImporterTestBase {}; + +TEST_P(DeclContextTest, removeDeclOfClassTemplateSpecialization) { + Decl *TU = getTuDecl( + R"( + namespace NS { + + template + struct S {}; + template struct S; + + inline namespace INS { + template + struct S {}; + template struct S; + } + + } + )", Lang_CXX11, "input0.cc"); + auto *NS = FirstDeclMatcher().match( + TU, namespaceDecl()); + auto *Spec = FirstDeclMatcher().match( + TU, classTemplateSpecializationDecl()); + ASSERT_TRUE(NS->containsDecl(Spec)); + + NS->removeDecl(Spec); + EXPECT_FALSE(NS->containsDecl(Spec)); +} + +INSTANTIATE_TEST_CASE_P( + ParameterizedTests, DeclContextTest, + ::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),); + } // end namespace ast_matchers } // end namespace clang