john.brawn created this revision.
john.brawn added reviewers: dexonsmith, rsmith, rjmccall.
Herald added a project: All.
john.brawn requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
UnresolvedSet::erase works by popping the last element then replacing the
element to be erased with that element. When the element to be erased is itself
the last element this leads to writing past the end of the set, causing an
assertion failure.
Fix this by making erase of the last element just pop that element.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154502
Files:
clang/include/clang/AST/UnresolvedSet.h
clang/unittests/AST/CMakeLists.txt
clang/unittests/AST/UnresolvedSetTest.cpp
Index: clang/unittests/AST/UnresolvedSetTest.cpp
===================================================================
--- /dev/null
+++ clang/unittests/AST/UnresolvedSetTest.cpp
@@ -0,0 +1,115 @@
+#include "clang/AST/UnresolvedSet.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+class NamedDecl {
+ int dummy;
+
+public:
+ NamedDecl() {}
+};
+} // namespace clang
+
+using namespace clang;
+
+class UnresolvedSetTest : public ::testing::Test {
+protected:
+ NamedDecl n0, n1, n2, n3;
+ UnresolvedSet<2> set;
+
+ void SetUp() override {
+ set.addDecl(&n0);
+ set.addDecl(&n1);
+ set.addDecl(&n2);
+ set.addDecl(&n3);
+ }
+};
+
+TEST_F(UnresolvedSetTest, Size) { EXPECT_EQ(set.size(), 4u); }
+
+TEST_F(UnresolvedSetTest, ArrayOperator) {
+ EXPECT_EQ(set[0].getDecl(), &n0);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+ EXPECT_EQ(set[2].getDecl(), &n2);
+ EXPECT_EQ(set[3].getDecl(), &n3);
+}
+
+TEST_F(UnresolvedSetTest, EraseIntegerFromStart) {
+ set.erase(0);
+ EXPECT_EQ(set.size(), 3u);
+ EXPECT_EQ(set[0].getDecl(), &n3);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+ EXPECT_EQ(set[2].getDecl(), &n2);
+
+ set.erase(0);
+ EXPECT_EQ(set.size(), 2u);
+ EXPECT_EQ(set[0].getDecl(), &n2);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+
+ set.erase(0);
+ EXPECT_EQ(set.size(), 1u);
+ EXPECT_EQ(set[0].getDecl(), &n1);
+
+ set.erase(0);
+ EXPECT_EQ(set.size(), 0u);
+}
+
+TEST_F(UnresolvedSetTest, EraseIntegerFromEnd) {
+ set.erase(3);
+ EXPECT_EQ(set.size(), 3u);
+ EXPECT_EQ(set[0].getDecl(), &n0);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+ EXPECT_EQ(set[2].getDecl(), &n2);
+
+ set.erase(2);
+ EXPECT_EQ(set.size(), 2u);
+ EXPECT_EQ(set[0].getDecl(), &n0);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+
+ set.erase(1);
+ EXPECT_EQ(set.size(), 1u);
+ EXPECT_EQ(set[0].getDecl(), &n0);
+
+ set.erase(0);
+ EXPECT_EQ(set.size(), 0u);
+}
+
+TEST_F(UnresolvedSetTest, EraseIteratorFromStart) {
+ set.erase(set.begin());
+ EXPECT_EQ(set.size(), 3u);
+ EXPECT_EQ(set[0].getDecl(), &n3);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+ EXPECT_EQ(set[2].getDecl(), &n2);
+
+ set.erase(set.begin());
+ EXPECT_EQ(set.size(), 2u);
+ EXPECT_EQ(set[0].getDecl(), &n2);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+
+ set.erase(set.begin());
+ EXPECT_EQ(set.size(), 1u);
+ EXPECT_EQ(set[0].getDecl(), &n1);
+
+ set.erase(set.begin());
+ EXPECT_EQ(set.size(), 0u);
+}
+
+TEST_F(UnresolvedSetTest, EraseIteratorFromEnd) {
+ set.erase(--set.end());
+ EXPECT_EQ(set.size(), 3u);
+ EXPECT_EQ(set[0].getDecl(), &n0);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+ EXPECT_EQ(set[2].getDecl(), &n2);
+
+ set.erase(--set.end());
+ EXPECT_EQ(set.size(), 2u);
+ EXPECT_EQ(set[0].getDecl(), &n0);
+ EXPECT_EQ(set[1].getDecl(), &n1);
+
+ set.erase(--set.end());
+ EXPECT_EQ(set.size(), 1u);
+ EXPECT_EQ(set[0].getDecl(), &n0);
+
+ set.erase(--set.end());
+ EXPECT_EQ(set.size(), 0u);
+}
Index: clang/unittests/AST/CMakeLists.txt
===================================================================
--- clang/unittests/AST/CMakeLists.txt
+++ clang/unittests/AST/CMakeLists.txt
@@ -35,6 +35,7 @@
StructuralEquivalenceTest.cpp
TemplateNameTest.cpp
TypePrinterTest.cpp
+ UnresolvedSetTest.cpp
)
clang_target_link_libraries(ASTTests
Index: clang/include/clang/AST/UnresolvedSet.h
===================================================================
--- clang/include/clang/AST/UnresolvedSet.h
+++ clang/include/clang/AST/UnresolvedSet.h
@@ -114,9 +114,17 @@
I.I->set(New, AS);
}
- void erase(unsigned I) { decls()[I] = decls().pop_back_val(); }
+ void erase(unsigned I) {
+ auto val = decls().pop_back_val();
+ if (I < size())
+ decls()[I] = val;
+ }
- void erase(iterator I) { *I.I = decls().pop_back_val(); }
+ void erase(iterator I) {
+ auto val = decls().pop_back_val();
+ if (I != end())
+ *I.I = val;
+ }
void setAccess(iterator I, AccessSpecifier AS) { I.I->setAccess(AS); }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
