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][Sema] Ignore previous partial specializations of member class templates explicitly specialized for a implicitly instantiated class template specialization #113464

Merged
merged 5 commits into from
Oct 30, 2024

Conversation

sdkrystian
Copy link
Member

Consider the following:

template<typename T>
struct A {
  template<typename U>
  struct B {
    static constexpr int x = 0; // #1
  };

  template<typename U>
  struct B<U*> {
    static constexpr int x = 1; // #2
  };
};

template<>
template<typename U>
struct A<long>::B {
  static constexpr int x = 2; // #3
};

static_assert(A<short>::B<int>::y == 0); // uses #1
static_assert(A<short>::B<int*>::y == 1); // uses #2

static_assert(A<long>::B<int>::y == 2); // uses #3
static_assert(A<long>::B<int*>::y == 2); // uses #3

According to [temp.spec.partial.member] p2:

If the primary member template is explicitly specialized for a given (implicit) specialization of the enclosing class template, the partial specializations of the member template are ignored for this specialization of the enclosing class template.
If a partial specialization of the member template is explicitly specialized for a given (implicit) specialization of the enclosing class template, the primary member template and its other partial specializations are still considered for this specialization of the enclosing class template.

The example above fails to compile because we currently don't implement [temp.spec.partial.member] p2. This patch implements the wording, fixing #51051.

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

llvmbot commented Oct 23, 2024

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Consider the following:

template&lt;typename T&gt;
struct A {
  template&lt;typename U&gt;
  struct B {
    static constexpr int x = 0; // #<!-- -->1
  };

  template&lt;typename U&gt;
  struct B&lt;U*&gt; {
    static constexpr int x = 1; // #<!-- -->2
  };
};

template&lt;&gt;
template&lt;typename U&gt;
struct A&lt;long&gt;::B {
  static constexpr int x = 2; // #<!-- -->3
};

static_assert(A&lt;short&gt;::B&lt;int&gt;::y == 0); // uses #<!-- -->1
static_assert(A&lt;short&gt;::B&lt;int*&gt;::y == 1); // uses #<!-- -->2

static_assert(A&lt;long&gt;::B&lt;int&gt;::y == 2); // uses #<!-- -->3
static_assert(A&lt;long&gt;::B&lt;int*&gt;::y == 2); // uses #<!-- -->3

According to [[temp.spec.partial.member] p2](http://eel.is/c++draft/temp.spec.partial.member#2):
> If the primary member template is explicitly specialized for a given (implicit) specialization of the enclosing class template, the partial specializations of the member template are ignored for this specialization of the enclosing class template.
If a partial specialization of the member template is explicitly specialized for a given (implicit) specialization of the enclosing class template, the primary member template and its other partial specializations are still considered for this specialization of the enclosing class template.

The example above fails to compile because we currently don't implement [[temp.spec.partial.member] p2](http://eel.is/c++draft/temp.spec.partial.member#2). This patch implements the wording, fixing #51051.


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

3 Files Affected:

  • (modified) clang/lib/Sema/SemaTemplate.cpp (+13)
  • (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+30-4)
  • (added) clang/test/CXX/temp/temp.decls/temp.spec.partial/temp.spec.partial.member/p2.cpp (+53)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 62f13610b5285c..705a29194f7d91 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4383,6 +4383,19 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, SourceLocation TemplateLoc,
 
   for (unsigned I = 0, N = PartialSpecs.size(); I != N; ++I) {
     VarTemplatePartialSpecializationDecl *Partial = PartialSpecs[I];
+    // C++ [temp.spec.partial.member]p2:
+    //   If the primary member template is explicitly specialized for a given
+    //   (implicit) specialization of the enclosing class template, the partial
+    //   specializations of the member template are ignored for this
+    //   specialization of the enclosing class template. If a partial
+    //   specialization of the member template is explicitly specialized for a
+    //   given (implicit) specialization of the enclosing class template, the
+    //   primary member template and its other partial specializations are still
+    //   considered for this specialization of the enclosing class template.
+    if (Template->isMemberSpecialization() &&
+        !Partial->isMemberSpecialization())
+      continue;
+
     TemplateDeductionInfo Info(FailedCandidates.getLocation());
 
     if (TemplateDeductionResult Result =
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 457a9968c32a4a..1dccb4dfca74c7 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3979,11 +3979,23 @@ bool Sema::usesPartialOrExplicitSpecialization(
     return true;
 
   SmallVector<ClassTemplatePartialSpecializationDecl *, 4> PartialSpecs;
-  ClassTemplateSpec->getSpecializedTemplate()
-                   ->getPartialSpecializations(PartialSpecs);
-  for (unsigned I = 0, N = PartialSpecs.size(); I != N; ++I) {
+  ClassTemplateDecl *CTD = ClassTemplateSpec->getSpecializedTemplate();
+  CTD->getPartialSpecializations(PartialSpecs);
+  for (ClassTemplatePartialSpecializationDecl *CTPSD : PartialSpecs) {
+    // C++ [temp.spec.partial.member]p2:
+    //   If the primary member template is explicitly specialized for a given
+    //   (implicit) specialization of the enclosing class template, the partial
+    //   specializations of the member template are ignored for this
+    //   specialization of the enclosing class template. If a partial
+    //   specialization of the member template is explicitly specialized for a
+    //   given (implicit) specialization of the enclosing class template, the
+    //   primary member template and its other partial specializations are still
+    //   considered for this specialization of the enclosing class template.
+    if (CTD->isMemberSpecialization() && !CTPSD->isMemberSpecialization())
+      continue;
+
     TemplateDeductionInfo Info(Loc);
-    if (DeduceTemplateArguments(PartialSpecs[I],
+    if (DeduceTemplateArguments(CTPSD,
                                 ClassTemplateSpec->getTemplateArgs().asArray(),
                                 Info) == TemplateDeductionResult::Success)
       return true;
@@ -4028,6 +4040,20 @@ getPatternForClassTemplateSpecialization(
     TemplateSpecCandidateSet FailedCandidates(PointOfInstantiation);
     for (unsigned I = 0, N = PartialSpecs.size(); I != N; ++I) {
       ClassTemplatePartialSpecializationDecl *Partial = PartialSpecs[I];
+      // C++ [temp.spec.partial.member]p2:
+      //   If the primary member template is explicitly specialized for a given
+      //   (implicit) specialization of the enclosing class template, the
+      //   partial specializations of the member template are ignored for this
+      //   specialization of the enclosing class template. If a partial
+      //   specialization of the member template is explicitly specialized for a
+      //   given (implicit) specialization of the enclosing class template, the
+      //   primary member template and its other partial specializations are
+      //   still considered for this specialization of the enclosing class
+      //   template.
+      if (Template->isMemberSpecialization() &&
+          !Partial->isMemberSpecialization())
+        continue;
+
       TemplateDeductionInfo Info(FailedCandidates.getLocation());
       if (TemplateDeductionResult Result = S.DeduceTemplateArguments(
               Partial, ClassTemplateSpec->getTemplateArgs().asArray(), Info);
diff --git a/clang/test/CXX/temp/temp.decls/temp.spec.partial/temp.spec.partial.member/p2.cpp b/clang/test/CXX/temp/temp.decls/temp.spec.partial/temp.spec.partial.member/p2.cpp
new file mode 100644
index 00000000000000..9a71da4bae37ca
--- /dev/null
+++ b/clang/test/CXX/temp/temp.decls/temp.spec.partial/temp.spec.partial.member/p2.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+template<typename T>
+struct A {
+  template<typename U>
+  struct B {
+    static constexpr int y = 0;
+  };
+
+  template<typename U>
+  struct B<U*> {
+    static constexpr int y = 1;
+  };
+
+  template<typename U>
+  static constexpr int x = 0;
+
+  template<typename U>
+  static constexpr int x<U*> = 1;
+};
+
+static_assert(A<short>::B<int>::y == 0);
+static_assert(A<short>::B<int*>::y == 1);
+static_assert(A<short>::x<int> == 0);
+static_assert(A<short>::x<int*> == 1);
+
+template<>
+template<typename U>
+struct A<long>::B {
+  static constexpr int y = 2;
+};
+
+template<>
+template<typename U>
+struct A<long>::B<U&> {
+  static constexpr int y = 3;
+};
+
+template<>
+template<typename U>
+constexpr int A<long>::x = 2;
+
+template<>
+template<typename U>
+constexpr int A<long>::x<U&> = 3;
+
+static_assert(A<long>::B<int>::y == 2);
+static_assert(A<long>::B<int*>::y == 2);
+static_assert(A<long>::B<int&>::y == 3);
+static_assert(A<long>::x<int> == 2);
+static_assert(A<long>::x<int*> == 2);
+static_assert(A<long>::x<int&> == 3);

@sdkrystian sdkrystian changed the title [Clang][Sema] Ignore previous partial specializations of member classtemplates explicitly specialized for a implicitly instantiated class template specialization [Clang][Sema] Ignore previous partial specializations of member class templates explicitly specialized for a implicitly instantiated class template specialization Oct 23, 2024
Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

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

Thanks for your patience, looks good to me!

@sdkrystian sdkrystian merged commit 7d1e283 into llvm:main Oct 30, 2024
6 of 9 checks passed
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…ates explicitly specialized for an implicitly instantiated class template specialization (llvm#113464)

Consider the following:
```
template<typename T>
struct A {
  template<typename U>
  struct B {
    static constexpr int x = 0; // llvm#1
  };

  template<typename U>
  struct B<U*> {
    static constexpr int x = 1; // llvm#2
  };
};

template<>
template<typename U>
struct A<long>::B {
  static constexpr int x = 2; // llvm#3
};

static_assert(A<short>::B<int>::y == 0); // uses llvm#1
static_assert(A<short>::B<int*>::y == 1); // uses llvm#2

static_assert(A<long>::B<int>::y == 2); // uses llvm#3
static_assert(A<long>::B<int*>::y == 2); // uses llvm#3
```

According to [temp.spec.partial.member] p2:
> If the primary member template is explicitly specialized for a given
(implicit) specialization of the enclosing class template, the partial
specializations of the member template are ignored for this
specialization of the enclosing class template.
If a partial specialization of the member template is explicitly
specialized for a given (implicit) specialization of the enclosing class
template, the primary member template and its other partial
specializations are still considered for this specialization of the
enclosing class template.

The example above fails to compile because we currently don't implement
[temp.spec.partial.member] p2. This patch implements the wording, fixing llvm#51051.
Copy link
Contributor

@mizvekov mizvekov left a comment

Choose a reason for hiding this comment

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

Sorry for the late review. The patch looks fine, thanks!

I have a minor concern about the 3x repetition here, and would be happy to see this refactored at some point.

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.

4 participants