From 41e3c9bd828b52ea8857338fa2db9b61bcc271f4 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Tue, 9 Jun 2026 23:49:00 -0700 Subject: [PATCH] Type-erase CARBON_CHECK message formatting. (#7325) `CARBON_CHECK` and `CARBON_FATAL` messages are formatted with `llvm::formatv`. Previously each check site that had a message instantiated its own copy of the formatv machinery -- a `formatv_object` over a tuple of per-argument format adapters, plus that tuple -- in every translation unit, keyed on the site's file, line, condition, and format strings. A translation unit with many checks paid for that machinery over and over. This restructures check failure so that the formatting machinery is compiled exactly once, and only a single small adapter is instantiated per distinct value type per translation unit: - `CheckFailImpl` (out-of-line) now takes the message's format string and an array of already-type-erased `format_adapter`s, and renders the whole failure message -- prefix plus the extra message -- directly into one stream. The extra message is rendered in place, so no separate string is ever materialized for it. - `FormatvInto` (in the `.cpp`) renders a format string over that adapter array. Rather than instantiate `llvm::formatv`, it drives the formatv replacement loop over the public `formatv_object_base::parseFormatString`, so this rendering code exists exactly once. (A TODO notes that we should add a type-erased entry point upstream in LLVM rather than reimplement the loop here.) - The lowering from the macro down to that out-of-line call is split so that the only per-check-site instantiation is trivial: - `CheckFail<...>` is instantiated once per site, since its file, line, condition, and format template-string parameters are unique to the site. It just lowers those compile-time strings to ordinary arguments and forwards to `CheckFailFormat`. - `CheckFailFormat` is instantiated once per distinct value-type sequence and shared across sites; it builds one type-erased adapter per value. - `CheckFailWithAdapters` collects pointers to those adapters into an array and calls `CheckFailImpl`. It is a distinct function so the adapter temporaries stay alive while pointers to their base class are in flight. Format semantics, including runtime format-string validation, are unchanged, and the rendered message is byte-for-byte identical. For `DCHECK` in optimized builds the check is dead code; its arguments are now routed through a trivial `IgnoreDeadCheckArgs` no-op rather than `CheckFail`. This still type-checks the arguments so they cannot bitrot, without instantiating any formatting machinery for them and without provoking unused-variable warnings. Measured full-rebuild impact (353 first-party translation units, fastbuild): -112.6s CPU, -6.9% relative to trunk. Assisted-by: Claude --- common/check_internal.cpp | 57 ++++++++++++++++++--- common/check_internal.h | 103 ++++++++++++++++++++++++++------------ 2 files changed, 122 insertions(+), 38 deletions(-) diff --git a/common/check_internal.cpp b/common/check_internal.cpp index ea744b3269e5..f16718735947 100644 --- a/common/check_internal.cpp +++ b/common/check_internal.cpp @@ -8,18 +8,61 @@ #include #include "common/ostream.h" +#include "llvm/Support/FormatCommon.h" +#include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Signals.h" +#include "llvm/Support/raw_ostream.h" namespace Carbon::Internal { -auto CheckFailImpl(const char* kind, const char* file, int line, - const char* condition_str, llvm::StringRef extra_message) +namespace { +// Renders `fmt` over the externally-built, type-erased `adapters` into `out`, +// with the same semantics as `llvm::formatv` (including runtime format-string +// validation). +// +// TODO: We should add a type-erased helper to upstream LLVM instead of rolling +// our own type-erased version of `format` here. +auto FormatvInto( + llvm::raw_ostream& out, llvm::StringRef format_str, + llvm::ArrayRef adapters) -> void { + for (const llvm::ReplacementItem& replacement : + llvm::formatv_object_base::parseFormatString(format_str, adapters.size(), + /*Validate=*/true)) { + if (replacement.Type == llvm::ReplacementType::Literal || + replacement.Index >= adapters.size()) { + out << replacement.Spec; + continue; + } + llvm::FmtAlign(*adapters[replacement.Index], replacement.Where, + replacement.Width, replacement.Pad) + .format(out, replacement.Options); + } +} +} // namespace + +auto CheckFailImpl( + const char* kind, const char* file, int line, const char* condition_str, + const char* extra_format, + llvm::ArrayRef extra_adapters) -> void { - // Render the final check string here. - std::string message = llvm::formatv( - "{0} failure at {1}:{2}{3}{4}{5}{6}\n", kind, file, line, - llvm::StringRef(condition_str).empty() ? "" : ": ", condition_str, - extra_message.empty() ? "" : ": ", extra_message); + // Render the final check string directly into one stream. The extra message + // is rendered in place from its format string and type-erased adapters, so + // we never materialize a separate string just for it. + // + // `llvm::raw_string_ostream` (rather than `common/raw_string_ostream.h`) is + // used to avoid a dependency cycle: `RawStringOstream` itself uses + // `CARBON_CHECK`. It is unbuffered, so `message` is populated directly. + std::string message; + llvm::raw_string_ostream message_stream(message); + message_stream << kind << " failure at " << file << ":" << line; + if (*condition_str != '\0') { + message_stream << ": " << condition_str; + } + if (*extra_format != '\0') { + message_stream << ": "; + FormatvInto(message_stream, extra_format, extra_adapters); + } + message_stream << "\n"; // This macro is defined by `--config=non-fatal-checks`. #ifdef CARBON_NON_FATAL_CHECKS diff --git a/common/check_internal.h b/common/check_internal.h index bcb04c9a8d2a..e94e713a6b17 100644 --- a/common/check_internal.h +++ b/common/check_internal.h @@ -39,13 +39,17 @@ CheckCondition(bool condition) // because we know that these are available as C strings and passing them that // way lets the code size of calling it be smaller: it only needs to materialize // a single pointer argument for each. The runtime cost of re-computing the size -// should be minimal. The extra message however might not be compile-time -// guaranteed to be a C string so we use a normal `StringRef` there. +// should be minimal. +// +// The user can provide an extra format string along with an array of +// type-erased format adapters. This will be rendered into the final message. #ifdef NDEBUG [[noreturn]] #endif -auto CheckFailImpl(const char* kind, const char* file, int line, - const char* condition_str, llvm::StringRef extra_message) +auto CheckFailImpl( + const char* kind, const char* file, int line, const char* condition_str, + const char* extra_format, + llvm::ArrayRef extra_adapters) -> void; // Allow converting format values; the default behaviour is to just pass them @@ -70,36 +74,71 @@ auto ConvertFormatValue(T&& t) -> auto { } } +// Collects pointers to the given type-erased format adapters and passes them, +// with the rest of the check metadata, to the out-of-line `CheckFailImpl`. +// +// We need a separate function accepting all the adapters as arguments to ensure +// those objects stay alive for pointers to their base class to be put into an +// array and passed to the type erased implementation. +template +#ifdef NDEBUG +[[noreturn]] +#endif +auto CheckFailWithAdapters(const char* kind, const char* file, int line, + const char* condition_str, const char* extra_format, + Adapters&&... adapters) -> void { + std::array + adapter_pointers = {&adapters...}; + CheckFailImpl(kind, file, line, condition_str, extra_format, + adapter_pointers); +} + +// Builds one type-erased format adapter per value -- forwarding each value +// through the conversion machinery -- and hands them to +// `CheckFailWithAdapters`. +// +// This is templated only on the value types, not on the per-check-site +// metadata (file, line, etc., which are passed as ordinary arguments), so the +// adapter-building is instantiated once per distinct sequence of value types in +// the TU. +template +#ifdef NDEBUG +[[noreturn]] +#endif +auto CheckFailFormat(const char* kind, const char* file, int line, + const char* condition_str, const char* extra_format, + Ts&&... values) -> void { + CheckFailWithAdapters(kind, file, line, condition_str, extra_format, + llvm::support::detail::build_format_adapter( + ConvertFormatValue(std::forward(values)))...); +} + // Prints a check failure, including rendering any user-provided message using // a format string. // -// Most of the parameters are passed as compile-time template strings to avoid -// runtime cost of parameter setup in optimized builds. Each of these are passed -// along to the underlying implementation to include in the final printed -// message. -// -// Any user-provided format string and values are directly passed to -// `llvm::formatv` which handles all of the formatting of output. +// The check-site metadata is passed as compile-time template strings to avoid +// runtime cost of parameter setup in optimized builds. This function is +// instantiated once per check site (its template arguments are unique to the +// site), so it is kept trivial: it just lowers those template strings to +// ordinary arguments and forwards everything to `CheckFailFormat`, where the +// adapter-building is shared across sites with the same value types. template #ifdef NDEBUG [[noreturn]] #endif [[gnu::cold, clang::noinline]] auto CheckFail(Ts&&... values) -> void { - if constexpr (llvm::StringRef(FormatStr).empty()) { - // Skip the format string rendering if empty. Note that we don't skip it - // even if there are no values as we want to have consistent handling of - // `{}`s in the format string. This case is about when there is no message - // at all, just the condition. - CheckFailImpl(Kind.c_str(), File.c_str(), Line, ConditionStr.c_str(), ""); - } else { - CheckFailImpl(Kind.c_str(), File.c_str(), Line, ConditionStr.c_str(), - llvm::formatv(FormatStr.c_str(), - ConvertFormatValue(std::forward(values))...) - .str()); - } + CheckFailFormat(Kind.c_str(), File.c_str(), Line, ConditionStr.c_str(), + FormatStr.c_str(), std::forward(values)...); } +// Type-checks the arguments of a `DCHECK` in optimized builds, where the check +// itself is dead code, without instantiating any formatting machinery for them +// and without provoking unused-variable warnings. It is only ever named from +// dead code, so it is never actually called. +template +auto IgnoreDeadCheckArgs(Ts&&... /*values*/) -> void {} + } // namespace Carbon::Internal // Evaluates the condition of a CHECK as a boolean value. @@ -148,21 +187,23 @@ template () + Carbon::Internal::IgnoreDeadCheckArgs() #define CARBON_INTERNAL_DEAD_DCHECK_IMPL_FORMAT(format_str, ...) \ - Carbon::Internal::CheckFail<"", "", 0, "", "">(__VA_ARGS__) + Carbon::Internal::IgnoreDeadCheckArgs(__VA_ARGS__) -// The CheckFail function itself is noreturn in NDEBUG. +// The `CheckFail` function itself is noreturn in NDEBUG. #define CARBON_INTERNAL_FATAL_NORETURN_SUFFIX() void() #else #define CARBON_INTERNAL_FATAL_NORETURN_SUFFIX() std::abort()