[PATCH] D145868: [clang][ASTImporter] Fix import of anonymous structures

Balázs Kéri via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 14 07:17:38 PDT 2023


balazske added a comment.

The problem is somewhat bigger and not fast to fix. This test shows what is problematic:

  TEST_P(ASTImporterOptionSpecificTestBase,
         ImportExistingTypedefToUnnamedRecordPtr) {
    const char *Code =
        R"(
        typedef const struct { int fff; } * const T;
        extern T x;
        )";
    Decl *ToTU = getToTuDecl(Code, Lang_C99);
    Decl *FromTU = getTuDecl(Code, Lang_C99);
  
    auto *FromX =
        FirstDeclMatcher<VarDecl>().match(FromTU, varDecl(hasName("x")));
    auto *ToX = Import(FromX, Lang_C99);
    EXPECT_TRUE(ToX);
  
    auto *Typedef1 =
        FirstDeclMatcher<TypedefDecl>().match(ToTU, typedefDecl(hasName("T")));
    auto *Typedef2 =
        LastDeclMatcher<TypedefDecl>().match(ToTU, typedefDecl(hasName("T")));
    EXPECT_EQ(Typedef1, Typedef2);
    
    auto *FromR =
        FirstDeclMatcher<RecordDecl>().match(FromTU, recordDecl(hasDescendant(fieldDecl(hasName("fff")))));
    auto *ToRExisting =
        FirstDeclMatcher<RecordDecl>().match(ToTU, recordDecl(hasDescendant(fieldDecl(hasName("fff")))));
    ASSERT_TRUE(FromR);
    auto *ToRImported = Import(FromR, Lang_C99);
    EXPECT_EQ(ToRExisting, ToRImported);
  }

The test fails because the last `EXPECT_EQ` fails (without the fix in this patch it crashes with the hasSameType assertion): The record is imported separately but the typedef was already imported before with a different record (in fact not imported, the import returned the existing typedef, this is caused by the existence of `PointerType` or any other type that has reference to unnamed struct but is not a `RecordType` at the typedef). The ToTU AST contains then two instances of the unnamed record (the last one is created when the second import in the test happens). The problem can be solved probably only if the import of existing equivalent AST nodes from another TU (in the root context, not in namespace with no name) is changed: The import should return the existing one and not create a new. I do not know how difficult is to change this.
But until a better fix is found the current solution is acceptable (with FIXME included).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145868/new/

https://reviews.llvm.org/D145868



More information about the cfe-commits mailing list