ICE macros: Use '<<' for error message

Lessens the friction of using these macros.
Also allows you to add a message to TINT_UNREACHABLE(), which you couldn't do before.

Change-Id: Ida4d63ec96e1d99af71503e8b80d7a5a712e6a47
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42020
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>

src/ast/module.cc

@@ -45,7 +45,7 @@ Module::Module(const Source& source, std::vector<CastableBase*> global_decls)
       global_variables_.push_back(var);
     } else {
       diag::List diagnostics;
-      TINT_ICE(diagnostics, "Unknown global declaration type");
+      TINT_ICE(diagnostics) << "Unknown global declaration type";
     }
   }
 }
@@ -108,7 +108,7 @@ void Module::Copy(CloneContext* ctx, const Module* src) {
     } else if (auto* var = decl->As<Variable>()) {
       AddGlobalVariable(ctx->Clone(var));
     } else {
-      TINT_ICE(ctx->dst->Diagnostics(), "Unknown global declaration type");
+      TINT_ICE(ctx->dst->Diagnostics()) << "Unknown global declaration type";
     }
   }
 }

src/clone_context.h

@@ -307,7 +307,7 @@ class CloneContext {
   TO* CheckedCast(FROM* obj) {
     TO* cast = obj->template As<TO>();
     if (!cast) {
-      TINT_ICE(Diagnostics(), "Cloned object was not of the expected type");
+      TINT_ICE(Diagnostics()) << "Cloned object was not of the expected type";
     }
     return cast;
   }

src/debug.cc

@@ -64,19 +64,21 @@ void SetInternalCompilerErrorReporter(InternalCompilerErrorReporter* reporter) {
   ice_reporter = reporter;
 }
 
-void InternalCompilerError(const char* filepath,
+InternalCompilerError::InternalCompilerError(const char* file,
-                           size_t line,
+                                             size_t line,
-                           const std::string& msg,
+                                             diag::List& diagnostics)
-                           diag::List& diagnostics) {
+    : file_(file), line_(line), diagnostics_(diagnostics) {}
-  auto* file = new Source::File(filepath, "");
+
+InternalCompilerError::~InternalCompilerError() {
+  auto* file = new Source::File(file_, "");
 
   SourceFileToDelete::Get().Add(file);
 
-  Source source{Source::Range{Source::Location{line}}, file};
+  Source source{Source::Range{Source::Location{line_}}, file};
-  diagnostics.add_ice(msg, source);
+  diagnostics_.add_ice(msg_.str(), source);
 
   if (ice_reporter) {
-    ice_reporter(diagnostics);
+    ice_reporter(diagnostics_);
   }
 }
 

src/debug.h

@@ -15,7 +15,8 @@
 #ifndef SRC_DEBUG_H_
 #define SRC_DEBUG_H_
 
-#include <string>
+#include <sstream>
+#include <utility>
 
 #include "src/diagnostic/diagnostic.h"
 #include "src/diagnostic/formatter.h"
@@ -37,17 +38,42 @@ void FreeInternalCompilerErrors();
 /// @param reporter the error reporter
 void SetInternalCompilerErrorReporter(InternalCompilerErrorReporter* reporter);
 
-/// InternalCompilerError adds the internal compiler error message to the
+/// InternalCompilerError is a helper for reporting internal compiler errors.
-/// diagnostics list, and then calls the InternalCompilerErrorReporter if one is
+/// Construct the InternalCompilerError with the source location of the ICE
-/// set.
+/// fault and append any error details with the `<<` operator.
-/// @param file the file containing the ICE
+/// When the InternalCompilerError is destructed, the concatenated error message
-/// @param line the line containing the ICE
+/// is appended to the diagnostics list with the severity of
-/// @param msg the ICE message
+/// tint::diag::Severity::InternalCompilerError, and if a
-/// @param diagnostics the list of diagnostics to append the ICE message to
+/// InternalCompilerErrorReporter is set, then it is called with the diagnostic
-void InternalCompilerError(const char* file,
+/// list.
-                           size_t line,
+class InternalCompilerError {
-                           const std::string& msg,
+ public:
-                           diag::List& diagnostics);
+  /// Constructor
+  /// @param file the file containing the ICE
+  /// @param line the line containing the ICE
+  /// @param diagnostics the list of diagnostics to append the ICE message to
+  InternalCompilerError(const char* file, size_t line, diag::List& diagnostics);
+
+  /// Destructor.
+  /// Adds the internal compiler error message to the diagnostics list, and then
+  /// calls the InternalCompilerErrorReporter if one is set.
+  ~InternalCompilerError();
+
+  /// Appends `arg` to the ICE message.
+  /// @param arg the argument to append to the ICE message
+  /// @returns this object so calls can be chained
+  template <typename T>
+  InternalCompilerError& operator<<(T&& arg) {
+    msg_ << std::forward<T>(arg);
+    return *this;
+  }
+
+ private:
+  char const* const file_;
+  size_t const line_;
+  diag::List& diagnostics_;
+  std::stringstream msg_;
+};
 
 }  // namespace tint
 
@@ -56,14 +82,17 @@ void InternalCompilerError(const char* file,
 /// InternalCompilerErrorReporter with the full diagnostic list if a reporter is
 /// set.
 /// The ICE message contains the callsite's file and line.
-#define TINT_ICE(diagnostics, msg) \
+/// Use the `<<` operator to append an error message to the ICE.
-  tint::InternalCompilerError(__FILE__, __LINE__, msg, diagnostics)
+#define TINT_ICE(diagnostics) \
+  tint::InternalCompilerError(__FILE__, __LINE__, diagnostics)
 
 /// TINT_UNREACHABLE() is a macro for appending a "TINT_UNREACHABLE"
 /// internal compiler error message to the diagnostics list `diagnostics`, and
 /// calling the InternalCompilerErrorReporter with the full diagnostic list if a
 /// reporter is set.
 /// The ICE message contains the callsite's file and line.
-#define TINT_UNREACHABLE(diagnostics) TINT_ICE(diagnostics, "TINT_UNREACHABLE")
+/// Use the `<<` operator to append an error message to the ICE.
+#define TINT_UNREACHABLE(diagnostics) \
+  TINT_ICE(diagnostics) << "TINT_UNREACHABLE "
 
 #endif  // SRC_DEBUG_H_

src/intrinsic_table.cc

@@ -1448,20 +1448,20 @@ semantic::Intrinsic* Impl::Overload::Match(ProgramBuilder& builder,
     if (type_it == matcher_state.open_types.end()) {
       // We have an overload that claims to have matched, but didn't actually
       // resolve the open type. This is a bug that needs fixing.
-      TINT_ICE(diagnostics, "IntrinsicTable overload matched for " +
+      TINT_ICE(diagnostics)
-                                CallSignature(builder, intrinsic, args) +
+          << "IntrinsicTable overload matched for "
-                                ", but didn't resolve the open type " +
+          << CallSignature(builder, intrinsic, args)
-                                str(open_type));
+          << ", but didn't resolve the open type " << str(open_type);
       return nullptr;
     }
     auto* resolved_type = type_it->second;
     if (resolved_type == nullptr) {
       // We have an overload that claims to have matched, but has a nullptr
       // resolved open type. This is a bug that needs fixing.
-      TINT_ICE(diagnostics, "IntrinsicTable overload matched for " +
+      TINT_ICE(diagnostics)
-                                CallSignature(builder, intrinsic, args) +
+          << "IntrinsicTable overload matched for "
-                                ", but open type " + str(open_type) +
+          << CallSignature(builder, intrinsic, args) << ", but open type "
-                                " is nullptr");
+          << str(open_type) << " is nullptr";
       return nullptr;
     }
     if (!matcher->Match(matcher_state, resolved_type)) {

src/transform/hlsl.cc

@@ -57,9 +57,8 @@ void Hlsl::PromoteArrayInitializerToConstVar(CloneContext& ctx) const {
     if (auto* src_init = src_node->As<ast::TypeConstructorExpression>()) {
       auto* src_sem_expr = ctx.src->Sem().Get(src_init);
       if (!src_sem_expr) {
-        TINT_ICE(
+        TINT_ICE(ctx.dst->Diagnostics())
-            ctx.dst->Diagnostics(),
+            << "ast::TypeConstructorExpression has no semantic expression node";
-            "ast::TypeConstructorExpression has no semantic expression node");
         continue;
       }
       auto* src_sem_stmt = src_sem_expr->Stmt();

src/writer/hlsl/generator_impl.cc

@@ -799,7 +799,7 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
         case semantic::IntrinsicType::kTextureDimensions:
           switch (texture_type->dim()) {
             case type::TextureDimension::kNone:
-              diagnostics_.add_error("texture dimension is kNone");
+              TINT_ICE(diagnostics_) << "texture dimension is kNone";
               return false;
             case type::TextureDimension::k1d:
               num_dimensions = 1;
@@ -835,7 +835,7 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
         case semantic::IntrinsicType::kTextureNumLayers:
           switch (texture_type->dim()) {
             default:
-              diagnostics_.add_error("texture dimension is not arrayed");
+              TINT_ICE(diagnostics_) << "texture dimension is not arrayed";
               return false;
             case type::TextureDimension::k1dArray:
               num_dimensions = 2;
@@ -852,7 +852,8 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
           add_mip_level_in = true;
           switch (texture_type->dim()) {
             default:
-              diagnostics_.add_error("texture dimension does not support mips");
+              TINT_ICE(diagnostics_)
+                  << "texture dimension does not support mips";
               return false;
             case type::TextureDimension::k2d:
             case type::TextureDimension::kCube:
@@ -870,8 +871,8 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
         case semantic::IntrinsicType::kTextureNumSamples:
           switch (texture_type->dim()) {
             default:
-              diagnostics_.add_error(
+              TINT_ICE(diagnostics_)
-                  "texture dimension does not support multisampling");
+                  << "texture dimension does not support multisampling";
               return false;
             case type::TextureDimension::k2d:
               num_dimensions = 3;
@@ -884,7 +885,7 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
           }
           break;
         default:
-          diagnostics_.add_error("unexpected intrinsic");
+          TINT_ICE(diagnostics_) << "unexpected intrinsic";
           return false;
       }
 
@@ -2468,9 +2469,8 @@ bool GeneratorImpl::EmitType(std::ostream& out,
     if (auto* st = tex->As<type::StorageTexture>()) {
       auto* component = image_format_to_rwtexture_type(st->image_format());
       if (component == nullptr) {
-        diagnostics_.add_error(
+        TINT_ICE(diagnostics_) << "Unsupported StorageTexture ImageFormat: "
-            "Unsupported StorageTexture ImageFormat: " +
+                               << static_cast<int>(st->image_format());
-            std::to_string(static_cast<int>(st->image_format())));
         return false;
       }
       out << "<" << component << ">";

src/writer/msl/generator_impl.cc

@@ -699,8 +699,8 @@ bool GeneratorImpl::EmitTextureCall(ast::CallExpression* expr,
       out_ << ".write(";
       break;
     default:
-      TINT_ICE(diagnostics_, "Unhandled texture intrinsic '" +
+      TINT_UNREACHABLE(diagnostics_)
-                                 std::string(intrinsic->str()) + "'");
+          << "Unhandled texture intrinsic '" << intrinsic->str() << "'";
       return false;
   }