[torque] Protect against printing Type* pointers

I've noticed a frequent mistake within Torque is to use Type* pointers
with ostream's operator<<, which causes it to print a hex pointer rather
than a descriptive string. This can cause confusing error messages for
users of the Torque compiler. This change is an idea to prevent future
incidences of that problem by adding a template overload that will cause
a compilation failure if anybody tries to use Type* in this way. It
found two incorrect uses of Type*, which I've corrected.

Bug: v8:7793
Change-Id: I85fafb333a89f8a3fed4346bdd154d70846a63d1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2748936
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#73574}

src/base/logging.h

@@ -51,6 +51,8 @@ V8_BASE_EXPORT V8_NOINLINE void V8_Dcheck(const char* file, int line,
 namespace v8 {
 namespace base {
 
+class CheckMessageStream : public std::ostringstream {};
+
 // Overwrite the default function that prints a stack trace.
 V8_BASE_EXPORT void SetPrintStackTrace(void (*print_stack_trace_)());
 
@@ -141,7 +143,7 @@ V8_BASE_EXPORT void SetDcheckFunction(void (*dcheck_Function)(const char*, int,
 namespace detail {
 template <typename... Ts>
 std::string PrintToString(Ts&&... ts) {
-  std::ostringstream oss;
+  CheckMessageStream oss;
   int unused_results[]{((oss << std::forward<Ts>(ts)), 0)...};
   (void)unused_results;  // Avoid "unused variable" warning.
   return oss.str();
@@ -164,7 +166,8 @@ auto GetUnderlyingEnumTypeForPrinting(T val) {
 template <typename T>
 typename std::enable_if<
     !std::is_function<typename std::remove_pointer<T>::type>::value &&
-        !std::is_enum<T>::value && has_output_operator<T>::value,
+        !std::is_enum<T>::value &&
+        has_output_operator<T, CheckMessageStream>::value,
     std::string>::type
 PrintCheckOperand(T val) {
   return detail::PrintToString(std::forward<T>(val));
@@ -185,7 +188,8 @@ PrintCheckOperand(T val) {
 
 // Define PrintCheckOperand<T> for enums with an output operator.
 template <typename T>
-typename std::enable_if<std::is_enum<T>::value && has_output_operator<T>::value,
+typename std::enable_if<std::is_enum<T>::value &&
+                            has_output_operator<T, CheckMessageStream>::value,
                         std::string>::type
 PrintCheckOperand(T val) {
   std::string val_str = detail::PrintToString(val);
@@ -205,15 +209,16 @@ PrintCheckOperand(T val) {
 
 // Define PrintCheckOperand<T> for enums without an output operator.
 template <typename T>
-typename std::enable_if<
+typename std::enable_if<std::is_enum<T>::value &&
-    std::is_enum<T>::value && !has_output_operator<T>::value, std::string>::type
+                            !has_output_operator<T, CheckMessageStream>::value,
+                        std::string>::type
 PrintCheckOperand(T val) {
   return detail::PrintToString(detail::GetUnderlyingEnumTypeForPrinting(val));
 }
 
 // Define default PrintCheckOperand<T> for non-printable types.
 template <typename T>
-typename std::enable_if<!has_output_operator<T>::value &&
+typename std::enable_if<!has_output_operator<T, CheckMessageStream>::value &&
                             !std::is_enum<T>::value,
                         std::string>::type
 PrintCheckOperand(T val) {
@@ -242,7 +247,7 @@ template <typename Lhs, typename Rhs>
 V8_NOINLINE std::string* MakeCheckOpString(Lhs lhs, Rhs rhs, char const* msg) {
   std::string lhs_str = PrintCheckOperand<Lhs>(lhs);
   std::string rhs_str = PrintCheckOperand<Rhs>(rhs);
-  std::ostringstream ss;
+  CheckMessageStream ss;
   ss << msg;
   constexpr size_t kMaxInlineLength = 50;
   if (lhs_str.size() <= kMaxInlineLength &&

src/base/template-utils.h

@@ -53,11 +53,11 @@ struct pass_value_or_ref {
 };
 
 // Uses expression SFINAE to detect whether using operator<< would work.
-template <typename T, typename = void>
+template <typename T, typename TStream = std::ostream, typename = void>
 struct has_output_operator : std::false_type {};
-template <typename T>
+template <typename T, typename TStream>
-struct has_output_operator<T, decltype(void(std::declval<std::ostream&>()
+struct has_output_operator<
-                                            << std::declval<T>()))>
+    T, TStream, decltype(void(std::declval<TStream&>() << std::declval<T>()))>
     : std::true_type {};
 
 // Fold all arguments from left to right with a given function.

src/torque/declaration-visitor.cc

@@ -141,7 +141,7 @@ void DeclarationVisitor::Visit(ExternalRuntimeDeclaration* decl) {
     ReportError(
         "runtime functions can only return strong tagged values, but "
         "found type ",
-        signature.return_type);
+        *signature.return_type);
   }
   for (const Type* parameter_type : signature.parameter_types.types) {
     if (!parameter_type->IsSubtypeOf(TypeOracle::GetStrongTaggedType())) {

src/torque/implementation-visitor.cc

@@ -2687,7 +2687,7 @@ VisitResult ImplementationVisitor::GenerateCall(
       if (!this_value.type()->IsSubtypeOf(method->aggregate_type())) {
         ReportError("this parameter must be a subtype of ",
                     *method->aggregate_type(), " but it is of type ",
-                    this_value.type());
+                    *this_value.type());
       }
     } else {
       AddCallParameter(callable, this_value, method->aggregate_type(),

src/torque/types.h

@@ -785,6 +785,20 @@ inline std::ostream& operator<<(std::ostream& os, const Type& t) {
   return os;
 }
 
+template <bool success = false>
+std::ostream& operator<<(std::ostream& os, const Type* t) {
+  static_assert(success,
+                "Using Type* with an ostream is usually a mistake. Did you "
+                "mean to use Type& instead? If you actually intended to print "
+                "a pointer, use static_cast<const void*>.");
+  return os;
+}
+
+// Don't emit an error if a Type* is printed due to CHECK macros.
+inline std::ostream& operator<<(base::CheckMessageStream& os, const Type* t) {
+  return os << static_cast<const void*>(t);
+}
+
 class VisitResult {
  public:
   VisitResult() = default;