From 2a36ff611d1bdba710ba78ed292447d0240c1aa2 Mon Sep 17 00:00:00 2001
From: Jon Ross-Perkins <jperkins@google.com>
Date: Thu, 17 Oct 2024 14:39:21 -0700
Subject: [PATCH] Remove a couple std::string uses in diagnostics. (#4421)

We can't completely remove std::string from diagnostics because it's
probably better to provide a string than something like a
StringLiteralValueId or NameId (because those may be opaque for someone
trying to present the diagnostic). So this change is just playing
whackamole on another couple things that could easily use the new format
providers.
---
 toolchain/check/handle_literal.cpp |  7 ++++---
 toolchain/check/handle_struct.cpp  | 16 ++++++++++------
 toolchain/source/BUILD             |  1 +
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/toolchain/check/handle_literal.cpp b/toolchain/check/handle_literal.cpp
index 1242aa934758..f6fab497e92d 100644
--- a/toolchain/check/handle_literal.cpp
+++ b/toolchain/check/handle_literal.cpp
@@ -5,6 +5,7 @@
 #include "toolchain/check/call.h"
 #include "toolchain/check/context.h"
 #include "toolchain/check/handle.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/typed_insts.h"
 
 namespace Carbon::Check {
@@ -127,10 +128,10 @@ static auto HandleIntOrUnsignedIntTypeLiteral(Context& context,
   if (!(context.ints().Get(size_id) & 3).isZero()) {
     CARBON_DIAGNOSTIC(IntWidthNotMultipleOf8, Error,
                       "bit width of integer type literal must be a multiple of "
-                      "8; use `Core.{0}({1})` instead",
-                      std::string, llvm::APSInt);
+                      "8; use `Core.{0:Int|UInt}({1})` instead",
+                      BoolAsSelect, llvm::APSInt);
     context.emitter().Emit(
-        node_id, IntWidthNotMultipleOf8, int_kind.is_signed() ? "Int" : "UInt",
+        node_id, IntWidthNotMultipleOf8, int_kind.is_signed(),
         llvm::APSInt(context.ints().Get(size_id), /*isUnsigned=*/true));
   }
   auto width_id = MakeI32Literal(context, node_id, size_id);
diff --git a/toolchain/check/handle_struct.cpp b/toolchain/check/handle_struct.cpp
index 66f6ec851fd3..ea72270a1a14 100644
--- a/toolchain/check/handle_struct.cpp
+++ b/toolchain/check/handle_struct.cpp
@@ -6,6 +6,7 @@
 #include "toolchain/check/context.h"
 #include "toolchain/check/convert.h"
 #include "toolchain/check/handle.h"
+#include "toolchain/diagnostics/format_providers.h"
 
 namespace Carbon::Check {
 
@@ -70,7 +71,7 @@ auto HandleParseNode(Context& context, Parse::StructTypeFieldId node_id)
 
 static auto DiagnoseDuplicateNames(Context& context,
                                    SemIR::InstBlockId type_block_id,
-                                   llvm::StringRef construct) -> bool {
+                                   bool is_struct_type_literal) -> bool {
   auto& sem_ir = context.sem_ir();
   auto fields = sem_ir.inst_blocks().Get(type_block_id);
   Map<SemIR::NameId, SemIR::InstId> names;
@@ -80,12 +81,13 @@ static auto DiagnoseDuplicateNames(Context& context,
     auto result = names.Insert(field_inst.name_id, field_inst_id);
     if (!result.is_inserted()) {
       CARBON_DIAGNOSTIC(StructNameDuplicate, Error,
-                        "duplicated field name `{1}` in {0}", std::string,
-                        SemIR::NameId);
+                        "duplicated field name `{1}` in "
+                        "{0:struct type literal|struct literal}",
+                        BoolAsSelect, SemIR::NameId);
       CARBON_DIAGNOSTIC(StructNamePrevious, Note,
                         "field with the same name here");
       context.emitter()
-          .Build(field_inst_id, StructNameDuplicate, construct.str(),
+          .Build(field_inst_id, StructNameDuplicate, is_struct_type_literal,
                  field_inst.name_id)
           .Note(result.value(), StructNamePrevious)
           .Emit();
@@ -103,7 +105,8 @@ auto HandleParseNode(Context& context, Parse::StructLiteralId node_id) -> bool {
   context.node_stack()
       .PopAndDiscardSoloNodeId<Parse::NodeKind::StructLiteralStart>();
   auto type_block_id = context.args_type_info_stack().Pop();
-  if (DiagnoseDuplicateNames(context, type_block_id, "struct literal")) {
+  if (DiagnoseDuplicateNames(context, type_block_id,
+                             /*is_struct_type_literal=*/false)) {
     context.node_stack().Push(node_id, SemIR::InstId::BuiltinError);
     return true;
   }
@@ -128,7 +131,8 @@ auto HandleParseNode(Context& context, Parse::StructTypeLiteralId node_id)
   CARBON_CHECK(refs_id != SemIR::InstBlockId::Empty,
                "{{}} is handled by StructLiteral.");
 
-  if (DiagnoseDuplicateNames(context, refs_id, "struct type literal")) {
+  if (DiagnoseDuplicateNames(context, refs_id,
+                             /*is_struct_type_literal=*/true)) {
     context.node_stack().Push(node_id, SemIR::InstId::BuiltinError);
     return true;
   }
diff --git a/toolchain/source/BUILD b/toolchain/source/BUILD
index c0fe8532465e..e96d7fc0c4a4 100644
--- a/toolchain/source/BUILD
+++ b/toolchain/source/BUILD
@@ -13,6 +13,7 @@ cc_library(
     deps = [
         "//common:error",
         "//toolchain/diagnostics:diagnostic_emitter",
+        "//toolchain/diagnostics:format_providers",
         "@llvm-project//llvm:Support",
     ],
 )