Implement basic bool and int formatting for diagnostics (#4411)

Note, this supports plurals, but doesn't apply it anywhere. I'm mainly
doing that to demonstrate the approach regarding syntax. See
format_providers.h for details.

Author: jonmeow

toolchain/check/BUILD

@@ -74,6 +74,7 @@ cc_library(
         "//toolchain/check:generic_region_stack",
         "//toolchain/check:scope_stack",
         "//toolchain/diagnostics:diagnostic_emitter",
+        "//toolchain/diagnostics:format_providers",
         "//toolchain/lex:token_kind",
         "//toolchain/lex:tokenized_buffer",
         "//toolchain/parse:node_kind",
@@ -116,6 +117,7 @@ cc_library(
         "//toolchain/base:pretty_stack_trace_function",
         "//toolchain/base:value_store",
         "//toolchain/diagnostics:diagnostic_emitter",
+        "//toolchain/diagnostics:format_providers",
         "//toolchain/lex:token_index",
         "//toolchain/lex:token_kind",
         "//toolchain/lex:tokenized_buffer",

toolchain/check/convert.cpp

@@ -14,6 +14,7 @@
 #include "toolchain/check/context.h"
 #include "toolchain/check/operator.h"
 #include "toolchain/check/pattern_match.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/copy_on_write_block.h"
 #include "toolchain/sem_ir/file.h"
 #include "toolchain/sem_ir/generic.h"
@@ -381,8 +382,13 @@ static auto ConvertStructToStructOrClass(Context& context,
                                          SemIR::StructType src_type,
                                          SemIR::StructType dest_type,
                                          SemIR::InstId value_id,
-                                         ConversionTarget target, bool is_class)
+                                         ConversionTarget target)
     -> SemIR::InstId {
+  static_assert(std::is_same_v<SemIR::ClassElementAccess, TargetAccessInstT> ||
+                std::is_same_v<SemIR::StructAccess, TargetAccessInstT>);
+  constexpr bool ToClass =
+      std::is_same_v<SemIR::ClassElementAccess, TargetAccessInstT>;
+
   auto& sem_ir = context.sem_ir();
   auto src_elem_fields = sem_ir.inst_blocks().Get(src_type.fields_id);
   auto dest_elem_fields = sem_ir.inst_blocks().Get(dest_type.fields_id);
@@ -406,14 +412,14 @@ static auto ConvertStructToStructOrClass(Context& context,
   // TODO: If not, include the name of the first source field that doesn't
   // exist in the destination or vice versa in the diagnostic.
   if (src_elem_fields.size() != dest_elem_fields.size()) {
-    CARBON_DIAGNOSTIC(StructInitElementCountMismatch, Error,
-                      "cannot initialize {0} with {1} field(s) from struct "
-                      "with {2} field(s).",
-                      llvm::StringLiteral, size_t, size_t);
-    context.emitter().Emit(
-        value_loc_id, StructInitElementCountMismatch,
-        is_class ? llvm::StringLiteral("class") : llvm::StringLiteral("struct"),
-        dest_elem_fields.size(), src_elem_fields.size());
+    CARBON_DIAGNOSTIC(
+        StructInitElementCountMismatch, Error,
+        "cannot initialize {0:class|struct} with {1} field(s) from struct "
+        "with {2} field(s).",
+        BoolAsSelect, size_t, size_t);
+    context.emitter().Emit(value_loc_id, StructInitElementCountMismatch,
+                           ToClass, dest_elem_fields.size(),
+                           src_elem_fields.size());
     return SemIR::InstId::BuiltinError;
   }
 
@@ -493,7 +499,7 @@ static auto ConvertStructToStructOrClass(Context& context,
     new_block.Set(i, init_id);
   }
 
-  if (is_class) {
+  if (ToClass) {
     target.init_block->InsertHere();
     CARBON_CHECK(is_init,
                  "Converting directly to a class value is not supported");
@@ -522,7 +528,7 @@ static auto ConvertStructToStruct(Context& context, SemIR::StructType src_type,
                                   SemIR::InstId value_id,
                                   ConversionTarget target) -> SemIR::InstId {
   return ConvertStructToStructOrClass<SemIR::StructAccess>(
-      context, src_type, dest_type, value_id, target, /*is_class=*/false);
+      context, src_type, dest_type, value_id, target);
 }
 
 // Performs a conversion from a struct to a class type. This function only
@@ -554,7 +560,7 @@ static auto ConvertStructToClass(Context& context, SemIR::StructType src_type,
   }
 
   auto result_id = ConvertStructToStructOrClass<SemIR::ClassElementAccess>(
-      context, src_type, dest_struct_type, value_id, target, /*is_class=*/true);
+      context, src_type, dest_struct_type, value_id, target);
 
   if (need_temporary) {
     target_block.InsertHere();
@@ -1154,13 +1160,11 @@ static auto ConvertSelf(Context& context, SemIR::LocId call_loc_id,
   bool addr_pattern = context.insts().Is<SemIR::AddrPattern>(self_param_id);
   DiagnosticAnnotationScope annotate_diagnostics(
       &context.emitter(), [&](auto& builder) {
-        CARBON_DIAGNOSTIC(
-            InCallToFunctionSelf, Note,
-            "initializing `{0}` parameter of method declared here",
-            llvm::StringLiteral);
-        builder.Note(self_param_id, InCallToFunctionSelf,
-                     addr_pattern ? llvm::StringLiteral("addr self")
-                                  : llvm::StringLiteral("self"));
+        CARBON_DIAGNOSTIC(InCallToFunctionSelf, Note,
+                          "initializing `{0:addr self|self}` parameter of "
+                          "method declared here",
+                          BoolAsSelect);
+        builder.Note(self_param_id, InCallToFunctionSelf, addr_pattern);
       });
 
   return CallerPatternMatch(context, callee_specific_id, self_param_id,

toolchain/check/eval.cpp

@@ -8,6 +8,7 @@
 #include "toolchain/check/diagnostic_helpers.h"
 #include "toolchain/check/generic.h"
 #include "toolchain/diagnostics/diagnostic_emitter.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/builtin_function_kind.h"
 #include "toolchain/sem_ir/function.h"
 #include "toolchain/sem_ir/generic.h"
@@ -745,18 +746,17 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIRLoc loc,
     // Bit shift.
     case SemIR::BuiltinFunctionKind::IntLeftShift:
     case SemIR::BuiltinFunctionKind::IntRightShift:
-      op_str = (builtin_kind == SemIR::BuiltinFunctionKind::IntLeftShift)
-                   ? llvm::StringLiteral("<<")
-                   : llvm::StringLiteral(">>");
       if (rhs_val.uge(lhs_val.getBitWidth()) ||
           (rhs_val.isNegative() && context.types().IsSignedInt(rhs.type_id))) {
-        CARBON_DIAGNOSTIC(CompileTimeShiftOutOfRange, Error,
-                          "shift distance not in range [0, {0}) in {1} {2} {3}",
-                          unsigned, TypedInt, llvm::StringLiteral, TypedInt);
-        context.emitter().Emit(loc, CompileTimeShiftOutOfRange,
-                               lhs_val.getBitWidth(),
-                               {.type = lhs.type_id, .value = lhs_val}, op_str,
-                               {.type = rhs.type_id, .value = rhs_val});
+        CARBON_DIAGNOSTIC(
+            CompileTimeShiftOutOfRange, Error,
+            "shift distance not in range [0, {0}) in {1} {2:<<|>>} {3}",
+            unsigned, TypedInt, BoolAsSelect, TypedInt);
+        context.emitter().Emit(
+            loc, CompileTimeShiftOutOfRange, lhs_val.getBitWidth(),
+            {.type = lhs.type_id, .value = lhs_val},
+            builtin_kind == SemIR::BuiltinFunctionKind::IntLeftShift,
+            {.type = rhs.type_id, .value = rhs_val});
         // TODO: Is it useful to recover by returning 0 or -1?
         return SemIR::ConstantId::Error;
       }

toolchain/check/handle_binding_pattern.cpp

@@ -6,6 +6,7 @@
 #include "toolchain/check/convert.h"
 #include "toolchain/check/handle.h"
 #include "toolchain/check/return.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/inst.h"
 
@@ -104,23 +105,19 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id,
           cast_type_id,
           [&] {
             CARBON_DIAGNOSTIC(IncompleteTypeInVarDecl, Error,
-                              "{0} has incomplete type {1}",
-                              llvm::StringLiteral, SemIR::TypeId);
-            return context.emitter().Build(
-                type_node, IncompleteTypeInVarDecl,
-                parent_class_decl ? llvm::StringLiteral("field")
-                                  : llvm::StringLiteral("variable"),
-                cast_type_id);
+                              "{0:field|variable} has incomplete type {1}",
+                              BoolAsSelect, SemIR::TypeId);
+            return context.emitter().Build(type_node, IncompleteTypeInVarDecl,
+                                           parent_class_decl.has_value(),
+                                           cast_type_id);
           },
           [&] {
             CARBON_DIAGNOSTIC(AbstractTypeInVarDecl, Error,
-                              "{0} has abstract type {1}", llvm::StringLiteral,
-                              SemIR::TypeId);
-            return context.emitter().Build(
-                type_node, AbstractTypeInVarDecl,
-                parent_class_decl ? llvm::StringLiteral("field")
-                                  : llvm::StringLiteral("variable"),
-                cast_type_id);
+                              "{0:field|variable} has abstract type {1}",
+                              BoolAsSelect, SemIR::TypeId);
+            return context.emitter().Build(type_node, AbstractTypeInVarDecl,
+                                           parent_class_decl.has_value(),
+                                           cast_type_id);
           });
       if (parent_class_decl) {
         CARBON_CHECK(context_node_kind == Parse::NodeKind::VariableIntroducer,

toolchain/check/merge.cpp

@@ -6,6 +6,7 @@
 
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/import_ref.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/sem_ir/ids.h"
 #include "toolchain/sem_ir/typed_insts.h"
 
@@ -193,10 +194,12 @@ static auto EntityHasParamError(Context& context, const DeclParams& info)
 
 // Returns false if a param differs for a redeclaration. The caller is expected
 // to provide a diagnostic.
-static auto CheckRedeclParam(
-    Context& context, llvm::StringLiteral param_diag_label, int32_t param_index,
-    SemIR::InstId new_param_pattern_id, SemIR::InstId prev_param_pattern_id,
-    SemIR::SpecificId prev_specific_id, bool diagnose) -> bool {
+static auto CheckRedeclParam(Context& context, bool is_implicit_param,
+                             int32_t param_index,
+                             SemIR::InstId new_param_pattern_id,
+                             SemIR::InstId prev_param_pattern_id,
+                             SemIR::SpecificId prev_specific_id, bool diagnose)
+    -> bool {
   // TODO: Consider differentiating between type and name mistakes. For now,
   // taking the simpler approach because I also think we may want to refactor
   // params.
@@ -205,15 +208,16 @@ static auto CheckRedeclParam(
       return;
     }
     CARBON_DIAGNOSTIC(RedeclParamDiffers, Error,
-                      "redeclaration differs at {0}parameter {1}",
-                      llvm::StringLiteral, int32_t);
-    CARBON_DIAGNOSTIC(RedeclParamPrevious, Note,
-                      "previous declaration's corresponding {0}parameter here",
-                      llvm::StringLiteral);
+                      "redeclaration differs at {0:implicit |}parameter {1}",
+                      BoolAsSelect, int32_t);
+    CARBON_DIAGNOSTIC(
+        RedeclParamPrevious, Note,
+        "previous declaration's corresponding {0:implicit |}parameter here",
+        BoolAsSelect);
     context.emitter()
-        .Build(new_param_pattern_id, RedeclParamDiffers, param_diag_label,
+        .Build(new_param_pattern_id, RedeclParamDiffers, is_implicit_param,
                param_index + 1)
-        .Note(prev_param_pattern_id, RedeclParamPrevious, param_diag_label)
+        .Note(prev_param_pattern_id, RedeclParamPrevious, is_implicit_param)
         .Emit();
   };
 
@@ -265,7 +269,7 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc,
                               SemIR::InstBlockId new_param_patterns_id,
                               SemIRLoc prev_decl_loc,
                               SemIR::InstBlockId prev_param_patterns_id,
-                              llvm::StringLiteral param_diag_label,
+                              bool is_implicit_param,
                               SemIR::SpecificId prev_specific_id, bool diagnose)
     -> bool {
   // This will often occur for empty params.
@@ -279,18 +283,18 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc,
       return false;
     }
     CARBON_DIAGNOSTIC(RedeclParamListDiffers, Error,
-                      "redeclaration differs because of {1}{0}parameter list",
-                      llvm::StringLiteral, llvm::StringLiteral);
+                      "redeclaration differs because of "
+                      "{1:'|missing '}{0:implicit |}parameter list",
+                      BoolAsSelect, BoolAsSelect);
     CARBON_DIAGNOSTIC(RedeclParamListPrevious, Note,
-                      "previously declared with{1} {0}parameter list",
-                      llvm::StringLiteral, llvm::StringLiteral);
+                      "previously declared "
+                      "{1:with|without} {0:implicit |}parameter list",
+                      BoolAsSelect, BoolAsSelect);
     context.emitter()
-        .Build(new_decl_loc, RedeclParamListDiffers, param_diag_label,
-               new_param_patterns_id.is_valid() ? llvm::StringLiteral("")
-                                                : "missing ")
-        .Note(
-            prev_decl_loc, RedeclParamListPrevious, param_diag_label,
-            prev_param_patterns_id.is_valid() ? llvm::StringLiteral("") : "out")
+        .Build(new_decl_loc, RedeclParamListDiffers, is_implicit_param,
+               new_param_patterns_id.is_valid())
+        .Note(prev_decl_loc, RedeclParamListPrevious, is_implicit_param,
+              prev_param_patterns_id.is_valid())
         .Emit();
     return false;
   }
@@ -307,22 +311,23 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc,
     }
     CARBON_DIAGNOSTIC(
         RedeclParamCountDiffers, Error,
-        "redeclaration differs because of {0}parameter count of {1}",
-        llvm::StringLiteral, int32_t);
-    CARBON_DIAGNOSTIC(RedeclParamCountPrevious, Note,
-                      "previously declared with {0}parameter count of {1}",
-                      llvm::StringLiteral, int32_t);
+        "redeclaration differs because of {0:implicit |}parameter count of {1}",
+        BoolAsSelect, int32_t);
+    CARBON_DIAGNOSTIC(
+        RedeclParamCountPrevious, Note,
+        "previously declared with {0:implicit |}parameter count of {1}",
+        BoolAsSelect, int32_t);
     context.emitter()
-        .Build(new_decl_loc, RedeclParamCountDiffers, param_diag_label,
+        .Build(new_decl_loc, RedeclParamCountDiffers, is_implicit_param,
                new_param_pattern_ids.size())
-        .Note(prev_decl_loc, RedeclParamCountPrevious, param_diag_label,
+        .Note(prev_decl_loc, RedeclParamCountPrevious, is_implicit_param,
               prev_param_pattern_ids.size())
         .Emit();
     return false;
   }
   for (auto [index, new_param_pattern_id, prev_param_pattern_id] :
        llvm::enumerate(new_param_pattern_ids, prev_param_pattern_ids)) {
-    if (!CheckRedeclParam(context, param_diag_label, index,
+    if (!CheckRedeclParam(context, is_implicit_param, index,
                           new_param_pattern_id, prev_param_pattern_id,
                           prev_specific_id, diagnose)) {
       return false;
@@ -410,15 +415,16 @@ auto CheckRedeclParamsMatch(Context& context, const DeclParams& new_entity,
       EntityHasParamError(context, prev_entity)) {
     return false;
   }
-  if (!CheckRedeclParams(context, new_entity.loc,
-                         new_entity.implicit_param_patterns_id, prev_entity.loc,
-                         prev_entity.implicit_param_patterns_id, "implicit ",
-                         prev_specific_id, diagnose)) {
+  if (!CheckRedeclParams(
+          context, new_entity.loc, new_entity.implicit_param_patterns_id,
+          prev_entity.loc, prev_entity.implicit_param_patterns_id,
+          /*is_implicit_param=*/true, prev_specific_id, diagnose)) {
     return false;
   }
   if (!CheckRedeclParams(context, new_entity.loc, new_entity.param_patterns_id,
-                         prev_entity.loc, prev_entity.param_patterns_id, "",
-                         prev_specific_id, diagnose)) {
+                         prev_entity.loc, prev_entity.param_patterns_id,
+                         /*is_implicit_param=*/false, prev_specific_id,
+                         diagnose)) {
     return false;
   }
   if (check_syntax &&

toolchain/diagnostics/BUILD

@@ -25,6 +25,7 @@ cc_library(
     ],
     deps = [
         ":diagnostic_kind",
+        ":format_providers",
         "//common:check",
         "//common:ostream",
         "@llvm-project//llvm:Support",
@@ -56,6 +57,31 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "format_providers",
+    srcs = ["format_providers.cpp"],
+    hdrs = ["format_providers.h"],
+    deps = [
+        "//common:check",
+        "//common:ostream",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
+cc_test(
+    name = "format_providers_test",
+    size = "small",
+    srcs = ["format_providers_test.cpp"],
+    deps = [
+        ":diagnostic_emitter",
+        ":format_providers",
+        ":mocks",
+        "//testing/base:gtest_main",
+        "@googletest//:gtest",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
 cc_library(
     name = "null_diagnostics",
     hdrs = ["null_diagnostics.h"],