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] Correctly look up primary template for variable template specializations #80359

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

sdkrystian
Copy link
Member

@sdkrystian sdkrystian commented Feb 1, 2024

Consider the following:

namespace N0 {
  namespace N1 {
    template<typename T>
    int x1 = 0;
  }
  using namespace N1;
}
template<>
int N0::x1<int>;

According to [dcl.meaning.general] p3.3:

  • If the declarator declares an explicit instantiation or a partial or explicit specialization, the declarator does not bind a name. If it declares a class member, the terminal name of the declarator-id is not looked up; otherwise, only those lookup results that are nominable in S are considered when identifying any function template specialization being declared.

In particular, the requirement for lookup results to be nominal in the lookup context of the terminal name of the declarator-id only applies to function template specializations -- not variable template specializations. We currently reject the above declaration, but we do (correctly) accept it if the using-directive is replaced with a using declaration naming N0::N1::x1. This patch makes it so the above specialization is (correctly) accepted.

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

llvmbot commented Feb 1, 2024

@llvm/pr-subscribers-clang

Author: Krystian Stasiowski (sdkrystian)

Changes

Consider the following:

namespace N0 {
  namespace N1 {
    template&lt;typename T&gt;
    int x1 = 0;
  }
  using namespace N1;
}
template&lt;&gt;
int N0::x1&lt;int&gt;;

According to [[dcl.meaning.general] p3.3](http://eel.is/c++draft/dcl.meaning.general#3.3):
> - If the declarator declares an explicit instantiation or a partial or explicit specialization, the declarator does not bind a name. If it declares a class member, the terminal name of the declarator-id is not looked up; otherwise, only those lookup results that are nominable in S are considered when identifying any function template specialization being declared.

In particular, the requirement for lookup results to be nominal in the lookup context of the terminal name of the declarator-id only applies to function template specializations -- not variable template specializations. We currently reject the above declaration. This patch makes it so the above specialization is (correctly) accepted.


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

4 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (+1-1)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+6-5)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+8-13)
  • (added) clang/test/CXX/dcl.decl/dcl.meaning/dcl.meaning.general/p3.cpp (+112)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 780a2f2d8ce27..7885cf6728b54 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8456,7 +8456,7 @@ class Sema final {
                                     SourceLocation RAngleLoc);
 
   DeclResult ActOnVarTemplateSpecialization(
-      Scope *S, Declarator &D, TypeSourceInfo *DI,
+      Scope *S, Declarator &D, TypeSourceInfo *DI, LookupResult &Previous,
       SourceLocation TemplateKWLoc, TemplateParameterList *TemplateParams,
       StorageClass SC, bool IsPartialSpecialization);
 
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fd1c47008d685..de6067ba13cbe 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7727,7 +7727,7 @@ NamedDecl *Sema::ActOnVariableDeclarator(
               ? TemplateParamLists[0]->getTemplateLoc()
               : SourceLocation();
       DeclResult Res = ActOnVarTemplateSpecialization(
-          S, D, TInfo, TemplateKWLoc, TemplateParams, SC,
+          S, D, TInfo, Previous, TemplateKWLoc, TemplateParams, SC,
           IsPartialSpecialization);
       if (Res.isInvalid())
         return nullptr;
@@ -8070,8 +8070,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
     D.setRedeclaration(CheckVariableDeclaration(NewVD, Previous));
   } else {
     // If this is an explicit specialization of a static data member, check it.
-    if (IsMemberSpecialization && !NewVD->isInvalidDecl() &&
-        CheckMemberSpecialization(NewVD, Previous))
+    if (IsMemberSpecialization && !IsVariableTemplateSpecialization &&
+        !NewVD->isInvalidDecl() && CheckMemberSpecialization(NewVD, Previous))
       NewVD->setInvalidDecl();
 
     // Merge the decl with the existing one if appropriate.
@@ -8086,7 +8086,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
         Previous.clear();
         NewVD->setInvalidDecl();
       }
-    } else if (D.getCXXScopeSpec().isSet()) {
+    } else if (D.getCXXScopeSpec().isSet() &&
+               !IsVariableTemplateSpecialization) {
       // No previous declaration in the qualifying scope.
       Diag(D.getIdentifierLoc(), diag::err_no_member)
         << Name << computeDeclContext(D.getCXXScopeSpec(), true)
@@ -8094,7 +8095,7 @@ NamedDecl *Sema::ActOnVariableDeclarator(
       NewVD->setInvalidDecl();
     }
 
-    if (!IsVariableTemplateSpecialization && !IsPlaceholderVariable)
+    if (!IsPlaceholderVariable)
       D.setRedeclaration(CheckVariableDeclaration(NewVD, Previous));
 
     // CheckVariableDeclaration will set NewVD as invalid if something is in
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 5616682e909aa..ed4201bc6b1f7 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4601,9 +4601,9 @@ void Sema::CheckDeductionGuideTemplate(FunctionTemplateDecl *TD) {
 }
 
 DeclResult Sema::ActOnVarTemplateSpecialization(
-    Scope *S, Declarator &D, TypeSourceInfo *DI, SourceLocation TemplateKWLoc,
-    TemplateParameterList *TemplateParams, StorageClass SC,
-    bool IsPartialSpecialization) {
+    Scope *S, Declarator &D, TypeSourceInfo *DI, LookupResult &Previous,
+    SourceLocation TemplateKWLoc, TemplateParameterList *TemplateParams,
+    StorageClass SC, bool IsPartialSpecialization) {
   // D must be variable template id.
   assert(D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId &&
          "Variable template specialization is declared with a template id.");
@@ -4783,17 +4783,12 @@ DeclResult Sema::ActOnVarTemplateSpecialization(
   // Note that this is an explicit specialization.
   Specialization->setSpecializationKind(TSK_ExplicitSpecialization);
 
-  if (PrevDecl) {
-    // Check that this isn't a redefinition of this specialization,
-    // merging with previous declarations.
-    LookupResult PrevSpec(*this, GetNameForDeclarator(D), LookupOrdinaryName,
-                          forRedeclarationInCurContext());
-    PrevSpec.addDecl(PrevDecl);
-    D.setRedeclaration(CheckVariableDeclaration(Specialization, PrevSpec));
-  } else if (Specialization->isStaticDataMember() &&
-             Specialization->isOutOfLine()) {
+  Previous.clear();
+  if (PrevDecl)
+    Previous.addDecl(PrevDecl);
+  else if (Specialization->isStaticDataMember() &&
+           Specialization->isOutOfLine())
     Specialization->setAccess(VarTemplate->getAccess());
-  }
 
   return Specialization;
 }
diff --git a/clang/test/CXX/dcl.decl/dcl.meaning/dcl.meaning.general/p3.cpp b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.meaning.general/p3.cpp
new file mode 100644
index 0000000000000..262f7cabf8213
--- /dev/null
+++ b/clang/test/CXX/dcl.decl/dcl.meaning/dcl.meaning.general/p3.cpp
@@ -0,0 +1,112 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+namespace N0 {
+  template<typename T>
+  void f0();
+
+  template<typename T>
+  int x0 = 0;
+
+  template<typename T>
+  class C0;
+}
+using namespace N0;
+
+template<>
+void f0<int>(); // expected-error {{no function template matches}}
+
+template<>
+int x0<int>;
+
+template<>
+class C0<int>;
+
+namespace N1 {
+  namespace N2 {
+    template<typename T>
+    void f2();
+
+    template<typename T>
+    int x2 = 0;
+
+    template<typename T>
+    class C2;
+  }
+  using namespace N2;
+}
+
+template<>
+void N1::f2<int>(); // expected-error {{no function template matches}}
+
+template<>
+int N1::x2<int>;
+
+template<>
+class N1::C2<int>;
+
+namespace N3 {
+  namespace N4 {
+    template<typename T>
+    void f4();
+
+    template<typename T>
+    int x4 = 0;
+
+    template<typename T>
+    class C4;
+  }
+  using N4::f4;
+  using N4::x4;
+  using N4::C4;
+}
+
+template<>
+void N3::f4<int>(); // expected-error {{no function template matches}}
+
+template<>
+int N3::x4<int>;
+
+template<>
+class N3::C4<int>;
+
+inline namespace N5 {
+  template<typename T>
+  void f5();
+
+  template<typename T>
+  int x5 = 0;
+
+  template<typename T>
+  class C5;
+}
+
+template<>
+void f5<int>();
+
+template<>
+int x5<int>;
+
+template<>
+class C5<int>;
+
+namespace N6 {
+  inline namespace N7 {
+    template<typename T>
+    void f7();
+
+    template<typename T>
+    int x7 = 0;
+
+    template<typename T>
+    class C7;
+  }
+}
+
+template<>
+void N6::f7<int>();
+
+template<>
+int N6::x7<int>;
+
+template<>
+class N6::C7<int>;

@sdkrystian
Copy link
Member Author

sdkrystian commented Feb 1, 2024

Still need to add a release note, but this should otherwise be complete

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.

Other than release note, this LGTM.

@sdkrystian sdkrystian force-pushed the variable-specialization-lookup branch from d7eae11 to f23e899 Compare February 2, 2024 16:52
@sdkrystian
Copy link
Member Author

@erichkeane Release note added

@sdkrystian sdkrystian merged commit 7ecfb66 into llvm:main Feb 2, 2024
4 of 5 checks passed
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
…e specializations (llvm#80359)

Consider the following:
```
namespace N0 {
  namespace N1 {
    template<typename T>
    int x1 = 0;
  }
  using namespace N1;
}
template<>
int N0::x1<int>;
```

According to [dcl.meaning.general] p3.3:
> - If the _declarator_ declares an explicit instantiation or a partial
or explicit specialization, the _declarator_ does not bind a name. If it
declares a class member, the terminal name of the _declarator-id_ is not
looked up; otherwise, **only those lookup results that are nominable in
`S` are considered when identifying any function template specialization
being declared**.

In particular, the requirement for lookup results to be nominal in the
lookup context of the terminal name of the _declarator-id_ only applies
to function template specializations -- not variable template
specializations. We currently reject the above declaration, but we do
(correctly) accept it if the using-directive is replaced with a `using`
declaration naming `N0::N1::x1`. This patch makes it so the above
specialization is (correctly) accepted.
@sdkrystian sdkrystian deleted the variable-specialization-lookup branch February 26, 2024 15:03
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.

None yet

3 participants