[clang] [Clang][Sema] Diagnose function/variable templates that shadow their own template parameters (PR #78274)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 16 05:33:02 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Krystian Stasiowski (sdkrystian)
<details>
<summary>Changes</summary>
Previously, we skipped through template parameter scopes (until we hit a declaration scope) prior to redeclaration lookup for declarators. For template declarations, the meant that their template parameters would not be found and shadowing would not be diagnosed. With these changes applied, the following declarations are correctly diagnosed:
```cpp
template<typename T> void T(); // error: declaration of 'T' shadows template parameter
template<typename U> int U; // error: declaration of 'U' shadows template parameter
```
The reason for skipping past non-declaration & template parameter scopes prior to lookup appears to have been because `GetTypeForDeclarator` needed this adjusted scope... but it doesn't actually use this parameter anymore. I added a separate commit that removes it from the declaration and all call sites.
The scope adjustment now happens prior to calling `ActOnFunctionDeclarator`/`ActOnVariableDeclarator`/`ActOnTypedefDeclarator` (just in case they depend on this behavior... I didn't check in depth).
---
Full diff: https://github.com/llvm/llvm-project/pull/78274.diff
13 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+1-1)
- (modified) clang/include/clang/Sema/Sema.h (+1-1)
- (modified) clang/lib/Sema/SemaDecl.cpp (+12-12)
- (modified) clang/lib/Sema/SemaDeclCXX.cpp (+5-5)
- (modified) clang/lib/Sema/SemaDeclObjC.cpp (+1-1)
- (modified) clang/lib/Sema/SemaExpr.cpp (+1-1)
- (modified) clang/lib/Sema/SemaExprCXX.cpp (+1-1)
- (modified) clang/lib/Sema/SemaLambda.cpp (+1-1)
- (modified) clang/lib/Sema/SemaObjCProperty.cpp (+1-1)
- (modified) clang/lib/Sema/SemaOpenMP.cpp (+2-2)
- (modified) clang/lib/Sema/SemaTemplate.cpp (+2-2)
- (modified) clang/lib/Sema/SemaType.cpp (+2-2)
- (modified) clang/test/CXX/temp/temp.res/temp.local/p6.cpp (+17-3)
``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ea57769a4a5795..4f62f4917029c0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -555,7 +555,7 @@ Improvements to Clang's diagnostics
- Clang now diagnoses unexpanded packs within the template argument lists of function template specializations.
- Clang now diagnoses attempts to bind a bitfield to an NTTP of a reference type as erroneous
converted constant expression and not as a reference to subobject.
-
+- Clang now diagnoses function/variable templates that shadow their own template parameters, e.g. ``template<class T> void T();``.
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b55f7584913b46..4dc4dded652b44 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2153,7 +2153,7 @@ class Sema final {
SourceLocation Loc);
QualType BuildBitIntType(bool IsUnsigned, Expr *BitWidth, SourceLocation Loc);
- TypeSourceInfo *GetTypeForDeclarator(Declarator &D, Scope *S);
+ TypeSourceInfo *GetTypeForDeclarator(Declarator &D);
TypeSourceInfo *GetTypeForDeclaratorCast(Declarator &D, QualType FromTy);
/// Package the given type and TSI into a ParsedType.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 722e2ac9e4ff8d..64208d42f44531 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -5797,7 +5797,7 @@ Decl *Sema::BuildAnonymousStructOrUnion(Scope *S, DeclSpec &DS,
// Mock up a declarator.
Declarator Dc(DS, ParsedAttributesView::none(), DeclaratorContext::Member);
StorageClass SC = StorageClassSpecToVarDeclStorageClass(DS);
- TypeSourceInfo *TInfo = GetTypeForDeclarator(Dc, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(Dc);
assert(TInfo && "couldn't build declarator info for anonymous struct/union");
// Create a declaration for this anonymous struct/union.
@@ -5894,7 +5894,7 @@ Decl *Sema::BuildMicrosoftCAnonymousStruct(Scope *S, DeclSpec &DS,
// Mock up a declarator.
Declarator Dc(DS, ParsedAttributesView::none(), DeclaratorContext::TypeName);
- TypeSourceInfo *TInfo = GetTypeForDeclarator(Dc, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(Dc);
assert(TInfo && "couldn't build declarator info for anonymous struct");
auto *ParentDecl = cast<RecordDecl>(CurContext);
@@ -6371,12 +6371,6 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
} else if (DiagnoseUnexpandedParameterPack(NameInfo, UPPC_DeclarationType))
return nullptr;
- // The scope passed in may not be a decl scope. Zip up the scope tree until
- // we find one that is.
- while ((S->getFlags() & Scope::DeclScope) == 0 ||
- (S->getFlags() & Scope::TemplateParamScope) != 0)
- S = S->getParent();
-
DeclContext *DC = CurContext;
if (D.getCXXScopeSpec().isInvalid())
D.setInvalidType();
@@ -6432,7 +6426,7 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
}
}
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType R = TInfo->getType();
if (DiagnoseUnexpandedParameterPack(D.getIdentifierLoc(), TInfo,
@@ -6529,6 +6523,12 @@ NamedDecl *Sema::HandleDeclarator(Scope *S, Declarator &D,
if (getLangOpts().CPlusPlus)
CheckExtraCXXDefaultArguments(D);
+ // The scope passed in may not be a decl scope. Zip up the scope tree until
+ // we find one that is.
+ while ((S->getFlags() & Scope::DeclScope) == 0 ||
+ (S->getFlags() & Scope::TemplateParamScope) != 0)
+ S = S->getParent();
+
NamedDecl *New;
bool AddToScope = true;
@@ -15041,7 +15041,7 @@ Decl *Sema::ActOnParamDeclarator(Scope *S, Declarator &D,
CheckFunctionOrTemplateParamDeclarator(S, D);
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType parmDeclType = TInfo->getType();
// Check for redeclaration of parameters, e.g. int foo(int x, int x);
@@ -18305,7 +18305,7 @@ FieldDecl *Sema::HandleField(Scope *S, RecordDecl *Record,
SourceLocation Loc = DeclStart;
if (II) Loc = D.getIdentifierLoc();
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType T = TInfo->getType();
if (getLangOpts().CPlusPlus) {
CheckExtraCXXDefaultArguments(D);
@@ -18669,7 +18669,7 @@ Decl *Sema::ActOnIvar(Scope *S, SourceLocation DeclStart, Declarator &D,
// FIXME: Unnamed fields can be handled in various different ways, for
// example, unnamed unions inject all members into the struct namespace!
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType T = TInfo->getType();
if (BitWidth) {
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index f229e734d06b28..a2ce96188b4f16 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -839,7 +839,7 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
Diag(DS.getVolatileSpecLoc(),
diag::warn_deprecated_volatile_structured_binding);
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType R = TInfo->getType();
if (DiagnoseUnexpandedParameterPack(D.getIdentifierLoc(), TInfo,
@@ -17011,7 +17011,7 @@ VarDecl *Sema::BuildExceptionDeclaration(Scope *S,
/// ActOnExceptionDeclarator - Parsed the exception-declarator in a C++ catch
/// handler.
Decl *Sema::ActOnExceptionDeclarator(Scope *S, Declarator &D) {
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
bool Invalid = D.isInvalidType();
// Check for unexpanded parameter packs.
@@ -17762,7 +17762,7 @@ Decl *Sema::ActOnFriendTypeDecl(Scope *S, const DeclSpec &DS,
// for a TUK_Friend.
Declarator TheDeclarator(DS, ParsedAttributesView::none(),
DeclaratorContext::Member);
- TypeSourceInfo *TSI = GetTypeForDeclarator(TheDeclarator, S);
+ TypeSourceInfo *TSI = GetTypeForDeclarator(TheDeclarator);
QualType T = TSI->getType();
if (TheDeclarator.isInvalidType())
return nullptr;
@@ -17827,7 +17827,7 @@ NamedDecl *Sema::ActOnFriendFunctionDecl(Scope *S, Declarator &D,
assert(DS.getStorageClassSpec() == DeclSpec::SCS_unspecified);
SourceLocation Loc = D.getIdentifierLoc();
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
// C++ [class.friend]p1
// A friend of a class is a function or class....
@@ -19182,7 +19182,7 @@ MSPropertyDecl *Sema::HandleMSProperty(Scope *S, RecordDecl *Record,
}
SourceLocation Loc = D.getIdentifierLoc();
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType T = TInfo->getType();
if (getLangOpts().CPlusPlus) {
CheckExtraCXXDefaultArguments(D);
diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index c3b95e168a6051..df190c80908788 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -5211,7 +5211,7 @@ Decl *Sema::ActOnObjCExceptionDecl(Scope *S, Declarator &D) {
if (getLangOpts().CPlusPlus)
CheckExtraCXXDefaultArguments(D);
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType ExceptionType = TInfo->getType();
VarDecl *New = BuildObjCExceptionDecl(TInfo, ExceptionType,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2aa192c3909cbe..499757aaf3e6bf 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16972,7 +16972,7 @@ void Sema::ActOnBlockArguments(SourceLocation CaretLoc, Declarator &ParamInfo,
assert(ParamInfo.getContext() == DeclaratorContext::BlockLiteral);
BlockScopeInfo *CurBlock = getCurBlock();
- TypeSourceInfo *Sig = GetTypeForDeclarator(ParamInfo, CurScope);
+ TypeSourceInfo *Sig = GetTypeForDeclarator(ParamInfo);
QualType T = Sig->getType();
// FIXME: We should allow unexpanded parameter packs here, but that would,
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 4ae04358d5df7c..c77a2370641624 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1931,7 +1931,7 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
}
}
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, /*Scope=*/nullptr);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType AllocType = TInfo->getType();
if (D.isInvalidType())
return ExprError();
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index e7b6443c984c91..5b95bae567b721 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -916,7 +916,7 @@ static TypeSourceInfo *getLambdaType(Sema &S, LambdaIntroducer &Intro,
}
}
- MethodTyInfo = S.GetTypeForDeclarator(ParamInfo, CurScope);
+ MethodTyInfo = S.GetTypeForDeclarator(ParamInfo);
assert(MethodTyInfo && "no type from lambda-declarator");
// Check for unexpanded parameter packs in the method type.
diff --git a/clang/lib/Sema/SemaObjCProperty.cpp b/clang/lib/Sema/SemaObjCProperty.cpp
index 22540af1cda8c6..349c7fc9c91bdb 100644
--- a/clang/lib/Sema/SemaObjCProperty.cpp
+++ b/clang/lib/Sema/SemaObjCProperty.cpp
@@ -180,7 +180,7 @@ Decl *Sema::ActOnProperty(Scope *S, SourceLocation AtLoc,
unsigned Attributes = ODS.getPropertyAttributes();
FD.D.setObjCWeakProperty((Attributes & ObjCPropertyAttribute::kind_weak) !=
0);
- TypeSourceInfo *TSI = GetTypeForDeclarator(FD.D, S);
+ TypeSourceInfo *TSI = GetTypeForDeclarator(FD.D);
QualType T = TSI->getType();
if (!getOwnershipRule(Attributes)) {
Attributes |= deducePropertyOwnershipFromType(*this, T);
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 365032c9642123..217fcb979deea2 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -7304,7 +7304,7 @@ void Sema::ActOnStartOfFunctionDefinitionInOpenMPDeclareVariantScope(
LookupOrdinaryName);
LookupParsedName(Lookup, S, &D.getCXXScopeSpec());
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType FType = TInfo->getType();
bool IsConstexpr =
@@ -22719,7 +22719,7 @@ Sema::DeclGroupPtrTy Sema::ActOnOpenMPDeclareReductionDirectiveEnd(
}
TypeResult Sema::ActOnOpenMPDeclareMapperVarDecl(Scope *S, Declarator &D) {
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType T = TInfo->getType();
if (D.isInvalidType())
return true;
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 80a48c268a648b..c0dcbda1fd6221 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1498,7 +1498,7 @@ NamedDecl *Sema::ActOnNonTypeTemplateParameter(Scope *S, Declarator &D,
unsigned Position,
SourceLocation EqualLoc,
Expr *Default) {
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
// Check that we have valid decl-specifiers specified.
auto CheckValidDeclSpecifiers = [this, &D] {
@@ -10521,7 +10521,7 @@ DeclResult Sema::ActOnExplicitInstantiation(Scope *S,
S = S->getParent();
// Determine the type of the declaration.
- TypeSourceInfo *T = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *T = GetTypeForDeclarator(D);
QualType R = T->getType();
if (R.isNull())
return true;
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 78702b41ab8200..c21f90fab5c648 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6070,7 +6070,7 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
///
/// The result of this call will never be null, but the associated
/// type may be a null type if there's an unrecoverable error.
-TypeSourceInfo *Sema::GetTypeForDeclarator(Declarator &D, Scope *S) {
+TypeSourceInfo *Sema::GetTypeForDeclarator(Declarator &D) {
// Determine the type of the declarator. Not all forms of declarator
// have a type.
@@ -6754,7 +6754,7 @@ TypeResult Sema::ActOnTypeName(Scope *S, Declarator &D) {
assert(D.getIdentifier() == nullptr &&
"Type name should have no identifier!");
- TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
+ TypeSourceInfo *TInfo = GetTypeForDeclarator(D);
QualType T = TInfo->getType();
if (D.isInvalidType())
return true;
diff --git a/clang/test/CXX/temp/temp.res/temp.local/p6.cpp b/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
index e2aa0ff344291d..0702966e568548 100644
--- a/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
+++ b/clang/test/CXX/temp/temp.res/temp.local/p6.cpp
@@ -127,16 +127,30 @@ template<int T> struct Z { // expected-note 16{{declared here}}
template<typename T> // expected-note {{declared here}}
void f(int T) {} // expected-error {{declaration of 'T' shadows template parameter}}
-// FIXME: These are ill-formed: a template-parameter shall not have the same name as the template name.
namespace A {
template<typename T> struct T {}; // expected-error{{declaration of 'T' shadows template parameter}}
// expected-note at -1{{template parameter is declared here}}
+ template<typename T> struct U {
+ template<typename V> struct V {}; // expected-error{{declaration of 'V' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ };
}
namespace B {
- template<typename T> void T() {}
+ template<typename T> void T() {} // expected-error{{declaration of 'T' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+
+ template<typename T> struct U {
+ template<typename V> void V(); // expected-error{{declaration of 'V' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ };
}
namespace C {
- template<typename T> int T;
+ template<typename T> int T; // expected-error{{declaration of 'T' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ template<typename T> struct U {
+ template<typename V> static int V; // expected-error{{declaration of 'V' shadows template parameter}}
+ // expected-note at -1{{template parameter is declared here}}
+ };
}
namespace PR28023 {
``````````
</details>
https://github.com/llvm/llvm-project/pull/78274
More information about the cfe-commits
mailing list