Skip to content

Commit d6fb091

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 d6fb091

File tree

10 files changed

+168
-49
lines changed

10 files changed

+168
-49
lines changed

‎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/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/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

‎clang/test/SemaTemplate/elaborated-type-specifier.cpp

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace PR6915 {
1010
struct D1 {
1111
enum X { value };
1212
};
13-
struct D2 {
13+
struct D2 {
1414
class X { }; // expected-note{{previous use is here}}
1515
};
1616
struct D3 { };
@@ -25,12 +25,12 @@ struct DeclOrDef {
2525
enum T::foo; // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
2626
// expected-error@-1{{forward declaration of enum cannot have a nested name specifier}}
2727
enum T::bar { // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
28-
value
28+
value
2929
};
3030
};
3131

3232
namespace PR6649 {
33-
template <typename T> struct foo {
33+
template <typename T> struct foo {
3434
class T::bar; // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
3535
// expected-error@-1{{forward declaration of class cannot have a nested name specifier}}
3636
class T::bar { int x; }; // expected-error{{nested name specifier for a declaration cannot depend on a template parameter}}
@@ -40,3 +40,60 @@ namespace PR6649 {
4040
namespace rdar8568507 {
4141
template <class T> struct A *makeA(T t);
4242
}
43+
44+
namespace canon {
45+
template <class T> void t1(struct T::X) {}
46+
// expected-note@-1 {{previous definition is here}}
47+
template <class T> void t1(class T::X) {}
48+
// expected-error@-1 {{redefinition of 't1'}}
49+
50+
template <class T> void t2(struct T::template X<int>) {}
51+
// expected-note@-1 {{previous definition is here}}
52+
template <class T> void t2(class T::template X<int>) {}
53+
// expected-error@-1 {{redefinition of 't2'}}
54+
55+
template <class T> constexpr int t3(typename T::X* = 0) { return 0; } // #canon-t3-0
56+
template <class T> constexpr int t3(struct T::X* = 0) { return 1; } // #canon-t3-1
57+
template <class T> constexpr int t3(union T::X* = 0) { return 2; } // #canon-t3-2
58+
template <class T> constexpr int t3(enum T::X* = 0) { return 3; } // #canon-t3-3
59+
60+
struct A { using X = int; };
61+
static_assert(t3<A>() == 0);
62+
63+
struct B { struct X {}; };
64+
static_assert(t3<B>() == 1);
65+
// expected-error@-1 {{call to 't3' is ambiguous}}
66+
// expected-note@#canon-t3-0 {{candidate function}}
67+
// expected-note@#canon-t3-1 {{candidate function}}
68+
69+
struct C { union X {}; };
70+
static_assert(t3<C>() == 2);
71+
// expected-error@-1 {{call to 't3' is ambiguous}}
72+
// expected-note@#canon-t3-0 {{candidate function}}
73+
// expected-note@#canon-t3-2 {{candidate function}}
74+
75+
struct D { enum X {}; };
76+
static_assert(t3<D>() == 3);
77+
// expected-error@-1 {{call to 't3' is ambiguous}}
78+
// expected-note@#canon-t3-0 {{candidate function}}
79+
// expected-note@#canon-t3-3 {{candidate function}}
80+
81+
template <class T> constexpr int t4(typename T::template X<int>* = 0) { return 0; }
82+
// expected-note@-1 3{{candidate function}}
83+
template <class T> constexpr int t4(struct T::template X<int>* = 0) { return 1; }
84+
// expected-note@-1 3{{candidate function}}
85+
template <class T> constexpr int t4(union T::template X<int>* = 0) { return 2; }
86+
// expected-note@-1 3{{candidate function}}
87+
88+
// FIXME: This should work.
89+
struct E { template <class T> using X = T; };
90+
static_assert(t4<E>() == 0); // expected-error {{call to 't4' is ambiguous}}
91+
92+
// FIXME: Should not match the union overload.
93+
struct F { template <class> struct X {}; };
94+
static_assert(t4<F>() == 1); // expected-error {{call to 't4' is ambiguous}}
95+
96+
// FIXME: Should not match the struct overload.
97+
struct G { template <class> union X {}; };
98+
static_assert(t4<G>() == 2); // expected-error {{call to 't4' is ambiguous}}
99+
} // namespace canon

0 commit comments

Comments
 (0)