[clang] fix diagnostic printing of expressions ignoring LangOpts #134693
Conversation
This file contains hidden or 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
mizvekov
requested review from
AaronBallman,
cor3ntin,
erichkeane,
shafik and
zyn0217
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-clang-tidy Author: Matheus Izvekov (mizvekov) ChangesCurrently 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. Full diff: https://github.com/llvm/llvm-project/pull/134693.diff 15 Files Affected:
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..049ba5ef9ea02 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
+ diagnsotics. 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@+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@+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@+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@+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@+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@-1{{because 'is_same_v<int, _Bool>' evaluated to false}}
+// expected-note@-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@-1{{because 'same_as<int, _Bool>' evaluated to false}}
+ // expected-note@-1{{because 'same_as<int, bool>' evaluated to false}}
static void foo(Us... u, int x) { };
// expected-note@-1{{candidate template ignored: deduced too few arguments}}
// expected-note@-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@-1 {{because 'false_v<requires { typename contains_template<int>::template temp<type_requirement::contains_template<int> >; }>' evaluated to false}}
- // expected-note@-2 {{because 'false_v<requires { typename contains_template<short>::template temp<type_requirement::contains_template<short> >; }>' evaluated to false}}
+ // expected-note@-1 {{because 'false_v<requires { typename contains_template<int>::template temp<type_requirement::contains_template<int>>; }>' evaluated to false}}
+ // expected-note@-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@-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@-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.
|
erichkeane
reviewed
Apr 7, 2025
mizvekov
force-pushed
the
users/mizvekov/diag-expr
branch
from
ab5d841 to
3ccb347
Compare
erichkeane
approved these changes
Apr 7, 2025
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.
mizvekov
force-pushed
the
users/mizvekov/diag-expr
branch
from
3ccb347 to
302fc36
Compare
llvmbot
added
the
libc++
shafik
reviewed
Apr 8, 2025
| /// 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); |
Collaborator
There was a problem hiding this comment.
So is this correct, to cast a pointer to uint64_t? It looks like we do this and in about half the other place we use intptr_t, which I would be much less suspicious of.
Collaborator
There was a problem hiding this comment.
intptr_t /uintptr_t I think avoids any 'round trip' errors by standard, but I think uint64_t is going to be the same type ANYWAY so this is probably harmless at least. I think the suggestion to use intptr_t/uintptr_t is a good one that should happen.
Contributor
Author
There was a problem hiding this comment.
If we care that uint64_t might not round trip a pointer in theory, even though it should always in practice, then do we care uintptr_t is not guaranteed to be provided in theory, even though it should always be in practice?
Collaborator
There was a problem hiding this comment.
If we care that uint64_t might not round trip a pointer in theory, even though it should always in practice, then do we care uintptr_t is not guaranteed to be provided in theory, even though it should always be in practice?
THAT we don't care about because : 1- it is a compile-time diagnosed missing 'thing', and 2- We know all our supported host platforms have (and presumably always will have) that type.
So it is a matter of "potentially ill-formed" vs "potential-UB".
Either way, it doesn't MATTER very much, but from a 'most correct' perspective (as frankly, uint64_t and uintptr_t are probably the same type, and I would bet good money that none of our platforms would ever change that/never give uintptr_t its own representation to take advantage of this UB), but just changing the cast type here is the right thing to do.
Contributor
Author
There was a problem hiding this comment.
It's not just here though, this is the same for all other kinds, and the same for the parameter type, and the same for where we store this.
It wouldn't change much regarding potential UB if we wrote uintptr_t here, but then this would implicitly cast to uint64_t anyway.
Now if we change that to uintptr_t, then that would be bad in 32 bit platforms, as this is used to pack other things besides pointers. So presumably we want to change the parameter and structure types here to something that's at least as big as both types, which would be uintmax perhaps?
Or maybe just assert somewhere that uint64 is at least as big as uintptr?
Collaborator
There was a problem hiding this comment.
As far as correctness: we need to smuggle pointers through a uintptr_t. Beyond that, there is no harm then again casting that immediately to the uint64_t.
So the 'fixit' suggestion is:
Suggested change
| DB.AddTaggedVal(reinterpret_cast<uint64_t>(E), DiagnosticsEngine::ak_expr); | |
| DB.AddTaggedVal(static_cast<uint64_t>(reinterpret_cast<uintptr_t>(E)), DiagnosticsEngine::ak_expr); |
The underlying type here is fine, and SHOULD stay uint64_t. But the reinterpret_cast of pointer-to-int itself is the problematic part. If you'd like, you can put a static-assert that sizeof(uintptr_t) is <= sizeof(uint64_t), but I'm not horribly worried about 128 bit pointers
Contributor
Author
There was a problem hiding this comment.
But presumably, I'd have to go on and do this for all diagnostics stream operators, not just this one, otherwise it would not be helpful to the next individual, who just
like me, did the same way it was done somewhere else :)
Collaborator
There was a problem hiding this comment.
We probably SHOULD, and you can do so without review if you care to. Else, we can just make @shafik do this, since he started it.
Collaborator
There was a problem hiding this comment.
I am not super sympathetic here, reinterpret_cast in a code review should have both the author and the reviewer on high alert and they should carefully examine the code and if we suspect the same pattern is used elsewhere we should confirm it is used consistently. Which in this case it is not.
The PR was turned over pretty quickly w/o giving any of the other long list of reviewers a chance to comment.
Contributor
Author
There was a problem hiding this comment.
Yes, I had carefully reviewed it and I think it was fine, given the circumstances.
For reference, here are all the other uses:
clang/include/clang/AST/Attr.h: DB.AddTaggedVal(reinterpret_cast<uint64_t>(At), DiagnosticsEngine::ak_attr);
clang/include/clang/AST/Expr.h: DB.AddTaggedVal(reinterpret_cast<uint64_t>(E), DiagnosticsEngine::ak_expr);
clang/include/clang/AST/NestedNameSpecifier.h: DB.AddTaggedVal(reinterpret_cast<uint64_t>(NNS),
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(reinterpret_cast<intptr_t>(Str),
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(I, DiagnosticsEngine::ak_sint);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(I, DiagnosticsEngine::ak_uint);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(static_cast<unsigned>(I), DiagnosticsEngine::ak_tokenkind);
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(reinterpret_cast<intptr_t>(II),
clang/include/clang/Basic/Diagnostic.h: DB.AddTaggedVal(reinterpret_cast<intptr_t>(DC),
clang/include/clang/Basic/PartialDiagnostic.h: DB.AddTaggedVal(DiagStorage->DiagArgumentsVal[i],
clang/include/clang/Sema/ParsedAttr.h: DB.AddTaggedVal(reinterpret_cast<uint64_t>(At.getAttrName()),
clang/include/clang/Sema/ParsedAttr.h: DB.AddTaggedVal(reinterpret_cast<uint64_t>(At->getAttrName()),
clang/include/clang/Sema/ParsedAttr.h: DB.AddTaggedVal(reinterpret_cast<uint64_t>(CI.getAttrName()),
clang/include/clang/Sema/ParsedAttr.h: DB.AddTaggedVal(reinterpret_cast<uint64_t>(CI->getAttrName()),So 6 uint64_t vs 3 intptr_t (excluding this patch). Without going on and changing everything else, there is no single answer here which would be satisfy everyone.
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.
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.