[PATCH] D136886: [ASTImporter] RFC: Correct importer to not duplicate sugared types

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers created this revision.
vabridgers added a reviewer: mizvekov.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
vabridgers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I'm not sure if this is a "correct" fix, but this change fixes the added
Unit test case in ASTImporterTest.cpp.

The change 69a6417406a1 
<https://reviews.llvm.org/rG69a6417406a1b0316a1fa6aeb63339d0e1d2abbd> - 
"[clang] Implement divergence for TypedefType
and UsingType", introduces a problem we've seen when the AST Importer is
used to import a sugared type with more than one level of syntactic
sugar.

The type va_list is imported like so

TypedefType 0x4a89410 'va_list' sugar

| -Typedef 0x4a891d0 'va_list' |
|

`-ElaboratedType 0x4a2d420 '__builtin_va_list' sugar

  `-TypedefType 0x4a2d3f0 '__builtin_va_list' sugar
    |-Typedef 0x4a2d398 '__builtin_va_list'
    `-ConstantArrayType 0x4a2d340 'struct __va_list_tag[1]' 1
      `-RecordType 0x4a2d180 'struct __va_list_tag'
        `-Record 0x4a2d0f8 '__va_list_tag'

and when "desugar" is used, comes back as follows.

VisitTypedefType T - desugared
ElaboratedType 0x4a2d420 '__builtin_va_list' sugar
`-TypedefType 0x4a2d3f0 '__builtin_va_list' sugar

  |-Typedef 0x4a2d398 '__builtin_va_list'
  `-ConstantArrayType 0x4a2d340 'struct __va_list_tag[1]' 1
    `-RecordType 0x4a2d180 'struct __va_list_tag'
      `-Record 0x4a2d0f8 '__va_list_tag'

So it appears the type must be desugared past the TypedefType to the
ElaboratedType, subsequently to the ConstantArrayType.

@mizvekov, could you please review this change and suggest different
approaches if this was not your intent?

This change seems to pass current Unit tests and LITs.

@mizvekov, if you wish to pick this up and finish, the UnitTest case
should be all you need for what we see.

Thanks


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136886

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
@@ -8075,6 +8075,36 @@
   EXPECT_FALSE(SharedStatePtr->isNewDecl(ToBar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, isSugaredImport) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      typedef __builtin_va_list va_list;
+      void dbgout(char* fmt, ...)
+      {
+        va_list va;
+      } 
+      )",
+      Lang_C99);
+  Decl *ToTU = getToTuDecl(
+      R"(
+      typedef __builtin_va_list va_list;
+      void dbgout(char* fmt, ...);
+      void maindbgout(char* str)
+      {
+        dbgout((char*)str);
+      }
+      )",
+      Lang_C99);
+  auto *FromOther = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("dbgout")));
+  ASSERT_TRUE(FromOther);
+
+  auto *ToOther = Import(FromOther, Lang_C99);
+  ASSERT_TRUE(ToOther);
+
+  // EXPECT_TRUE(SharedStatePtr->isSugaredImport(ToOther));
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1358,7 +1358,9 @@
   Expected<TypedefNameDecl *> ToDeclOrErr = import(T->getDecl());
   if (!ToDeclOrErr)
     return ToDeclOrErr.takeError();
-  ExpectedType ToUnderlyingTypeOrErr = import(T->desugar());
+  // ExpectedType ToUnderlyingTypeOrErr = import(T->desugar());
+  ExpectedType ToUnderlyingTypeOrErr =
+      import(QualType(T, 0).getDesugaredType(T->getDecl()->getASTContext()));
   if (!ToUnderlyingTypeOrErr)
     return ToUnderlyingTypeOrErr.takeError();
 



Index: clang/unittests/AST/ASTImporterTest.cpp
===================================================================
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -8075,6 +8075,36 @@
   EXPECT_FALSE(SharedStatePtr->isNewDecl(ToBar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, isSugaredImport) {
+  Decl *FromTU = getTuDecl(
+      R"(
+      typedef __builtin_va_list va_list;
+      void dbgout(char* fmt, ...)
+      {
+        va_list va;
+      } 
+      )",
+      Lang_C99);
+  Decl *ToTU = getToTuDecl(
+      R"(
+      typedef __builtin_va_list va_list;
+      void dbgout(char* fmt, ...);
+      void maindbgout(char* str)
+      {
+        dbgout((char*)str);
+      }
+      )",
+      Lang_C99);
+  auto *FromOther = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("dbgout")));
+  ASSERT_TRUE(FromOther);
+
+  auto *ToOther = Import(FromOther, Lang_C99);
+  ASSERT_TRUE(ToOther);
+
+  // EXPECT_TRUE(SharedStatePtr->isSugaredImport(ToOther));
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1358,7 +1358,9 @@
   Expected<TypedefNameDecl *> ToDeclOrErr = import(T->getDecl());
   if (!ToDeclOrErr)
     return ToDeclOrErr.takeError();
-  ExpectedType ToUnderlyingTypeOrErr = import(T->desugar());
+  // ExpectedType ToUnderlyingTypeOrErr = import(T->desugar());
+  ExpectedType ToUnderlyingTypeOrErr =
+      import(QualType(T, 0).getDesugaredType(T->getDecl()->getASTContext()));
   if (!ToUnderlyingTypeOrErr)
     return ToUnderlyingTypeOrErr.takeError();
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits