[clang] c5d83cd - [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 28 01:55:41 PDT 2022
Author: Balázs Kéri
Date: 2022-03-28T10:55:26+02:00
New Revision: c5d83cdca457fd024a3f76c5e2ba649462ecde67
URL: https://github.com/llvm/llvm-project/commit/c5d83cdca457fd024a3f76c5e2ba649462ecde67
DIFF: https://github.com/llvm/llvm-project/commit/c5d83cdca457fd024a3f76c5e2ba649462ecde67.diff
LOG: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.
The "in-class initializer" expression should be set in the field of a
default initialization expression before this expression node is created.
The `CXXDefaultInitExpr` objects are created after the AST is loaded and
at import not present in the "To" AST. And the in-class initializers of
the used fields can be missing too, these must be set at import.
This fixes a github issue #54061.
Reviewed By: martong
Differential Revision: https://reviews.llvm.org/D120824
Added:
clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp
clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt
clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp
Modified:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 9b421c7dec7c9..a5b6e3fa3a687 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3646,19 +3646,23 @@ ExpectedDecl ASTNodeImporter::VisitFieldDecl(FieldDecl *D) {
// initializer of a FieldDecl might not had been instantiated in the
// "To" context. However, the "From" context might instantiated that,
// thus we have to merge that.
+ // Note: `hasInClassInitializer()` is not the same as non-null
+ // `getInClassInitializer()` value.
if (Expr *FromInitializer = D->getInClassInitializer()) {
- // We don't have yet the initializer set.
- if (FoundField->hasInClassInitializer() &&
- !FoundField->getInClassInitializer()) {
- if (ExpectedExpr ToInitializerOrErr = import(FromInitializer))
+ if (ExpectedExpr ToInitializerOrErr = import(FromInitializer)) {
+ // Import of the FromInitializer may result in the setting of
+ // InClassInitializer. If not, set it here.
+ assert(FoundField->hasInClassInitializer() &&
+ "Field should have an in-class initializer if it has an "
+ "expression for it.");
+ if (!FoundField->getInClassInitializer())
FoundField->setInClassInitializer(*ToInitializerOrErr);
- else {
- // We can't return error here,
- // since we already mapped D as imported.
- // FIXME: warning message?
- consumeError(ToInitializerOrErr.takeError());
- return FoundField;
- }
+ } else {
+ // We can't return error here,
+ // since we already mapped D as imported.
+ // FIXME: warning message?
+ consumeError(ToInitializerOrErr.takeError());
+ return FoundField;
}
}
return FoundField;
@@ -8127,8 +8131,23 @@ ExpectedStmt ASTNodeImporter::VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E) {
if (!UsedContextOrErr)
return UsedContextOrErr.takeError();
- return CXXDefaultInitExpr::Create(
- Importer.getToContext(), *ToBeginLocOrErr, *ToFieldOrErr, *UsedContextOrErr);
+ FieldDecl *ToField = *ToFieldOrErr;
+ assert(ToField->hasInClassInitializer() &&
+ "Field should have in-class initializer if there is a default init "
+ "expression that uses it.");
+ if (!ToField->getInClassInitializer()) {
+ // The in-class initializer may be not yet set in "To" AST even if the
+ // field is already there. This must be set here to make construction of
+ // CXXDefaultInitExpr work.
+ auto ToInClassInitializerOrErr =
+ import(E->getField()->getInClassInitializer());
+ if (!ToInClassInitializerOrErr)
+ return ToInClassInitializerOrErr.takeError();
+ ToField->setInClassInitializer(*ToInClassInitializerOrErr);
+ }
+
+ return CXXDefaultInitExpr::Create(Importer.getToContext(), *ToBeginLocOrErr,
+ ToField, *UsedContextOrErr);
}
ExpectedStmt ASTNodeImporter::VisitCXXNamedCastExpr(CXXNamedCastExpr *E) {
diff --git a/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp
new file mode 100644
index 0000000000000..e0e356ff555cf
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp
@@ -0,0 +1,26 @@
+namespace QHashPrivate {
+template <typename> int b;
+struct Data;
+} // namespace QHashPrivate
+
+struct QDomNodePrivate {};
+template <typename = struct QString> struct QMultiHash {
+ QHashPrivate::Data *d = nullptr;
+};
+
+struct QDomNamedNodeMapPrivate {
+ QMultiHash<> map;
+};
+struct QDomElementPrivate : QDomNodePrivate {
+ QDomElementPrivate();
+ void importee();
+ QMultiHash<> *m_attr = nullptr;
+};
+// --------- common part end ---------
+
+QDomElementPrivate::QDomElementPrivate() : m_attr{new QMultiHash<>} {}
+void QDomElementPrivate::importee() { (void)QMultiHash<>{}; }
+struct foo {
+ QDomElementPrivate m = {};
+ static const int value = (QHashPrivate::b<foo>, 22);
+};
diff --git a/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt
new file mode 100644
index 0000000000000..7c8403c2fe684
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt
@@ -0,0 +1,4 @@
+14:c:@S at foo@value ctu-cxxdefaultinitexpr-import.cpp.ast
+35:c:@S at QDomElementPrivate@F at importee# ctu-cxxdefaultinitexpr-import.cpp.ast
+45:c:@S at QDomElementPrivate@F at QDomElementPrivate# ctu-cxxdefaultinitexpr-import.cpp.ast
+39:c:@S at QDomNodePrivate@F at QDomNodePrivate# ctu-cxxdefaultinitexpr-import.cpp.ast
diff --git a/clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp b/clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp
new file mode 100644
index 0000000000000..e785a76310bbb
--- /dev/null
+++ b/clang/test/Analysis/ctu-cxxdefaultinitexpr.cpp
@@ -0,0 +1,35 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -std=c++17 \
+// RUN: -emit-pch -o %t/ctudir/ctu-cxxdefaultinitexpr-import.cpp.ast %S/Inputs/ctu-cxxdefaultinitexpr-import.cpp
+// RUN: cp %S/Inputs/ctu-cxxdefaultinitexpr-import.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c++17 -analyze \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN: -analyzer-config ctu-dir=%t/ctudir \
+// RUN: -verify %s
+
+// Check that importing this code does not cause crash.
+// expected-no-diagnostics
+
+namespace QHashPrivate {
+template <typename> int b;
+struct Data;
+} // namespace QHashPrivate
+
+struct QDomNodePrivate {};
+template <typename = struct QString> struct QMultiHash {
+ QHashPrivate::Data *d = nullptr;
+};
+
+struct QDomNamedNodeMapPrivate {
+ QMultiHash<> map;
+};
+struct QDomElementPrivate : QDomNodePrivate {
+ QDomElementPrivate();
+ void importee();
+ QMultiHash<> *m_attr = nullptr;
+};
+// --------- common part end ---------
+
+void importer(QDomElementPrivate x) { x.importee(); }
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index b52e0b5d18e39..1e66701cd7128 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -530,6 +530,21 @@ TEST_P(ImportExpr, ImportInitListExpr) {
has(floatLiteral(equals(1.0)))))))));
}
+const internal::VariadicDynCastAllOfMatcher<Expr, CXXDefaultInitExpr>
+ cxxDefaultInitExpr;
+
+TEST_P(ImportExpr, ImportCXXDefaultInitExpr) {
+ MatchVerifier<Decl> Verifier;
+ testImport("class declToImport { int DefInit = 5; }; declToImport X;",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ cxxRecordDecl(hasDescendant(cxxConstructorDecl(
+ hasAnyConstructorInitializer(cxxCtorInitializer(
+ withInitializer(cxxDefaultInitExpr())))))));
+ testImport(
+ "struct X { int A = 5; }; X declToImport{};", Lang_CXX17, "", Lang_CXX17,
+ Verifier,
+ varDecl(hasInitializer(initListExpr(hasInit(0, cxxDefaultInitExpr())))));
+}
const internal::VariadicDynCastAllOfMatcher<Expr, VAArgExpr> vaArgExpr;
@@ -7529,6 +7544,92 @@ TEST_P(ASTImporterOptionSpecificTestBase,
EXPECT_TRUE(ToA->isCompleteDefinition());
}
+TEST_P(ASTImporterOptionSpecificTestBase, ImportInClassInitializerFromField) {
+ // Encounter import of a field when the field already exists but has the
+ // in-class initializer expression not yet set. Such case can occur in the AST
+ // of generated template specializations.
+ // The first code forces to create a template specialization of
+ // `A<int>` but without implicit constructors.
+ // The second ("From") code contains a variable of type `A<int>`, this
+ // results in a template specialization that has constructors and
+ // CXXDefaultInitExpr nodes.
+ Decl *ToTU = getToTuDecl(
+ R"(
+ void f();
+ template<typename> struct A { int X = 1; };
+ struct B { A<int> Y; };
+ )",
+ Lang_CXX11);
+ auto *ToX = FirstDeclMatcher<FieldDecl>().match(
+ ToTU,
+ fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl())));
+ ASSERT_TRUE(ToX->hasInClassInitializer());
+ ASSERT_FALSE(ToX->getInClassInitializer());
+
+ Decl *FromTU = getTuDecl(
+ R"(
+ void f();
+ template<typename> struct A { int X = 1; };
+ struct B { A<int> Y; };
+ //
+ A<int> Z;
+ )",
+ Lang_CXX11, "input1.cc");
+ auto *FromX = FirstDeclMatcher<FieldDecl>().match(
+ FromTU,
+ fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl())));
+
+ auto *ToXImported = Import(FromX, Lang_CXX11);
+ EXPECT_EQ(ToXImported, ToX);
+ EXPECT_TRUE(ToX->getInClassInitializer());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ImportInClassInitializerFromCXXDefaultInitExpr) {
+ // Encounter AST import of a CXXDefaultInitExpr where the "to-field"
+ // of it exists but has the in-class initializer not set yet.
+ Decl *ToTU = getToTuDecl(
+ R"(
+ namespace N {
+ template<typename> int b;
+ struct X;
+ }
+ template<typename> struct A { N::X *X = nullptr; };
+ struct B { A<int> Y; };
+ )",
+ Lang_CXX14);
+ auto *ToX = FirstDeclMatcher<FieldDecl>().match(
+ ToTU,
+ fieldDecl(hasName("X"), hasParent(classTemplateSpecializationDecl())));
+ ASSERT_TRUE(ToX->hasInClassInitializer());
+ ASSERT_FALSE(ToX->getInClassInitializer());
+
+ Decl *FromTU = getTuDecl(
+ R"(
+ namespace N {
+ template<typename> int b;
+ struct X;
+ }
+ template<typename> struct A { N::X *X = nullptr; };
+ struct B { A<int> Y; };
+ //
+ void f() {
+ (void)A<int>{};
+ }
+ struct C {
+ C(): attr(new A<int>{}){}
+ A<int> *attr;
+ const int value = N::b<C>;
+ };
+ )",
+ Lang_CXX14, "input1.cc");
+ auto *FromF = FirstDeclMatcher<FunctionDecl>().match(
+ FromTU, functionDecl(hasName("f"), isDefinition()));
+ auto *ToF = Import(FromF, Lang_CXX11);
+ EXPECT_TRUE(ToF);
+ EXPECT_TRUE(ToX->getInClassInitializer());
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
More information about the cfe-commits
mailing list