[clang] Preserve Qualifiers and type sugar in TemplateNames #93433
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesThis patch improves the preservation of qualifiers and loss of type sugar in TemplateNames. This problem is analogous to https://reviews.llvm.org/D112374 and this patch takes a very similar approach to that patch, except the impact here is much lesser. When a TemplateName was written bare, without qualifications, we wouldn't produce a QualifiedTemplate which could be used to disambiguate it from a Canonical TemplateName. This had effects in the TemplateName printer, which had workarounds to deal with this, and wouldn't print the TemplateName as-written in most situations. There are also some related fixes to help preserve this type sugar along the way into diagnostics, so that this patch can be properly tested.
Patch is 53.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/93433.diff 36 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 403a107edef17..257e1df6e6503 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -802,6 +802,8 @@ Bug Fixes to AST Handling
- Clang now properly preserves ``FoundDecls`` within a ``ConceptReference``. (#GH82628)
- The presence of the ``typename`` keyword is now stored in ``TemplateTemplateParmDecl``.
- Fixed malformed AST generated for anonymous union access in templates. (#GH90842)
+- Improved preservation of qualifiers and sugar in TemplateNames, including
+ template keyword.
Miscellaneous Bug Fixes
^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/TemplateName.h b/clang/include/clang/AST/TemplateName.h
index b7732e54ba107..6bb03f90ac36a 100644
--- a/clang/include/clang/AST/TemplateName.h
+++ b/clang/include/clang/AST/TemplateName.h
@@ -332,7 +332,7 @@ class TemplateName {
/// unexpanded parameter pack (for C++0x variadic templates).
bool containsUnexpandedParameterPack() const;
- enum class Qualified { None, AsWritten, Fully };
+ enum class Qualified { None, AsWritten };
/// Print the template name.
///
/// \param OS the output stream to which the template name will be
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ec083f7cc09b7..e6296868000c5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8988,6 +8988,9 @@ class Sema final : public SemaBase {
const TemplateArgumentListInfo *TemplateArgs);
void diagnoseMissingTemplateArguments(TemplateName Name, SourceLocation Loc);
+ void diagnoseMissingTemplateArguments(const CXXScopeSpec &SS,
+ bool TemplateKeyword, TemplateDecl *TD,
+ SourceLocation Loc);
ExprResult BuildTemplateIdExpr(const CXXScopeSpec &SS,
SourceLocation TemplateKWLoc, LookupResult &R,
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a2398fef623ea..06780ceba4074 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -5006,9 +5006,6 @@ ASTContext::getTemplateSpecializationType(TemplateName Template,
QualType Underlying) const {
assert(!Template.getAsDependentTemplateName() &&
"No dependent template names here!");
- // Look through qualified template names.
- if (QualifiedTemplateName *QTN = Template.getAsQualifiedTemplateName())
- Template = QTN->getUnderlyingTemplate();
const auto *TD = Template.getAsTemplateDecl();
bool IsTypeAlias = TD && TD->isTypeAlias();
@@ -5044,10 +5041,6 @@ QualType ASTContext::getCanonicalTemplateSpecializationType(
assert(!Template.getAsDependentTemplateName() &&
"No dependent template names here!");
- // Look through qualified template names.
- if (QualifiedTemplateName *QTN = Template.getAsQualifiedTemplateName())
- Template = TemplateName(QTN->getUnderlyingTemplate());
-
// Build the canonical template specialization type.
TemplateName CanonTemplate = getCanonicalTemplateName(Template);
bool AnyNonCanonArgs = false;
@@ -5262,10 +5255,12 @@ TemplateArgument ASTContext::getInjectedTemplateArg(NamedDecl *Param) {
Arg = TemplateArgument(E);
} else {
auto *TTP = cast<TemplateTemplateParmDecl>(Param);
+ TemplateName Name = getQualifiedTemplateName(
+ nullptr, /*TemplateKeyword=*/false, TemplateName(TTP));
if (TTP->isParameterPack())
- Arg = TemplateArgument(TemplateName(TTP), std::optional<unsigned>());
+ Arg = TemplateArgument(Name, std::optional<unsigned>());
else
- Arg = TemplateArgument(TemplateName(TTP));
+ Arg = TemplateArgument(Name);
}
if (Param->isTemplateParameterPack())
@@ -9304,7 +9299,8 @@ TemplateName ASTContext::getAssumedTemplateName(DeclarationName Name) const {
TemplateName ASTContext::getQualifiedTemplateName(NestedNameSpecifier *NNS,
bool TemplateKeyword,
TemplateName Template) const {
- assert(NNS && "Missing nested-name-specifier in qualified template name");
+ assert(Template.getKind() == TemplateName::Template ||
+ Template.getKind() == TemplateName::UsingTemplate);
// FIXME: Canonicalization?
llvm::FoldingSetNodeID ID;
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 95ffd4784641f..d952f7e181848 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -627,9 +627,10 @@ ClassTemplateDecl::getInjectedClassNameSpecialization() {
TemplateParameterList *Params = getTemplateParameters();
SmallVector<TemplateArgument, 16> TemplateArgs;
Context.getInjectedTemplateArgs(Params, TemplateArgs);
- CommonPtr->InjectedClassNameType
- = Context.getTemplateSpecializationType(TemplateName(this),
- TemplateArgs);
+ TemplateName Name = Context.getQualifiedTemplateName(
+ /*NNS=*/nullptr, /*TemplateKeyword=*/false, TemplateName(this));
+ CommonPtr->InjectedClassNameType =
+ Context.getTemplateSpecializationType(Name, TemplateArgs);
return CommonPtr->InjectedClassNameType;
}
diff --git a/clang/lib/AST/ODRHash.cpp b/clang/lib/AST/ODRHash.cpp
index 246e56231539a..1249531eab09f 100644
--- a/clang/lib/AST/ODRHash.cpp
+++ b/clang/lib/AST/ODRHash.cpp
@@ -146,10 +146,17 @@ void ODRHash::AddTemplateName(TemplateName Name) {
case TemplateName::Template:
AddDecl(Name.getAsTemplateDecl());
break;
+ case TemplateName::QualifiedTemplate: {
+ QualifiedTemplateName *QTN = Name.getAsQualifiedTemplateName();
+ if (NestedNameSpecifier *NNS = QTN->getQualifier())
+ AddNestedNameSpecifier(NNS);
+ AddBoolean(QTN->hasTemplateKeyword());
+ AddTemplateName(QTN->getUnderlyingTemplate());
+ break;
+ }
// TODO: Support these cases.
case TemplateName::OverloadedTemplate:
case TemplateName::AssumedTemplate:
- case TemplateName::QualifiedTemplate:
case TemplateName::DependentTemplate:
case TemplateName::SubstTemplateTemplateParm:
case TemplateName::SubstTemplateTemplateParmPack:
diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index a7ee973b7f7d0..4d4991d8c38b5 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -547,7 +547,7 @@ void TemplateArgument::print(const PrintingPolicy &Policy, raw_ostream &Out,
const auto *TTP = cast<TemplateTemplateParmDecl>(TD);
Out << "template-parameter-" << TTP->getDepth() << "-" << TTP->getIndex();
} else {
- TN.print(Out, Policy, TemplateName::Qualified::Fully);
+ TN.print(Out, Policy);
}
break;
}
diff --git a/clang/lib/AST/TemplateName.cpp b/clang/lib/AST/TemplateName.cpp
index 2f0e4181e9408..3aae998eceeb0 100644
--- a/clang/lib/AST/TemplateName.cpp
+++ b/clang/lib/AST/TemplateName.cpp
@@ -235,8 +235,8 @@ TemplateNameDependence TemplateName::getDependence() const {
auto D = TemplateNameDependence::None;
switch (getKind()) {
case TemplateName::NameKind::QualifiedTemplate:
- D |= toTemplateNameDependence(
- getAsQualifiedTemplateName()->getQualifier()->getDependence());
+ if (NestedNameSpecifier *NNS = getAsQualifiedTemplateName()->getQualifier())
+ D |= toTemplateNameDependence(NNS->getDependence());
break;
case TemplateName::NameKind::DependentTemplate:
D |= toTemplateNameDependence(
@@ -292,9 +292,8 @@ void TemplateName::Profile(llvm::FoldingSetNodeID &ID) {
void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
Qualified Qual) const {
- auto Kind = getKind();
- TemplateDecl *Template = nullptr;
- if (Kind == TemplateName::Template || Kind == TemplateName::UsingTemplate) {
+ if (NameKind Kind = getKind();
+ Kind == TemplateName::Template || Kind == TemplateName::UsingTemplate) {
// After `namespace ns { using std::vector }`, what is the fully-qualified
// name of the UsingTemplateName `vector` within ns?
//
@@ -304,46 +303,43 @@ void TemplateName::print(raw_ostream &OS, const PrintingPolicy &Policy,
// Similar to the UsingType behavior, using declarations are used to import
// names more often than to export them, thus using the original name is
// most useful in this case.
- Template = getAsTemplateDecl();
- }
-
- if (Template)
- if (Policy.CleanUglifiedParameters &&
- isa<TemplateTemplateParmDecl>(Template) && Template->getIdentifier())
- OS << Template->getIdentifier()->deuglifiedName();
- else if (Qual == Qualified::Fully &&
- getDependence() !=
- TemplateNameDependenceScope::DependentInstantiation)
- Template->printQualifiedName(OS, Policy);
- else
+ TemplateDecl *Template = getAsTemplateDecl();
+ if (Qual == Qualified::None)
OS << *Template;
- else if (QualifiedTemplateName *QTN = getAsQualifiedTemplateName()) {
- if (Qual == Qualified::Fully &&
- getDependence() !=
- TemplateNameDependenceScope::DependentInstantiation) {
- QTN->getUnderlyingTemplate().getAsTemplateDecl()->printQualifiedName(
- OS, Policy);
- return;
- }
- if (Qual == Qualified::AsWritten)
- QTN->getQualifier()->print(OS, Policy);
+ else
+ Template->printQualifiedName(OS, Policy);
+ } else if (QualifiedTemplateName *QTN = getAsQualifiedTemplateName()) {
+ if (NestedNameSpecifier *NNS = QTN->getQualifier();
+ Qual != Qualified::None && NNS)
+ NNS->print(OS, Policy);
if (QTN->hasTemplateKeyword())
OS << "template ";
- OS << *QTN->getUnderlyingTemplate().getAsTemplateDecl();
+
+ TemplateName Underlying = QTN->getUnderlyingTemplate();
+ assert(Underlying.getKind() == TemplateName::Template ||
+ Underlying.getKind() == TemplateName::UsingTemplate);
+
+ TemplateDecl *UTD = Underlying.getAsTemplateDecl();
+ if (IdentifierInfo *II = UTD->getIdentifier();
+ Policy.CleanUglifiedParameters && II &&
+ isa<TemplateTemplateParmDecl>(UTD))
+ OS << II->deuglifiedName();
+ else
+ OS << *UTD;
} else if (DependentTemplateName *DTN = getAsDependentTemplateName()) {
- if (Qual == Qualified::AsWritten && DTN->getQualifier())
- DTN->getQualifier()->print(OS, Policy);
+ if (NestedNameSpecifier *NNS = DTN->getQualifier())
+ NNS->print(OS, Policy);
OS << "template ";
if (DTN->isIdentifier())
OS << DTN->getIdentifier()->getName();
else
OS << "operator " << getOperatorSpelling(DTN->getOperator());
- } else if (SubstTemplateTemplateParmStorage *subst
- = getAsSubstTemplateTemplateParm()) {
+ } else if (SubstTemplateTemplateParmStorage *subst =
+ getAsSubstTemplateTemplateParm()) {
subst->getReplacement().print(OS, Policy, Qual);
- } else if (SubstTemplateTemplateParmPackStorage *SubstPack
- = getAsSubstTemplateTemplateParmPack())
+ } else if (SubstTemplateTemplateParmPackStorage *SubstPack =
+ getAsSubstTemplateTemplateParmPack())
OS << *SubstPack->getParameterPack();
else if (AssumedTemplateStorage *Assumed = getAsAssumedTemplateName()) {
Assumed->getDeclName().print(OS, Policy);
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index ed343ffb74124..1a1bef4dd297b 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1988,7 +1988,7 @@ void TextNodeDumper::VisitAutoType(const AutoType *T) {
void TextNodeDumper::VisitDeducedTemplateSpecializationType(
const DeducedTemplateSpecializationType *T) {
- if (T->getTemplateName().getKind() == TemplateName::UsingTemplate)
+ if (T->getTemplateName().getAsUsingShadowDecl())
OS << " using";
}
@@ -1996,7 +1996,7 @@ void TextNodeDumper::VisitTemplateSpecializationType(
const TemplateSpecializationType *T) {
if (T->isTypeAlias())
OS << " alias";
- if (T->getTemplateName().getKind() == TemplateName::UsingTemplate)
+ if (T->getTemplateName().getAsUsingShadowDecl())
OS << " using";
OS << " ";
T->getTemplateName().dump(OS);
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 04f105c128872..2097b29b7e0b6 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -4251,7 +4251,8 @@ TemplateSpecializationType::TemplateSpecializationType(
assert((T.getKind() == TemplateName::Template ||
T.getKind() == TemplateName::SubstTemplateTemplateParm ||
T.getKind() == TemplateName::SubstTemplateTemplateParmPack ||
- T.getKind() == TemplateName::UsingTemplate) &&
+ T.getKind() == TemplateName::UsingTemplate ||
+ T.getKind() == TemplateName::QualifiedTemplate) &&
"Unexpected template name for TemplateSpecializationType");
auto *TemplateArgs = reinterpret_cast<TemplateArgument *>(this + 1);
diff --git a/clang/lib/AST/TypePrinter.cpp b/clang/lib/AST/TypePrinter.cpp
index 5ed56b367a46a..58d01705d607b 100644
--- a/clang/lib/AST/TypePrinter.cpp
+++ b/clang/lib/AST/TypePrinter.cpp
@@ -1586,14 +1586,14 @@ void TypePrinter::printTemplateId(const TemplateSpecializationType *T,
IncludeStrongLifetimeRAII Strong(Policy);
TemplateDecl *TD = T->getTemplateName().getAsTemplateDecl();
- // FIXME: Null TD never excercised in test suite.
+ // FIXME: Null TD never exercised in test suite.
if (FullyQualify && TD) {
if (!Policy.SuppressScope)
AppendScope(TD->getDeclContext(), OS, TD->getDeclName());
OS << TD->getName();
} else {
- T->getTemplateName().print(OS, Policy);
+ T->getTemplateName().print(OS, Policy, TemplateName::Qualified::None);
}
DefaultTemplateArgsPolicyRAII TemplateArgs(Policy);
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 2a87b26f17a2b..e29ddd81a3f88 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -538,8 +538,9 @@ ParsedType Sema::getTypeName(const IdentifierInfo &II, SourceLocation NameLoc,
} else if (AllowDeducedTemplate) {
if (auto *TD = getAsTypeTemplateDecl(IIDecl)) {
assert(!FoundUsingShadow || FoundUsingShadow->getTargetDecl() == TD);
- TemplateName Template =
- FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD);
+ TemplateName Template = Context.getQualifiedTemplateName(
+ SS ? SS->getScopeRep() : nullptr, /*TemplateKeyword=*/false,
+ FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD));
T = Context.getDeducedTemplateSpecializationType(Template, QualType(),
false);
// Don't wrap in a further UsingType.
@@ -1137,12 +1138,10 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, CXXScopeSpec &SS,
dyn_cast<UsingShadowDecl>(*Result.begin());
assert(!FoundUsingShadow ||
TD == cast<TemplateDecl>(FoundUsingShadow->getTargetDecl()));
- Template =
- FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD);
- if (SS.isNotEmpty())
- Template = Context.getQualifiedTemplateName(SS.getScopeRep(),
- /*TemplateKeyword=*/false,
- Template);
+ Template = Context.getQualifiedTemplateName(
+ SS.getScopeRep(),
+ /*TemplateKeyword=*/false,
+ FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD));
} else {
// All results were non-template functions. This is a function template
// name.
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 8ab429e2a136e..631fd4e354927 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -11547,12 +11547,12 @@ bool Sema::CheckDeductionGuideDeclarator(Declarator &D, QualType &R,
TemplateName SpecifiedName = RetTST.getTypePtr()->getTemplateName();
bool TemplateMatches =
Context.hasSameTemplateName(SpecifiedName, GuidedTemplate);
- auto TKind = SpecifiedName.getKind();
- // A Using TemplateName can't actually be valid (either it's qualified, or
- // we're in the wrong scope). But we have diagnosed these problems
- // already.
- bool SimplyWritten = TKind == TemplateName::Template ||
- TKind == TemplateName::UsingTemplate;
+
+ const QualifiedTemplateName *Qualifiers =
+ SpecifiedName.getAsQualifiedTemplateName();
+ assert(Qualifiers && "expected QualifiedTemplate");
+ bool SimplyWritten = !Qualifiers->hasTemplateKeyword() &&
+ Qualifiers->getQualifier() == nullptr;
if (SimplyWritten && TemplateMatches)
AcceptableReturnType = true;
else {
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ded4f59833ac0..fb4154757775b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3284,10 +3284,10 @@ ExprResult Sema::BuildDeclarationNameExpr(
return CreateRecoveryExpr(NameInfo.getBeginLoc(), NameInfo.getEndLoc(), {});
}
- if (TemplateDecl *Template = dyn_cast<TemplateDecl>(D)) {
+ if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D)) {
// Specifically diagnose references to class templates that are missing
// a template argument list.
- diagnoseMissingTemplateArguments(TemplateName(Template), Loc);
+ diagnoseMissingTemplateArguments(SS, /*TemplateKeyword=*/false, TD, Loc);
return ExprError();
}
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 9aa60204bf29d..3ae1af26d0096 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1194,7 +1194,8 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
if (VarTemplateDecl *VarTempl = dyn_cast<VarTemplateDecl>(MemberDecl)) {
if (!TemplateArgs) {
- diagnoseMissingTemplateArguments(TemplateName(VarTempl), MemberLoc);
+ diagnoseMissingTemplateArguments(
+ SS, /*TemplateKeyword=*/TemplateKWLoc.isValid(), VarTempl, MemberLoc);
return ExprError();
}
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 39e9dbed0c3e0..3e3ed77de710e 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -292,7 +292,7 @@ TemplateNameKind Sema::isTemplateName(Scope *S,
Template =
FoundUsingShadow ? TemplateName(FoundUsingShadow) : TemplateName(TD);
assert(!FoundUsingShadow || FoundUsingShadow->getTargetDecl() == TD);
- if (SS.isSet() && !SS.isInvalid()) {
+ if (!SS.isInvalid()) {
NestedNameSpecifier *Qualifier = SS.getScopeRep();
Template = Context.getQualifiedTemplateName(Qualifier, hasTemplateKeyword,
Template);
@@ -342,8 +342,11 @@ bool Sema::isDeductionGuideName(Scope *S, const IdentifierInfo &Name,
if (!TD || !getAsTypeTemplateDecl(TD))
return false;
- if (Template)
- *Template = TemplateTy::make(TemplateName(TD));
+ if (Template) {
+ TemplateName Name = Context.getQualifiedTemplateName(
+ SS.getScopeRep(), /*TemplateKeyword=*/false, TemplateName(TD));
+ *Template = TemplateTy::make(Name);
+ }
return true;
}
@@ -983,10 +986,6 @@ ParsedTemplateArgument Sema::ActOnTemplateTypeArgument(TypeResult ParsedType) {
if (auto DTST = TL.getAs<DeducedTemplateSpecializationTypeLoc>()) {
TemplateName Name = DTST.getTypePtr()->getTemplateName();
- if (SS.isSet())
- Name = Context.getQualifiedTemplateName(SS.getScopeRep(),
- /*HasTemplateKeyword=*/false,
- Name);
ParsedTemplateArgument Result(SS, TemplateTy::make(Name),
DTST.getTemplateNameLoc());
if (EllipsisLoc.isValid())
@@ -5621,6 +5620,15 @@ void Sema::diagnoseMissingTemplateArguments(TemplateName Name,
}
}
+void Sema::diagnoseMissingTemplateArguments(const CXXScopeSpec &SS,
+ bool TemplateKeyword,
+ TemplateD...
[truncated]
|
mizvekov
force-pushed
the
users/mizvekov/clang-improve-ast-dumper-template-arguments
branch
from
031e7c2
to
f9892eb
Compare
mizvekov
force-pushed
the
users/mizvekov/clang-improved-template-name-qualifiers
branch
from
a1fe052
to
91fa7c9
Compare
llvmbot
added
the
libc++
There was a problem hiding this comment.
The change makes handling of template names more uniform, which overall looks like a great change.
However, we are changing the contract of QualifiedTemplateName (it can have a null qualifier now) and this seems both counter-intuitive from the API design perspective and potentially adding bugs in various places where the qualifier is used without checking for null.
I am not necessarily saying that's wrong, but maybe we need a different name for the class or a different node for the case where there is no qualifier.
We could discuss this in the relevant comment thread.
mizvekov
force-pushed
the
users/mizvekov/clang-improve-ast-dumper-template-arguments
branch
from
f9892eb
to
86e3852
Compare
mizvekov
force-pushed
the
users/mizvekov/clang-improved-template-name-qualifiers
branch
from
91fa7c9
to
40e3c0f
Compare
Endilll
reviewed
May 27, 2024
ilya-biryukov
approved these changes
May 29, 2024
mizvekov
force-pushed
the
users/mizvekov/clang-improve-ast-dumper-template-arguments
branch
from
86e3852
to
fabcce0
Compare
mizvekov
force-pushed
the
users/mizvekov/clang-improved-template-name-qualifiers
branch
from
40e3c0f
to
2f2aa69
Compare
Base automatically changed from
users/mizvekov/clang-improve-ast-dumper-template-arguments
to
main
May 29, 2024 18:23
This patch improves the preservation of qualifiers and loss of type sugar in TemplateNames. This problem is analogous to https://reviews.llvm.org/D112374 and this patch takes a very similar approach to that patch, except the impact here is much lesser. When a TemplateName was written bare, without qualifications, we wouldn't produce a QualifiedTemplate which could be used to disambiguate it from a Canonical TemplateName. This had effects in the TemplateName printer, which had workarounds to deal with this, and wouldn't print the TemplateName as-written in most situations. There are also some related fixes to help preserve this type sugar along the way into diagnostics, so that this patch can be properly tested. - Fix dropping the template keyword. - Fix type deduction to preserve sugar in TST TemplateNames.
mizvekov
force-pushed
the
users/mizvekov/clang-improved-template-name-qualifiers
branch
from
2f2aa69
to
8322ce1
Compare
You can test this locally with the following command:git-clang-format --diff 1a2f3309765fdc143fdc3809211fb85d2e2ca341 8322ce12520d1d4ab96b58195bd91f62a7b95c06 -- clang/include/clang/AST/TemplateName.h clang/include/clang/Sema/Sema.h clang/lib/AST/ASTContext.cpp clang/lib/AST/DeclTemplate.cpp clang/lib/AST/ODRHash.cpp clang/lib/AST/TemplateBase.cpp clang/lib/AST/TemplateName.cpp clang/lib/AST/TextNodeDumper.cpp clang/lib/AST/Type.cpp clang/lib/AST/TypePrinter.cpp clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExpr.cpp clang/lib/Sema/SemaExprMember.cpp clang/lib/Sema/SemaTemplate.cpp clang/lib/Sema/SemaTemplateDeduction.cpp clang/lib/Sema/SemaType.cpp clang/lib/Sema/TreeTransform.h clang/test/AST/ast-dump-ctad-alias.cpp clang/test/AST/ast-dump-decl.cpp clang/test/AST/ast-dump-expr.cpp clang/test/AST/ast-dump-template-decls.cpp clang/test/AST/ast-dump-template-name.cpp clang/test/AST/ast-dump-using-template.cpp clang/test/CXX/drs/cwg1xx.cpp clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp clang/test/CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp clang/test/Index/print-type.cpp clang/test/OpenMP/declare_mapper_messages.cpp clang/test/Parser/cxx-template-template-recovery.cpp clang/test/SemaCXX/cxx1y-variable-templates_in_class.cpp clang/test/SemaTemplate/cwg2398.cpp clang/test/SemaTemplate/instantiate-requires-expr.cpp clang/test/SemaTemplate/nested-implicit-deduction-guides.cpp clang/unittests/AST/TemplateNameTest.cpp libcxx/test/std/containers/associative/map/map.cons/deduct.verify.cpp libcxx/test/std/containers/associative/multimap/multimap.cons/deduct.verify.cpp libcxx/test/std/containers/associative/multiset/multiset.cons/deduct.verify.cpp libcxx/test/std/containers/associative/set/set.cons/deduct.verify.cpp libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/deduct.verify.cpp libcxx/test/std/containers/container.adaptors/queue/queue.cons/deduct.verify.cpp libcxx/test/std/containers/container.adaptors/stack/stack.cons/deduct.verify.cpp libcxx/test/std/containers/sequences/array/array.cons/deduct.verify.cpp libcxx/test/std/containers/sequences/deque/deque.cons/deduct.verify.cpp libcxx/test/std/containers/sequences/forwardlist/forwardlist.cons/deduct.verify.cpp libcxx/test/std/containers/sequences/list/list.cons/deduct.verify.cpp libcxx/test/std/containers/sequences/vector/vector.cons/deduct.verify.cpp libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/deduct.verify.cpp libcxx/test/std/containers/unord/unord.multimap/unord.multimap.cnstr/deduct.verify.cpp libcxx/test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.verify.cpp libcxx/test/std/containers/unord/unord.set/unord.set.cnstr/deduct.verify.cpp libcxx/test/std/ranges/range.adaptors/range.join/ctad.verify.cpp libcxx/test/std/re/re.regex/re.regex.construct/deduct.verify.cpp libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.verify.cppView the diff from clang-format here.diff --git a/libcxx/test/std/containers/associative/map/map.cons/deduct.verify.cpp b/libcxx/test/std/containers/associative/map/map.cons/deduct.verify.cpp
index b314e6fba6..8dfc015685 100644
--- a/libcxx/test/std/containers/associative/map/map.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/associative/map/map.cons/deduct.verify.cpp
@@ -42,63 +42,65 @@ int main(int, char**)
{
{
// cannot deduce Key and T from nothing
- std::map m; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ std::map
+ m; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// cannot deduce Key and T from just (Compare)
std::map m(std::less<int>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// cannot deduce Key and T from just (Compare, Allocator)
std::map m(std::less<int>{}, std::allocator<PC>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// cannot deduce Key and T from just (Allocator)
std::map m(std::allocator<PC>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// refuse to rebind the allocator if Allocator::value_type is not exactly what we expect
const P arr[] = { {1,1L}, {2,2L}, {3,3L} };
std::map m(arr, arr + 3, std::allocator<P>());
- // expected-error-re@map:*{{static assertion failed{{( due to requirement '.*')?}}{{.*}}Allocator::value_type must be same type as value_type}}
+ // expected-error-re@map:*{{static assertion failed{{( due to requirement '.*')?}}{{.*}}Allocator::value_type must be same type as value_type}}
}
{
// cannot convert from some arbitrary unrelated type
NotAnAllocator a;
- std::map m(a); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ std::map m(
+ a); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::map m{ {1,1L}, {2,2L}, {3,3L} };
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::map m({ {1,1L}, {2,2L}, {3,3L} }, std::less<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::map m({ {1,1L}, {2,2L}, {3,3L} }, std::less<int>(), std::allocator<PC>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::map m({ {1,1L}, {2,2L}, {3,3L} }, std::allocator<PC>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// since we have parens, not braces, this deliberately does not find the initializer_list constructor
std::map m(P{1,1L});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
{
// since we have parens, not braces, this deliberately does not find the initializer_list constructor
std::map m(PC{1,1L});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}map'}}
}
return 0;
diff --git a/libcxx/test/std/containers/associative/multimap/multimap.cons/deduct.verify.cpp b/libcxx/test/std/containers/associative/multimap/multimap.cons/deduct.verify.cpp
index 795ac19240..18893873d5 100644
--- a/libcxx/test/std/containers/associative/multimap/multimap.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/associative/multimap/multimap.cons/deduct.verify.cpp
@@ -42,22 +42,23 @@ int main(int, char**)
{
{
// cannot deduce Key and T from nothing
- std::multimap m; // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ std::multimap
+ m; // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// cannot deduce Key and T from just (Compare)
std::multimap m(std::less<int>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// cannot deduce Key and T from just (Compare, Allocator)
std::multimap m(std::less<int>{}, std::allocator<PC>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// cannot deduce Key and T from just (Allocator)
std::multimap m(std::allocator<PC>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// refuse to rebind the allocator if Allocator::value_type is not exactly what we expect
@@ -68,37 +69,38 @@ int main(int, char**)
{
// cannot convert from some arbitrary unrelated type
NotAnAllocator a;
- std::multimap m(a); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ std::multimap m(
+ a); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::multimap m{ {1,1L}, {2,2L}, {3,3L} };
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::multimap m({ {1,1L}, {2,2L}, {3,3L} }, std::less<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::multimap m({ {1,1L}, {2,2L}, {3,3L} }, std::less<int>(), std::allocator<PC>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// cannot deduce that the inner braced things should be std::pair and not something else
std::multimap m({ {1,1L}, {2,2L}, {3,3L} }, std::allocator<PC>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// since we have parens, not braces, this deliberately does not find the initializer_list constructor
std::multimap m(P{1,1L});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
{
// since we have parens, not braces, this deliberately does not find the initializer_list constructor
std::multimap m(PC{1,1L});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}multimap'}}
}
return 0;
diff --git a/libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/deduct.verify.cpp b/libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/deduct.verify.cpp
index 73487597ca..dbdcebcbbc 100644
--- a/libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/deduct.verify.cpp
@@ -22,33 +22,49 @@ int main(int, char**)
{
// queue(Compare, Container, const Alloc);
// The '45' is not an allocator
- std::priority_queue pri(std::greater<int>(), std::deque<int>({1,2,3}), 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
+std::priority_queue pri(
+ std::greater<int>(),
+ std::deque<int>({1, 2, 3}),
+ 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
}
{
// queue(const queue&, const Alloc&);
// The '45' is not an allocator
std::priority_queue<int> source;
- std::priority_queue pri(source, 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
+ std::priority_queue pri(
+ source,
+ 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
}
{
// priority_queue(Iter, Iter, Comp)
// int is not an iterator
- std::priority_queue pri(15, 17, std::greater<double>()); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
+std::priority_queue pri(
+ 15,
+ 17,
+ std::greater<
+ double>()); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
}
{
// priority_queue(Iter, Iter, Comp, Container)
// float is not an iterator
- std::priority_queue pri(23.f, 2.f, std::greater<float>(), std::deque<float>()); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
+std::priority_queue pri(
+ 23.f,
+ 2.f,
+ std::greater<float>(),
+ std::deque<
+ float>()); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
}
// Test the implicit deduction guides
{
// priority_queue (allocator &)
- std::priority_queue pri((std::allocator<int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
-// Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
+std::priority_queue pri(
+ (std::allocator<
+ int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}priority_queue'}}
+ // Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
// Also, we can't use {} instead of parens, because that constructs a
// stack<allocator<int>, allocator<allocator<int>>>
}
diff --git a/libcxx/test/std/containers/container.adaptors/queue/queue.cons/deduct.verify.cpp b/libcxx/test/std/containers/container.adaptors/queue/queue.cons/deduct.verify.cpp
index f85b2cbeb2..1ef8f8a7e2 100644
--- a/libcxx/test/std/containers/container.adaptors/queue/queue.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/container.adaptors/queue/queue.cons/deduct.verify.cpp
@@ -22,21 +22,27 @@ int main(int, char**)
{
// queue(const Container&, const Alloc&);
// The '45' is not an allocator
- std::queue que(std::list<int>{1,2,3}, 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}queue'}}
+std::queue que(
+ std::list<int>{1, 2, 3},
+ 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}queue'}}
}
{
// queue(const queue&, const Alloc&);
// The '45' is not an allocator
std::queue<int> source;
- std::queue que(source, 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}queue'}}
+ std::queue que(
+ source,
+ 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}queue'}}
}
// Test the implicit deduction guides
{
// queue (allocator &)
- std::queue que((std::allocator<int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}queue'}}
-// Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
+std::queue que(
+ (std::allocator<
+ int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}queue'}}
+ // Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
// Also, we can't use {} instead of parens, because that constructs a
// stack<allocator<int>, allocator<allocator<int>>>
}
diff --git a/libcxx/test/std/containers/container.adaptors/stack/stack.cons/deduct.verify.cpp b/libcxx/test/std/containers/container.adaptors/stack/stack.cons/deduct.verify.cpp
index 390df86dd0..3523272e19 100644
--- a/libcxx/test/std/containers/container.adaptors/stack/stack.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/container.adaptors/stack/stack.cons/deduct.verify.cpp
@@ -28,21 +28,27 @@ int main(int, char**)
{
// stack(const Container&, const Alloc&);
// The '45' is not an allocator
- std::stack stk(std::list<int>({1,2,3}), 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}stack'}}
+std::stack stk(
+ std::list<int>({1, 2, 3}),
+ 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}stack'}}
}
{
// stack(const stack&, const Alloc&);
// The '45' is not an allocator
std::stack<int> source;
- std::stack stk(source, 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}stack'}}
+ std::stack stk(
+ source,
+ 45); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}stack'}}
}
// Test the implicit deduction guides
{
// stack (allocator &)
- std::stack stk((std::allocator<int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}stack'}}
-// Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
+std::stack stk(
+ (std::allocator<
+ int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}stack'}}
+ // Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
// Also, we can't use {} instead of parens, because that constructs a
// stack<allocator<int>, allocator<allocator<int>>>
}
diff --git a/libcxx/test/std/containers/sequences/array/array.cons/deduct.verify.cpp b/libcxx/test/std/containers/sequences/array/array.cons/deduct.verify.cpp
index f59b761fad..3fdee4a2b2 100644
--- a/libcxx/test/std/containers/sequences/array/array.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/sequences/array/array.cons/deduct.verify.cpp
@@ -24,7 +24,10 @@
int main(int, char**)
{
{
- std::array arr{1,2,3L}; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}array'}}
+ std::array arr{
+ 1,
+ 2,
+ 3L}; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}array'}}
}
return 0;
diff --git a/libcxx/test/std/containers/sequences/deque/deque.cons/deduct.verify.cpp b/libcxx/test/std/containers/sequences/deque/deque.cons/deduct.verify.cpp
index f65e230112..13a1f5d87a 100644
--- a/libcxx/test/std/containers/sequences/deque/deque.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/sequences/deque/deque.cons/deduct.verify.cpp
@@ -29,8 +29,10 @@ int main(int, char**)
// Test the implicit deduction guides
{
// deque (allocator &)
- std::deque deq((std::allocator<int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}deque'}}
-// Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
+std::deque deq(
+ (std::allocator<
+ int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}deque'}}
+ // Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
// Also, we can't use {} instead of parens, because that constructs a
// deque<allocator<int>, allocator<allocator<int>>>
}
diff --git a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.cons/deduct.verify.cpp b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.cons/deduct.verify.cpp
index b3c3f73270..73ebaf9588 100644
--- a/libcxx/test/std/containers/sequences/forwardlist/forwardlist.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/sequences/forwardlist/forwardlist.cons/deduct.verify.cpp
@@ -29,8 +29,10 @@ int main(int, char**)
// Test the implicit deduction guides
{
// forward_list (allocator &)
- std::forward_list fwl((std::allocator<int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}forward_list'}}
-// Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
+std::forward_list fwl(
+ (std::allocator<
+ int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}forward_list'}}
+ // Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
// Also, we can't use {} instead of parens, because that constructs a
// forward_list<allocator<int>, allocator<allocator<int>>>
}
diff --git a/libcxx/test/std/containers/sequences/list/list.cons/deduct.verify.cpp b/libcxx/test/std/containers/sequences/list/list.cons/deduct.verify.cpp
index 370cd38612..56cce57e4d 100644
--- a/libcxx/test/std/containers/sequences/list/list.cons/deduct.verify.cpp
+++ b/libcxx/test/std/containers/sequences/list/list.cons/deduct.verify.cpp
@@ -29,8 +29,10 @@ int main(int, char**)
// Test the implicit deduction guides
{
// list (allocator &)
- std::list lst((std::allocator<int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}list'}}
-// Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
+std::list lst(
+ (std::allocator<
+ int>())); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}list'}}
+ // Note: The extra parens are necessary, since otherwise clang decides it is a function declaration.
// Also, we can't use {} instead of parens, because that constructs a
// deque<allocator<int>, allocator<allocator<int>>>
}
diff --git a/libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/deduct.verify.cpp b/libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/deduct.verify.cpp
index dc0ffd2681..35bafd2d5f 100644
--- a/libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/deduct.verify.cpp
+++ b/libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/deduct.verify.cpp
@@ -64,41 +64,43 @@ int main(int, char**)
using P = std::pair<const int, int>;
{
// cannot deduce Key from nothing
- std::unordered_map m; // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ std::unordered_map
+ m; // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
{
// cannot deduce Key from just (Size)
- std::unordered_map m(42); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ std::unordered_map m(
+ 42); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
{
// cannot deduce Key from just (Size, Hash)
std::unordered_map m(42, std::hash<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred)
std::unordered_map m(42, std::hash<int>(), std::equal_to<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred, Allocator)
std::unordered_map m(42, std::hash<int>(), std::equal_to<int>(), std::allocator<P>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
{
// cannot deduce Key from just (Allocator)
std::unordered_map m(std::allocator<P>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
{
// cannot deduce Key from just (Size, Allocator)
std::unordered_map m(42, std::allocator<P>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
{
// cannot deduce Key from just (Size, Hash, Allocator)
std::unordered_map m(42, std::hash<int>(), std::allocator<P>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_map'}}
}
return 0;
diff --git a/libcxx/test/std/containers/unord/unord.multimap/unord.multimap.cnstr/deduct.verify.cpp b/libcxx/test/std/containers/unord/unord.multimap/unord.multimap.cnstr/deduct.verify.cpp
index efcbbbce0e..213a360e72 100644
--- a/libcxx/test/std/containers/unord/unord.multimap/unord.multimap.cnstr/deduct.verify.cpp
+++ b/libcxx/test/std/containers/unord/unord.multimap/unord.multimap.cnstr/deduct.verify.cpp
@@ -64,41 +64,43 @@ int main(int, char**)
using P = std::pair<const int, int>;
{
// cannot deduce Key from nothing
- std::unordered_multimap m; // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ std::unordered_multimap
+ m; // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
{
// cannot deduce Key from just (Size)
- std::unordered_multimap m(42); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ std::unordered_multimap m(
+ 42); // expected-error-re{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
{
// cannot deduce Key from just (Size, Hash)
std::unordered_multimap m(42, std::hash<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred)
std::unordered_multimap m(42, std::hash<int>(), std::equal_to<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred, Allocator)
std::unordered_multimap m(42, std::hash<int>(), std::equal_to<int>(), std::allocator<P>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
{
// cannot deduce Key from just (Allocator)
std::unordered_multimap m(std::allocator<P>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
{
// cannot deduce Key from just (Size, Allocator)
std::unordered_multimap m(42, std::allocator<P>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
{
// cannot deduce Key from just (Size, Hash, Allocator)
std::unordered_multimap m(42, std::hash<int>(), std::allocator<P>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multimap'}}
}
return 0;
diff --git a/libcxx/test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.verify.cpp b/libcxx/test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.verify.cpp
index 8fd7d1d6c2..4941c7c4e3 100644
--- a/libcxx/test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.verify.cpp
+++ b/libcxx/test/std/containers/unord/unord.multiset/unord.multiset.cnstr/deduct.verify.cpp
@@ -55,42 +55,42 @@ int main(int, char**)
{
// cannot deduce Key from nothing
std::unordered_multiset s;
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
{
// cannot deduce Key from just (Size)
std::unordered_multiset s(42);
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
{
// cannot deduce Key from just (Size, Hash)
std::unordered_multiset s(42, std::hash<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred)
std::unordered_multiset s(42, std::hash<int>(), std::equal_to<>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred, Allocator)
std::unordered_multiset s(42, std::hash<int>(), std::equal_to<>(), std::allocator<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
{
// cannot deduce Key from just (Allocator)
std::unordered_multiset s(std::allocator<int>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
{
// cannot deduce Key from just (Size, Allocator)
std::unordered_multiset s(42, std::allocator<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
{
// cannot deduce Key from just (Size, Hash, Allocator)
std::unordered_multiset s(42, std::hash<short>(), std::allocator<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_multiset'}}
}
return 0;
diff --git a/libcxx/test/std/containers/unord/unord.set/unord.set.cnstr/deduct.verify.cpp b/libcxx/test/std/containers/unord/unord.set/unord.set.cnstr/deduct.verify.cpp
index 26e5f9ae6c..760ffba880 100644
--- a/libcxx/test/std/containers/unord/unord.set/unord.set.cnstr/deduct.verify.cpp
+++ b/libcxx/test/std/containers/unord/unord.set/unord.set.cnstr/deduct.verify.cpp
@@ -55,42 +55,42 @@ int main(int, char**)
{
// cannot deduce Key from nothing
std::unordered_set s;
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
{
// cannot deduce Key from just (Size)
std::unordered_set s(42);
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
{
// cannot deduce Key from just (Size, Hash)
std::unordered_set s(42, std::hash<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred)
std::unordered_set s(42, std::hash<int>(), std::equal_to<>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
{
// cannot deduce Key from just (Size, Hash, Pred, Allocator)
std::unordered_set s(42, std::hash<int>(), std::equal_to<>(), std::allocator<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
{
// cannot deduce Key from just (Allocator)
std::unordered_set s(std::allocator<int>{});
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
{
// cannot deduce Key from just (Size, Allocator)
std::unordered_set s(42, std::allocator<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
{
// cannot deduce Key from just (Size, Hash, Allocator)
std::unordered_set s(42, std::hash<short>(), std::allocator<int>());
- // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
+ // expected-error-re@-1{{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}unordered_set'}}
}
return 0;
diff --git a/libcxx/test/std/ranges/range.adaptors/range.join/ctad.verify.cpp b/libcxx/test/std/ranges/range.adaptors/range.join/ctad.verify.cpp
index de0c108e98..de208ba40a 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.join/ctad.verify.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.join/ctad.verify.cpp
@@ -27,5 +27,6 @@ struct Range {
void testExplicitCTAD() {
Range<Range<int>> r;
- std::ranges::join_view v = r; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::ranges::)?}}join_view'}}
+ std::ranges::join_view v =
+ r; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::ranges::)?}}join_view'}}
}
diff --git a/libcxx/test/std/re/re.regex/re.regex.construct/deduct.verify.cpp b/libcxx/test/std/re/re.regex/re.regex.construct/deduct.verify.cpp
index 8ef87cfc64..59ba3a2238 100644
--- a/libcxx/test/std/re/re.regex/re.regex.construct/deduct.verify.cpp
+++ b/libcxx/test/std/re/re.regex/re.regex.construct/deduct.verify.cpp
@@ -27,13 +27,19 @@ int main(int, char**)
{
// basic_regex(ForwardIterator, ForwardIterator)
// <int> is not an iterator
- std::basic_regex re(23, 34); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}basic_regex'}}
+ std::basic_regex re(
+ 23,
+ 34); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}basic_regex'}}
}
{
// basic_regex(ForwardIterator, ForwardIterator, flag_type)
// <double> is not an iterator
- std::basic_regex re(23.0, 34.0, std::regex_constants::basic); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}basic_regex'}}
+ std::basic_regex re(
+ 23.0,
+ 34.0,
+ std::regex_constants::
+ basic); // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}basic_regex'}}
}
return 0;
diff --git a/libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.verify.cpp b/libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.verify.cpp
index 364f9b2e95..9a89351e13 100644
--- a/libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.verify.cpp
+++ b/libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.verify.cpp
@@ -25,7 +25,8 @@ int main(int, char**)
// Test the implicit deduction guides
{
// optional()
- std::optional opt; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}optional'}}
+std::optional
+ opt; // expected-error-re {{no viable constructor or deduction guide for deduction of template arguments of '{{(std::)?}}optional'}}
}
{
|
mizvekov
deleted the
users/mizvekov/clang-improved-template-name-qualifiers
branch
|
This change seems to have some compile-time overhead. I'm not seeing a lot on CTMark, but during the clang bootstrap there are many files with ~1% regressions (http://llvm-compile-time-tracker.com/compare_clang.php?from=ec8fe598a94d2826f8e4f79367a5a45a6b32d284&to=9c4a716c1292096fcbdf415b63b7b0122b03310f&stat=instructions%3Au&sortBy=interestingness). Just an FYI in case this is unexpected. |
|
@nikic Thanks for the heads-up. I expected some impact. There are some ideas to regain that loss in follow up work. |
|
@mizvekov After I apply you changes in this patch, #91677 is still crash(although it makes sense with template <typename> struct t1 {
template <typename>
struct t2 {};
};
template <typename T>
t1<T>::template t2<T> f1();
void f2() {
f1<bool>();
}The reason seems that we transform |
Sure, but let's take the discussion to the other PR. |
|
This commit broke the UBSan buildbot: https://lab.llvm.org/buildbot/#/builders/85/builds/24378 Can you please take a look? |
mizvekov
added a commit
to mizvekov/llvm-project
that referenced
this pull request
May 31, 2024
This change was missed in llvm#93433.
|
so after this patch, clang seems to be crashing on: template <typename T, typename Y> struct map {};
template <typename T> class foo {};
template <template <typename...> class MapType, typename Value>
Value bar(MapType<int, Value> map);
template <template <typename...> class MapType, typename Value>
Value bar(MapType<int, foo<Value>> map);
void aux() {
map<int, foo<int>> input;
bar(input);
}stack trace: @yozhu this is the same assertion you mentioned in #94659, but doesn't really involve default template args, so I am not sure if this is the same kind of reproducer. |
Thanks for the reproducer. Yeah this is about default argument deduction, which doesn't involve default arguments as written in source. I confirm the crash and it's indeed caused by changes in this patch. The fix is very simple and I will be posting a patch shortly. |
mizvekov
added a commit
to mizvekov/llvm-project
that referenced
this pull request
Jun 7, 2024
This fixes a regression introduced with the changes in llvm#93433 around preservation of TemplateName sugar in template type deduction. Since the argument side TST is non-canonical, we have to extract the arguments from it's canonical type. This was done for the deduction of the TST arguments, but we missed it for the default arguments used in the deduction of the TST name.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch improves the preservation of qualifiers and loss of type sugar in TemplateNames.
This problem is analogous to https://reviews.llvm.org/D112374 and this patch takes a very similar approach to that patch, except the impact here is much lesser.
When a TemplateName was written bare, without qualifications, we wouldn't produce a QualifiedTemplate which could be used to disambiguate it from a Canonical TemplateName. This had effects in the TemplateName printer, which had workarounds to deal with this, and wouldn't print the TemplateName as-written in most situations.
There are also some related fixes to help preserve this type sugar along the way into diagnostics, so that this patch can be properly tested.