wchilders created this revision.
wchilders added reviewers: yaxunl, hans, rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
wchilders requested review of this revision.
Previously expressions were being printed in diagnostics via an implicit
conversion to an expression template argument -- which would then be printed
via some code with a comment suggesting the code path was only for completeness
and rarely/never used; this was both roundabout, and subtle. This patch
provides a more direct overload, though the patch is still imperfect, as it
does not solve the associated LangOpts issue.
Additionally, to prevent similar issues in the future, this patch makes
conversions from expressions to template arguments explicit.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D90976
Files:
clang/include/clang/AST/ASTNodeTraverser.h
clang/include/clang/AST/Expr.h
clang/include/clang/AST/TemplateBase.h
clang/lib/AST/Expr.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/TemplateBase.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/lib/Sema/TreeTransform.h
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3542,7 +3542,7 @@
if (Result.isInvalid())
return TemplateArgumentLoc();
- return TemplateArgumentLoc(Result.get(), Result.get());
+ return TemplateArgumentLoc(TemplateArgument(Result.get()), Result.get());
}
case TemplateArgument::Template:
@@ -3822,7 +3822,8 @@
Expr *Pattern = Expansion->getPattern();
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
- getSema().collectUnexpandedParameterPacks(Pattern, Unexpanded);
+ getSema().collectUnexpandedParameterPacks(
+ TemplateArgument(Pattern), Unexpanded);
assert(!Unexpanded.empty() && "Pack expansion without parameter packs?");
// Determine whether the set of unexpanded parameter packs can and should
@@ -12420,7 +12421,8 @@
->getTypeLoc()
.castAs<PackExpansionTypeLoc>();
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
- SemaRef.collectUnexpandedParameterPacks(OldVD->getInit(), Unexpanded);
+ SemaRef.collectUnexpandedParameterPacks(
+ TemplateArgument(OldVD->getInit()), Unexpanded);
// Determine whether the set of unexpanded parameter packs can and should
// be expanded.
@@ -13016,8 +13018,8 @@
E->getPackLoc());
if (DRE.isInvalid())
return ExprError();
- ArgStorage = new (getSema().Context) PackExpansionExpr(
- getSema().Context.DependentTy, DRE.get(), E->getPackLoc(), None);
+ ArgStorage = TemplateArgument(new (getSema().Context) PackExpansionExpr(
+ getSema().Context.DependentTy, DRE.get(), E->getPackLoc(), None));
}
PackArgs = ArgStorage;
}
@@ -13155,7 +13157,8 @@
Expr *Pattern = E->getPattern();
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
- getSema().collectUnexpandedParameterPacks(Pattern, Unexpanded);
+ getSema().collectUnexpandedParameterPacks(
+ TemplateArgument(Pattern), Unexpanded);
assert(!Unexpanded.empty() && "Pack expansion without parameter packs?");
// Determine whether the set of unexpanded parameter packs can and should
@@ -13351,8 +13354,10 @@
if (OrigElement.isPackExpansion()) {
// This key/value element is a pack expansion.
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
- getSema().collectUnexpandedParameterPacks(OrigElement.Key, Unexpanded);
- getSema().collectUnexpandedParameterPacks(OrigElement.Value, Unexpanded);
+ getSema().collectUnexpandedParameterPacks(
+ TemplateArgument(OrigElement.Key), Unexpanded);
+ getSema().collectUnexpandedParameterPacks(
+ TemplateArgument(OrigElement.Value), Unexpanded);
assert(!Unexpanded.empty() && "Pack expansion without parameter packs?");
// Determine whether the set of unexpanded parameter packs can
Index: clang/lib/Sema/SemaTemplateVariadic.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateVariadic.cpp
+++ clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -1089,7 +1089,7 @@
Expr *Pattern = Expansion->getPattern();
Ellipsis = Expansion->getEllipsisLoc();
NumExpansions = Expansion->getNumExpansions();
- return TemplateArgumentLoc(Pattern, Pattern);
+ return TemplateArgumentLoc(TemplateArgument(Pattern), Pattern);
}
case TemplateArgument::TemplateExpansion:
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -109,8 +109,8 @@
SmallVector<UnexpandedParameterPack, 2> Unexpanded;
if (Aligned->isAlignmentExpr())
- S.collectUnexpandedParameterPacks(Aligned->getAlignmentExpr(),
- Unexpanded);
+ S.collectUnexpandedParameterPacks(
+ TemplateArgument(Aligned->getAlignmentExpr()), Unexpanded);
else
S.collectUnexpandedParameterPacks(Aligned->getAlignmentType()->getTypeLoc(),
Unexpanded);
@@ -5402,7 +5402,8 @@
TypeLoc BaseTL = Init->getTypeSourceInfo()->getTypeLoc();
SmallVector<UnexpandedParameterPack, 4> Unexpanded;
collectUnexpandedParameterPacks(BaseTL, Unexpanded);
- collectUnexpandedParameterPacks(Init->getInit(), Unexpanded);
+ collectUnexpandedParameterPacks(
+ TemplateArgument(Init->getInit()), Unexpanded);
bool ShouldExpand = false;
bool RetainExpansion = false;
Optional<unsigned> NumExpansions;
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -460,8 +460,9 @@
S.Context.NullPtrTy, NTTP->getLocation()),
NullPtrType, CK_NullToPointer)
.get();
+ TemplateArgument TV(Value);
return DeduceNonTypeTemplateArgument(S, TemplateParams, NTTP,
- DeducedTemplateArgument(Value),
+ DeducedTemplateArgument(TV),
Value->getType(), Info, Deduced);
}
@@ -473,8 +474,9 @@
Sema &S, TemplateParameterList *TemplateParams,
const NonTypeTemplateParmDecl *NTTP, Expr *Value, TemplateDeductionInfo &Info,
SmallVectorImpl<DeducedTemplateArgument> &Deduced) {
+ TemplateArgument TV(Value);
return DeduceNonTypeTemplateArgument(S, TemplateParams, NTTP,
- DeducedTemplateArgument(Value),
+ DeducedTemplateArgument(TV),
Value->getType(), Info, Deduced);
}
Index: clang/lib/AST/TemplateBase.cpp
===================================================================
--- clang/lib/AST/TemplateBase.cpp
+++ clang/lib/AST/TemplateBase.cpp
@@ -319,7 +319,7 @@
return getAsType()->castAs<PackExpansionType>()->getPattern();
case Expression:
- return cast<PackExpansionExpr>(getAsExpr())->getPattern();
+ return TemplateArgument(cast<PackExpansionExpr>(getAsExpr())->getPattern());
case TemplateExpansion:
return TemplateArgument(getAsTemplateOrTemplatePattern());
@@ -482,16 +482,7 @@
return DB << Arg.getAsTemplateOrTemplatePattern() << "...";
case TemplateArgument::Expression: {
- // This shouldn't actually ever happen, so it's okay that we're
- // regurgitating an expression here.
- // FIXME: We're guessing at LangOptions!
- SmallString<32> Str;
- llvm::raw_svector_ostream OS(Str);
- LangOptions LangOpts;
- LangOpts.CPlusPlus = true;
- PrintingPolicy Policy(LangOpts);
- Arg.getAsExpr()->printPretty(OS, nullptr, Policy);
- return DB << OS.str();
+ return DB << Arg.getAsExpr();
}
case TemplateArgument::Pack: {
Index: clang/lib/AST/JSONNodeDumper.cpp
===================================================================
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -872,7 +872,7 @@
if (D->hasDefaultArgument())
JOS.attributeObject("defaultArg", [=] {
- Visit(D->getDefaultArgument(), SourceRange(),
+ Visit(TemplateArgument(D->getDefaultArgument()), SourceRange(),
D->getDefaultArgStorage().getInheritedFrom(),
D->defaultArgumentWasInherited() ? "inherited from" : "previous");
});
Index: clang/lib/AST/ItaniumMangle.cpp
===================================================================
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3487,8 +3487,8 @@
// U<Len>matrix_type<row expr><column expr><element type>
StringRef VendorQualifier = "matrix_type";
Out << "U" << VendorQualifier.size() << VendorQualifier;
- mangleTemplateArg(T->getRowExpr());
- mangleTemplateArg(T->getColumnExpr());
+ mangleTemplateArg(TemplateArgument(T->getRowExpr()));
+ mangleTemplateArg(TemplateArgument(T->getColumnExpr()));
mangleType(T->getElementType());
}
Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -4837,3 +4837,15 @@
alignof(OMPIteratorExpr));
return new (Mem) OMPIteratorExpr(EmptyShell(), NumIterators);
}
+
+const StreamingDiagnostic &clang::operator<<(const StreamingDiagnostic &DB,
+ const Expr *E) {
+ // FIXME: We're guessing at LangOptions!
+ SmallString<32> Str;
+ llvm::raw_svector_ostream OS(Str);
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus = true;
+ PrintingPolicy Policy(LangOpts);
+ E->printPretty(OS, nullptr, Policy);
+ return DB << OS.str();
+}
Index: clang/include/clang/AST/TemplateBase.h
===================================================================
--- clang/include/clang/AST/TemplateBase.h
+++ clang/include/clang/AST/TemplateBase.h
@@ -217,7 +217,7 @@
/// This form of template argument only occurs in template argument
/// lists used for dependent types and for expression; it will not
/// occur in a non-dependent, canonical template argument list.
- TemplateArgument(Expr *E) {
+ explicit TemplateArgument(Expr *E) {
TypeOrValue.Kind = Expression;
TypeOrValue.V = reinterpret_cast<uintptr_t>(E);
}
Index: clang/include/clang/AST/Expr.h
===================================================================
--- clang/include/clang/AST/Expr.h
+++ clang/include/clang/AST/Expr.h
@@ -6377,6 +6377,9 @@
friend class ASTStmtWriter;
};
+const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ const Expr* E);
+
} // end namespace clang
#endif // LLVM_CLANG_AST_EXPR_H
Index: clang/include/clang/AST/ASTNodeTraverser.h
===================================================================
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -557,7 +557,7 @@
if (const auto *E = D->getPlaceholderTypeConstraint())
Visit(E);
if (D->hasDefaultArgument())
- Visit(D->getDefaultArgument(), SourceRange(),
+ Visit(TemplateArgument(D->getDefaultArgument()), SourceRange(),
D->getDefaultArgStorage().getInheritedFrom(),
D->defaultArgumentWasInherited() ? "inherited from" : "previous");
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
