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>
diff --git a/src/ast/module.cc b/src/ast/module.cc index 996ecc9..91b8fdb 100644 --- a/src/ast/module.cc +++ b/src/ast/module.cc
@@ -45,7 +45,7 @@ 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 @@ } 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"; } } }
diff --git a/src/clone_context.h b/src/clone_context.h index a533bff..e1b4da0 100644 --- a/src/clone_context.h +++ b/src/clone_context.h
@@ -307,7 +307,7 @@ 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; }
diff --git a/src/debug.cc b/src/debug.cc index b6d9b01..c33c7de 100644 --- a/src/debug.cc +++ b/src/debug.cc
@@ -64,19 +64,21 @@ ice_reporter = reporter; } -void InternalCompilerError(const char* filepath, - size_t line, - const std::string& msg, - diag::List& diagnostics) { - auto* file = new Source::File(filepath, ""); +InternalCompilerError::InternalCompilerError(const char* file, + size_t line, + diag::List& diagnostics) + : file_(file), line_(line), diagnostics_(diagnostics) {} + +InternalCompilerError::~InternalCompilerError() { + auto* file = new Source::File(file_, ""); SourceFileToDelete::Get().Add(file); - Source source{Source::Range{Source::Location{line}}, file}; - diagnostics.add_ice(msg, source); + Source source{Source::Range{Source::Location{line_}}, file}; + diagnostics_.add_ice(msg_.str(), source); if (ice_reporter) { - ice_reporter(diagnostics); + ice_reporter(diagnostics_); } }
diff --git a/src/debug.h b/src/debug.h index 3e7e7de..ba3aed1 100644 --- a/src/debug.h +++ b/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 @@ /// @param reporter the error reporter void SetInternalCompilerErrorReporter(InternalCompilerErrorReporter* reporter); -/// InternalCompilerError adds the internal compiler error message to the -/// diagnostics list, and then calls the InternalCompilerErrorReporter if one is -/// set. -/// @param file the file containing the ICE -/// @param line the line containing the ICE -/// @param msg the ICE message -/// @param diagnostics the list of diagnostics to append the ICE message to -void InternalCompilerError(const char* file, - size_t line, - const std::string& msg, - diag::List& diagnostics); +/// InternalCompilerError is a helper for reporting internal compiler errors. +/// Construct the InternalCompilerError with the source location of the ICE +/// fault and append any error details with the `<<` operator. +/// When the InternalCompilerError is destructed, the concatenated error message +/// is appended to the diagnostics list with the severity of +/// tint::diag::Severity::InternalCompilerError, and if a +/// InternalCompilerErrorReporter is set, then it is called with the diagnostic +/// list. +class InternalCompilerError { + public: + /// 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 @@ /// 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) \ - tint::InternalCompilerError(__FILE__, __LINE__, msg, diagnostics) +/// Use the `<<` operator to append an error message to the ICE. +#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_
diff --git a/src/intrinsic_table.cc b/src/intrinsic_table.cc index 0475721..34550b1 100644 --- a/src/intrinsic_table.cc +++ b/src/intrinsic_table.cc
@@ -1448,20 +1448,20 @@ 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 " + - CallSignature(builder, intrinsic, args) + - ", but didn't resolve the open type " + - str(open_type)); + TINT_ICE(diagnostics) + << "IntrinsicTable overload matched for " + << CallSignature(builder, intrinsic, args) + << ", 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 " + - CallSignature(builder, intrinsic, args) + - ", but open type " + str(open_type) + - " is nullptr"); + TINT_ICE(diagnostics) + << "IntrinsicTable overload matched for " + << CallSignature(builder, intrinsic, args) << ", but open type " + << str(open_type) << " is nullptr"; return nullptr; } if (!matcher->Match(matcher_state, resolved_type)) {
diff --git a/src/transform/hlsl.cc b/src/transform/hlsl.cc index b0ce373..af67bcc 100644 --- a/src/transform/hlsl.cc +++ b/src/transform/hlsl.cc
@@ -57,9 +57,8 @@ 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( - ctx.dst->Diagnostics(), - "ast::TypeConstructorExpression has no semantic expression node"); + TINT_ICE(ctx.dst->Diagnostics()) + << "ast::TypeConstructorExpression has no semantic expression node"; continue; } auto* src_sem_stmt = src_sem_expr->Stmt();
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index d8ee19d..9a60d21 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc
@@ -799,7 +799,7 @@ 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 @@ 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 @@ 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 @@ case semantic::IntrinsicType::kTextureNumSamples: switch (texture_type->dim()) { default: - diagnostics_.add_error( - "texture dimension does not support multisampling"); + TINT_ICE(diagnostics_) + << "texture dimension does not support multisampling"; return false; case type::TextureDimension::k2d: num_dimensions = 3; @@ -884,7 +885,7 @@ } break; default: - diagnostics_.add_error("unexpected intrinsic"); + TINT_ICE(diagnostics_) << "unexpected intrinsic"; return false; } @@ -2468,9 +2469,8 @@ if (auto* st = tex->As<type::StorageTexture>()) { auto* component = image_format_to_rwtexture_type(st->image_format()); if (component == nullptr) { - diagnostics_.add_error( - "Unsupported StorageTexture ImageFormat: " + - std::to_string(static_cast<int>(st->image_format()))); + TINT_ICE(diagnostics_) << "Unsupported StorageTexture ImageFormat: " + << static_cast<int>(st->image_format()); return false; } out << "<" << component << ">";
diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index ac835bd..f21598f 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc
@@ -699,8 +699,8 @@ out_ << ".write("; break; default: - TINT_ICE(diagnostics_, "Unhandled texture intrinsic '" + - std::string(intrinsic->str()) + "'"); + TINT_UNREACHABLE(diagnostics_) + << "Unhandled texture intrinsic '" << intrinsic->str() << "'"; return false; }