[PATCH] D119675: Also remove the empty StoredDeclsList entry from the lookup table
This revision was automatically updated to reflect the committed changes. Closed by commit rG8c0eb32d2aa0: Also remove the empty StoredDeclsList entry from the lookup table (authored by v.g.vassilev). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119675/new/ https://reviews.llvm.org/D119675 Files: clang/lib/AST/DeclBase.cpp clang/unittests/AST/ASTImporterTest.cpp Index: clang/unittests/AST/ASTImporterTest.cpp === --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -4279,6 +4279,10 @@ // This asserts in the old implementation. DC->removeDecl(A0); EXPECT_FALSE(DC->containsDecl(A0)); + + // Make sure we do not leave a StoredDeclsList with no entries. + DC->removeDecl(A1); + ASSERT_EQ(Map->find(A1->getDeclName()), Map->end()); } struct ImportFunctionTemplateSpecializations Index: clang/lib/AST/DeclBase.cpp === --- clang/lib/AST/DeclBase.cpp +++ clang/lib/AST/DeclBase.cpp @@ -1537,7 +1537,11 @@ if (Map) { StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName()); assert(Pos != Map->end() && "no lookup entry for decl"); -Pos->second.remove(ND); +StoredDeclsList = Pos->second; +List.remove(ND); +// Clean up the entry if there are no more decls. +if (List.isNull()) + Map->erase(Pos); } } while (DC->isTransparentContext() && (DC = DC->getParent())); } Index: clang/unittests/AST/ASTImporterTest.cpp === --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -4279,6 +4279,10 @@ // This asserts in the old implementation. DC->removeDecl(A0); EXPECT_FALSE(DC->containsDecl(A0)); + + // Make sure we do not leave a StoredDeclsList with no entries. + DC->removeDecl(A1); + ASSERT_EQ(Map->find(A1->getDeclName()), Map->end()); } struct ImportFunctionTemplateSpecializations Index: clang/lib/AST/DeclBase.cpp === --- clang/lib/AST/DeclBase.cpp +++ clang/lib/AST/DeclBase.cpp @@ -1537,7 +1537,11 @@ if (Map) { StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName()); assert(Pos != Map->end() && "no lookup entry for decl"); -Pos->second.remove(ND); +StoredDeclsList = Pos->second; +List.remove(ND); +// Clean up the entry if there are no more decls. +if (List.isNull()) + Map->erase(Pos); } } while (DC->isTransparentContext() && (DC = DC->getParent())); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119675: Also remove the empty StoredDeclsList entry from the lookup table
martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. LGTM! And please accept my apologies @v.g.vassilev for taking it months to review this small change. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119675/new/ https://reviews.llvm.org/D119675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119675: Also remove the empty StoredDeclsList entry from the lookup table
v.g.vassilev added a comment. Herald added a project: All. ping. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119675/new/ https://reviews.llvm.org/D119675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D119675: Also remove the empty StoredDeclsList entry from the lookup table
v.g.vassilev created this revision. v.g.vassilev added reviewers: rsmith, balazske. Herald added a reviewer: shafik. v.g.vassilev requested review of this revision. In case where we have removed all declarations for a given declaration name entry we should remove the whole StoredDeclsMap entry. This patch improves consistency in the lookup tables and helps cling/clang-repl error recovery. Repository: rC Clang https://reviews.llvm.org/D119675 Files: clang/lib/AST/DeclBase.cpp clang/unittests/AST/ASTImporterTest.cpp Index: clang/unittests/AST/ASTImporterTest.cpp === --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -4243,6 +4243,10 @@ // This asserts in the old implementation. DC->removeDecl(A0); EXPECT_FALSE(DC->containsDecl(A0)); + + // Make sure we do not leave a StoredDeclsList with no entries. + DC->removeDecl(A1); + ASSERT_EQ(Map->find(A1->getDeclName()), Map->end()); } struct ImportFunctionTemplateSpecializations Index: clang/lib/AST/DeclBase.cpp === --- clang/lib/AST/DeclBase.cpp +++ clang/lib/AST/DeclBase.cpp @@ -1524,7 +1524,11 @@ if (Map) { StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName()); assert(Pos != Map->end() && "no lookup entry for decl"); -Pos->second.remove(ND); +StoredDeclsList = Pos->second; +List.remove(ND); +// Clean up the entry if there are no more decls. +if (List.isNull()) + Map->erase(Pos); } } while (DC->isTransparentContext() && (DC = DC->getParent())); } Index: clang/unittests/AST/ASTImporterTest.cpp === --- clang/unittests/AST/ASTImporterTest.cpp +++ clang/unittests/AST/ASTImporterTest.cpp @@ -4243,6 +4243,10 @@ // This asserts in the old implementation. DC->removeDecl(A0); EXPECT_FALSE(DC->containsDecl(A0)); + + // Make sure we do not leave a StoredDeclsList with no entries. + DC->removeDecl(A1); + ASSERT_EQ(Map->find(A1->getDeclName()), Map->end()); } struct ImportFunctionTemplateSpecializations Index: clang/lib/AST/DeclBase.cpp === --- clang/lib/AST/DeclBase.cpp +++ clang/lib/AST/DeclBase.cpp @@ -1524,7 +1524,11 @@ if (Map) { StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName()); assert(Pos != Map->end() && "no lookup entry for decl"); -Pos->second.remove(ND); +StoredDeclsList = Pos->second; +List.remove(ND); +// Clean up the entry if there are no more decls. +if (List.isNull()) + Map->erase(Pos); } } while (DC->isTransparentContext() && (DC = DC->getParent())); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits