Switch `CARBON_VLOG` to support a format string API. (#4283)

The goal is to replace our stream operator APIs with format string APIs
that can be made to have much less impact on inlining and other
optimizations of the performance critical path through the code.

Several experiments show that the most compact representation we can
arrange for is one that calls an uninlined function and passes a minimal
number of arguments to it. It doesn't help to do any work to minimize
the arguments such as building a lambda -- the cost of extra code to
merge the arguments is likely to outweigh the benefit.

Initial experiments showed that switching a hot but uninlined function
to this new API enabled inlining and the subsequent performance
improvement.

This also adds a 'TemplateString` utility that allows using a string
literal as a template parameter. This is useful to remove the format
string itself from the arguments passed to the function by passing it as
a template argument instead.

Currently, support is left in place for both APIs because with
`CARBON_VLOG` we can detect whether or not any message was provided
expecting a format string. This should allow incrementally migrating
code to this API. I've added some test coverage in this PR, but I'll
separate out any switching of parts of the codebase over.

The goal is to eventually replace all the usages and remove the
streaming support entirely.

This PR doesn't update `CARBON_CHECK` in the same way because it is
substantially more complex to switch. I have a few experimental PRs
looking at that and will discuss how best to approach this with the
specific challenges check presents separately. But the goal is for all
of the macro-based output APIs to move to format strings rather than
streams.

---------

Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>

common/BUILD

@@ -469,6 +469,25 @@ cc_test(
     ],
 )
 
+cc_library(
+    name = "template_string",
+    hdrs = ["template_string.h"],
+    deps = [
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_test(
+    name = "template_string_test",
+    size = "small",
+    srcs = ["template_string_test.cpp"],
+    deps = [
+        ":template_string",
+        "//testing/base:gtest_main",
+        "@googletest//:gtest",
+    ],
+)
+
 cc_library(
     name = "variant_helpers",
     hdrs = ["variant_helpers.h"],
@@ -555,6 +574,7 @@ cc_library(
     hdrs = ["vlog.h"],
     deps = [
         ":ostream",
+        ":template_string",
         "@llvm-project//llvm:Support",
     ],
 )

common/template_string.h

@@ -0,0 +1,80 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_COMMON_TEMPLATE_STRING_H_
+#define CARBON_COMMON_TEMPLATE_STRING_H_
+
+#include "llvm/ADT/StringRef.h"
+
+namespace Carbon {
+
+// Represents a compile-time string in a form suitable for use as a non-type
+// template argument.
+//
+// These arguments are required to be a "structural type", and so we copy the
+// string contents into a public array of `char`s. For details, see:
+// https://en.cppreference.com/w/cpp/language/template_parameters#Non-type_template_parameter
+//
+// Designed to support implicitly deduced construction from a string literal
+// template argument. This type will implicitly convert to an `llvm::StringRef`
+// for accessing the string contents, and also provides a dedicated `c_str()`
+// method to access the string as a C string.
+//
+// Example usage:
+// ```cpp
+// template <TemplateString Str> auto F() -> void {
+//   llvm::cout() << Str;
+// }
+//
+// auto Example() -> void {
+//   F<"string contents here">();
+// }
+// ```
+template <int N>
+struct TemplateString {
+  // Constructs the template string from a string literal.
+  //
+  // Intentionally allows implicit conversion from string literals for use as a
+  // non-type template parameter.
+  //
+  // The closest we can get to explicitly accepting a string literal is to
+  // accept an array of `const char`s, so we additionally use Clang's constexpr
+  // `enable_if` attribute to require the array to be usable as a C string with
+  // the expected length. This checks both for null-termination and no embedded
+  // `0` bytes.
+  //
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  constexpr TemplateString(const char (&str)[N + 1]) __attribute__((
+      enable_if(__builtin_strlen(str) == N,
+                "character array is not null-terminated valid C string"))) {
+    // Rely on Clang's constexpr `__builtin_memcpy` to minimize compile time
+    // overhead copying the string contents around.
+    __builtin_memcpy(storage_, str, N + 1);
+  }
+
+  // This type is designed to act as a `StringRef` implicitly while having the
+  // storage necessary to be used as a template parameter.
+  //
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  constexpr operator llvm::StringRef() const {
+    return llvm::StringRef(storage_, N);
+  }
+
+  // Accesses the string data directly as a compile-time C string.
+  constexpr auto c_str() const -> const char* { return storage_; }
+
+  // Note that this must be public for the type to be structural and available
+  // as a template argument, but this is not part of the public API.
+  char storage_[N + 1];
+};
+
+// Allow deducing `N` when implicitly constructing these so that we can directly
+// use a string literal in a template argument. The array needs an extra char
+// for the null terminator.
+template <int M>
+TemplateString(const char (&str)[M]) -> TemplateString<M - 1>;
+
+}  // namespace Carbon
+
+#endif  // CARBON_COMMON_TEMPLATE_STRING_H_

common/template_string_test.cpp

@@ -0,0 +1,76 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "common/template_string.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace Carbon {
+namespace {
+
+using ::testing::StrEq;
+
+template <TemplateString S>
+constexpr auto FromTemplate() -> llvm::StringRef {
+  return S;
+}
+
+template <TemplateString S>
+constexpr auto CStrFromTemplate() -> const char* {
+  return S.c_str();
+}
+
+// An overload that will be active when it is passed a valid `TemplateString`.
+// Returns a true type to allow detection of a valid `TemplateString` argument.
+template <TemplateString /*Unused*/>
+constexpr auto IsValidTemplateString(int /*unused*/) -> std::true_type {
+  return {};
+}
+
+// A struct that can be used as a template parameter for any template argument.
+struct AnythingAsTemplateArg {
+  // An implicit constructor that can accept any argument and discards it.
+  template <typename T>
+  // NOLINTNEXTLINE(google-explicit-constructor,bugprone-forwarding-reference-overload)
+  constexpr AnythingAsTemplateArg(T&& /*unused*/) {}
+};
+
+// An overload that will be active for any template argument. Returns a false
+// type and is used to detect when a template argument cannot correctly match a
+// `TemplateString`.
+template <AnythingAsTemplateArg /*Unused*/>
+constexpr auto IsValidTemplateString(...) -> std::false_type {
+  return {};
+}
+
+// Compile time tests with `static_assert`
+static_assert(FromTemplate<"test">().size() == 4,
+              "Not usable in a `constexpr` context.");
+static_assert(__builtin_strlen(CStrFromTemplate<"test">()) == 4,
+              "Not usable in a `constexpr` context.");
+
+// The string must not contain embedded nulls.
+static_assert(IsValidTemplateString<"test">(0));
+static_assert(!IsValidTemplateString<"test\0test">(0));
+
+// The string must be null-terminated.
+using FourChars = char[4];
+static_assert(IsValidTemplateString<FourChars{'t', 'e', 's', 0}>(0));
+static_assert(!IsValidTemplateString<FourChars{'t', 'e', 's', 't'}>(0));
+
+TEST(TemplateStringTest, Test) {
+  EXPECT_THAT(FromTemplate<"test">(), StrEq("test"));
+  EXPECT_THAT(CStrFromTemplate<"test">(), StrEq("test"));
+
+  constexpr char GoodStr[5] = {'t', 'e', 's', 't', '\0'};
+  static_assert(IsValidTemplateString<GoodStr>(0));
+  EXPECT_THAT(FromTemplate<GoodStr>(), StrEq("test"));
+
+  constexpr char BadStr[4] = {'t', 'e', 's', 't'};
+  static_assert(!IsValidTemplateString<BadStr>(0));
+}
+
+}  // namespace
+}  // namespace Carbon

common/vlog.h

@@ -12,11 +12,24 @@ namespace Carbon {
 // Logs when verbose logging is enabled (vlog_stream_ is non-null).
 //
 // For example:
-//   CARBON_VLOG() << "Verbose message";
-#define CARBON_VLOG()                             \
-  __builtin_expect(vlog_stream_ == nullptr, true) \
-      ? (void)0                                   \
-      : CARBON_VLOG_INTERNAL_STREAM(vlog_stream_)
+//   CARBON_VLOG("Verbose message: {0}", "extra information");
+//
+// The first argument must be a string literal format string valid for passing
+// to `llvm::formatv`. If it contains any substitutions, those should be passed
+// as subsequent arguments.
+//
+// Also supports a legacy syntax where no arguments are passed and the desired
+// logging is streamed into the call:
+//   CARBON_VLOG() << "Legacy verbose message";
+//
+// However, the streaming syntax has higher overhead and can inhibit inlining.
+// Code should prefer the format string form, and eventually when all code has
+// migrated the streaming interface will be removed.
+#define CARBON_VLOG(...)                                                    \
+  __builtin_expect(vlog_stream_ == nullptr, true)                           \
+      ? (void)0                                                             \
+      : CARBON_VLOG_INTERNAL##__VA_OPT__(_CALL)(vlog_stream_ __VA_OPT__(, ) \
+                                                    __VA_ARGS__)
 
 }  // namespace Carbon
 

common/vlog_internal.h

@@ -6,11 +6,16 @@
 #define CARBON_COMMON_VLOG_INTERNAL_H_
 
 #include "common/ostream.h"
+#include "common/template_string.h"
+#include "llvm/Support/FormatVariadic.h"
 
 namespace Carbon::Internal {
 
 // Wraps a stream and exiting for fatal errors. Should only be used by check.h
 // macros.
+//
+// TODO: Remove this when the last streaming `vlog` is replaced with a function
+// call variant.
 class VLoggingStream {
  public:
   // Internal type used in macros to dispatch to the `operator|` overload.
@@ -38,17 +43,33 @@ class VLoggingStream {
   }
 
  private:
-  [[noreturn]] auto Done() -> void;
-
   llvm::raw_ostream* stream_;
 };
 
+// Implements verbose logging.
+//
+// This is designed to minimize the overhead in callers by being a
+// forcibly outlined routine that takes a minimal number of parameters.
+//
+// Internally uses `llvm::formatv` to render the format string with any value
+// arguments, and streams the result to the provided stream.
+template <TemplateString FormatStr, typename... Ts>
+[[gnu::cold, clang::noinline, clang::preserve_most]] auto VLogImpl(
+    llvm::raw_ostream* stream, Ts&&... values) -> void {
+  *stream << llvm::formatv(FormatStr.c_str(), std::forward<Ts>(values)...);
+}
+
 }  // namespace Carbon::Internal
 
-// Raw logging stream. This should be used when building forms of vlog
-// macros.
-#define CARBON_VLOG_INTERNAL_STREAM(stream)    \
+// Raw logging stream. This should be used when building the streaming forms of
+// vlog macros.
+#define CARBON_VLOG_INTERNAL(stream)           \
   Carbon::Internal::VLoggingStream::Helper() | \
       Carbon::Internal::VLoggingStream(stream)
 
+// Raw logging call. This should be used when building the format-string forms
+// of vlog macros.
+#define CARBON_VLOG_INTERNAL_CALL(stream, FormatStr, ...) \
+  Carbon::Internal::VLogImpl<"" FormatStr>(stream __VA_OPT__(, ) __VA_ARGS__)
+
 #endif  // CARBON_COMMON_VLOG_INTERNAL_H_

common/vlog_test.cpp

@@ -24,7 +24,9 @@ class VLogger {
     }
   }
 
-  void VLog() { CARBON_VLOG() << "Test\n"; }
+  void VLog() { CARBON_VLOG("Test\n"); }
+  void VLogFormatArgs() { CARBON_VLOG("Test {0} {1} {2}\n", 1, 2, 3); }
+  void VLogStream() { CARBON_VLOG() << "Test\n"; }
 
   auto TakeStr() -> std::string { return buffer_.TakeStr(); }
 
@@ -38,6 +40,10 @@ TEST(VLogTest, Enabled) {
   VLogger vlog(/*enable=*/true);
   vlog.VLog();
   EXPECT_THAT(vlog.TakeStr(), StrEq("Test\n"));
+  vlog.VLogFormatArgs();
+  EXPECT_THAT(vlog.TakeStr(), StrEq("Test 1 2 3\n"));
+  vlog.VLogStream();
+  EXPECT_THAT(vlog.TakeStr(), StrEq("Test\n"));
 }
 
 TEST(VLogTest, Disabled) {