[PATCH] D116155: [clang][AST][ASTImporter] Set record to complete during import of its members.
Balázs Kéri via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Dec 22 03:19:53 PST 2021
balazske created this revision.
Herald added subscribers: steakhal, martong, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
At import of a member it may require that the record is already set to complete.
(For example 'computeDependence' at create of some Expr nodes.)
The record at this time may not be completely imported, the result of layout
calculations can be incorrect, but at least no crash occurs this way.
A good solution would be if fields of every encountered record are imported
before other members of all records. This is much more difficult to implement.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116155
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
@@ -7475,6 +7475,57 @@
ToDGOther);
}
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ImportRecordWithLayoutRequestingExpr) {
+ TranslationUnitDecl *FromTU = getTuDecl(
+ R"(
+ struct A {
+ int idx;
+ static void foo(A x) {
+ (void)&"text"[x.idx];
+ }
+ };
+ )",
+ Lang_CXX11);
+
+ auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A")));
+
+ // Test that during import of 'foo' the record layout can be obtained without
+ // crash.
+ auto *ToA = Import(FromA, Lang_CXX11);
+ EXPECT_TRUE(ToA);
+ EXPECT_TRUE(ToA->isCompleteDefinition());
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase,
+ ImportRecordWithLayoutRequestingExprDifferentRecord) {
+ TranslationUnitDecl *FromTU = getTuDecl(
+ R"(
+ struct B;
+ struct A {
+ int idx;
+ B *b;
+ };
+ struct B {
+ static void foo(A x) {
+ (void)&"text"[x.idx];
+ }
+ };
+ )",
+ Lang_CXX11);
+
+ auto *FromA = FirstDeclMatcher<CXXRecordDecl>().match(
+ FromTU, cxxRecordDecl(hasName("A")));
+
+ // Test that during import of 'foo' the record layout (of 'A') can be obtained
+ // without crash. It is not possible to have all of the fields of 'A' imported
+ // at that time (without big code changes).
+ auto *ToA = Import(FromA, Lang_CXX11);
+ EXPECT_TRUE(ToA);
+ EXPECT_TRUE(ToA->isCompleteDefinition());
+}
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
DefaultTestValuesForRunOptions);
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -2012,6 +2012,13 @@
}
To->startDefinition();
+ // Set the definition to complete even if it is really not complete during
+ // import. Some AST constructs (expressions) require the record layout
+ // to be calculated (see 'clang::computeDependence') at the time they are
+ // constructed. Import of such AST node is possible during import of the
+ // same record, there is no way to have a completely defined record (all
+ // fields imported) at that time without multiple AST import passes.
+ To->setCompleteDefinition(true);
// Complete the definition even if error is returned.
// The RecordDecl may be already part of the AST so it is better to
// have it in complete state even if something is wrong with it.
@@ -2076,9 +2083,12 @@
ToCXX->setBases(Bases.data(), Bases.size());
}
- if (shouldForceImportDeclContext(Kind))
+ if (shouldForceImportDeclContext(Kind)) {
+ // Set to false here to force "completion" of the record.
+ To->setCompleteDefinition(false);
if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
return Err;
+ }
return Error::success();
}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116155.395827.patch
Type: text/x-patch
Size: 3120 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211222/9874a434/attachment.bin>
More information about the cfe-commits
mailing list