Skip to content

Commit c9e4621

Browse files
mizvekovmizvekov
committed
[clang] retain type sugar in auto / template argument deduction
This implements the following changes: * AutoType retains sugared deduced-as-type. * Template argument deduction machinery analyses the sugared type all the way down. It would previously lose the sugar on first recursion. * Undeduced AutoType will be properly canonicalized, including the constraint template arguments. * Remove the decltype node created from the decltype(auto) deduction. As a result, we start seeing sugared types in a lot more test cases, including some which showed very unfriendly `type-parameter-*-*` types. Signed-off-by: Matheus Izvekov <[email protected]> Reviewed By: rsmith, #libc, ldionne Differential Revision: https://reviews.llvm.org/D110216
1 parent fcd07f8 commit c9e4621

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+851
-853
lines changed

‎clang-tools-extra/clang-tidy/cppcoreguidelines/OwningMemoryCheck.cpp

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,9 @@ void OwningMemoryCheck::registerMatchers(MatchFinder *Finder) {
134134
// Matching on initialization operations where the initial value is a newly
135135
// created owner, but the LHS is not an owner.
136136
Finder->addMatcher(
137-
traverse(
138-
TK_AsIs,
139-
namedDecl(
140-
varDecl(eachOf(allOf(hasInitializer(CreatesOwner),
141-
unless(IsOwnerType)),
142-
allOf(hasInitializer(ConsideredOwner),
143-
hasType(autoType().bind("deduced_type")))))
144-
.bind("bad_owner_creation_variable"))),
137+
traverse(TK_AsIs, namedDecl(varDecl(allOf(hasInitializer(CreatesOwner),
138+
unless(IsOwnerType)))
139+
.bind("bad_owner_creation_variable"))),
145140
this);
146141

147142
// Match on all function calls that expect owners as arguments, but didn't
@@ -324,13 +319,6 @@ bool OwningMemoryCheck::handleAssignmentFromNewOwner(const BoundNodes &Nodes) {
324319

325320
// FIXME: FixitHint to rewrite the type of the initialized variable
326321
// as 'gsl::owner<OriginalType>'
327-
328-
// If the type of the variable was deduced, the wrapping owner typedef is
329-
// eliminated, therefore the check emits a special note for that case.
330-
if (Nodes.getNodeAs<AutoType>("deduced_type")) {
331-
diag(BadOwnerInitialization->getBeginLoc(),
332-
"type deduction did not result in an owner", DiagnosticIDs::Note);
333-
}
334322
return true;
335323
}
336324

‎clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ void ProBoundsPointerArithmeticCheck::registerMatchers(MatchFinder *Finder) {
2020
if (!getLangOpts().CPlusPlus)
2121
return;
2222

23-
const auto AllPointerTypes = anyOf(
24-
hasType(pointerType()), hasType(autoType(hasDeducedType(pointerType()))),
25-
hasType(decltypeType(hasUnderlyingType(pointerType()))));
23+
const auto AllPointerTypes =
24+
anyOf(hasType(pointerType()),
25+
hasType(autoType(
26+
hasDeducedType(hasUnqualifiedDesugaredType(pointerType())))),
27+
hasType(decltypeType(hasUnderlyingType(pointerType()))));
2628

2729
// Flag all operators +, -, +=, -=, ++, -- that result in a pointer
2830
Finder->addMatcher(

‎clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -125,18 +125,22 @@ void QualifiedAutoCheck::registerMatchers(MatchFinder *Finder) {
125125
};
126126

127127
auto IsBoundToType = refersToType(equalsBoundNode("type"));
128+
auto UnlessFunctionType = unless(hasUnqualifiedDesugaredType(functionType()));
129+
auto IsAutoDeducedToPointer = [](const auto &...InnerMatchers) {
130+
return autoType(hasDeducedType(
131+
hasUnqualifiedDesugaredType(pointerType(pointee(InnerMatchers...)))));
132+
};
128133

129134
Finder->addMatcher(
130-
ExplicitSingleVarDecl(hasType(autoType(hasDeducedType(
131-
pointerType(pointee(unless(functionType())))))),
135+
ExplicitSingleVarDecl(hasType(IsAutoDeducedToPointer(UnlessFunctionType)),
132136
"auto"),
133137
this);
134138

135139
Finder->addMatcher(
136140
ExplicitSingleVarDeclInTemplate(
137-
allOf(hasType(autoType(hasDeducedType(pointerType(
138-
pointee(hasUnqualifiedType(qualType().bind("type")),
139-
unless(functionType())))))),
141+
allOf(hasType(IsAutoDeducedToPointer(
142+
hasUnqualifiedType(qualType().bind("type")),
143+
UnlessFunctionType)),
140144
anyOf(hasAncestor(
141145
functionDecl(hasAnyTemplateArgument(IsBoundToType))),
142146
hasAncestor(classTemplateSpecializationDecl(

‎clang-tools-extra/clangd/FindTarget.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ struct TargetFinder {
374374
void VisitDeducedType(const DeducedType *DT) {
375375
// FIXME: In practice this doesn't work: the AutoType you find inside
376376
// TypeLoc never has a deduced type. https://llvm.org/PR42914
377-
Outer.add(DT->getDeducedType(), Flags | Rel::Underlying);
377+
Outer.add(DT->getDeducedType(), Flags);
378378
}
379379
void VisitDeducedTemplateSpecializationType(
380380
const DeducedTemplateSpecializationType *DTST) {

‎clang-tools-extra/clangd/InlayHints.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,9 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
3838
// For structured bindings, print canonical types. This is important because
3939
// for bindings that use the tuple_element protocol, the non-canonical types
4040
// would be "tuple_element<I, A>::type".
41-
// For "auto", we often prefer sugared types, but the AST doesn't currently
42-
// retain them in DeducedType. However, not setting PrintCanonicalTypes for
43-
// "auto" at least allows SuppressDefaultTemplateArgs (set by default) to
44-
// have an effect.
41+
// For "auto", we often prefer sugared types.
42+
// Not setting PrintCanonicalTypes for "auto" allows
43+
// SuppressDefaultTemplateArgs (set by default) to have an effect.
4544
StructuredBindingPolicy = TypeHintPolicy;
4645
StructuredBindingPolicy.PrintCanonicalTypes = true;
4746
}

‎clang-tools-extra/clangd/unittests/ASTTests.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ TEST(GetDeducedType, KwAutoKwDecltypeExpansion) {
4343
namespace ns1 { struct S {}; }
4444
^auto v = ns1::S{};
4545
)cpp",
46-
"struct ns1::S",
46+
"ns1::S",
4747
},
4848
{
4949
R"cpp( // decltype on struct
@@ -63,15 +63,15 @@ TEST(GetDeducedType, KwAutoKwDecltypeExpansion) {
6363
ns1::S& j = i;
6464
^decltype(auto) k = j;
6565
)cpp",
66-
"struct ns1::S &",
66+
"ns1::S &",
6767
},
6868
{
6969
R"cpp( // auto on template class
7070
class X;
7171
template<typename T> class Foo {};
7272
^auto v = Foo<X>();
7373
)cpp",
74-
"class Foo<class X>",
74+
"Foo<class X>",
7575
},
7676
{
7777
R"cpp( // auto on initializer list.
@@ -177,8 +177,7 @@ TEST(GetDeducedType, KwAutoKwDecltypeExpansion) {
177177
using Bar = Foo;
178178
^auto x = Bar();
179179
)cpp",
180-
// FIXME: it'd be nice if this resolved to the alias instead
181-
"struct Foo",
180+
"Bar",
182181
},
183182
};
184183
for (Test T : Tests) {

‎clang-tools-extra/clangd/unittests/HoverTests.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ class Foo {})cpp";
461461
[](HoverInfo &HI) {
462462
HI.Name = "auto";
463463
HI.Kind = index::SymbolKind::TypeAlias;
464-
HI.Definition = "class Foo<int>";
464+
HI.Definition = "Foo<int>";
465465
}},
466466
// auto on specialized template
467467
{R"cpp(
@@ -474,7 +474,7 @@ class Foo {})cpp";
474474
[](HoverInfo &HI) {
475475
HI.Name = "auto";
476476
HI.Kind = index::SymbolKind::TypeAlias;
477-
HI.Definition = "class Foo<int>";
477+
HI.Definition = "Foo<int>";
478478
}},
479479

480480
// macro
@@ -648,7 +648,7 @@ class Foo {})cpp";
648648
[](HoverInfo &HI) {
649649
HI.Name = "auto";
650650
HI.Kind = index::SymbolKind::TypeAlias;
651-
HI.Definition = "class Foo<X>";
651+
HI.Definition = "Foo<X>";
652652
}},
653653
{// Falls back to primary template, when the type is not instantiated.
654654
R"cpp(
@@ -2024,7 +2024,7 @@ TEST(Hover, All) {
20242024
[](HoverInfo &HI) {
20252025
HI.Name = "auto";
20262026
HI.Kind = index::SymbolKind::TypeAlias;
2027-
HI.Definition = "int";
2027+
HI.Definition = "int_type";
20282028
}},
20292029
{
20302030
R"cpp(// auto on alias
@@ -2035,7 +2035,7 @@ TEST(Hover, All) {
20352035
[](HoverInfo &HI) {
20362036
HI.Name = "auto";
20372037
HI.Kind = index::SymbolKind::TypeAlias;
2038-
HI.Definition = "struct cls";
2038+
HI.Definition = "cls_type";
20392039
HI.Documentation = "auto on alias";
20402040
}},
20412041
{
@@ -2047,7 +2047,7 @@ TEST(Hover, All) {
20472047
[](HoverInfo &HI) {
20482048
HI.Name = "auto";
20492049
HI.Kind = index::SymbolKind::TypeAlias;
2050-
HI.Definition = "struct templ<int>";
2050+
HI.Definition = "templ<int>";
20512051
HI.Documentation = "auto on alias";
20522052
}},
20532053
{

‎clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,14 @@ TEST(TypeHints, NoQualifiers) {
466466
}
467467
}
468468
)cpp",
469-
ExpectedHint{": S1", "x"}, ExpectedHint{": Inner<int>", "y"});
469+
ExpectedHint{": S1", "x"},
470+
// FIXME: We want to suppress scope specifiers
471+
// here because we are into the whole
472+
// brevity thing, but the ElaboratedType
473+
// printer does not honor the SuppressScope
474+
// flag by design, so we need to extend the
475+
// PrintingPolicy to support this use case.
476+
ExpectedHint{": S2::Inner<int>", "y"});
470477
}
471478

472479
TEST(TypeHints, Lambda) {

‎clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ TEST_F(ExpandAutoTypeTest, Test) {
5757
EXPECT_UNAVAILABLE("au^to x = []{};");
5858
// inline namespaces
5959
EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
60-
"Visible x = inl_ns::Visible();");
60+
"inl_ns::Visible x = inl_ns::Visible();");
6161
// local class
6262
EXPECT_EQ(apply("namespace x { void y() { struct S{}; ^auto z = S(); } }"),
6363
"namespace x { void y() { struct S{}; S z = S(); } }");
@@ -67,8 +67,9 @@ TEST_F(ExpandAutoTypeTest, Test) {
6767

6868
EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
6969
"ns::Class * foo() { ns::Class * c = foo(); }");
70-
EXPECT_EQ(apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
71-
"void ns::Func() { Class::Nested * x = new ns::Class::Nested{}; }");
70+
EXPECT_EQ(
71+
apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
72+
"void ns::Func() { ns::Class::Nested * x = new ns::Class::Nested{}; }");
7273

7374
EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
7475
// expanding types in structured bindings is syntactically invalid.

‎clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,9 @@ void test_assignment_and_initialization() {
9191
// FIXME:, flow analysis for the case of reassignment. Value must be released before
9292
owned_int6 = owned_int3; // BAD, because reassignment without resource release
9393

94-
auto owned_int7 = returns_owner1(); // Bad, since type deduction eliminates the owner wrapper
95-
// CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'int *' with a newly created 'gsl::owner<>'
96-
// CHECK-NOTES: [[@LINE-2]]:3: note: type deduction did not result in an owner
94+
auto owned_int7 = returns_owner1(); // Ok, since type deduction does not eliminate the owner wrapper
9795

98-
const auto owned_int8 = returns_owner2(); // Bad, since type deduction eliminates the owner wrapper
99-
// CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'int *const' with a newly created 'gsl::owner<>'
100-
// CHECK-NOTES: [[@LINE-2]]:3: note: type deduction did not result in an owner
96+
const auto owned_int8 = returns_owner2(); // Ok, since type deduction does not eliminate the owner wrapper
10197

10298
gsl::owner<int *> owned_int9 = returns_owner1(); // Ok
10399
int *unowned_int3 = returns_owner1(); // Bad
@@ -285,15 +281,12 @@ void test_class_with_owner() {
285281
ClassWithOwner C2{A}; // Bad, since the owner would be initialized with an non-owner, but catched in the class
286282
ClassWithOwner C3{gsl::owner<ArbitraryClass *>(new ArbitraryClass)}; // Ok
287283

288-
const auto Owner1 = C3.buggy_but_returns_owner(); // BAD, deduces Owner1 to ArbitraryClass *const
289-
// CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'ArbitraryClass *const' with a newly created 'gsl::owner<>'
290-
// CHECK-NOTES: [[@LINE-2]]:3: note: type deduction did not result in an owner
284+
const auto Owner1 = C3.buggy_but_returns_owner(); // Ok, deduces Owner1 to owner<ArbitraryClass *> const
291285

292-
auto Owner2 = C2.buggy_but_returns_owner(); // BAD, deduces Owner2 to ArbitraryClass *
293-
// CHECK-NOTES: [[@LINE-1]]:3: warning: initializing non-owner 'ArbitraryClass *' with a newly created 'gsl::owner<>'
294-
// CHECK-NOTES: [[@LINE-2]]:3: note: type deduction did not result in an owner
286+
auto Owner2 = C2.buggy_but_returns_owner(); // Ok, deduces Owner2 to owner<ArbitraryClass *>
295287

296-
Owner2 = &A; // Ok, since type deduction did NOT result in owner<int*>
288+
Owner2 = &A; // BAD, since type deduction resulted in owner<ArbitraryClass *>
289+
// CHECK-NOTES: [[@LINE-1]]:3: warning: expected assignment source to be of type 'gsl::owner<>'; got 'ArbitraryClass *'
297290

298291
gsl::owner<ArbitraryClass *> Owner3 = C1.buggy_but_returns_owner(); // Ok, still an owner
299292
Owner3 = &A; // Bad, since assignment of non-owner to owner

‎clang/include/clang/AST/ASTContext.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,6 +1531,12 @@ class ASTContext : public RefCountedBase<ASTContext> {
15311531
QualType getFunctionTypeInternal(QualType ResultTy, ArrayRef<QualType> Args,
15321532
const FunctionProtoType::ExtProtoInfo &EPI,
15331533
bool OnlyWantCanonical) const;
1534+
QualType
1535+
getAutoTypeInternal(QualType DeducedType, AutoTypeKeyword Keyword,
1536+
bool IsDependent, bool IsPack = false,
1537+
ConceptDecl *TypeConstraintConcept = nullptr,
1538+
ArrayRef<TemplateArgument> TypeConstraintArgs = {},
1539+
bool IsCanon = false) const;
15341540

15351541
public:
15361542
/// Return the unique reference to the type for the specified type

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

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4944,29 +4944,29 @@ class SubstTemplateTypeParmPackType : public Type, public llvm::FoldingSetNode {
49444944
/// type-dependent, there is no deduced type and the type is canonical. In
49454945
/// the latter case, it is also a dependent type.
49464946
class DeducedType : public Type {
4947+
QualType DeducedAsType;
4948+
49474949
protected:
49484950
DeducedType(TypeClass TC, QualType DeducedAsType,
4949-
TypeDependence ExtraDependence)
4950-
: Type(TC,
4951-
// FIXME: Retain the sugared deduced type?
4952-
DeducedAsType.isNull() ? QualType(this, 0)
4953-
: DeducedAsType.getCanonicalType(),
4951+
TypeDependence ExtraDependence, QualType Canon)
4952+
: Type(TC, Canon,
49544953
ExtraDependence | (DeducedAsType.isNull()
49554954
? TypeDependence::None
49564955
: DeducedAsType->getDependence() &
4957-
~TypeDependence::VariablyModified)) {}
4956+
~TypeDependence::VariablyModified)),
4957+
DeducedAsType(DeducedAsType) {}
49584958

49594959
public:
4960-
bool isSugared() const { return !isCanonicalUnqualified(); }
4961-
QualType desugar() const { return getCanonicalTypeInternal(); }
4962-
4963-
/// Get the type deduced for this placeholder type, or null if it's
4964-
/// either not been deduced or was deduced to a dependent type.
4965-
QualType getDeducedType() const {
4966-
return !isCanonicalUnqualified() ? getCanonicalTypeInternal() : QualType();
4960+
bool isSugared() const { return !DeducedAsType.isNull(); }
4961+
QualType desugar() const {
4962+
return isSugared() ? DeducedAsType : QualType(this, 0);
49674963
}
4964+
4965+
/// Get the type deduced for this placeholder type, or null if it
4966+
/// has not been deduced.
4967+
QualType getDeducedType() const { return DeducedAsType; }
49684968
bool isDeduced() const {
4969-
return !isCanonicalUnqualified() || isDependentType();
4969+
return !DeducedAsType.isNull() || isDependentType();
49704970
}
49714971

49724972
static bool classof(const Type *T) {
@@ -4983,7 +4983,7 @@ class alignas(8) AutoType : public DeducedType, public llvm::FoldingSetNode {
49834983
ConceptDecl *TypeConstraintConcept;
49844984

49854985
AutoType(QualType DeducedAsType, AutoTypeKeyword Keyword,
4986-
TypeDependence ExtraDependence, ConceptDecl *CD,
4986+
TypeDependence ExtraDependence, QualType Canon, ConceptDecl *CD,
49874987
ArrayRef<TemplateArgument> TypeConstraintArgs);
49884988

49894989
const TemplateArgument *getArgBuffer() const {
@@ -5057,7 +5057,9 @@ class DeducedTemplateSpecializationType : public DeducedType,
50575057
toTypeDependence(Template.getDependence()) |
50585058
(IsDeducedAsDependent
50595059
? TypeDependence::DependentInstantiation
5060-
: TypeDependence::None)),
5060+
: TypeDependence::None),
5061+
DeducedAsType.isNull() ? QualType(this, 0)
5062+
: DeducedAsType.getCanonicalType()),
50615063
Template(Template) {}
50625064

50635065
public:

0 commit comments

Comments
 (0)