[Clang][Sema] Diagnose use of template keyword after declarative nested-name-specifiers #78595
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sdkrystian
changed the title
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) ChangesAccording to [[temp.names] p5](http://eel.is/c++draft/temp.names#5): [[expr.prim.id.qual] p2](http://eel.is/c++draft/expr.prim.id.qual#2) defines a declarative nested-name-specifier as follows: Note: I believe this definition is defective as it doesn't include nested-name-specifiers appearing in elaborated-type-specifiers that declare partial/explicit specializations and explicit instantiations. See my post to the core reflector here. Minus a few bugs that are addressed by this PR, this is how we implement it. This means that declarations like: template<typename>
struct A
{
template<typename>
struct B
{
void f();
};
};
template<typename T>
template<typename U>
void A<T>::template B<U>::f() { } // error: 'template' cannot be used in a declarative nested name specifierare ill-formed. This PR add diagnostics for such declarations. Regarding the aforementioned "few bugs that are addressed by this PR" in order to correctly implement this:
struct A
{
template<typename T>
struct B
{
template<typename U>
struct C;
};
};
template<typename T>
template<typename U>
struct decltype(A())::B<T>::C { };
Patch is 20.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78595.diff 14 Files Affected:
diff --git a/clang/include/clang/AST/TypeLoc.h b/clang/include/clang/AST/TypeLoc.h
index 471deb14aba51f..4b95e2d0a7da41 100644
--- a/clang/include/clang/AST/TypeLoc.h
+++ b/clang/include/clang/AST/TypeLoc.h
@@ -1684,7 +1684,7 @@ class TemplateSpecializationTypeLoc :
}
void initializeLocal(ASTContext &Context, SourceLocation Loc) {
- setTemplateKeywordLoc(Loc);
+ setTemplateKeywordLoc(SourceLocation());
setTemplateNameLoc(Loc);
setLAngleLoc(Loc);
setRAngleLoc(Loc);
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 03b0122d1c08f7..9f6263c123979a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8223,6 +8223,8 @@ def err_invalid_declarator_in_block : Error<
"definition or redeclaration of %0 not allowed inside a block">;
def err_not_tag_in_scope : Error<
"no %select{struct|interface|union|class|enum}0 named %1 in %2">;
+def err_template_in_declarative_nns : Error<
+ "'template' cannot be used in a declarative nested name specifier">;
def err_no_typeid_with_fno_rtti : Error<
"use of typeid requires -frtti">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2c0ad022bcf19d..86d9ff8cbad4f1 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2911,7 +2911,8 @@ class Sema final {
bool DiagnoseClassNameShadow(DeclContext *DC, DeclarationNameInfo Info);
bool diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
DeclarationName Name, SourceLocation Loc,
- bool IsTemplateId);
+ TemplateIdAnnotation *TemplateId,
+ bool IsMemberSpecialization);
void
diagnoseIgnoredQualifiers(unsigned DiagID, unsigned Quals,
SourceLocation FallbackLoc,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 9e6e4ab882c0be..f6b5b65bc81a8e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -6264,13 +6264,16 @@ bool Sema::DiagnoseClassNameShadow(DeclContext *DC,
///
/// \param Loc The location of the name of the entity being declared.
///
-/// \param IsTemplateId Whether the name is a (simple-)template-id, and thus
-/// we're declaring an explicit / partial specialization / instantiation.
+/// \param IsMemberSpecialization Whether we are declaring a member specialization.
+///
+/// \param TemplateId The template-id, if any.
///
/// \returns true if we cannot safely recover from this error, false otherwise.
bool Sema::diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
DeclarationName Name,
- SourceLocation Loc, bool IsTemplateId) {
+ SourceLocation Loc,
+ TemplateIdAnnotation *TemplateId,
+ bool IsMemberSpecialization) {
DeclContext *Cur = CurContext;
while (isa<LinkageSpecDecl>(Cur) || isa<CapturedDecl>(Cur))
Cur = Cur->getParent();
@@ -6299,7 +6302,7 @@ bool Sema::diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
// Check whether the qualifying scope encloses the scope of the original
// declaration. For a template-id, we perform the checks in
// CheckTemplateSpecializationScope.
- if (!Cur->Encloses(DC) && !IsTemplateId) {
+ if (!Cur->Encloses(DC) && !(TemplateId || IsMemberSpecialization)) {
if (Cur->isRecord())
Diag(Loc, diag::err_member_qualification)
<< Name << SS.getRange();
@@ -6345,12 +6348,42 @@ bool Sema::diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC,
return false;
}
+ // C++23 [temp.names]p5:
+ // The keyword template shall not appear immediately after a declarative
+ // nested-name-specifier.
+ if (TemplateId && TemplateId->TemplateKWLoc.isValid()) {
+ Diag(Loc, diag::err_template_in_declarative_nns)
+ << FixItHint::CreateRemoval(TemplateId->TemplateKWLoc);
+ }
+
+ NestedNameSpecifierLoc SpecLoc(SS.getScopeRep(), SS.location_data());
+ while (SpecLoc.getPrefix()) {
+ // C++23 [temp.names]p5:
+ // The keyword template shall not appear immediately after a declarative
+ // nested-name-specifier.
+ // FIXME: nested-name-specifiers in friend declarations are declarative,
+ // but we don't call diagnoseQualifiedDeclaration for them. We should.
+ if (SpecLoc.getNestedNameSpecifier()->getKind() ==
+ NestedNameSpecifier::TypeSpecWithTemplate) {
+ SourceLocation TemplateKWLoc;
+ auto TL = SpecLoc.getTypeLoc();
+ if (const auto DTSTL =
+ TL.getAsAdjusted<DependentTemplateSpecializationTypeLoc>())
+ TemplateKWLoc = DTSTL.getTemplateKeywordLoc();
+ else if (const auto TSTL =
+ TL.getAsAdjusted<TemplateSpecializationTypeLoc>())
+ TemplateKWLoc = TSTL.getTemplateKeywordLoc();
+ else
+ TemplateKWLoc = TL.getBeginLoc();
+
+ Diag(Loc, diag::err_template_in_declarative_nns)
+ << FixItHint::CreateRemoval(TemplateKWLoc);
+ }
+ SpecLoc = SpecLoc.getPrefix();
+ }
// C++11 [dcl.meaning]p1:
// [...] "The nested-name-specifier of the qualified declarator-id shall
// not begin with a decltype-specifer"
- NestedNameSpecifierLoc SpecLoc(SS.getScopeRep(), SS.location_data());
- while (SpecLoc.getPrefix())
- SpecLoc = SpecLoc.getPrefix();
if (isa_and_nonnull<DecltypeType>(
SpecLoc.getNestedNameSpecifier()->getAsType()))
Diag(Loc, diag::err_decltype_in_declarator)
@@ -6418,9 +6451,12 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
return nullptr;
}
if (!D.getDeclSpec().isFriendSpecified()) {
+ TemplateIdAnnotation *TemplateId =
+ D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId
+ ? D.getName().TemplateId : nullptr;
if (diagnoseQualifiedDeclaration(
D.getCXXScopeSpec(), DC, Name, D.getIdentifierLoc(),
- D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId)) {
+ TemplateId, /*IsMemberSpecialization=*/false)) {
if (DC->isRecord())
return nullptr;
@@ -17952,6 +17988,7 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc,
// nested-name-specifier against the current context.
if ((TUK == TUK_Definition || TUK == TUK_Declaration) &&
diagnoseQualifiedDeclaration(SS, DC, OrigName, Loc,
+ /*TemplateId=*/nullptr,
isMemberSpecialization))
Invalid = true;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index a2ce96188b4f16..f8b2e3f8059ebb 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -3621,14 +3621,16 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,
// class X {
// int X::member;
// };
- if (DeclContext *DC = computeDeclContext(SS, false))
+ if (DeclContext *DC = computeDeclContext(SS, false)) {
+ TemplateIdAnnotation *TemplateId =
+ D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId
+ ? D.getName().TemplateId : nullptr;
diagnoseQualifiedDeclaration(SS, DC, Name, D.getIdentifierLoc(),
- D.getName().getKind() ==
- UnqualifiedIdKind::IK_TemplateId);
- else
+ TemplateId, /*IsMemberSpecialization=*/false);
+ } else {
Diag(D.getIdentifierLoc(), diag::err_member_qualification)
<< Name << SS.getRange();
-
+ }
SS.clear();
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index c0dcbda1fd6221..95fe29eeaba4c1 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1890,8 +1890,11 @@ DeclResult Sema::CheckClassTemplate(
ContextRAII SavedContext(*this, SemanticContext);
if (RebuildTemplateParamsInCurrentInstantiation(TemplateParams))
Invalid = true;
- } else if (TUK != TUK_Friend && TUK != TUK_Reference)
- diagnoseQualifiedDeclaration(SS, SemanticContext, Name, NameLoc, false);
+ }
+
+ if (TUK != TUK_Friend && TUK != TUK_Reference)
+ diagnoseQualifiedDeclaration(SS, SemanticContext, Name, NameLoc,
+ /*TemplateId-*/nullptr, /*IsMemberSpecialization*/false);
LookupQualifiedName(Previous, SemanticContext);
} else {
@@ -8771,6 +8774,16 @@ DeclResult Sema::ActOnClassTemplateSpecialization(
bool isMemberSpecialization = false;
bool isPartialSpecialization = false;
+ if (SS.isSet()) {
+ if (TUK != TUK_Reference && TUK != TUK_Friend &&
+ diagnoseQualifiedDeclaration(SS, ClassTemplate->getDeclContext(),
+ ClassTemplate->getDeclName(),
+ TemplateNameLoc,
+ &TemplateId,
+ /*IsMemberSpecialization=*/false))
+ return true;
+ }
+
// Check the validity of the template headers that introduce this
// template.
// FIXME: We probably shouldn't complain about these headers for
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1a1bc87d2b3203..4117dcdc143e41 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4376,8 +4376,14 @@ NestedNameSpecifierLoc TreeTransform<Derived>::TransformNestedNameSpecifierLoc(
SS.Adopt(ETL.getQualifierLoc());
TL = ETL.getNamedTypeLoc();
}
- SS.Extend(SemaRef.Context, /*FIXME:*/ SourceLocation(), TL,
- Q.getLocalEndLoc());
+ SourceLocation TemplateKWLoc;
+ if (const auto TSTL = TL.getAs<TemplateSpecializationTypeLoc>())
+ TemplateKWLoc = TSTL.getTemplateKeywordLoc();
+ else if (const auto DTSTL =
+ TL.getAs<DependentTemplateSpecializationTypeLoc>())
+ TemplateKWLoc = DTSTL.getTemplateKeywordLoc();
+
+ SS.Extend(SemaRef.Context, TemplateKWLoc, TL, Q.getLocalEndLoc());
break;
}
// If the nested-name-specifier is an invalid type def, don't emit an
diff --git a/clang/test/CXX/drs/dr23xx.cpp b/clang/test/CXX/drs/dr23xx.cpp
index 03077ae9239a45..a14fb58e38af0b 100644
--- a/clang/test/CXX/drs/dr23xx.cpp
+++ b/clang/test/CXX/drs/dr23xx.cpp
@@ -182,8 +182,8 @@ struct Bad2 { int a, b; };
} // namespace dr2386
namespace std {
template <typename T> struct tuple_size;
-template <> struct std::tuple_size<dr2386::Bad1> {};
-template <> struct std::tuple_size<dr2386::Bad2> {
+template <> struct tuple_size<dr2386::Bad1> {};
+template <> struct tuple_size<dr2386::Bad2> {
static const int value = 42;
};
} // namespace std
diff --git a/clang/test/CXX/drs/dr7xx.cpp b/clang/test/CXX/drs/dr7xx.cpp
index 926bff1cc479c5..2cbdc218ab7b5b 100644
--- a/clang/test/CXX/drs/dr7xx.cpp
+++ b/clang/test/CXX/drs/dr7xx.cpp
@@ -105,7 +105,8 @@ namespace dr727 { // dr727: partial
// expected-note@#dr727-N {{explicitly specialized declaration is here}}
template<> struct A::C<double>;
- // expected-error@-1 {{class template specialization of 'C' not in class 'A' or an enclosing namespace}}
+ // expected-error@-1 {{non-friend class member 'C' cannot have a qualified name}}
+ // expected-error@-2 {{class template specialization of 'C' not in class 'A' or an enclosing namespace}}
// expected-note@#dr727-C {{explicitly specialized declaration is here}}
template<> void A::f<double>();
// expected-error@-1 {{o function template matches function template specialization 'f'}}
@@ -116,7 +117,8 @@ namespace dr727 { // dr727: partial
// expected-note@#dr727-N {{explicitly specialized declaration is here}}
template<typename T> struct A::C<T***>;
- // expected-error@-1 {{class template partial specialization of 'C' not in class 'A' or an enclosing namespace}}
+ // expected-error@-1 {{non-friend class member 'C' cannot have a qualified name}}
+ // expected-error@-2 {{class template partial specialization of 'C' not in class 'A' or an enclosing namespace}}
// expected-note@#dr727-C {{explicitly specialized declaration is here}}
template<typename T> static int A::N<T***>;
// expected-error@-1 {{non-friend class member 'N' cannot have a qualified name}}
diff --git a/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1.cpp b/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1.cpp
index 17645639fb82f3..3e687a5ba6776c 100644
--- a/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1.cpp
+++ b/clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1.cpp
@@ -3,21 +3,21 @@ template<typename T, typename U> // expected-note{{previous template}}
class X0 {
public:
typedef int size_type;
-
+
X0(int);
~X0();
-
+
void f0(const T&, const U&);
-
+
T& operator[](int i) const;
-
+
void f1(size_type) const;
void f2(size_type) const;
void f3(size_type) const;
void f4() ;
-
+
operator T*() const;
-
+
T value;
};
@@ -41,7 +41,7 @@ template<class X, class Y, class Z> // expected-error{{too many template paramet
void X0<X, Y>::f3(size_type) const {
}
-template<class X, class Y>
+template<class X, class Y>
void X0<Y, X>::f4() { } // expected-error{{does not refer}}
// FIXME: error message should probably say, "redefinition of 'X0<T, U>::f0'"
@@ -68,14 +68,14 @@ namespace N { template <class X> void A<X>::a() {} }
// PR5566
template<typename T>
-struct X1 {
+struct X1 {
template<typename U>
struct B { void f(); };
};
template<typename T>
template<typename U>
-void X1<T>::template B<U>::f() { }
+void X1<T>::template B<U>::f() { } // expected-error{{'template' cannot be used in a declarative nested name specifier}}
// PR5527
template <template <class> class T>
diff --git a/clang/test/CXX/temp/temp.names/p5.cpp b/clang/test/CXX/temp/temp.names/p5.cpp
new file mode 100644
index 00000000000000..9001fe0ff7afad
--- /dev/null
+++ b/clang/test/CXX/temp/temp.names/p5.cpp
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template<typename T> struct A {
+ template<typename U> struct B {
+ struct C;
+ template<typename V> struct D;
+ template<typename V> struct D<V*>;
+
+ void f();
+ template<typename V> void g();
+
+ static int x;
+ template<typename V> static int y;
+ template<typename V> static int y<V*>;
+
+ enum class E;
+ };
+};
+
+template<typename T>
+template<typename U>
+struct A<T>::template B<U>::C { }; // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+struct A<int>::template B<bool>::C; // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+struct A<int>::template B<bool>::C { }; // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+template<typename V>
+struct A<T>::template B<U>::D { }; // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+template<typename V>
+struct A<T>::template B<U>::D<V*> { }; // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+template<typename V>
+struct A<int>::template B<bool>::D { }; // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+template<typename V>
+struct A<int>::template B<bool>::D<V*> { }; // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+void A<T>::template B<U>::f() { } // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+void A<int>::template B<bool>::f() { } // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+template<typename V>
+void A<T>::template B<U>::g() { } // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+template<typename V>
+void A<int>::template B<bool>::g() { } // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+int A<T>::template B<U>::x = 0; // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+template<typename V>
+int A<T>::template B<U>::y = 0; // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+template<typename V>
+int A<T>::template B<U>::y<V*> = 0; // expected-error{{'template' cannot be used in a declarative}}
+
+template<typename T>
+template<typename U>
+enum class A<T>::template B<U>::E { a }; // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+enum class A<int>::template B<bool>::E; // expected-error{{'template' cannot be used in a declarative}}
+
+template<>
+template<>
+enum class A<int>::template B<bool>::E { a }; // expected-error{{'template' cannot be used in a declarative}}
diff --git a/clang/test/CXX/temp/temp.spec/part.spec.cpp b/clang/test/CXX/temp/temp.spec/part.spec.cpp
index f62050af69cff5..4b0fdb902633a1 100644
--- a/clang/test/CXX/temp/temp.spec/part.spec.cpp
+++ b/clang/test/CXX/temp/temp.spec/part.spec.cpp
@@ -478,4 +478,4 @@ template <typename T> class PCTT6<TestClass::PrivateClass, T> {
};
template <typename T1> template <typename, typename> class PCTT6<TestClass::PrivateClass, T1>::NCT4 final {};
// expected-error@+1 2{{is a private member of}}
-template <typename T1> template <typename T2> struct PCTT6<TestClass::PrivateClass, T1>::template NCT3<T2, TestClass::TemplatePrivateClass<TestClass::TemplateProtectedClass<TestClass::PublicClass>>> : PCTT6<TestClass::PrivateClass, T1>::NCT4<T2, TestClass::TemplatePrivateClass<int>> {};
+template <typename T1> template <typename T2> struct PCTT6<TestClass::PrivateClass, T1>::NCT3<T2, TestClass::TemplatePrivateClass<TestClass::TemplateProtectedClass<TestClass::PublicClass>>> : PCTT6<TestClass::PrivateClass, T1>::NCT4<T2, TestClass::TemplatePrivateClass<int>> {};
diff --git a/clang/test/SemaCXX/static-assert.cpp b/clang/test/SemaCXX/static-assert.cpp
index 6e1701602ae30c..0d384b6b499f78 100644
--- a/clang/test/SemaCXX/static-assert.cpp
+++ b/clang/test/SemaCXX/static-assert.cpp
@@ -197,7 +197,7 @@ struct NestedTemplates1 {
template <typename T, typename U, int a>
void foo2() {
static_assert(::ns::NestedTemplates1<T, a>::NestedTemplates2::template NestedTemplates3<U>::value, "message");
- // expected-error@-1{{static assertion failed due to requirement '::ns::NestedTemplates1<int, 3>::NestedTemplates2::NestedTemplates3<float>::value': message}}
+ // expected-error@-1{{static assertion failed due to requirement '::ns::NestedTemplates1<int, 3>::NestedTemplates2::template NestedTemplates3<float>::value': message}}
}
template void foo2<int, float, 3>();
// expected-note@-1{{in instantiation of function template specialization 'foo2<int, float, 3>' requested here}}
diff --git a/clang/test/SemaTemplate/class-template-spec.cpp b/clang/test/SemaTemplate/class-template-spec.cpp
index e96ef44b7a251b..56b8207bd9a435 100644
--- a/clang/test/SemaTemplate/class-template-spec.cpp
+++ b/clang/test/SemaTemplate/class-template-spec.cpp
@@ -74,14 +74,14 @@ namespace N {
// Diagnose specialization errors
struct A<double> { }; // expected-error{{template specialization requires 'template<>'}}
-template<> struct ::A<double>;
+template<> struct ::A<double>; // expected-warning {{extra qualification on member}}
namespace N {
template<typename T> struct B; // expected-note {{explicitly specialized}}
- template<> struct ::N::B<char>; // okay
- template<> struct ::N::B<short>; // okay
- template<> struct ::N::B<int>; // okay
+ template<> struct ::N::B<char>; // expected-warning {{extra qualification on member}}
+ template<> struct ::N::B<short>; // expected-warning {{extra qualification on member}}
+ template<> struct ::N::B<int>; // expected-warning {{extra qualification on member}}
...
[truncated]
|
|
(still need to add a release note) |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
sdkrystian
force-pushed
the
nns-with-template
branch
from
8a5ad66 to
c0babac
Compare
sdkrystian
changed the title
Endilll
reviewed
Jan 18, 2024
|
Thank for this patch I think this might be better done as a pedantic warning. I wonder if we should add tests for https://cplusplus.github.io/CWG/issues/1707.html in this patch. |
|
@cor3ntin Pedantic warning would probably be best as other implementations are not great at diagnosing this |
sdkrystian
force-pushed
the
nns-with-template
branch
2 times, most recently
from
c07119d to
f72b5e6
Compare
|
@cor3ntin I changed the diagnostic to a pedantic warning and added a release note.
We don't check for the template keyword when parsing an elaborated-type-specifier without a nested-name-specifier, so such a declaration is parsed as an unnamed class. I feel that it's beyond the scope of this PR anyways, as this is only concerned with qualified declarations. |
sdkrystian
force-pushed
the
nns-with-template
branch
2 times, most recently
from
be86902 to
2211a93
Compare
cor3ntin
reviewed
Jan 30, 2024
sdkrystian
force-pushed
the
nns-with-template
branch
3 times, most recently
from
ba33d31 to
529d16d
Compare
|
@cor3ntin Addressed your comments. I also added the error to a diagnostics group ( |
sdkrystian
force-pushed
the
nns-with-template
branch
from
529d16d to
3c211c6
Compare
|
@erichkeane cor3ntin approved... please take a look when you have time |
erichkeane
approved these changes
Feb 2, 2024
…ed-name-specifiers
sdkrystian
force-pushed
the
nns-with-template
branch
from
3c211c6 to
44689d0
Compare
agozillon
pushed a commit
to agozillon/llvm-project
that referenced
this pull request
Feb 5, 2024
…ed-name-specifiers (llvm#78595) According to [temp.names] p5: > The keyword template shall not appear immediately after a declarative nested-name-specifier. [expr.prim.id.qual] p2 defines a declarative nested-name-specifier as follows: > A nested-name-specifier is declarative if it is part of > - a class-head-name, > - an enum-head-name, > - a qualified-id that is the id-expression of a declarator-id, or > - a declarative nested-name-specifier. Note: I believe this definition is defective as it doesn't include _nested-name-specifiers_ appearing in _elaborated-type-specifiers_ that declare partial/explicit specializations and explicit instantiations. See my post to the core reflector. Minus a few bugs that are addressed by this PR, this is how we implement it. This means that declarations like: ``` template<typename> struct A { template<typename> struct B { void f(); }; }; template<typename T> template<typename U> void A<T>::template B<U>::f() { } // error: 'template' cannot be used after a declarative nested name specifier ``` are ill-formed. This PR add diagnostics for such declarations. The name of the diagnostic group is `template-in-declaration-name`. Regarding the aforementioned "few bugs that are addressed by this PR" in order to correctly implement this: - `CheckClassTemplate` did not call `diagnoseQualifiedDeclaration` when the semantic context was dependent. This allowed for constructs like: ``` struct A { template<typename T> struct B { template<typename U> struct C; }; }; template<typename T> template<typename U> struct decltype(A())::B<T>::C { }; ``` - `ActOnClassTemplateSpecialization` did not call `diagnoseQualifiedDeclaration` at all, allowing for qualified partial/explicit specializations at class scope and other related nonsense - `TreeTransform::TransformNestedNameSpecifierLoc` would rebuild a `NestedNameSpecifier::TypeSpecWithTemplate` as a `NestedNameSpecifier::TypeSpec` - `TemplateSpecializationTypeLoc::initializeLocal` would set the `template` keyword `SourceLocation` to the provided `Loc` parameter, which would result in a `TemplateSpecializationTypeLoc` obtained via `ASTContext::getTrivialTypeSourceInfo` being displayed as always having a `template` prefix (since the presence of the keyword is not stored anywhere else).
|
It looks like this caused some new This doesn't use This behavior change is covered by the test diffs here in clang/test/CXX/drs/dr23xx.cpp and clang/test/SemaTemplate/class-template-spec.cpp. |
|
@rupprecht This is intended behavior. We previously did not check qualified declarations of explicit/partial specializations of class templates for redundant qualification & invalid nested-name-specifiers (e.g. ones that begin with |
|
Ok, thanks for the quick confirmation! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to [temp.names] p5:
[expr.prim.id.qual] p2 defines a declarative nested-name-specifier as follows:
Note: I believe this definition is defective as it doesn't include nested-name-specifiers appearing in elaborated-type-specifiers that declare partial/explicit specializations and explicit instantiations. See my post to the core reflector here. Minus a few bugs that are addressed by this PR, this is how we implement it.
This means that declarations like:
are ill-formed. This PR add diagnostics for such declarations. The name of the diagnostic group is
template-in-declaration-name.Regarding the aforementioned "few bugs that are addressed by this PR" in order to correctly implement this:
CheckClassTemplatedid not calldiagnoseQualifiedDeclarationwhen the semantic context was dependent. This allowed for constructs like:ActOnClassTemplateSpecializationdid not calldiagnoseQualifiedDeclarationat all, allowing for qualified partial/explicit specializations at class scope and other related nonsenseTreeTransform::TransformNestedNameSpecifierLocwould rebuild aNestedNameSpecifier::TypeSpecWithTemplateas aNestedNameSpecifier::TypeSpecTemplateSpecializationTypeLoc::initializeLocalwould set thetemplatekeywordSourceLocationto the providedLocparameter, which would result in aTemplateSpecializationTypeLocobtained viaASTContext::getTrivialTypeSourceInfobeing displayed as always having atemplateprefix (since the presence of the keyword is not stored anywhere else).