[PATCH] D86624: [clang] Exclude invalid destructors from lookups.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeed0af6179ca: [clang] Exclude invalid destructors from
lookups. (authored by adamcz).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86624/new/
https://reviews.llvm.org/D86624
Files:
clang/lib/AST/DeclBase.cpp
clang/test/PCH/cxx-invalid-destructor.cpp
clang/test/PCH/cxx-invalid-destructor.h
Index: clang/test/PCH/cxx-invalid-destructor.h
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.h
@@ -0,0 +1,7 @@
+struct Base {
+ ~Base();
+};
+
+struct Foo : public Base {
+ ~Base();
+};
Index: clang/test/PCH/cxx-invalid-destructor.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t
%S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t
%S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
+Foo f;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1487,6 +1487,13 @@
if (FD->isFunctionTemplateSpecialization())
return true;
+ // Hide destructors that are invalid. There should always be one destructor,
+ // but if it is an invalid decl, another one is created. We need to hide the
+ // invalid one from places that expect exactly one destructor, like the
+ // serialization code.
+ if (isa(D) && D->isInvalidDecl())
+return true;
+
return false;
}
Index: clang/test/PCH/cxx-invalid-destructor.h
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.h
@@ -0,0 +1,7 @@
+struct Base {
+ ~Base();
+};
+
+struct Foo : public Base {
+ ~Base();
+};
Index: clang/test/PCH/cxx-invalid-destructor.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t %S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t %S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
+Foo f;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1487,6 +1487,13 @@
if (FD->isFunctionTemplateSpecialization())
return true;
+ // Hide destructors that are invalid. There should always be one destructor,
+ // but if it is an invalid decl, another one is created. We need to hide the
+ // invalid one from places that expect exactly one destructor, like the
+ // serialization code.
+ if (isa(D) && D->isInvalidDecl())
+return true;
+
return false;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86624: [clang] Exclude invalid destructors from lookups.
adamcz updated this revision to Diff 287983.
adamcz marked 2 inline comments as done.
adamcz added a comment.
addressed review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86624/new/
https://reviews.llvm.org/D86624
Files:
clang/lib/AST/DeclBase.cpp
clang/test/PCH/cxx-invalid-destructor.cpp
clang/test/PCH/cxx-invalid-destructor.h
Index: clang/test/PCH/cxx-invalid-destructor.h
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.h
@@ -0,0 +1,7 @@
+struct Base {
+ ~Base();
+};
+
+struct Foo : public Base {
+ ~Base();
+};
Index: clang/test/PCH/cxx-invalid-destructor.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t
%S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t
%S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
+Foo f;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1487,6 +1487,13 @@
if (FD->isFunctionTemplateSpecialization())
return true;
+ // Hide destructors that are invalid. There should always be one destructor,
+ // but if it is an invalid decl, another one is created. We need to hide the
+ // invalid one from places that expect exactly one destructor, like the
+ // serialization code.
+ if (isa(D) && D->isInvalidDecl())
+return true;
+
return false;
}
Index: clang/test/PCH/cxx-invalid-destructor.h
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.h
@@ -0,0 +1,7 @@
+struct Base {
+ ~Base();
+};
+
+struct Foo : public Base {
+ ~Base();
+};
Index: clang/test/PCH/cxx-invalid-destructor.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t %S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t %S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
+Foo f;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1487,6 +1487,13 @@
if (FD->isFunctionTemplateSpecialization())
return true;
+ // Hide destructors that are invalid. There should always be one destructor,
+ // but if it is an invalid decl, another one is created. We need to hide the
+ // invalid one from places that expect exactly one destructor, like the
+ // serialization code.
+ if (isa(D) && D->isInvalidDecl())
+return true;
+
return false;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86624: [clang] Exclude invalid destructors from lookups.
adamcz added inline comments. Comment at: clang/test/PCH/cxx-invalid-destructor.cpp:2 +// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t %S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors +// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t %S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch + sammccall wrote: > nit: -fno-validate-pch shouldn't be needed IIUC We need this, otherwise the PCH file is rejected for containing errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86624/new/ https://reviews.llvm.org/D86624 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86624: [clang] Exclude invalid destructors from lookups.
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang/lib/AST/DeclBase.cpp:1489
return true;
+ if (isa(D) && D->isInvalidDecl())
+return true;
split into separate block, add a comment why
Comment at: clang/test/PCH/cxx-invalid-destructor.cpp:2
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t
%S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t
%S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
nit: -fno-validate-pch shouldn't be needed IIUC
Comment at: clang/test/PCH/cxx-invalid-destructor.h:6
+
+class Bar {
+public:
nit: we can eliminate Bar I think?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86624/new/
https://reviews.llvm.org/D86624
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86624: [clang] Exclude invalid destructors from lookups.
adamcz created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
adamcz requested review of this revision.
This fixes a crash when declaring a destructor with a wrong name, then
writing result to pch file and loading it again. The PCH storage uses
DeclarationNameKey as key and it is the same key for both the invalid
destructor and the implicit one that was created because the other one
was invalid. When querying for the Foo::~Foo we end up getting
Foo::~Bar, which is then rejected and we end up with nullptr in
CXXRecordDecl::GetDestructor().
Fixes https://bugs.llvm.org/show_bug.cgi?id=47270
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86624
Files:
clang/lib/AST/DeclBase.cpp
clang/test/PCH/cxx-invalid-destructor.cpp
clang/test/PCH/cxx-invalid-destructor.h
Index: clang/test/PCH/cxx-invalid-destructor.h
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.h
@@ -0,0 +1,14 @@
+class Base {
+public:
+ ~Base();
+};
+
+class Bar {
+public:
+ ~Bar();
+};
+
+class Foo : public Base {
+public:
+ ~Bar();
+};
Index: clang/test/PCH/cxx-invalid-destructor.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t
%S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t
%S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
+Foo f;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1486,6 +1486,8 @@
if (auto *FD = dyn_cast(D))
if (FD->isFunctionTemplateSpecialization())
return true;
+ if (isa(D) && D->isInvalidDecl())
+return true;
return false;
}
Index: clang/test/PCH/cxx-invalid-destructor.h
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.h
@@ -0,0 +1,14 @@
+class Base {
+public:
+ ~Base();
+};
+
+class Bar {
+public:
+ ~Bar();
+};
+
+class Foo : public Base {
+public:
+ ~Bar();
+};
Index: clang/test/PCH/cxx-invalid-destructor.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx-invalid-destructor.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t %S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t %S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
+Foo f;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1486,6 +1486,8 @@
if (auto *FD = dyn_cast(D))
if (FD->isFunctionTemplateSpecialization())
return true;
+ if (isa(D) && D->isInvalidDecl())
+return true;
return false;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
