Skip to content

Commit

Permalink
[Clang][Sema] Diagnose use of template keyword after declarative nest…
Browse files Browse the repository at this point in the history
…ed-name-specifiers (#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).
  • Loading branch information
sdkrystian authored Feb 2, 2024
1 parent faeb3d1 commit 1156bbc
Show file tree
Hide file tree
Showing 17 changed files with 306 additions and 32 deletions.
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ Improvements to Clang's diagnostics
prints.

- Clang now diagnoses member template declarations with multiple declarators.
- Clang now diagnoses use of the ``template`` keyword after declarative nested name specifiers.

Improvements to Clang's time-trace
----------------------------------
Expand Down
5 changes: 4 additions & 1 deletion clang/include/clang/AST/TypeLoc.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,9 @@ class TypeLoc {
/// pointer types, but not through decltype or typedefs.
AutoTypeLoc getContainedAutoTypeLoc() const;

/// Get the SourceLocation of the template keyword (if any).
SourceLocation getTemplateKeywordLoc() const;

/// Initializes this to state that every location in this
/// type is the given location.
///
Expand Down Expand Up @@ -1691,7 +1694,7 @@ class TemplateSpecializationTypeLoc :
}

void initializeLocal(ASTContext &Context, SourceLocation Loc) {
setTemplateKeywordLoc(Loc);
setTemplateKeywordLoc(SourceLocation());
setTemplateNameLoc(Loc);
setLAngleLoc(Loc);
setRAngleLoc(Loc);
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -8247,6 +8247,9 @@ 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 ext_template_after_declarative_nns : ExtWarn<
"'template' cannot be used after a declarative nested name specifier">,
InGroup<DiagGroup<"template-in-declaration-name">>;

def err_no_typeid_with_fno_rtti : Error<
"use of typeid requires -frtti">;
Expand Down
3 changes: 2 additions & 1 deletion clang/include/clang/Sema/Sema.h
Original file line number Diff line number Diff line change
Expand Up @@ -2960,7 +2960,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,
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/AST/TypeLoc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -738,3 +738,12 @@ AutoTypeLoc TypeLoc::getContainedAutoTypeLoc() const {
return AutoTypeLoc();
return Res.getAs<AutoTypeLoc>();
}

SourceLocation TypeLoc::getTemplateKeywordLoc() const {
if (const auto TSTL = getAsAdjusted<TemplateSpecializationTypeLoc>())
return TSTL.getTemplateKeywordLoc();
if (const auto DTSTL =
getAsAdjusted<DependentTemplateSpecializationTypeLoc>())
return DTSTL.getTemplateKeywordLoc();
return SourceLocation();
}
4 changes: 3 additions & 1 deletion clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6663,12 +6663,14 @@ void Parser::ParseDirectDeclarator(Declarator &D) {
}

bool HadScope = D.getCXXScopeSpec().isValid();
SourceLocation TemplateKWLoc;
if (ParseUnqualifiedId(D.getCXXScopeSpec(),
/*ObjectType=*/nullptr,
/*ObjectHadErrors=*/false,
/*EnteringContext=*/true,
/*AllowDestructorName=*/true, AllowConstructorName,
AllowDeductionGuide, nullptr, D.getName()) ||
AllowDeductionGuide, &TemplateKWLoc,
D.getName()) ||
// Once we're past the identifier, if the scope was bad, mark the
// whole declarator bad.
D.getCXXScopeSpec().isInvalid()) {
Expand Down
49 changes: 39 additions & 10 deletions clang/lib/Sema/SemaDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6196,13 +6196,17 @@ 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();
Expand Down Expand Up @@ -6231,7 +6235,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();
Expand Down Expand Up @@ -6277,12 +6281,32 @@ 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.
//
// First check the template-id (if any), and then check each component of the
// nested-name-specifier in reverse order.
//
// FIXME: nested-name-specifiers in friend declarations are declarative,
// but we don't call diagnoseQualifiedDeclaration for them. We should.
if (TemplateId && TemplateId->TemplateKWLoc.isValid())
Diag(Loc, diag::ext_template_after_declarative_nns)
<< FixItHint::CreateRemoval(TemplateId->TemplateKWLoc);

NestedNameSpecifierLoc SpecLoc(SS.getScopeRep(), SS.location_data());
while (SpecLoc.getPrefix()) {
if (SpecLoc.getNestedNameSpecifier()->getKind() ==
NestedNameSpecifier::TypeSpecWithTemplate)
Diag(Loc, diag::ext_template_after_declarative_nns)
<< FixItHint::CreateRemoval(
SpecLoc.getTypeLoc().getTemplateKeywordLoc());

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)
Expand Down Expand Up @@ -6350,9 +6374,13 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
return nullptr;
}
if (!D.getDeclSpec().isFriendSpecified()) {
if (diagnoseQualifiedDeclaration(
D.getCXXScopeSpec(), DC, Name, D.getIdentifierLoc(),
D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId)) {
TemplateIdAnnotation *TemplateId =
D.getName().getKind() == UnqualifiedIdKind::IK_TemplateId
? D.getName().TemplateId
: nullptr;
if (diagnoseQualifiedDeclaration(D.getCXXScopeSpec(), DC, Name,
D.getIdentifierLoc(), TemplateId,
/*IsMemberSpecialization=*/false)) {
if (DC->isRecord())
return nullptr;

Expand Down Expand Up @@ -17957,6 +17985,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;

Expand Down
14 changes: 9 additions & 5 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3621,14 +3621,18 @@ 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();
}

Expand Down
17 changes: 15 additions & 2 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1890,8 +1890,12 @@ 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 {
Expand Down Expand Up @@ -8826,6 +8830,15 @@ 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
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Sema/TreeTransform.h
Original file line number Diff line number Diff line change
Expand Up @@ -4394,7 +4394,8 @@ NestedNameSpecifierLoc TreeTransform<Derived>::TransformNestedNameSpecifierLoc(
SS.Adopt(ETL.getQualifierLoc());
TL = ETL.getNamedTypeLoc();
}
SS.Extend(SemaRef.Context, /*FIXME:*/ SourceLocation(), TL,

SS.Extend(SemaRef.Context, TL.getTemplateKeywordLoc(), TL,
Q.getLocalEndLoc());
break;
}
Expand Down
4 changes: 2 additions & 2 deletions clang/test/CXX/drs/dr23xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions clang/test/CXX/drs/dr7xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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'}}
Expand All @@ -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}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ struct X1 {

template<typename T>
template<typename U>
void X1<T>::template B<U>::f() { }
void X1<T>::template B<U>::f() { } // expected-warning{{'template' cannot be used after a declarative}}

// PR5527
template <template <class> class T>
Expand Down
Loading

0 comments on commit 1156bbc

Please sign in to comment.