[clang] [clang-tools-extra] [clang] fix diagnostic printing of expressions ignoring LangOpts (PR #134693)
Matheus Izvekov via cfe-commits
cfe-commits at lists.llvm.org
Mon Apr 7 10:13:38 PDT 2025
https://github.com/mizvekov updated https://github.com/llvm/llvm-project/pull/134693
>From 3ccb34773d143d3ef6a153ecc433db4791ee85f5 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Mon, 7 Apr 2025 13:08:52 -0300
Subject: [PATCH] [clang] fix diagnostic printing of expressions ignoring
LangOpts
Currently when printing a template argument of expression type,
the expression is converted immediately into a string to be sent
to the diagnostic engine, unsing a fake LangOpts.
This makes the expression printing look incorrect for the current language,
besides being inneficient, as we don't actually need to print
the expression if the diagnostic would be ignored.
This fixes a nastiness with the TemplateArgument constructor for
expressions being implicit, and all current users just passing
an expression to a diagnostic were implicitly going through the
template argument path.
The expressions are also being printed unquoted. This will be fixed
in a subsequent patch, as the test churn is much larger.
---
.../clang-tidy/ClangTidyDiagnosticConsumer.cpp | 3 +++
clang/docs/ReleaseNotes.rst | 3 +++
clang/include/clang/AST/Expr.h | 8 ++++++++
clang/include/clang/AST/TemplateBase.h | 2 +-
clang/include/clang/Basic/Diagnostic.h | 5 ++++-
clang/lib/AST/ASTDiagnostic.cpp | 8 ++++++++
clang/lib/AST/TemplateBase.cpp | 16 +++-------------
clang/lib/Basic/Diagnostic.cpp | 1 +
clang/lib/Sema/SemaTemplateDeduction.cpp | 8 ++++----
clang/lib/Sema/SemaTemplateVariadic.cpp | 2 +-
clang/lib/Sema/TreeTransform.h | 6 +++---
clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl | 10 +++++-----
.../instantiate-expanded-type-constraint.cpp | 4 ++--
.../SemaTemplate/instantiate-requires-expr.cpp | 4 ++--
.../trailing-return-short-circuit.cpp | 4 ++--
15 files changed, 50 insertions(+), 34 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 71e852545203e..abd6d7b4cd60f 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -22,6 +22,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/Attr.h"
+#include "clang/AST/Expr.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticOptions.h"
@@ -528,6 +529,8 @@ void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) {
case clang::DiagnosticsEngine::ak_addrspace:
Builder << static_cast<LangAS>(Info.getRawArg(Index));
break;
+ case clang::DiagnosticsEngine::ak_expr:
+ Builder << reinterpret_cast<const Expr *>(Info.getRawArg(Index));
}
}
}
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f8f4dfbafb4f8..e671183522565 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -280,6 +280,9 @@ Improvements to Clang's diagnostics
- Clang now better preserves the sugared types of pointers to member.
- Clang now better preserves the presence of the template keyword with dependent
prefixes.
+- Clang now respects the current language mode when printing expressions in
+ diagnostics. This fixes a bunch of `bool` being printed as `_Bool`, and also
+ a bunch of HLSL types being printed as their C++ equivalents.
- When printing types for diagnostics, clang now doesn't suppress the scopes of
template arguments contained within nested names.
- The ``-Wshift-bool`` warning has been added to warn about shifting a boolean. (#GH28334)
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index dedbff5944af8..20f70863a05b3 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -7379,6 +7379,14 @@ class RecoveryExpr final : public Expr,
friend class ASTStmtWriter;
};
+/// Insertion operator for diagnostics. This allows sending
+/// Expr into a diagnostic with <<.
+inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ const Expr *E) {
+ DB.AddTaggedVal(reinterpret_cast<uint64_t>(E), DiagnosticsEngine::ak_expr);
+ return DB;
+}
+
} // end namespace clang
#endif // LLVM_CLANG_AST_EXPR_H
diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h
index a800a16fc3e7a..bea624eb04942 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -262,7 +262,7 @@ class TemplateArgument {
/// 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, bool IsDefaulted = false) {
+ explicit TemplateArgument(Expr *E, bool IsDefaulted = false) {
TypeOrValue.Kind = Expression;
TypeOrValue.IsDefaulted = IsDefaulted;
TypeOrValue.V = reinterpret_cast<uintptr_t>(E);
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index 848acce3c4f13..19524856a9bb3 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -284,7 +284,10 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
ak_qualtype_pair,
/// Attr *
- ak_attr
+ ak_attr,
+
+ /// Expr *
+ ak_expr,
};
/// Represents on argument value, which is a union discriminated
diff --git a/clang/lib/AST/ASTDiagnostic.cpp b/clang/lib/AST/ASTDiagnostic.cpp
index b4e7360e126fb..ccfef9c7ae361 100644
--- a/clang/lib/AST/ASTDiagnostic.cpp
+++ b/clang/lib/AST/ASTDiagnostic.cpp
@@ -508,6 +508,14 @@ void clang::FormatASTNodeDiagnosticArgument(
NeedQuotes = false;
break;
}
+ case DiagnosticsEngine::ak_expr: {
+ const Expr *E = reinterpret_cast<Expr *>(Val);
+ assert(E && "Received null Expr!");
+ E->printPretty(OS, /*Helper=*/nullptr, Context.getPrintingPolicy());
+ // FIXME: Include quotes when printing expressions.
+ NeedQuotes = false;
+ break;
+ }
}
if (NeedQuotes) {
diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index 0be0a83b7010d..42da4e6ca9964 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -478,7 +478,7 @@ TemplateArgument TemplateArgument::getPackExpansionPattern() const {
return getAsType()->castAs<PackExpansionType>()->getPattern();
case Expression:
- return cast<PackExpansionExpr>(getAsExpr())->getPattern();
+ return TemplateArgument(cast<PackExpansionExpr>(getAsExpr())->getPattern());
case TemplateExpansion:
return TemplateArgument(getAsTemplateOrTemplatePattern());
@@ -654,18 +654,8 @@ static const T &DiagTemplateArg(const T &DB, const TemplateArgument &Arg) {
case TemplateArgument::TemplateExpansion:
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();
- }
+ case TemplateArgument::Expression:
+ return DB << Arg.getAsExpr();
case TemplateArgument::Pack: {
// FIXME: We're guessing at LangOptions!
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 9e2f134135647..4b4a85aaccf8b 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -1247,6 +1247,7 @@ FormatDiagnostic(const char *DiagStr, const char *DiagEnd,
case DiagnosticsEngine::ak_nestednamespec:
case DiagnosticsEngine::ak_declcontext:
case DiagnosticsEngine::ak_attr:
+ case DiagnosticsEngine::ak_expr:
getDiags()->ConvertArgToString(Kind, getRawArg(ArgNo),
StringRef(Modifier, ModifierLen),
StringRef(Argument, ArgumentLen),
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 200168087136b..6236bce743438 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -493,8 +493,8 @@ DeduceNullPtrTemplateArgument(Sema &S, TemplateParameterList *TemplateParams,
: CK_NullToPointer)
.get();
return DeduceNonTypeTemplateArgument(
- S, TemplateParams, NTTP, DeducedTemplateArgument(Value), Value->getType(),
- Info, PartialOrdering, Deduced, HasDeducedAnyParam);
+ S, TemplateParams, NTTP, TemplateArgument(Value), Value->getType(), Info,
+ PartialOrdering, Deduced, HasDeducedAnyParam);
}
/// Deduce the value of the given non-type template parameter
@@ -508,8 +508,8 @@ DeduceNonTypeTemplateArgument(Sema &S, TemplateParameterList *TemplateParams,
SmallVectorImpl<DeducedTemplateArgument> &Deduced,
bool *HasDeducedAnyParam) {
return DeduceNonTypeTemplateArgument(
- S, TemplateParams, NTTP, DeducedTemplateArgument(Value), Value->getType(),
- Info, PartialOrdering, Deduced, HasDeducedAnyParam);
+ S, TemplateParams, NTTP, TemplateArgument(Value), Value->getType(), Info,
+ PartialOrdering, Deduced, HasDeducedAnyParam);
}
/// Deduce the value of the given non-type template parameter
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 3d4a245eb8bd5..3040a30454b0c 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -1286,7 +1286,7 @@ TemplateArgumentLoc Sema::getTemplateArgumentPackExpansionPattern(
Expr *Pattern = Expansion->getPattern();
Ellipsis = Expansion->getEllipsisLoc();
NumExpansions = Expansion->getNumExpansions();
- return TemplateArgumentLoc(Pattern, Pattern);
+ return TemplateArgumentLoc(TemplateArgument(Pattern), Pattern);
}
case TemplateArgument::TemplateExpansion:
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 57fcc4b3b3682..2889a16dd0271 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3981,7 +3981,7 @@ class TreeTransform {
if (Result.isInvalid())
return TemplateArgumentLoc();
- return TemplateArgumentLoc(Result.get(), Result.get());
+ return TemplateArgumentLoc(TemplateArgument(Result.get()), Result.get());
}
case TemplateArgument::Template:
@@ -16125,8 +16125,8 @@ TreeTransform<Derived>::TransformSizeOfPackExpr(SizeOfPackExpr *E) {
E->getPackLoc());
if (DRE.isInvalid())
return ExprError();
- ArgStorage = new (getSema().Context)
- PackExpansionExpr(DRE.get(), E->getPackLoc(), std::nullopt);
+ ArgStorage = TemplateArgument(new (getSema().Context) PackExpansionExpr(
+ DRE.get(), E->getPackLoc(), std::nullopt));
}
PackArgs = ArgStorage;
}
diff --git a/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl b/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
index 941e0a975d5f4..34930d8963688 100644
--- a/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/RWBuffers.hlsl
@@ -60,13 +60,13 @@ RWBuffer<TemplatedVector<int> > r9;
// arrays not allowed
// expected-error at +3 {{constraints not satisfied for class template 'RWBuffer'}}
// expected-note@*:* {{because 'half[4]' does not satisfy '__is_typed_resource_element_compatible'}}
-// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(__fp16[4])' evaluated to false}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(half[4])' evaluated to false}}
RWBuffer<half[4]> r10;
typedef vector<int, 8> int8;
// expected-error at +3 {{constraints not satisfied for class template 'RWBuffer'}}
// expected-note@*:* {{because 'vector<int, 8>' (vector of 8 'int' values) does not satisfy '__is_typed_resource_element_compatible'}}
-// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(int __attribute__((ext_vector_type(8))))' evaluated to false}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(vector<int, 8>)' evaluated to false}}
RWBuffer<int8> r11;
typedef int MyInt;
@@ -74,12 +74,12 @@ RWBuffer<MyInt> r12;
// expected-error at +3 {{constraints not satisfied for class template 'RWBuffer'}}
// expected-note@*:* {{because 'bool' does not satisfy '__is_typed_resource_element_compatible'}}
-// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(_Bool)' evaluated to false}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(bool)' evaluated to false}}
RWBuffer<bool> r13;
// expected-error at +3 {{constraints not satisfied for class template 'RWBuffer'}}
// expected-note@*:* {{because 'vector<bool, 2>' (vector of 2 'bool' values) does not satisfy '__is_typed_resource_element_compatible'}}
-// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(_Bool __attribute__((ext_vector_type(2))))' evaluated to false}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(vector<bool, 2>)' evaluated to false}}
RWBuffer<vector<bool, 2>> r14;
enum numbers { one, two, three };
@@ -91,7 +91,7 @@ RWBuffer<numbers> r15;
// expected-error at +3 {{constraints not satisfied for class template 'RWBuffer'}}
// expected-note@*:* {{because 'vector<double, 3>' (vector of 3 'double' values) does not satisfy '__is_typed_resource_element_compatible'}}
-// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(double __attribute__((ext_vector_type(3))))' evaluated to false}}
+// expected-note@*:* {{because '__builtin_hlsl_is_typed_resource_element_compatible(vector<double, 3>)' evaluated to false}}
RWBuffer<double3> r16;
diff --git a/clang/test/SemaTemplate/instantiate-expanded-type-constraint.cpp b/clang/test/SemaTemplate/instantiate-expanded-type-constraint.cpp
index 73fef87b97822..3edf243982958 100644
--- a/clang/test/SemaTemplate/instantiate-expanded-type-constraint.cpp
+++ b/clang/test/SemaTemplate/instantiate-expanded-type-constraint.cpp
@@ -8,7 +8,7 @@ constexpr bool is_same_v<T, T> = true;
template<typename T, typename U>
concept same_as = is_same_v<T, U>;
-// expected-note at -1{{because 'is_same_v<int, _Bool>' evaluated to false}}
+// expected-note at -1{{because 'is_same_v<int, bool>' evaluated to false}}
template<typename T, typename... Us>
concept either = (is_same_v<T, Us> || ...);
@@ -16,7 +16,7 @@ concept either = (is_same_v<T, Us> || ...);
template<typename... Ts>
struct T {
template<same_as<Ts>... Us>
- // expected-note at -1{{because 'same_as<int, _Bool>' evaluated to false}}
+ // expected-note at -1{{because 'same_as<int, bool>' evaluated to false}}
static void foo(Us... u, int x) { };
// expected-note at -1{{candidate template ignored: deduced too few arguments}}
// expected-note at -2{{candidate template ignored: constraints not satisfied}}
diff --git a/clang/test/SemaTemplate/instantiate-requires-expr.cpp b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
index ab5fac1f9e63e..47689b93db50f 100644
--- a/clang/test/SemaTemplate/instantiate-requires-expr.cpp
+++ b/clang/test/SemaTemplate/instantiate-requires-expr.cpp
@@ -72,8 +72,8 @@ namespace type_requirement {
template<typename T> requires
false_v<requires { typename T::template temp<T>; }>
- // expected-note at -1 {{because 'false_v<requires { typename contains_template<int>::template temp<type_requirement::contains_template<int> >; }>' evaluated to false}}
- // expected-note at -2 {{because 'false_v<requires { typename contains_template<short>::template temp<type_requirement::contains_template<short> >; }>' evaluated to false}}
+ // expected-note at -1 {{because 'false_v<requires { typename contains_template<int>::template temp<type_requirement::contains_template<int>>; }>' evaluated to false}}
+ // expected-note at -2 {{because 'false_v<requires { typename contains_template<short>::template temp<type_requirement::contains_template<short>>; }>' evaluated to false}}
struct r2 {};
using r2i1 = r2<contains_template<int>>; // expected-error{{constraints not satisfied for class template 'r2' [with T = type_requirement::contains_template<int>]}}
diff --git a/clang/test/SemaTemplate/trailing-return-short-circuit.cpp b/clang/test/SemaTemplate/trailing-return-short-circuit.cpp
index 0d1c9b52b0e85..4ef7888d23dc5 100644
--- a/clang/test/SemaTemplate/trailing-return-short-circuit.cpp
+++ b/clang/test/SemaTemplate/trailing-return-short-circuit.cpp
@@ -39,13 +39,13 @@ void usage() {
Foo(true);
// expected-error at -1{{no matching function for call to 'Foo'}}
// expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = bool]}}
- // expected-note@#FOO_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+ // expected-note@#FOO_REQ{{because 'sizeof(bool) > 2' (1 > 2) evaluated to false}}
// expected-note@#FOO_REQ{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
TrailingReturn(true);
// expected-error at -1{{no matching function for call to 'TrailingReturn'}}
// expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = bool]}}
- // expected-note@#TRAILING_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+ // expected-note@#TRAILING_REQ{{because 'sizeof(bool) > 2' (1 > 2) evaluated to false}}
// expected-note@#TRAILING_REQ_VAL{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
// Fails the 1st check, fails 2nd because ::value is false.
More information about the cfe-commits
mailing list