Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][ASTImporter] import InstantiatedFromMember of ClassTemplateSpecializationDecl #76493

Conversation

jcsxky
Copy link
Contributor

@jcsxky jcsxky commented Dec 28, 2023

import of ClassTemplateSpecializationDecl didn't set InstantiatedFromMember and this makes ast-dump crash. import and set InstantiatedFromMember. fix issue

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Dec 28, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 28, 2023

@llvm/pr-subscribers-clang

Author: Qizhi Hu (jcsxky)

Changes

import of ClassTemplateSpecializationDecl didn't set InstantiatedFromMember and this makes ast-dump crash. import and set InstantiatedFromMember. fix issue


Full diff: https://github.com/llvm/llvm-project/pull/76493.diff

2 Files Affected:

  • (modified) clang/lib/AST/ASTImporter.cpp (+5)
  • (modified) clang/unittests/AST/ASTImporterTest.cpp (+32)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index b61180c4f3491d..9ffae72346f2af 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6141,6 +6141,11 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl(
                                                   InsertPos))
       // Add this partial specialization to the class template.
       ClassTemplate->AddPartialSpecialization(PartSpec2, InsertPos);
+    if (Expected<ClassTemplatePartialSpecializationDecl *> ToInstOrErr =
+            import(PartialSpec->getInstantiatedFromMember()))
+      PartSpec2->setInstantiatedFromMember(*ToInstOrErr);
+    else
+      return ToInstOrErr.takeError();
 
     updateLookupTableForTemplateParameters(*ToTPList);
   } else { // Not a partial specialization.
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index ed8ecb080e268d..1221174326b90f 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -9342,6 +9342,38 @@ TEST_P(ASTImporterOptionSpecificTestBase, ImportConflictTypeAliasTemplate) {
   EXPECT_FALSE(ImportedCallable);
 }
 
+AST_MATCHER(ClassTemplateSpecializationDecl, hasInstantiatedFromMember) {
+  if (auto Instantiate = Node.getInstantiatedFrom()) {
+    if (auto *FromPartialSpecialization =
+            Instantiate.get<ClassTemplatePartialSpecializationDecl *>()) {
+      return nullptr != FromPartialSpecialization->getInstantiatedFromMember();
+    }
+  }
+  return false;
+}
+
+TEST_P(ASTImporterOptionSpecificTestBase, ImportInstantiatedFromMember) {
+  const char *Code =
+      R"(
+      template <typename> struct B {
+        template <typename, bool = false> union D;
+        template <typename T> union D<T> {};
+        D<int> d;
+      };
+      B<int> b;
+      )";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX17);
+  auto *FromA = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match(
+      FromTU, classTemplateSpecializationDecl(hasName("D"),
+                                              hasInstantiatedFromMember()));
+  auto *FromPartialSpecialization =
+      cast<ClassTemplatePartialSpecializationDecl *>(
+          FromA->getInstantiatedFrom());
+  auto *ImportedPartialSpecialization =
+      Import(FromPartialSpecialization, Lang_CXX17);
+  EXPECT_TRUE(ImportedPartialSpecialization->getInstantiatedFromMember());
+}
+
 INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ASTImporterLookupTableTest,
                          DefaultTestValuesForRunOptions);
 

@jcsxky jcsxky force-pushed the import_InstantiatedFromMember_of_ClassTemplateSpecializationDecl branch from fa5ec63 to 057f932 Compare December 28, 2023 08:23
@jcsxky jcsxky requested a review from balazske December 28, 2023 08:54
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I will let @balazske approve

};
B<int> b;
)";
Decl *FromTU = getTuDecl(Code, Lang_CXX17);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick, I don't think this requires C++17 even though the original test case uses C++17.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed, use C++11 instead.

@jcsxky jcsxky force-pushed the import_InstantiatedFromMember_of_ClassTemplateSpecializationDecl branch 2 times, most recently from 9a9a203 to a891331 Compare December 29, 2023 01:25
B<int> b;
)";
Decl *FromTU = getTuDecl(Code, Lang_CXX11);
auto *FromA = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto *FromA = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match(
auto *FromD = FirstDeclMatcher<ClassTemplateSpecializationDecl>().match(

hasInstantiatedFromMember()));
auto *FromPartialSpecialization =
cast<ClassTemplatePartialSpecializationDecl *>(
FromA->getInstantiatedFrom());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FromA->getInstantiatedFrom());
FromD->getInstantiatedFrom());

auto *FromPartialSpecialization =
cast<ClassTemplatePartialSpecializationDecl *>(
FromA->getInstantiatedFrom());
auto *ImportedPartialSpecialization =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ASSERT_TRUE(FromPartialSpecialization->getInstantiatedFromMember());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All has been fixed.

@jcsxky jcsxky force-pushed the import_InstantiatedFromMember_of_ClassTemplateSpecializationDecl branch from a891331 to ef18bd2 Compare January 3, 2024 22:55
@jcsxky jcsxky merged commit 4de971c into llvm:main Jan 4, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Clang][ASTMergeAction] Assertion `inst_from != nullptr' failed.
4 participants