danix800 updated this revision to Diff 543834.
danix800 added a comment.
Restore in-class initializer importing when minimal importing.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156201/new/
https://reviews.llvm.org/D156201
Files:
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp
Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "clang/AST/RecordLayout.h"
#include "clang/ASTMatchers/ASTMatchers.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/SmallVectorMemoryBuffer.h"
@@ -1539,6 +1540,83 @@
Verifier.match(To, cxxRecordDecl(hasFieldOrder({"a", "b", "c"}))));
}
+TEST_P(ASTImporterOptionSpecificTestBase, FieldCircularRef) {
+ Decl *From, *To;
+ std::tie(From, To) = getImportedDecl(
+ R"s(
+ struct declToImport {
+ int a{a};
+ int b = c;
+ int c = b + c + a;
+ };
+ )s",
+ Lang_CXX11, "", Lang_CXX11);
+
+ auto FieldBWithInit = has(fieldDecl(
+ hasName("b"),
+ hasInClassInitializer(ignoringImpCasts(memberExpr(
+ hasObjectExpression(cxxThisExpr()), member(hasName("c")))))));
+ auto FieldCWithInit = has(fieldDecl(
+ hasName("c"),
+ hasInClassInitializer(binaryOperator(hasLHS(binaryOperator())))));
+ auto Pattern = cxxRecordDecl(FieldBWithInit, FieldCWithInit,
+ hasFieldOrder({"a", "b", "c"}));
+ MatchVerifier<Decl> Verifier;
+ ASSERT_TRUE(Verifier.match(From, Pattern));
+ EXPECT_TRUE(Verifier.match(To, Pattern));
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ImportFieldDirectlyWithInitializerContainingCircularRef) {
+ auto Code = R"s(
+ struct declToImport {
+ int a{a};
+ int b = c;
+ int c = b + c + a;
+ };
+ )s";
+
+ auto *FromField = FirstDeclMatcher<FieldDecl>().match(
+ getTuDecl(Code, Lang_CXX11), fieldDecl(hasName("b")));
+ auto *ToField = Import(FromField, Lang_CXX11);
+
+ auto Pattern = fieldDecl(
+ hasName("b"),
+ hasInClassInitializer(ignoringImpCasts(memberExpr(
+ hasObjectExpression(cxxThisExpr()), member(hasName("c"))))));
+
+ MatchVerifier<Decl> Verifier;
+ ASSERT_TRUE(Verifier.match(FromField, Pattern));
+ EXPECT_TRUE(Verifier.match(ToField, Pattern));
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, FieldCircularIndirectRef) {
+ Decl *From, *To;
+ std::tie(From, To) = getImportedDecl(
+ R"s(
+ static int ref_A();
+ static int ref_B();
+ struct A {
+ int a = ref_B();
+ };
+ struct B {
+ int b = ref_A();
+ };
+ int ref_B() { B b; return b.b; }
+ int ref_A() { A a; return a.a; }
+ )s",
+ Lang_CXX11, "", Lang_CXX11, "A");
+
+ auto InitByRefB =
+ hasInClassInitializer(callExpr(callee(functionDecl(hasName("ref_B")))));
+ auto FieldAWithInit = has(fieldDecl(hasName("a"), InitByRefB));
+ auto Pattern =
+ cxxRecordDecl(hasName("A"), FieldAWithInit, hasFieldOrder({"a"}));
+ MatchVerifier<Decl> Verifier;
+ ASSERT_TRUE(Verifier.match(From, Pattern));
+ EXPECT_TRUE(Verifier.match(To, Pattern));
+}
+
TEST_P(ASTImporterOptionSpecificTestBase,
CXXRecordDeclFieldAndIndirectFieldOrder) {
Decl *From, *To;
@@ -8028,6 +8106,45 @@
Import(FromF, Lang_CXX11);
}
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ImportCirularRefFieldsWithoutCorruptedRecordLayoutCacheTest) {
+ // Import sequence: A => A.b => B => B.f() => ... => UnaryOperator(&) => ...
+ //
+ // UnaryOperator(&) force computing RecordLayout of 'A' while it's still not
+ // completely imported.
+ auto Code =
+ R"(
+ class B;
+ class A {
+ B* b;
+ int c;
+ };
+ class B {
+ A *f() { return &((B *)0)->a; }
+ A a;
+ };
+ )";
+
+ auto *FromR = FirstDeclMatcher<CXXRecordDecl>().match(
+ getTuDecl(Code, Lang_CXX11), cxxRecordDecl(hasName("A")));
+ FromR = FromR->getDefinition();
+ auto &FromAST = FromR->getASTContext();
+ auto *ToR = Import(FromR, Lang_CXX11);
+ auto &ToAST = ToR->getASTContext();
+
+ uint64_t SecondFieldOffset = FromAST.getTypeSize(FromAST.VoidPtrTy);
+
+ EXPECT_TRUE(FromR->isCompleteDefinition());
+ const auto &FromLayout = FromAST.getASTRecordLayout(FromR);
+ EXPECT_TRUE(FromLayout.getFieldOffset(0) == 0);
+ EXPECT_TRUE(FromLayout.getFieldOffset(1) == SecondFieldOffset);
+
+ EXPECT_TRUE(ToR->isCompleteDefinition());
+ const auto &ToLayout = ToAST.getASTRecordLayout(ToR);
+ EXPECT_TRUE(ToLayout.getFieldOffset(0) == 0);
+ EXPECT_TRUE(ToLayout.getFieldOffset(1) == SecondFieldOffset);
+}
+
TEST_P(ASTImporterOptionSpecificTestBase,
ImportRecordWithLayoutRequestingExpr) {
TranslationUnitDecl *FromTU = getTuDecl(
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1881,30 +1881,6 @@
}
}
- // We reorder declarations in RecordDecls because they may have another order
- // in the "to" context than they have in the "from" context. This may happen
- // e.g when we import a class like this:
- // struct declToImport {
- // int a = c + b;
- // int b = 1;
- // int c = 2;
- // };
- // During the import of `a` we import first the dependencies in sequence,
- // thus the order would be `c`, `b`, `a`. We will get the normal order by
- // first removing the already imported members and then adding them in the
- // order as they appear in the "from" context.
- //
- // Keeping field order is vital because it determines structure layout.
- //
- // Here and below, we cannot call field_begin() method and its callers on
- // ToDC if it has an external storage. Calling field_begin() will
- // automatically load all the fields by calling
- // LoadFieldsFromExternalStorage(). LoadFieldsFromExternalStorage() would
- // call ASTImporter::Import(). This is because the ExternalASTSource
- // interface in LLDB is implemented by the means of the ASTImporter. However,
- // calling an import at this point would result in an uncontrolled import, we
- // must avoid that.
-
auto ToDCOrErr = Importer.ImportContext(FromDC);
if (!ToDCOrErr) {
consumeError(std::move(ChildErrors));
@@ -1912,23 +1888,26 @@
}
DeclContext *ToDC = *ToDCOrErr;
- // Remove all declarations, which may be in wrong order in the
- // lexical DeclContext and then add them in the proper order.
- for (auto *D : FromDC->decls()) {
- if (!MightNeedReordering(D))
- continue;
+ // Import in-class initializers for fields.
+ for (auto *D : FromDC->decls()) {
assert(D && "DC contains a null decl");
- if (Decl *ToD = Importer.GetAlreadyImportedOrNull(D)) {
- // Remove only the decls which we successfully imported.
- assert(ToDC == ToD->getLexicalDeclContext() && ToDC->containsDecl(ToD));
- // Remove the decl from its wrong place in the linked list.
- ToDC->removeDecl(ToD);
- // Add the decl to the end of the linked list.
- // This time it will be at the proper place because the enclosing for
- // loop iterates in the original (good) order of the decls.
- ToDC->addDeclInternal(ToD);
- }
+ auto *FromField = dyn_cast<FieldDecl>(D);
+ if (!FromField || !FromField->hasInClassInitializer())
+ continue;
+ Decl *ToD = Importer.GetAlreadyImportedOrNull(D);
+ if (!ToD || ToDC != ToD->getLexicalDeclContext() ||
+ !ToDC->containsDecl(ToD))
+ continue;
+ auto *ToField = cast<FieldDecl>(ToD);
+ Error Err = Error::success();
+ auto ToInitializer = importChecked(Err, FromField->getInClassInitializer());
+ // Might already been imported.
+ if (ToField->getInClassInitializer())
+ continue;
+ if (ToInitializer)
+ ToField->setInClassInitializer(ToInitializer);
+ HandleChildErrors.handleChildImportResult(ChildErrors, std::move(Err));
}
// Import everything else.
@@ -3925,7 +3904,10 @@
auto ToTInfo = importChecked(Err, D->getTypeSourceInfo());
auto ToBitWidth = importChecked(Err, D->getBitWidth());
auto ToInnerLocStart = importChecked(Err, D->getInnerLocStart());
- auto ToInitializer = importChecked(Err, D->getInClassInitializer());
+ // Defer Initializer to avoid circular reference on full import.
+ Expr *ToInitializer = nullptr;
+ if (Importer.isMinimalImport())
+ ToInitializer = importChecked(Err, D->getInClassInitializer());
if (Err)
return std::move(Err);
const Type *ToCapturedVLAType = nullptr;
@@ -7394,10 +7376,15 @@
if (Err)
return std::move(Err);
- return UnaryOperator::Create(
- Importer.getToContext(), ToSubExpr, E->getOpcode(), ToType,
- E->getValueKind(), E->getObjectKind(), ToOperatorLoc, E->canOverflow(),
- E->getFPOptionsOverride());
+ auto *UO = UnaryOperator::CreateEmpty(Importer.getToContext(),
+ E->hasStoredFPFeatures());
+ UO->setType(ToType);
+ UO->setSubExpr(ToSubExpr);
+ UO->setOpcode(E->getOpcode());
+ UO->setOperatorLoc(ToOperatorLoc);
+ UO->setCanOverflow(E->canOverflow());
+
+ return UO;
}
ExpectedStmt
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
