[PATCH] D140562: [clang][ASTImporter] Improve import of InjectedClassNameType.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: steakhal, martong, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

During AST import multiple different InjectedClassNameType objects
could be created for a single class template. This can cause problems
and failed assertions when these types are compared and found to be
not the same (because the instance is different and there is no
canonical type).
The import of this type does not use the factory method in ASTContext,
probably because the preconditions are not fulfilled at that state.
The fix tries to make the code in ASTImporter work more like the code
in ASTContext::getInjectedClassNameType. If a type is stored at the
Decl or previous Decl object, it is reused instead of creating a new
one. This avoids crash at least a part of the cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140562

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,46 @@
   EXPECT_FALSE(SharedStatePtr->isNewDecl(ToBar));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportInjectedClassNameType) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template <class T>
+      struct A {
+        typedef A A1;
+        void f(A1 *);
+      };
+      )",
+      Lang_CXX11);
+  Decl *FromTU = getTuDecl(
+      R"(
+      template <class T>
+      struct A {
+        typedef A A1;
+        void f(A1 *);
+      };
+      template<class T>
+      void A<T>::f(A::A1 *) {}
+      )",
+      Lang_CXX11);
+
+  auto *FromF = FirstDeclMatcher<FunctionDecl>().match(
+      FromTU, functionDecl(hasName("f"), isDefinition()));
+  auto *ToF = Import(FromF, Lang_CXX11);
+  EXPECT_TRUE(ToF);
+  ASTContext &ToCtx = ToF->getDeclContext()->getParentASTContext();
+
+  auto *ToA1 =
+      FirstDeclMatcher<TypedefDecl>().match(ToTU, typedefDecl(hasName("A1")));
+  QualType ToInjTypedef = ToA1->getUnderlyingType().getCanonicalType();
+  QualType ToInjParmVar =
+      ToF->parameters()[0]->getType().getDesugaredType(ToCtx);
+  ToInjParmVar =
+      ToInjParmVar->getAs<PointerType>()->getPointeeType().getCanonicalType();
+  EXPECT_TRUE(isa<InjectedClassNameType>(ToInjTypedef));
+  EXPECT_TRUE(isa<InjectedClassNameType>(ToInjParmVar));
+  EXPECT_TRUE(ToCtx.hasSameType(ToInjTypedef, ToInjParmVar));
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -1468,6 +1468,22 @@
   // FIXME: ASTContext::getInjectedClassNameType is not suitable for AST reading
   // See comments in InjectedClassNameType definition for details
   // return Importer.getToContext().getInjectedClassNameType(D, InjType);
+
+  // Take the actions in ASTContext::getInjectedClassNameType here as far as
+  // possible. Try to reuse a previous type if it exists.
+  // We want to avoid that multiple InjectedClassNameType objects are created
+  // for the same Decl object. It is not verified if this works always because
+  // differences in the import order and chain of PreviousDecl.
+
+  CXXRecordDecl *ToDecl = *ToDeclOrErr;
+  const Type *TypeForDecl = ToDecl->getTypeForDecl();
+  if (!TypeForDecl && ToDecl->getPreviousDecl())
+    TypeForDecl = ToDecl->getPreviousDecl()->getTypeForDecl();
+  if (TypeForDecl) {
+    assert(isa<InjectedClassNameType>(TypeForDecl));
+    return QualType(TypeForDecl, 0);
+  }
+
   enum {
     TypeAlignmentInBits = 4,
     TypeAlignment = 1 << TypeAlignmentInBits


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D140562.484866.patch
Type: text/x-patch
Size: 2887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221222/7a0402bd/attachment.bin>