[clang] fix diagnostic printing of expressions ignoring LangOpts (#134693)

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-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));
     }
   }
 }

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)

clang/include/clang/AST/Expr.h

@@ -7379,6 +7379,14 @@ private:
   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

clang/include/clang/AST/TemplateBase.h

@@ -262,7 +262,7 @@ public:
   /// 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);

clang/include/clang/Basic/Diagnostic.h

@@ -284,7 +284,10 @@ public:
     ak_qualtype_pair,
 
     /// Attr *
-    ak_attr
+    ak_attr,
+
+    /// Expr *
+    ak_expr,
   };
 
   /// Represents on argument value, which is a union discriminated

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) {

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!

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),

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

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:

clang/lib/Sema/TreeTransform.h

@@ -3981,7 +3981,7 @@ public:
       if (Result.isInvalid())
         return TemplateArgumentLoc();
 
-      return TemplateArgumentLoc(Result.get(), Result.get());
+      return TemplateArgumentLoc(TemplateArgument(Result.get()), Result.get());
     }
 
     case TemplateArgument::Template:
@@ -16131,8 +16131,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;
     }

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;
 
 

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}}

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>]}}

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.

libcxx/test/libcxx/algorithms/cpp17_iterator_concepts.verify.cpp

@@ -173,7 +173,7 @@ void check_bidirectional_iterator_requirements() {
   _LIBCPP_REQUIRE_CPP17_BIDIRECTIONAL_ITERATOR(missing_postdecrement, ""); // expected-error {{static assertion failed}}
   // expected-note@*:* {{cannot decrement value of type 'missing_postdecrement'}}
   _LIBCPP_REQUIRE_CPP17_BIDIRECTIONAL_ITERATOR(not_returning_iter_reference, ""); // expected-error {{static assertion failed}}
-  // expected-note@*:* {{because type constraint 'same_as<int, __iter_reference<not_returning_iter_reference> >' was not satisfied}}
+  // expected-note-re@*:* {{because type constraint 'same_as<int, __iter_reference<not_returning_iter_reference>{{ ?}}>' was not satisfied}}
   // clang-format on
 }