Skip to content

Commit 0b41606

Browse files
committed
Revert "Reland [clang][ASTImport] Add support for import of empty records" (llvm#100903)
This reverts commit 88e5206. The original change went in a while ago (last year) in https://reviews.llvm.org/D145057. The specific reason I'm proposing a revert is that this is now causing exactly the issue that @balazske predicted in https://reviews.llvm.org/D145057#4164717: > Problematic case is if the attribute has pointer to a Decl or Type that is imported here in a state when the field is already created but not initialized. Another problem is that attributes are added a second time in Import(Decl *) This now came up in the testing of LLDB support for llvm#93069. There, `__compressed_pair`s are now replaced with fields that have an `alignof(...)` and `[[no_unique_address]]` attribute. In the specific failing case, we're importing following `std::list` method: ``` size_type& __sz() _NOEXCEPT { return __size_; } ``` During this process, we create a new `__size_` `FieldDecl` (but don't initialize it yet). Then we go down the `ImportAttrs` codepath added in D145057. This imports the `alignof` expression which then references the uninitialized `__size_` and we trigger an assertion. Important to note, this codepath was added specifically to support `[[no_unique_address]]` in LLDB, and was supposed to land with https://reviews.llvm.org/D143347. But the LLDB side of that never landed, and the way we plan to support `[[no_unique_address]]` doesn't require things like the `markEmpty` method added here. So really, this is a dead codepath, which as pointed out in the original review isn't fully sound. (cherry picked from commit 31769e4)
1 parent e1a9c0d commit 0b41606

File tree

4 files changed

+9
-49
lines changed

4 files changed

+9
-49
lines changed

clang/include/clang/AST/ASTImporter.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,6 @@ class TypeSourceInfo;
258258
FoundDeclsTy findDeclsInToCtx(DeclContext *DC, DeclarationName Name);
259259

260260
void AddToLookupTable(Decl *ToD);
261-
llvm::Error ImportAttrs(Decl *ToD, Decl *FromD);
262261

263262
protected:
264263
/// Can be overwritten by subclasses to implement their own import logic.

clang/include/clang/AST/DeclCXX.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,10 +1188,6 @@ class CXXRecordDecl : public RecordDecl {
11881188
///
11891189
/// \note This does NOT include a check for union-ness.
11901190
bool isEmpty() const { return data().Empty; }
1191-
/// Marks this record as empty. This is used by DWARFASTParserClang
1192-
/// when parsing records with empty fields having [[no_unique_address]]
1193-
/// attribute
1194-
void markEmpty() { data().Empty = true; }
11951191

11961192
void setInitMethod(bool Val) { data().HasInitMethod = Val; }
11971193
bool hasInitMethod() const { return data().HasInitMethod; }

clang/lib/AST/ASTImporter.cpp

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4201,12 +4201,6 @@ ExpectedDecl ASTNodeImporter::VisitFieldDecl(FieldDecl *D) {
42014201
D->getInClassInitStyle()))
42024202
return ToField;
42034203

4204-
// We need [[no_unqiue_address]] attributes to be added to FieldDecl, before
4205-
// we add fields in CXXRecordDecl::addedMember, otherwise record will be
4206-
// marked as having non-zero size.
4207-
Err = Importer.ImportAttrs(ToField, D);
4208-
if (Err)
4209-
return std::move(Err);
42104204
ToField->setAccess(D->getAccess());
42114205
ToField->setLexicalDeclContext(LexicalDC);
42124206
ToField->setImplicit(D->isImplicit());
@@ -9423,19 +9417,6 @@ TranslationUnitDecl *ASTImporter::GetFromTU(Decl *ToD) {
94239417
return FromDPos->second->getTranslationUnitDecl();
94249418
}
94259419

9426-
Error ASTImporter::ImportAttrs(Decl *ToD, Decl *FromD) {
9427-
if (!FromD->hasAttrs() || ToD->hasAttrs())
9428-
return Error::success();
9429-
for (const Attr *FromAttr : FromD->getAttrs()) {
9430-
auto ToAttrOrErr = Import(FromAttr);
9431-
if (ToAttrOrErr)
9432-
ToD->addAttr(*ToAttrOrErr);
9433-
else
9434-
return ToAttrOrErr.takeError();
9435-
}
9436-
return Error::success();
9437-
}
9438-
94399420
Expected<Decl *> ASTImporter::Import(Decl *FromD) {
94409421
if (!FromD)
94419422
return nullptr;
@@ -9569,8 +9550,15 @@ Expected<Decl *> ASTImporter::Import(Decl *FromD) {
95699550
}
95709551
// Make sure that ImportImpl registered the imported decl.
95719552
assert(ImportedDecls.count(FromD) != 0 && "Missing call to MapImported?");
9572-
if (auto Error = ImportAttrs(ToD, FromD))
9573-
return std::move(Error);
9553+
9554+
if (FromD->hasAttrs())
9555+
for (const Attr *FromAttr : FromD->getAttrs()) {
9556+
auto ToAttrOrErr = Import(FromAttr);
9557+
if (ToAttrOrErr)
9558+
ToD->addAttr(*ToAttrOrErr);
9559+
else
9560+
return ToAttrOrErr.takeError();
9561+
}
95749562

95759563
// Notify subclasses.
95769564
Imported(FromD, ToD);

clang/unittests/AST/ASTImporterTest.cpp

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -9376,29 +9376,6 @@ TEST_P(ASTImporterOptionSpecificTestBase, VaListCpp) {
93769376
ToVaList->getUnderlyingType(), ToBuiltinVaList->getUnderlyingType()));
93779377
}
93789378

9379-
TEST_P(ASTImporterOptionSpecificTestBase,
9380-
ImportDefinitionOfEmptyClassWithNoUniqueAddressField) {
9381-
Decl *FromTU = getTuDecl(
9382-
R"(
9383-
struct B {};
9384-
struct A { B b; };
9385-
)",
9386-
Lang_CXX20);
9387-
9388-
CXXRecordDecl *FromD = FirstDeclMatcher<CXXRecordDecl>().match(
9389-
FromTU, cxxRecordDecl(hasName("A")));
9390-
9391-
for (auto *FD : FromD->fields())
9392-
FD->addAttr(clang::NoUniqueAddressAttr::Create(FromD->getASTContext(),
9393-
clang::SourceRange()));
9394-
FromD->markEmpty();
9395-
9396-
CXXRecordDecl *ToD = Import(FromD, Lang_CXX20);
9397-
EXPECT_TRUE(ToD->isEmpty());
9398-
for (auto *FD : ToD->fields())
9399-
EXPECT_EQ(true, FD->hasAttr<NoUniqueAddressAttr>());
9400-
}
9401-
94029379
TEST_P(ASTImporterOptionSpecificTestBase, ImportExistingTypedefToRecord) {
94039380
const char *Code =
94049381
R"(

0 commit comments

Comments
 (0)