Skip to content

Commit 2178bee

Browse files
mizvekovmizvekov
committed
[clang] Fix elaborated keyword canonicalization.
class and struct keywords are functionally equivalent, so relying on their equivalence was IFNDR. This changes it so we treat them as equivalent, as at least that disallows overloading, which is diagnosable. This patch also considers a few drive by fixes to preservation of the presence / abscence of the typename keyword, and improves a few assertions in that area. Also improves preservervation of sugar on initializer_list deduction in order to avoid a few changes in diagnostics.
1 parent 0bdab9c commit 2178bee

File tree

13 files changed

+182
-60
lines changed

13 files changed

+182
-60
lines changed

‎clang-tools-extra/clang-tidy/modernize/UseConstraintsCheck.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,10 @@ static std::optional<TemplateSpecializationTypeLoc>
5454
matchEnableIfSpecializationImplTypename(TypeLoc TheType) {
5555
if (const auto Dep = TheType.getAs<DependentNameTypeLoc>()) {
5656
const IdentifierInfo *Identifier = Dep.getTypePtr()->getIdentifier();
57+
ElaboratedTypeKeyword Keyword = Dep.getTypePtr()->getKeyword();
5758
if (!Identifier || Identifier->getName() != "type" ||
58-
Dep.getTypePtr()->getKeyword() != ElaboratedTypeKeyword::Typename) {
59+
(Keyword != ElaboratedTypeKeyword::Typename &&
60+
Keyword != ElaboratedTypeKeyword::None)) {
5961
return std::nullopt;
6062
}
6163
TheType = Dep.getQualifierLoc().getTypeLoc();
@@ -108,8 +110,10 @@ matchEnableIfSpecializationImplTrait(TypeLoc TheType) {
108110

109111
if (const auto *AliasedType =
110112
dyn_cast<DependentNameType>(Specialization->getAliasedType())) {
113+
ElaboratedTypeKeyword Keyword = AliasedType->getKeyword();
111114
if (AliasedType->getIdentifier()->getName() != "type" ||
112-
AliasedType->getKeyword() != ElaboratedTypeKeyword::Typename) {
115+
(Keyword != ElaboratedTypeKeyword::Typename &&
116+
Keyword != ElaboratedTypeKeyword::None)) {
113117
return std::nullopt;
114118
}
115119
} else {

‎clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,10 @@ Bug Fixes to C++ Support
476476
- Fixes matching of nested template template parameters. (#GH130362)
477477
- Correctly diagnoses template template paramters which have a pack parameter
478478
not in the last position.
479+
- Disallow overloading on struct vs class on dependent types, which is IFNDR, as
480+
this makes the problem diagnosable.
481+
- Improved preservation of the presence or abscence of typename specifier when
482+
printing types in diagnostics.
479483
- Clang now correctly parses ``if constexpr`` expressions in immediate function context. (#GH123524)
480484
- Fixed an assertion failure affecting code that uses C++23 "deducing this". (#GH130272)
481485
- Clang now properly instantiates destructors for initialized members within non-delegating constructors. (#GH93251)

‎clang/include/clang/AST/Type.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,13 +2839,15 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
28392839
template <typename T> const T *getAs() const;
28402840

28412841
/// Look through sugar for an instance of TemplateSpecializationType which
2842-
/// is not a type alias.
2842+
/// is not a type alias, or null if there is no such type.
2843+
/// This is used when you want as-written template arguments or the template
2844+
/// name for a class template specialization.
28432845
const TemplateSpecializationType *
28442846
getAsNonAliasTemplateSpecializationType() const;
28452847

28462848
const TemplateSpecializationType *
28472849
castAsNonAliasTemplateSpecializationType() const {
2848-
auto TST = getAsNonAliasTemplateSpecializationType();
2850+
const auto *TST = getAsNonAliasTemplateSpecializationType();
28492851
assert(TST && "not a TemplateSpecializationType");
28502852
return TST;
28512853
}

‎clang/lib/AST/ASTContext.cpp

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5747,6 +5747,30 @@ ASTContext::getMacroQualifiedType(QualType UnderlyingTy,
57475747
return QualType(newType, 0);
57485748
}
57495749

5750+
static ElaboratedTypeKeyword
5751+
getCanonicalElaboratedTypeKeyword(ElaboratedTypeKeyword Keyword) {
5752+
switch (Keyword) {
5753+
// These are just themselves.
5754+
case ElaboratedTypeKeyword::None:
5755+
case ElaboratedTypeKeyword::Struct:
5756+
case ElaboratedTypeKeyword::Union:
5757+
case ElaboratedTypeKeyword::Enum:
5758+
case ElaboratedTypeKeyword::Interface:
5759+
return Keyword;
5760+
5761+
// These are equivalent.
5762+
case ElaboratedTypeKeyword::Typename:
5763+
return ElaboratedTypeKeyword::None;
5764+
5765+
// These are functionally equivalent, so relying on their equivalence is
5766+
// IFNDR. By making them equivalent, we disallow overloading, which at least
5767+
// can produce a diagnostic.
5768+
case ElaboratedTypeKeyword::Class:
5769+
return ElaboratedTypeKeyword::Struct;
5770+
}
5771+
llvm_unreachable("unexpected keyword kind");
5772+
}
5773+
57505774
QualType ASTContext::getDependentNameType(ElaboratedTypeKeyword Keyword,
57515775
NestedNameSpecifier *NNS,
57525776
const IdentifierInfo *Name) const {
@@ -5758,10 +5782,13 @@ QualType ASTContext::getDependentNameType(ElaboratedTypeKeyword Keyword,
57585782
DependentNameTypes.FindNodeOrInsertPos(ID, InsertPos))
57595783
return QualType(T, 0);
57605784

5785+
ElaboratedTypeKeyword CanonKeyword =
5786+
getCanonicalElaboratedTypeKeyword(Keyword);
5787+
NestedNameSpecifier *CanonNNS = getCanonicalNestedNameSpecifier(NNS);
5788+
57615789
QualType Canon;
5762-
if (NestedNameSpecifier *CanonNNS = getCanonicalNestedNameSpecifier(NNS);
5763-
CanonNNS != NNS) {
5764-
Canon = getDependentNameType(Keyword, CanonNNS, Name);
5790+
if (CanonKeyword != Keyword || CanonNNS != NNS) {
5791+
Canon = getDependentNameType(CanonKeyword, CanonNNS, Name);
57655792
[[maybe_unused]] DependentNameType *T =
57665793
DependentNameTypes.FindNodeOrInsertPos(ID, InsertPos);
57675794
assert(!T && "broken canonicalization");
@@ -5800,27 +5827,27 @@ QualType ASTContext::getDependentTemplateSpecializationType(
58005827

58015828
QualType Canon;
58025829
if (!IsCanonical) {
5803-
ElaboratedTypeKeyword CanonKeyword = Keyword != ElaboratedTypeKeyword::None
5804-
? Keyword
5805-
: ElaboratedTypeKeyword::Typename;
5830+
ElaboratedTypeKeyword CanonKeyword =
5831+
getCanonicalElaboratedTypeKeyword(Keyword);
58065832
NestedNameSpecifier *CanonNNS = getCanonicalNestedNameSpecifier(NNS);
58075833
bool AnyNonCanonArgs = false;
58085834
auto CanonArgs =
58095835
::getCanonicalTemplateArguments(*this, Args, AnyNonCanonArgs);
58105836

5811-
if (AnyNonCanonArgs || CanonNNS != NNS || !Name.hasTemplateKeyword() ||
5812-
CanonKeyword != Keyword) {
5837+
if (CanonKeyword != Keyword || AnyNonCanonArgs || CanonNNS != NNS ||
5838+
!Name.hasTemplateKeyword()) {
58135839
Canon = getDependentTemplateSpecializationType(
58145840
CanonKeyword, {CanonNNS, Name.getName(), /*HasTemplateKeyword=*/true},
5815-
CanonArgs, /*IsCanonical=*/true);
5841+
CanonArgs,
5842+
/*IsCanonical=*/true);
58165843
// Find the insert position again.
58175844
[[maybe_unused]] auto *Nothing =
58185845
DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID,
58195846
InsertPos);
58205847
assert(!Nothing && "canonical type broken");
58215848
}
58225849
} else {
5823-
assert(Keyword != ElaboratedTypeKeyword::None);
5850+
assert(Keyword == getCanonicalElaboratedTypeKeyword(Keyword));
58245851
assert(Name.hasTemplateKeyword());
58255852
assert(NNS == getCanonicalNestedNameSpecifier(NNS));
58265853
#ifndef NDEBUG
@@ -7657,7 +7684,7 @@ ASTContext::getCanonicalNestedNameSpecifier(NestedNameSpecifier *NNS) const {
76577684
if (const auto *DTST = T->getAs<DependentTemplateSpecializationType>()) {
76587685
const DependentTemplateStorage &DTN = DTST->getDependentTemplateName();
76597686
QualType NewT = getDependentTemplateSpecializationType(
7660-
ElaboratedTypeKeyword::Typename,
7687+
ElaboratedTypeKeyword::None,
76617688
{/*NNS=*/nullptr, DTN.getName(), /*HasTemplateKeyword=*/true},
76627689
DTST->template_arguments(), /*IsCanonical=*/true);
76637690
assert(NewT.isCanonical());

‎clang/lib/AST/Type.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,13 +1940,10 @@ TagDecl *Type::getAsTagDecl() const {
19401940

19411941
const TemplateSpecializationType *
19421942
Type::getAsNonAliasTemplateSpecializationType() const {
1943-
for (const auto *T = this; /**/; /**/) {
1944-
const TemplateSpecializationType *TST =
1945-
T->getAs<TemplateSpecializationType>();
1946-
if (!TST || !TST->isTypeAlias())
1947-
return TST;
1948-
T = TST->desugar().getTypePtr();
1949-
}
1943+
const auto *TST = getAs<TemplateSpecializationType>();
1944+
while (TST && TST->isTypeAlias())
1945+
TST = TST->desugar()->getAs<TemplateSpecializationType>();
1946+
return TST;
19501947
}
19511948

19521949
bool Type::hasAttr(attr::Kind AK) const {

‎clang/lib/AST/TypeLoc.cpp

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -546,37 +546,47 @@ void UnaryTransformTypeLoc::initializeLocal(ASTContext &Context,
546546
Context.getTrivialTypeSourceInfo(getTypePtr()->getBaseType(), Loc));
547547
}
548548

549+
template <class TL>
550+
static void initializeElaboratedKeyword(TL T, SourceLocation Loc) {
551+
T.setElaboratedKeywordLoc(T.getTypePtr()->getKeyword() !=
552+
ElaboratedTypeKeyword::None
553+
? Loc
554+
: SourceLocation());
555+
}
556+
557+
static NestedNameSpecifierLoc
558+
initializeQualifier(ASTContext &Context, NestedNameSpecifier *Qualifier,
559+
SourceLocation Loc) {
560+
if (!Qualifier)
561+
return NestedNameSpecifierLoc();
562+
NestedNameSpecifierLocBuilder Builder;
563+
Builder.MakeTrivial(Context, Qualifier, Loc);
564+
return Builder.getWithLocInContext(Context);
565+
}
566+
549567
void ElaboratedTypeLoc::initializeLocal(ASTContext &Context,
550568
SourceLocation Loc) {
551569
if (isEmpty())
552570
return;
553-
setElaboratedKeywordLoc(Loc);
554-
NestedNameSpecifierLocBuilder Builder;
555-
Builder.MakeTrivial(Context, getTypePtr()->getQualifier(), Loc);
556-
setQualifierLoc(Builder.getWithLocInContext(Context));
571+
initializeElaboratedKeyword(*this, Loc);
572+
setQualifierLoc(
573+
initializeQualifier(Context, getTypePtr()->getQualifier(), Loc));
557574
}
558575

559576
void DependentNameTypeLoc::initializeLocal(ASTContext &Context,
560577
SourceLocation Loc) {
561-
setElaboratedKeywordLoc(Loc);
562-
NestedNameSpecifierLocBuilder Builder;
563-
Builder.MakeTrivial(Context, getTypePtr()->getQualifier(), Loc);
564-
setQualifierLoc(Builder.getWithLocInContext(Context));
578+
initializeElaboratedKeyword(*this, Loc);
579+
setQualifierLoc(
580+
initializeQualifier(Context, getTypePtr()->getQualifier(), Loc));
565581
setNameLoc(Loc);
566582
}
567583

568584
void
569585
DependentTemplateSpecializationTypeLoc::initializeLocal(ASTContext &Context,
570586
SourceLocation Loc) {
571-
setElaboratedKeywordLoc(Loc);
572-
if (NestedNameSpecifier *Qualifier =
573-
getTypePtr()->getDependentTemplateName().getQualifier()) {
574-
NestedNameSpecifierLocBuilder Builder;
575-
Builder.MakeTrivial(Context, Qualifier, Loc);
576-
setQualifierLoc(Builder.getWithLocInContext(Context));
577-
} else {
578-
setQualifierLoc(NestedNameSpecifierLoc());
579-
}
587+
initializeElaboratedKeyword(*this, Loc);
588+
setQualifierLoc(initializeQualifier(
589+
Context, getTypePtr()->getDependentTemplateName().getQualifier(), Loc));
580590
setTemplateKeywordLoc(Loc);
581591
setTemplateNameLoc(Loc);
582592
setLAngleLoc(Loc);

‎clang/lib/Sema/SemaDecl.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,15 +247,15 @@ static ParsedType recoverFromTypeInKnownDependentBase(Sema &S,
247247
return nullptr;
248248

249249
// We found some types in dependent base classes. Recover as if the user
250-
// wrote 'typename MyClass::II' instead of 'II'. We'll fully resolve the
251-
// lookup during template instantiation.
250+
// wrote 'MyClass::II' instead of 'II', and this implicit typename was
251+
// allowed. We'll fully resolve the lookup during template instantiation.
252252
S.Diag(NameLoc, diag::ext_found_in_dependent_base) << &II;
253253

254254
ASTContext &Context = S.Context;
255255
auto *NNS = NestedNameSpecifier::Create(
256256
Context, nullptr, cast<Type>(Context.getRecordType(RD)));
257257
QualType T =
258-
Context.getDependentNameType(ElaboratedTypeKeyword::Typename, NNS, &II);
258+
Context.getDependentNameType(ElaboratedTypeKeyword::None, NNS, &II);
259259

260260
CXXScopeSpec SS;
261261
SS.MakeTrivial(Context, NNS, SourceRange(NameLoc));

‎clang/lib/Sema/SemaInit.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9897,7 +9897,7 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
98979897

98989898
auto TemplateName = DeducedTST->getTemplateName();
98999899
if (TemplateName.isDependent())
9900-
return SubstAutoTypeDependent(TSInfo->getType());
9900+
return SubstAutoTypeSourceInfoDependent(TSInfo)->getType();
99019901

99029902
// We can only perform deduction for class templates or alias templates.
99039903
auto *Template =
@@ -9942,7 +9942,7 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
99429942
Diag(TSInfo->getTypeLoc().getBeginLoc(),
99439943
diag::warn_cxx14_compat_class_template_argument_deduction)
99449944
<< TSInfo->getTypeLoc().getSourceRange() << 0;
9945-
return SubstAutoTypeDependent(TSInfo->getType());
9945+
return SubstAutoTypeSourceInfoDependent(TSInfo)->getType();
99469946
}
99479947

99489948
// FIXME: Perform "exact type" matching first, per CWG discussion?
@@ -10253,7 +10253,8 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
1025310253
// The placeholder is replaced by the return type of the function selected
1025410254
// by overload resolution for class template deduction.
1025510255
QualType DeducedType =
10256-
SubstAutoType(TSInfo->getType(), Best->Function->getReturnType());
10256+
SubstAutoTypeSourceInfo(TSInfo, Best->Function->getReturnType())
10257+
->getType();
1025710258
Diag(TSInfo->getTypeLoc().getBeginLoc(),
1025810259
diag::warn_cxx14_compat_class_template_argument_deduction)
1025910260
<< TSInfo->getTypeLoc().getSourceRange() << 1 << DeducedType;

‎clang/lib/Sema/SemaTemplate.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4892,7 +4892,7 @@ bool Sema::CheckTemplateTypeArgument(
48924892

48934893
// Recover by synthesizing a type using the location information that we
48944894
// already have.
4895-
ArgType = Context.getDependentNameType(ElaboratedTypeKeyword::Typename,
4895+
ArgType = Context.getDependentNameType(ElaboratedTypeKeyword::None,
48964896
SS.getScopeRep(), II);
48974897
TypeLocBuilder TLB;
48984898
DependentNameTypeLoc TL = TLB.push<DependentNameTypeLoc>(ArgType);
@@ -10672,10 +10672,8 @@ TypeResult Sema::ActOnTypenameType(Scope *S, SourceLocation TypenameLoc,
1067210672
NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
1067310673
TypeSourceInfo *TSI = nullptr;
1067410674
QualType T =
10675-
CheckTypenameType((TypenameLoc.isValid() ||
10676-
IsImplicitTypename == ImplicitTypenameContext::Yes)
10677-
? ElaboratedTypeKeyword::Typename
10678-
: ElaboratedTypeKeyword::None,
10675+
CheckTypenameType(TypenameLoc.isValid() ? ElaboratedTypeKeyword::Typename
10676+
: ElaboratedTypeKeyword::None,
1067910677
TypenameLoc, QualifierLoc, II, IdLoc, &TSI,
1068010678
/*DeducedTSTContext=*/true);
1068110679
if (T.isNull())
@@ -10713,6 +10711,9 @@ Sema::ActOnTypenameType(Scope *S, SourceLocation TypenameLoc,
1071310711
TemplateArgumentListInfo TemplateArgs(LAngleLoc, RAngleLoc);
1071410712
translateTemplateArguments(TemplateArgsIn, TemplateArgs);
1071510713

10714+
auto Keyword = TypenameLoc.isValid() ? ElaboratedTypeKeyword::Typename
10715+
: ElaboratedTypeKeyword::None;
10716+
1071610717
TemplateName Template = TemplateIn.get();
1071710718
if (DependentTemplateName *DTN = Template.getAsDependentTemplateName()) {
1071810719
// Construct a dependent template specialization type.
@@ -10726,7 +10727,7 @@ Sema::ActOnTypenameType(Scope *S, SourceLocation TypenameLoc,
1072610727
}
1072710728

1072810729
QualType T = Context.getDependentTemplateSpecializationType(
10729-
ElaboratedTypeKeyword::Typename, *DTN, TemplateArgs.arguments());
10730+
Keyword, *DTN, TemplateArgs.arguments());
1073010731

1073110732
// Create source-location information for this type.
1073210733
TypeLocBuilder Builder;
@@ -10758,8 +10759,7 @@ Sema::ActOnTypenameType(Scope *S, SourceLocation TypenameLoc,
1075810759
for (unsigned I = 0, N = TemplateArgs.size(); I != N; ++I)
1075910760
SpecTL.setArgLocInfo(I, TemplateArgs[I].getLocInfo());
1076010761

10761-
T = Context.getElaboratedType(ElaboratedTypeKeyword::Typename,
10762-
SS.getScopeRep(), T);
10762+
T = Context.getElaboratedType(Keyword, SS.getScopeRep(), T);
1076310763
ElaboratedTypeLoc TL = Builder.push<ElaboratedTypeLoc>(T);
1076410764
TL.setElaboratedKeywordLoc(TypenameLoc);
1076510765
TL.setQualifierLoc(SS.getWithLocInContext(Context));
@@ -10853,6 +10853,8 @@ Sema::CheckTypenameType(ElaboratedTypeKeyword Keyword,
1085310853
NestedNameSpecifierLoc QualifierLoc,
1085410854
const IdentifierInfo &II,
1085510855
SourceLocation IILoc, bool DeducedTSTContext) {
10856+
assert((Keyword != ElaboratedTypeKeyword::None) == KeywordLoc.isValid());
10857+
1085610858
CXXScopeSpec SS;
1085710859
SS.Adopt(QualifierLoc);
1085810860

‎clang/test/Analysis/anonymous-decls.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,13 @@ int main() {
7474
// CHECK-NEXT: 4: * [B3.3] (OperatorCall)
7575
// CHECK-NEXT: 5: auto &;
7676
// CHECK-NEXT: 6: get<0UL>
77-
// CHECK-NEXT: 7: [B3.6] (ImplicitCastExpr, FunctionToPointerDecay, typename tuple_element<0L, pair<int, int> >::type (*)(pair<int, int> &))
77+
// CHECK-NEXT: 7: [B3.6] (ImplicitCastExpr, FunctionToPointerDecay, tuple_element<0L, pair<int, int> >::type (*)(pair<int, int> &))
7878
// CHECK-NEXT: 8: decomposition-a-b
7979
// CHECK-NEXT: 9: [B3.7]([B3.8])
8080
// CHECK-NEXT: 10: [B3.9]
8181
// CHECK-NEXT: 11: std::tuple_element<0, std::pair<int, int>>::type a = get<0UL>(decomposition-a-b);
8282
// CHECK-NEXT: 12: get<1UL>
83-
// CHECK-NEXT: 13: [B3.12] (ImplicitCastExpr, FunctionToPointerDecay, typename tuple_element<1L, pair<int, int> >::type (*)(pair<int, int> &))
83+
// CHECK-NEXT: 13: [B3.12] (ImplicitCastExpr, FunctionToPointerDecay, tuple_element<1L, pair<int, int> >::type (*)(pair<int, int> &))
8484
// CHECK-NEXT: 14: decomposition-a-b
8585
// CHECK-NEXT: 15: [B3.13]([B3.14])
8686
// CHECK-NEXT: 16: [B3.15]

‎clang/test/SemaTemplate/dependent-template-recover.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ namespace templ_spec {
146146

147147
// FIXME: Why error recovery for the non-typename case is so bad?
148148
A<T::template X<int>> t3; // expected-error {{did you forget 'typename'}}
149-
// expected-error@-1 {{'A<typename T::X>' (aka 'void')}}
149+
// expected-error@-1 {{'A<T::X>' (aka 'void')}}
150150

151151
A<T::X<int>> t4; // expected-error {{use 'template' keyword}} expected-error {{did you forget 'typename'}}
152-
// expected-error@-1 {{'A<typename T::X>' (aka 'void')}}
152+
// expected-error@-1 {{'A<T::X>' (aka 'void')}}
153153
};
154154
} // namespace templ_spec

0 commit comments

Comments
 (0)