Remove uses of StringLiteral in format strings.

Author: jonmeow

toolchain/check/call.cpp

@@ -9,6 +9,7 @@
 #include "toolchain/check/convert.h"
 #include "toolchain/check/deduce.h"
 #include "toolchain/check/function.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/builtin_function_kind.h"
 #include "toolchain/sem_ir/builtin_inst_kind.h"
 #include "toolchain/sem_ir/entity_with_params_base.h"
@@ -18,6 +19,15 @@
 
 namespace Carbon::Check {
 
+namespace {
+// Entity kinds for the diagnostic. Converted to an int for a select.
+enum class EntityKind : uint8_t {
+  Function,
+  GenericClass,
+  GenericInterface,
+};
+}  // namespace
+
 // Resolves the callee expression in a call to a specific callee, or diagnoses
 // if no specific callee can be identified. This verifies the arity of the
 // callee and determines any compile-time arguments, but doesn't check that the
@@ -30,7 +40,7 @@ namespace Carbon::Check {
 // callee is not generic, or `nullopt` if an error has been diagnosed.
 static auto ResolveCalleeInCall(Context& context, SemIR::LocId loc_id,
                                 const SemIR::EntityWithParamsBase& entity,
-                                llvm::StringLiteral entity_kind_for_diagnostic,
+                                EntityKind entity_kind_for_diagnostic,
                                 SemIR::GenericId entity_generic_id,
                                 SemIR::SpecificId enclosing_specific_id,
                                 SemIR::InstId self_id,
@@ -42,16 +52,20 @@ static auto ResolveCalleeInCall(Context& context, SemIR::LocId loc_id,
   auto params = context.inst_blocks().GetOrEmpty(callee_info.param_refs_id);
   if (arg_ids.size() != params.size()) {
     CARBON_DIAGNOSTIC(CallArgCountMismatch, Error,
-                      "{0} argument(s) passed to {1} expecting "
-                      "{2} argument(s).",
-                      int, llvm::StringLiteral, int);
-    CARBON_DIAGNOSTIC(InCallToEntity, Note, "calling {0} declared here",
-                      llvm::StringLiteral);
+                      "{0} argument(s) passed to "
+                      "{1:=0:function|=1:generic class|=2:generic interface}"
+                      " expecting {2} argument(s).",
+                      int, IntAsSelect, int);
+    CARBON_DIAGNOSTIC(
+        InCallToEntity, Note,
+        "calling {0:=0:function|=1:generic class|=2:generic interface}"
+        " declared here",
+        IntAsSelect);
     context.emitter()
         .Build(loc_id, CallArgCountMismatch, arg_ids.size(),
-               entity_kind_for_diagnostic, params.size())
+               static_cast<int>(entity_kind_for_diagnostic), params.size())
         .Note(callee_info.callee_loc, InCallToEntity,
-              entity_kind_for_diagnostic)
+              static_cast<int>(entity_kind_for_diagnostic))
         .Emit();
     return std::nullopt;
   }
@@ -79,8 +93,9 @@ static auto PerformCallToGenericClass(Context& context, SemIR::LocId loc_id,
     -> SemIR::InstId {
   const auto& generic_class = context.classes().Get(class_id);
   auto callee_specific_id = ResolveCalleeInCall(
-      context, loc_id, generic_class, "generic class", generic_class.generic_id,
-      enclosing_specific_id, /*self_id=*/SemIR::InstId::Invalid, arg_ids);
+      context, loc_id, generic_class, EntityKind::GenericClass,
+      generic_class.generic_id, enclosing_specific_id,
+      /*self_id=*/SemIR::InstId::Invalid, arg_ids);
   if (!callee_specific_id) {
     return SemIR::InstId::BuiltinError;
   }
@@ -98,8 +113,9 @@ static auto PerformCallToGenericInterface(
     llvm::ArrayRef<SemIR::InstId> arg_ids) -> SemIR::InstId {
   const auto& interface = context.interfaces().Get(interface_id);
   auto callee_specific_id = ResolveCalleeInCall(
-      context, loc_id, interface, "generic interface", interface.generic_id,
-      enclosing_specific_id, /*self_id=*/SemIR::InstId::Invalid, arg_ids);
+      context, loc_id, interface, EntityKind::GenericInterface,
+      interface.generic_id, enclosing_specific_id,
+      /*self_id=*/SemIR::InstId::Invalid, arg_ids);
   if (!callee_specific_id) {
     return SemIR::InstId::BuiltinError;
   }
@@ -142,7 +158,7 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
   // If the callee is a generic function, determine the generic argument values
   // for the call.
   auto callee_specific_id = ResolveCalleeInCall(
-      context, loc_id, callable, "function", callable.generic_id,
+      context, loc_id, callable, EntityKind::Function, callable.generic_id,
       callee_function.enclosing_specific_id, callee_function.self_id, arg_ids);
   if (!callee_specific_id) {
     return SemIR::InstId::BuiltinError;

toolchain/check/check.cpp

@@ -23,6 +23,7 @@
 #include "toolchain/check/sem_ir_diagnostic_converter.h"
 #include "toolchain/diagnostics/diagnostic.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/lex/token_kind.h"
 #include "toolchain/parse/node_ids.h"
 #include "toolchain/parse/tree.h"
@@ -1189,19 +1190,18 @@ static auto BuildApiMapAndDiagnosePackaging(
       bool is_api_with_impl_ext = !is_impl && filename.ends_with(ImplExt);
       auto want_ext = is_impl ? ImplExt : ApiExt;
       if (is_api_with_impl_ext || !filename.ends_with(want_ext)) {
-        CARBON_DIAGNOSTIC(IncorrectExtension, Error,
-                          "file extension of `{0}` required for `{1}`",
-                          llvm::StringLiteral, Lex::TokenKind);
+        CARBON_DIAGNOSTIC(
+            IncorrectExtension, Error,
+            "file extension of `{0:.impl|}.carbon` required for {0:`impl`|api}",
+            BoolAsSelect);
         auto diag = unit_info.emitter.Build(
             packaging ? packaging->names.node_id : Parse::NodeId::Invalid,
-            IncorrectExtension, want_ext,
-            is_impl ? Lex::TokenKind::Impl : Lex::TokenKind::Api);
+            IncorrectExtension, is_impl);
         if (is_api_with_impl_ext) {
-          CARBON_DIAGNOSTIC(IncorrectExtensionImplNote, Note,
-                            "file extension of `{0}` only allowed for `{1}`",
-                            llvm::StringLiteral, Lex::TokenKind);
-          diag.Note(Parse::NodeId::Invalid, IncorrectExtensionImplNote, ImplExt,
-                    Lex::TokenKind::Impl);
+          CARBON_DIAGNOSTIC(
+              IncorrectExtensionImplNote, Note,
+              "file extension of `.impl.carbon` only allowed for `impl`");
+          diag.Note(Parse::NodeId::Invalid, IncorrectExtensionImplNote);
         }
         diag.Emit();
       }

toolchain/check/eval.cpp

@@ -682,65 +682,65 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIRLoc loc,
 
   bool overflow = false;
   llvm::APInt result_val;
-  llvm::StringLiteral op_str = "<error>";
+  Lex::TokenKind op_token = Lex::TokenKind::Not;
   switch (builtin_kind) {
     // Arithmetic.
     case SemIR::BuiltinFunctionKind::IntSAdd:
       result_val = lhs_val.sadd_ov(rhs_val, overflow);
-      op_str = "+";
+      op_token = Lex::TokenKind::Plus;
       break;
     case SemIR::BuiltinFunctionKind::IntSSub:
       result_val = lhs_val.ssub_ov(rhs_val, overflow);
-      op_str = "-";
+      op_token = Lex::TokenKind::Minus;
       break;
     case SemIR::BuiltinFunctionKind::IntSMul:
       result_val = lhs_val.smul_ov(rhs_val, overflow);
-      op_str = "*";
+      op_token = Lex::TokenKind::Star;
       break;
     case SemIR::BuiltinFunctionKind::IntSDiv:
       result_val = lhs_val.sdiv_ov(rhs_val, overflow);
-      op_str = "/";
+      op_token = Lex::TokenKind::Slash;
       break;
     case SemIR::BuiltinFunctionKind::IntSMod:
       result_val = lhs_val.srem(rhs_val);
       // LLVM weirdly lacks `srem_ov`, so we work it out for ourselves:
       // <signed min> % -1 overflows because <signed min> / -1 overflows.
       overflow = lhs_val.isMinSignedValue() && rhs_val.isAllOnes();
-      op_str = "%";
+      op_token = Lex::TokenKind::Percent;
       break;
     case SemIR::BuiltinFunctionKind::IntUAdd:
       result_val = lhs_val + rhs_val;
-      op_str = "+";
+      op_token = Lex::TokenKind::Plus;
       break;
     case SemIR::BuiltinFunctionKind::IntUSub:
       result_val = lhs_val - rhs_val;
-      op_str = "-";
+      op_token = Lex::TokenKind::Minus;
       break;
     case SemIR::BuiltinFunctionKind::IntUMul:
       result_val = lhs_val * rhs_val;
-      op_str = "*";
+      op_token = Lex::TokenKind::Star;
       break;
     case SemIR::BuiltinFunctionKind::IntUDiv:
       result_val = lhs_val.udiv(rhs_val);
-      op_str = "/";
+      op_token = Lex::TokenKind::Slash;
       break;
     case SemIR::BuiltinFunctionKind::IntUMod:
       result_val = lhs_val.urem(rhs_val);
-      op_str = "%";
+      op_token = Lex::TokenKind::Percent;
       break;
 
     // Bitwise.
     case SemIR::BuiltinFunctionKind::IntAnd:
       result_val = lhs_val & rhs_val;
-      op_str = "&";
+      op_token = Lex::TokenKind::And;
       break;
     case SemIR::BuiltinFunctionKind::IntOr:
       result_val = lhs_val | rhs_val;
-      op_str = "|";
+      op_token = Lex::TokenKind::Pipe;
       break;
     case SemIR::BuiltinFunctionKind::IntXor:
       result_val = lhs_val ^ rhs_val;
-      op_str = "^";
+      op_token = Lex::TokenKind::Caret;
       break;
 
     // Bit shift.
@@ -777,9 +777,9 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIRLoc loc,
   if (overflow) {
     CARBON_DIAGNOSTIC(CompileTimeIntegerOverflow, Error,
                       "integer overflow in calculation {0} {1} {2}", TypedInt,
-                      llvm::StringLiteral, TypedInt);
+                      Lex::TokenKind, TypedInt);
     context.emitter().Emit(loc, CompileTimeIntegerOverflow,
-                           {.type = lhs.type_id, .value = lhs_val}, op_str,
+                           {.type = lhs.type_id, .value = lhs_val}, op_token,
                            {.type = rhs.type_id, .value = rhs_val});
   }
 

toolchain/check/testdata/packages/fail_extension.carbon

@@ -7,11 +7,11 @@
 // TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/packages/fail_extension.carbon
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/packages/fail_extension.carbon
-// CHECK:STDERR: fail_main.incorrect: error: file extension of `.carbon` required for `api`
+// CHECK:STDERR: fail_main.incorrect: error: file extension of `.carbon` required for api
 // CHECK:STDERR:
 // CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: `Main//default` previously provided by `fail_main.incorrect`
 // CHECK:STDERR:
-// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: file extension of `.carbon` required for `api`
+// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: file extension of `.carbon` required for api
 // CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: note: file extension of `.impl.carbon` only allowed for `impl`
 // CHECK:STDERR:
 
@@ -21,7 +21,7 @@
 
 // --- fail_main_lib.incorrect
 
-// CHECK:STDERR: fail_main_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for `api`
+// CHECK:STDERR: fail_main_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for api
 // CHECK:STDERR: library "main_lib";
 // CHECK:STDERR: ^~~~~~~
 // CHECK:STDERR:
@@ -37,7 +37,7 @@ impl library "[[@TEST_NAME]]";
 
 // --- fail_package.incorrect
 
-// CHECK:STDERR: fail_package.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for `api`
+// CHECK:STDERR: fail_package.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for api
 // CHECK:STDERR: package Package;
 // CHECK:STDERR: ^~~~~~~
 // CHECK:STDERR:
@@ -53,7 +53,7 @@ impl package Package;
 
 // --- fail_package_lib.incorrect
 
-// CHECK:STDERR: fail_package_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for `api`
+// CHECK:STDERR: fail_package_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for api
 // CHECK:STDERR: package Package library "package_lib";
 // CHECK:STDERR: ^~~~~~~
 // CHECK:STDERR:
@@ -69,7 +69,7 @@ impl package Package library "[[@TEST_NAME]]";
 
 // --- fail_swapped_ext.impl.carbon
 
-// CHECK:STDERR: fail_swapped_ext.impl.carbon:[[@LINE+5]]:1: error: file extension of `.carbon` required for `api`
+// CHECK:STDERR: fail_swapped_ext.impl.carbon:[[@LINE+5]]:1: error: file extension of `.carbon` required for api
 // CHECK:STDERR: package SwappedExt;
 // CHECK:STDERR: ^~~~~~~
 // CHECK:STDERR: fail_swapped_ext.impl.carbon: note: file extension of `.impl.carbon` only allowed for `impl`

toolchain/diagnostics/diagnostic.h

@@ -119,8 +119,10 @@ struct DiagnosticBase {
   explicit constexpr DiagnosticBase(DiagnosticKind kind, DiagnosticLevel level,
                                     llvm::StringLiteral format)
       : Kind(kind), Level(level), Format(format) {
-    static_assert((... && !std::is_same_v<Args, llvm::StringRef>),
-                  "Use std::string or llvm::StringLiteral for diagnostics to "
+    static_assert((... && !(std::is_same_v<Args, llvm::StringRef> ||
+                            std::is_same_v<Args, llvm::StringLiteral>)),
+                  "For diagnostics, use a format provider (see "
+                  "toolchain/diagnostics/format_providers.h) or std::string to "
                   "avoid lifetime issues.");
   }
 

toolchain/diagnostics/diagnostic_emitter_test.cpp

@@ -55,7 +55,7 @@ TEST_F(DiagnosticEmitterTest, EmitSimpleWarning) {
 }
 
 TEST_F(DiagnosticEmitterTest, EmitOneArgDiagnostic) {
-  CARBON_DIAGNOSTIC(TestDiagnostic, Error, "arg: `{0}`", llvm::StringLiteral);
+  CARBON_DIAGNOSTIC(TestDiagnostic, Error, "arg: `{0}`", std::string);
   EXPECT_CALL(consumer_, HandleDiagnostic(IsSingleDiagnostic(
                              DiagnosticKind::TestDiagnostic,
                              DiagnosticLevel::Error, 1, 1, "arg: `str`")));

toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp

@@ -17,7 +17,7 @@ namespace {
 using ::Carbon::Testing::IsSingleDiagnostic;
 using ::testing::InSequence;
 
-CARBON_DIAGNOSTIC(TestDiagnostic, Error, "{0}", llvm::StringLiteral);
+CARBON_DIAGNOSTIC(TestDiagnostic, Error, "M{0}", int);
 
 struct FakeDiagnosticConverter : DiagnosticConverter<DiagnosticLoc> {
   auto ConvertLoc(DiagnosticLoc loc, ContextFnT /*context_fn*/) const
@@ -32,12 +32,12 @@ TEST(SortedDiagnosticEmitterTest, SortErrors) {
   SortingDiagnosticConsumer sorting_consumer(consumer);
   DiagnosticEmitter<DiagnosticLoc> emitter(converter, sorting_consumer);
 
-  emitter.Emit({"f", "line", 2, 1}, TestDiagnostic, "M1");
-  emitter.Emit({"f", "line", 1, 1}, TestDiagnostic, "M2");
-  emitter.Emit({"f", "line", 1, 3}, TestDiagnostic, "M3");
-  emitter.Emit({"f", "line", 3, 4}, TestDiagnostic, "M4");
-  emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, "M5");
-  emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, "M6");
+  emitter.Emit({"f", "line", 2, 1}, TestDiagnostic, 1);
+  emitter.Emit({"f", "line", 1, 1}, TestDiagnostic, 2);
+  emitter.Emit({"f", "line", 1, 3}, TestDiagnostic, 3);
+  emitter.Emit({"f", "line", 3, 4}, TestDiagnostic, 4);
+  emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, 5);
+  emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, 6);
 
   InSequence s;
   EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic(

toolchain/lex/token_kind.def

@@ -167,7 +167,6 @@ CARBON_KEYWORD_TOKEN(Abstract,            "abstract")
 CARBON_KEYWORD_TOKEN(Addr,                "addr")
 CARBON_TOKEN_WITH_VIRTUAL_NODE(
   CARBON_KEYWORD_TOKEN(And,               "and"))
-CARBON_KEYWORD_TOKEN(Api,                 "api")
 CARBON_KEYWORD_TOKEN(As,                  "as")
 CARBON_KEYWORD_TOKEN(Auto,                "auto")
 CARBON_KEYWORD_TOKEN(Bool,                "bool")

toolchain/parse/context.cpp

@@ -77,9 +77,9 @@ auto Context::ConsumeAndAddCloseSymbol(Lex::TokenIndex expected_open,
     // TODO: Include the location of the matching opening delimiter in the
     // diagnostic.
     CARBON_DIAGNOSTIC(ExpectedCloseSymbol, Error,
-                      "unexpected tokens before `{0}`", llvm::StringLiteral);
+                      "unexpected tokens before `{0}`", Lex::TokenKind);
     emitter_->Emit(*position_, ExpectedCloseSymbol,
-                   open_token_kind.closing_symbol().fixed_spelling());
+                   open_token_kind.closing_symbol());
 
     SkipTo(tokens().GetMatchedClosingToken(expected_open));
     AddNode(close_kind, Consume(), /*has_error=*/true);

toolchain/parse/handle_binding_pattern.cpp

@@ -2,6 +2,7 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/parse/context.h"
 #include "toolchain/parse/handle.h"
 
@@ -25,12 +26,13 @@ auto HandleBindingPattern(Context& context) -> void {
   }
 
   // Handle an invalid pattern introducer for parameters and variables.
-  auto on_error = [&](llvm::StringLiteral expected) {
+  auto on_error = [&](bool expected_name) {
     if (!state.has_error) {
       CARBON_DIAGNOSTIC(ExpectedBindingPattern, Error,
-                        "expected {0} in binding pattern", llvm::StringLiteral);
+                        "expected {0:name|`:` or `:!`} in binding pattern",
+                        BoolAsSelect);
       context.emitter().Emit(*context.position(), ExpectedBindingPattern,
-                             expected);
+                             expected_name);
       state.has_error = true;
     }
   };
@@ -51,7 +53,7 @@ auto HandleBindingPattern(Context& context) -> void {
     // Add a placeholder for the name.
     context.AddLeafNode(NodeKind::IdentifierName, *context.position(),
                         /*has_error=*/true);
-    on_error("name");
+    on_error(/*expected_name=*/true);
   }
 
   if (auto kind = context.PositionKind();
@@ -64,7 +66,7 @@ auto HandleBindingPattern(Context& context) -> void {
     context.PushState(state);
     context.PushStateForExpr(PrecedenceGroup::ForType());
   } else {
-    on_error("`:` or `:!`");
+    on_error(/*expected_name=*/false);
     // Add a placeholder for the type.
     context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
                         /*has_error=*/true);