Implement basic bool and int formatting for diagnostics

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).",
+        FormatBool, size_t, size_t);
+    context.emitter().Emit(value_loc_id, StructInitElementCountMismatch,
+                           {.value = 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,12 @@ 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);
+        CARBON_DIAGNOSTIC(InCallToFunctionSelf, Note,
+                          "initializing `{0:addr self|self}` parameter of "
+                          "method declared here",
+                          FormatBool);
         builder.Note(self_param_id, InCallToFunctionSelf,
-                     addr_pattern ? llvm::StringLiteral("addr self")
-                                  : llvm::StringLiteral("self"));
+                     {.value = 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, FormatBool, TypedInt);
+        context.emitter().Emit(
+            loc, CompileTimeShiftOutOfRange, lhs_val.getBitWidth(),
+            {.type = lhs.type_id, .value = lhs_val},
+            {.value = 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);
+                              "{0:field|variable} has incomplete type {1}",
+                              FormatBool, SemIR::TypeId);
             return context.emitter().Build(
                 type_node, IncompleteTypeInVarDecl,
-                parent_class_decl ? llvm::StringLiteral("field")
-                                  : llvm::StringLiteral("variable"),
-                cast_type_id);
+                {.value = parent_class_decl.has_value()}, cast_type_id);
           },
           [&] {
             CARBON_DIAGNOSTIC(AbstractTypeInVarDecl, Error,
-                              "{0} has abstract type {1}", llvm::StringLiteral,
-                              SemIR::TypeId);
+                              "{0:field|variable} has abstract type {1}",
+                              FormatBool, SemIR::TypeId);
             return context.emitter().Build(
                 type_node, AbstractTypeInVarDecl,
-                parent_class_decl ? llvm::StringLiteral("field")
-                                  : llvm::StringLiteral("variable"),
-                cast_type_id);
+                {.value = 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,11 +6,35 @@
 
 #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"
 
 namespace Carbon::Check {
 
+// Explicit or implicit parameters. Used in diagnostics.
+enum class ParamKind : uint8_t {
+  Explicit,
+  Implicit,
+};
+
+}  // namespace Carbon::Check
+
+// `ParamKind` is formatted in diagnostics as `{0}parameters`, and we only add
+// text for implicit paramseters.
+template <>
+struct llvm::format_provider<Carbon::Check::ParamKind> {
+  using ParamKind = Carbon::Check::ParamKind;
+  static void format(const ParamKind& kind, raw_ostream& out,
+                     StringRef /*style*/) {
+    if (kind == ParamKind::Implicit) {
+      out << "implicit ";
+    }
+  }
+};
+
+namespace Carbon::Check {
+
 CARBON_DIAGNOSTIC(RedeclPrevDecl, Note, "previously declared here");
 
 // Diagnoses a redeclaration which is redundant.
@@ -193,10 +217,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, ParamKind param_kind,
+                             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 +231,15 @@ static auto CheckRedeclParam(
       return;
     }
     CARBON_DIAGNOSTIC(RedeclParamDiffers, Error,
-                      "redeclaration differs at {0}parameter {1}",
-                      llvm::StringLiteral, int32_t);
+                      "redeclaration differs at {0}parameter {1}", ParamKind,
+                      int32_t);
     CARBON_DIAGNOSTIC(RedeclParamPrevious, Note,
                       "previous declaration's corresponding {0}parameter here",
-                      llvm::StringLiteral);
+                      ParamKind);
     context.emitter()
-        .Build(new_param_pattern_id, RedeclParamDiffers, param_diag_label,
+        .Build(new_param_pattern_id, RedeclParamDiffers, param_kind,
                param_index + 1)
-        .Note(prev_param_pattern_id, RedeclParamPrevious, param_diag_label)
+        .Note(prev_param_pattern_id, RedeclParamPrevious, param_kind)
         .Emit();
   };
 
@@ -265,7 +291,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,
+                              ParamKind param_kind,
                               SemIR::SpecificId prev_specific_id, bool diagnose)
     -> bool {
   // This will often occur for empty params.
@@ -278,19 +304,18 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc,
     if (!diagnose) {
       return false;
     }
-    CARBON_DIAGNOSTIC(RedeclParamListDiffers, Error,
-                      "redeclaration differs because of {1}{0}parameter list",
-                      llvm::StringLiteral, llvm::StringLiteral);
+    CARBON_DIAGNOSTIC(
+        RedeclParamListDiffers, Error,
+        "redeclaration differs because of {1:'|missing '}{0}parameter list",
+        ParamKind, FormatBool);
     CARBON_DIAGNOSTIC(RedeclParamListPrevious, Note,
-                      "previously declared with{1} {0}parameter list",
-                      llvm::StringLiteral, llvm::StringLiteral);
+                      "previously declared {1:with|without} {0}parameter list",
+                      ParamKind, FormatBool);
     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, param_kind,
+               {.value = new_param_patterns_id.is_valid()})
+        .Note(prev_decl_loc, RedeclParamListPrevious, param_kind,
+              {.value = prev_param_patterns_id.is_valid()})
         .Emit();
     return false;
   }
@@ -307,24 +332,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);
+        "redeclaration differs because of {0}parameter count of {1}", ParamKind,
+        int32_t);
     CARBON_DIAGNOSTIC(RedeclParamCountPrevious, Note,
                       "previously declared with {0}parameter count of {1}",
-                      llvm::StringLiteral, int32_t);
+                      ParamKind, int32_t);
     context.emitter()
-        .Build(new_decl_loc, RedeclParamCountDiffers, param_diag_label,
+        .Build(new_decl_loc, RedeclParamCountDiffers, param_kind,
                new_param_pattern_ids.size())
-        .Note(prev_decl_loc, RedeclParamCountPrevious, param_diag_label,
+        .Note(prev_decl_loc, RedeclParamCountPrevious, param_kind,
               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,
-                          new_param_pattern_id, prev_param_pattern_id,
-                          prev_specific_id, diagnose)) {
+    if (!CheckRedeclParam(context, param_kind, index, new_param_pattern_id,
+                          prev_param_pattern_id, prev_specific_id, diagnose)) {
       return false;
     }
   }
@@ -412,13 +436,13 @@ auto CheckRedeclParamsMatch(Context& context, const DeclParams& new_entity,
   }
   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)) {
+                         prev_entity.implicit_param_patterns_id,
+                         ParamKind::Implicit, 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,
+                         ParamKind::Explicit, 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,30 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "format_providers",
+    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"],