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] Fix crash when import VarTemplateDecl in record #67522

Merged
merged 1 commit into from
Oct 7, 2023

Conversation

jcsxky
Copy link
Contributor

@jcsxky jcsxky commented Sep 27, 2023

[clang][ASTImporter] Fix crash when import VarTemplateDecl in record
static VarTemplateDecl in record isn't a definition, when imported before, it will crash in ASTContext::setTemplateOrSpecializationInfo due to setting specialization while it already exists. This patch skip this specific case.

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

llvmbot commented Sep 27, 2023

@llvm/pr-subscribers-clang

Changes

[clang][ASTImporter] fix clash when import VarTemplateDecl in record
VarTemplateDecl in Record isn't a definition, when imported, it will crash in ASTContext::setTemplateOrSpecializationInfo due to setting specialization while it already exists. This patch skip this specific case.


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

2 Files Affected:

  • (modified) clang/lib/AST/ASTImporter.cpp (+4-2)
  • (modified) clang/unittests/AST/ASTImporterTest.cpp (+44-52)
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c7c2aecc8b179a4..b207c39cdf90154 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -6236,10 +6236,12 @@ ExpectedDecl ASTNodeImporter::VisitVarTemplateDecl(VarTemplateDecl *D) {
         // The Decl in the "From" context has a definition, but in the
         // "To" context we already have a definition.
         VarTemplateDecl *FoundDef = getTemplateDefinition(FoundTemplate);
-        if (D->isThisDeclarationADefinition() && FoundDef)
+
+        if ((D->isThisDeclarationADefinition() && FoundDef) ||
+            FoundTemplate->getDeclContext()->isRecord())
           // FIXME Check for ODR error if the two definitions have
           // different initializers?
-          return Importer.MapImported(D, FoundDef);
+          return Importer.MapImported(D, FoundTemplate);
 
         FoundByLookup = FoundTemplate;
         break;
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index c90b5aaeb624306..f2fafaf4329b740 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -4355,58 +4355,6 @@ TEST_P(ImportFriendClasses, DeclsFromFriendsShouldBeInRedeclChains) {
   EXPECT_TRUE(Imported->getPreviousDecl());
 }
 
-TEST_P(ImportFriendClasses, SkipComparingFriendTemplateDepth) {
-  Decl *ToTU = getToTuDecl(
-      R"(
-      template <class T, T U>
-      class A;
-
-      template <class T, T U>
-      class A {
-      public:
-        template <class P, P Q>
-        friend class A;
-
-        A(T x)  :x(x) {}
-
-      private:
-        T x;
-      };
-      )",
-      Lang_CXX11);
-
-  auto *Fwd = FirstDeclMatcher<ClassTemplateDecl>().match(
-      ToTU,
-      classTemplateDecl(has(cxxRecordDecl(hasDefinition(), hasName("A")))));
-  Decl *FromTU = getTuDecl(
-      R"(
-      template <class T, T U>
-      class A;
-
-      template <class T, T U>
-      class A {
-      public:
-        template <class P, P Q>
-        friend class A;
-
-        A(T x) : x(x) {}
-
-      private:
-        T x;
-      };
-
-      A<int,3> a1(0);
-      )",
-      Lang_CXX11, "input1.cc");
-  auto *FromA = FirstDeclMatcher<ClassTemplateDecl>().match(
-      FromTU,
-      classTemplateDecl(has(cxxRecordDecl(hasDefinition(), hasName("A")))));
-  auto *ToA = Import(FromA, Lang_CXX11);
-  EXPECT_TRUE(ToA);
-  EXPECT_EQ(Fwd->getTemplatedDecl()->getTypeForDecl(),
-            ToA->getTemplatedDecl()->getTypeForDecl());
-}
-
 TEST_P(ImportFriendClasses,
        ImportOfClassTemplateDefinitionShouldConnectToFwdFriend) {
   Decl *ToTU = getToTuDecl(
@@ -4988,6 +4936,50 @@ TEST_P(ASTImporterOptionSpecificTestBase,
   }
 }
 
+TEST_P(ImportFriendClasses, RecordVarTemplateDecl) {
+  Decl *ToTU = getToTuDecl(
+      R"(
+      template <class T>
+      class A {
+      public:
+        template <class U>
+        struct B {
+          static U Value;
+        };
+
+        template <class W>
+        static constexpr bool X = !B<W>::Value;
+      };
+      )",
+      Lang_CXX14);
+
+  auto *Fwd = FirstDeclMatcher<VarTemplateDecl>().match(
+      ToTU, varTemplateDecl(hasName("X")));
+  Decl *FromTU = getTuDecl(
+      R"(
+      template <class T>
+      class A {
+      public:
+        template <class U>
+        struct B {
+          static U Value;
+        };
+
+        template <class W>
+        static constexpr bool X = !B<W>::Value;
+      };
+      )",
+      Lang_CXX14, "input1.cc");
+  auto *FromA = FirstDeclMatcher<VarTemplateDecl>().match(
+      FromTU, varTemplateDecl(hasName("X")));
+  auto *ToA = Import(FromA, Lang_CXX11);
+  EXPECT_TRUE(ToA);
+  EXPECT_EQ(Fwd->getTemplatedDecl(),
+            ToA->getTemplatedDecl());
+  EXPECT_EQ(Fwd->getTemplatedDecl()->getDefinition(),
+            ToA->getTemplatedDecl()->getDefinition());
+}
+
 TEST_P(ASTImporterOptionSpecificTestBase, VarTemplateParameterDeclContext) {
   constexpr auto Code =
       R"(

@jcsxky jcsxky force-pushed the fix-crash-when-import-var-template-decl branch 5 times, most recently from 23c1002 to f9ff9c8 Compare September 27, 2023 07:47
@jcsxky jcsxky changed the title [clang][ASTImporter] fix clash when import VarTemplateDecl in record [clang][ASTImporter] Fix clash when import VarTemplateDecl in record Sep 27, 2023
@github-actions
Copy link

github-actions bot commented Sep 27, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@jcsxky jcsxky force-pushed the fix-crash-when-import-var-template-decl branch from f9ff9c8 to bbde18a Compare September 27, 2023 08:05
@jcsxky jcsxky requested a review from a team September 27, 2023 08:51
@jcsxky jcsxky changed the title [clang][ASTImporter] Fix clash when import VarTemplateDecl in record [clang][ASTImporter] Fix crash when import VarTemplateDecl in record Sep 27, 2023
@jcsxky jcsxky requested review from balazske, AaronBallman, shafik, cor3ntin and NagyDonat and removed request for a team September 27, 2023 12:18
@jcsxky jcsxky force-pushed the fix-crash-when-import-var-template-decl branch from bbde18a to 9dc31bf Compare October 3, 2023 08:40
@jcsxky jcsxky force-pushed the fix-crash-when-import-var-template-decl branch from 9dc31bf to 4a14372 Compare October 3, 2023 10:05
@balazske balazske self-requested a review October 5, 2023 07:19
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.

3 participants