Просмотр исходного кода

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.
Jon Ross-Perkins 1 год назад
Родитель
Сommit
96964ee534

+ 2 - 0
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",

+ 23 - 19
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,

+ 10 - 10
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;
       }

+ 11 - 14
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,

+ 42 - 36
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 &&

+ 26 - 0
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"],

+ 92 - 0
toolchain/diagnostics/format_providers.cpp

@@ -0,0 +1,92 @@
+// 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 "toolchain/diagnostics/format_providers.h"
+
+#include "common/check.h"
+#include "llvm/ADT/StringExtras.h"
+
+auto llvm::format_provider<Carbon::BoolAsSelect>::format(
+    const Carbon::BoolAsSelect& wrapper, raw_ostream& out, StringRef style)
+    -> void {
+  if (style.empty()) {
+    llvm::format_provider<bool>::format(wrapper.value, out, style);
+    return;
+  }
+
+  // Remove wrapping quotes if present.
+  if (style.starts_with('\'') && style.ends_with('\'')) {
+    style = style.drop_front().drop_back();
+  }
+
+  auto sep = style.find('|');
+  CARBON_CHECK(
+      sep != llvm::StringRef::npos,
+      "BoolAsSelect requires a `|` separating true and false results: `{0}`",
+      style);
+  CARBON_CHECK(style.find('|', sep + 1) == llvm::StringRef::npos,
+               "BoolAsSelect only allows one `|`: `{0}`", style);
+
+  if (wrapper.value) {
+    out << style.take_front(sep);
+  } else {
+    out << style.drop_front(sep + 1);
+  }
+}
+
+auto llvm::format_provider<Carbon::IntAsSelect>::format(
+    const Carbon::IntAsSelect& wrapper, raw_ostream& out, StringRef style)
+    -> void {
+  if (style.empty()) {
+    llvm::format_provider<int>::format(wrapper.value, out, style);
+    return;
+  }
+
+  // Remove wrapping quotes if present.
+  if (style.starts_with('\'') && style.ends_with('\'')) {
+    style = style.drop_front().drop_back();
+  }
+
+  auto cursor = style;
+  while (!cursor.empty()) {
+    auto case_sep = cursor.find("|");
+    auto token = cursor.substr(0, case_sep);
+    if (case_sep == llvm::StringRef::npos) {
+      cursor = llvm::StringRef();
+    } else {
+      cursor = cursor.drop_front(case_sep + 1);
+    }
+
+    auto pair_sep = token.find(':');
+    CARBON_CHECK(pair_sep != llvm::StringRef::npos,
+                 "IntAsSelect requires a `:` separating each comparison and "
+                 "output string: `{0}`",
+                 style);
+
+    auto comp = token.take_front(pair_sep);
+    auto output_string = token.drop_front(pair_sep + 1);
+
+    if (comp.empty()) {
+      // Default case.
+      CARBON_CHECK(cursor.empty(),
+                   "IntAsSelect requires the default case be last: `{0}`",
+                   style);
+      out << output_string;
+      return;
+    } else if (comp.consume_front("=")) {
+      // Equality comparison.
+      int value;
+      CARBON_CHECK(to_integer(comp, value),
+                   "IntAsSelect has invalid value in comparison: `{0}`", style);
+      if (value == wrapper.value) {
+        out << output_string;
+        return;
+      }
+    } else {
+      CARBON_FATAL("IntAsSelect has unrecognized comparison: `{0}`", style);
+    }
+  }
+
+  CARBON_FATAL("IntAsSelect doesn't handle `{0}`: `{1}`", wrapper.value, style);
+}

+ 76 - 0
toolchain/diagnostics/format_providers.h

@@ -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
+
+#ifndef CARBON_TOOLCHAIN_DIAGNOSTICS_FORMAT_PROVIDERS_H_
+#define CARBON_TOOLCHAIN_DIAGNOSTICS_FORMAT_PROVIDERS_H_
+
+#include "common/ostream.h"
+#include "llvm/Support/FormatVariadicDetails.h"
+
+namespace Carbon {
+
+// Selects a formatv string based on the value. If the format style is not
+// provided, as in `{0}`, the value uses standard formatting.
+//
+// When used, the true and false outputs are separated by a `|`.
+//
+// For example, `{0:true|false}` would yield standard bool formatting.
+//
+// If needed, the _full_ style string can be wrapped with `'` in order to
+// preserve prefix or suffix whitespace (which is stripped by formatv). For
+// example, `{0:' true | false '}` retains whitespace which would be dropped
+// before `true` and after `false`.
+struct BoolAsSelect {
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  BoolAsSelect(bool value) : value(value) {}
+
+  bool value;
+};
+
+// Selects a formatv string based on the value. If the format style is not
+// provided, as in `{0}`, the value uses standard formatting.
+//
+// The style is a series of match cases, separated by `|`. Each case is a pair
+// formatted as `<selector>:<output string>`.
+//
+// Supported selectors are:
+// - `=<value>`: Matches when the value is correct.
+// - Empty for the default. This is optional, although it's a fatal error to not
+//   handle a value. If provided, it must be last.
+//
+// For example, `{0:=0:zero|=1:one|:other}` breaks down into:
+// - `=0` -> `zero`
+// - `=1` -> `one`
+// - default -> `other`
+//
+// As another example, `{0:=1:is|:are}` is a way to handle plural-based output.
+//
+// If needed, the _full_ style string can be wrapped with `'` in order to
+// preserve prefix or suffix whitespace (which is stripped by formatv). For
+// example, `{0:'=0: zero |=1: one '}` retains whitespace which would be dropped
+// after `one`.
+struct IntAsSelect {
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  IntAsSelect(int value) : value(value) {}
+
+  int value;
+};
+
+}  // namespace Carbon
+
+// See BoolAsSelect.
+template <>
+struct llvm::format_provider<Carbon::BoolAsSelect> {
+  static auto format(const Carbon::BoolAsSelect& wrapper, raw_ostream& out,
+                     StringRef style) -> void;
+};
+
+// See IntAsSelect.
+template <>
+struct llvm::format_provider<Carbon::IntAsSelect> {
+  static auto format(const Carbon::IntAsSelect& wrapper, raw_ostream& out,
+                     StringRef style) -> void;
+};
+
+#endif  // CARBON_TOOLCHAIN_DIAGNOSTICS_FORMAT_PROVIDERS_H_

+ 86 - 0
toolchain/diagnostics/format_providers_test.cpp

@@ -0,0 +1,86 @@
+// 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 "toolchain/diagnostics/format_providers.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "llvm/Support/FormatVariadic.h"
+
+namespace Carbon {
+namespace {
+
+using ::testing::Eq;
+
+TEST(BoolAsSelect, Cases) {
+  constexpr char Format[] = "{0:a|b}";
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(true)).str(), Eq("a"));
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(false)).str(), Eq("b"));
+}
+
+TEST(BoolAsSelect, CasesWithNormalFormat) {
+  constexpr char Format[] = "{0} {0:a|b}";
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(true)).str(), Eq("true a"));
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(false)).str(), Eq("false b"));
+}
+
+TEST(BoolAsSelect, Spaces) {
+  constexpr char Format[] = "{0: a | b }";
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(true)).str(), Eq("a "));
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(false)).str(), Eq(" b"));
+}
+
+TEST(BoolAsSelect, QuotedSpaces) {
+  constexpr char Format[] = "{0:' a | b '}";
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(true)).str(), Eq(" a "));
+  EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(false)).str(), Eq(" b "));
+}
+
+TEST(IntAsSelect, OnlyDefault) {
+  constexpr char Format[] = "{0::default}";
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("default"));
+}
+
+TEST(IntAsSelect, OneEquals) {
+  constexpr char Format[] = "{0:=0:zero}";
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("zero"));
+}
+
+TEST(IntAsSelect, TwoEquals) {
+  constexpr char Format[] = "{0:=0:zero|=1:one}";
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("zero"));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(1)).str(), Eq("one"));
+}
+
+TEST(IntAsSelect, TwoEqualsAndDefault) {
+  constexpr char Format[] = "{0:=0:zero|=1:one|:default}";
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("zero"));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(1)).str(), Eq("one"));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(2)).str(), Eq("default"));
+}
+
+TEST(IntAsSelect, Spaces) {
+  constexpr char Format[] = "{0:=0: zero |=1: one |: default }";
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq(" zero "));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(1)).str(), Eq(" one "));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(2)).str(), Eq(" default"));
+}
+
+TEST(IntAsSelect, QuotedSpaces) {
+  constexpr char Format[] = "{0:'=0: zero |=1: one |: default '}";
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq(" zero "));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(1)).str(), Eq(" one "));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(2)).str(), Eq(" default "));
+}
+
+TEST(IntAsSelect, PluralExample) {
+  constexpr char Format[] = "{0} argument{0:=1:|:s}";
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("0 arguments"));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(1)).str(), Eq("1 argument"));
+  EXPECT_THAT(llvm::formatv(Format, IntAsSelect(2)).str(), Eq("2 arguments"));
+}
+
+}  // namespace
+}  // namespace Carbon

+ 2 - 0
toolchain/parse/BUILD

@@ -62,6 +62,7 @@ cc_library(
         "//common:check",
         "//common:ostream",
         "//common:vlog",
+        "//toolchain/diagnostics:format_providers",
         "//toolchain/lex:token_kind",
         "//toolchain/lex:tokenized_buffer",
         "@llvm-project//llvm:Support",
@@ -89,6 +90,7 @@ cc_library(
         "//toolchain/base:pretty_stack_trace_function",
         "//toolchain/base:value_store",
         "//toolchain/diagnostics:diagnostic_emitter",
+        "//toolchain/diagnostics:format_providers",
         "//toolchain/lex:token_kind",
         "//toolchain/lex:tokenized_buffer",
     ],

+ 21 - 48
toolchain/parse/context.cpp

@@ -9,6 +9,7 @@
 #include "common/check.h"
 #include "common/ostream.h"
 #include "llvm/ADT/STLExtras.h"
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/lex/token_kind.h"
 #include "toolchain/lex/tokenized_buffer.h"
 #include "toolchain/parse/node_ids.h"
@@ -19,37 +20,6 @@
 
 namespace Carbon::Parse {
 
-// A relative location for characters in errors.
-enum class RelativeLoc : int8_t {
-  Around,
-  After,
-  Before,
-};
-
-}  // namespace Carbon::Parse
-
-// Adapts RelativeLoc for use with formatv.
-template <>
-struct llvm::format_provider<Carbon::Parse::RelativeLoc> {
-  using RelativeLoc = Carbon::Parse::RelativeLoc;
-  static void format(const RelativeLoc& loc, raw_ostream& out,
-                     StringRef /*style*/) {
-    switch (loc) {
-      case RelativeLoc::Around:
-        out << "around";
-        break;
-      case RelativeLoc::After:
-        out << "after";
-        break;
-      case RelativeLoc::Before:
-        out << "before";
-        break;
-    }
-  }
-};
-
-namespace Carbon::Parse {
-
 Context::Context(Tree& tree, Lex::TokenizedBuffer& tokens,
                  Lex::TokenDiagnosticEmitter& emitter,
                  llvm::raw_ostream* vlog_stream)
@@ -309,13 +279,16 @@ auto Context::DiagnoseOperatorFixity(OperatorFixity fixity) -> void {
     // Infix operators must satisfy the infix operator rules.
     if (!IsLexicallyValidInfixOperator()) {
       CARBON_DIAGNOSTIC(BinaryOperatorRequiresWhitespace, Error,
-                        "whitespace missing {0} binary operator", RelativeLoc);
-      emitter_->Emit(*position_, BinaryOperatorRequiresWhitespace,
-                     tokens().HasLeadingWhitespace(*position_)
-                         ? RelativeLoc::After
-                         : (tokens().HasTrailingWhitespace(*position_)
-                                ? RelativeLoc::Before
-                                : RelativeLoc::Around));
+                        "whitespace missing {0:=-1:before|=0:around|=1:after} "
+                        "binary operator",
+                        IntAsSelect);
+      IntAsSelect pos(0);
+      if (tokens().HasLeadingWhitespace(*position_)) {
+        pos.value = 1;
+      } else if (tokens().HasTrailingWhitespace(*position_)) {
+        pos.value = -1;
+      }
+      emitter_->Emit(*position_, BinaryOperatorRequiresWhitespace, pos);
     }
   } else {
     bool prefix = fixity == OperatorFixity::Prefix;
@@ -324,18 +297,18 @@ auto Context::DiagnoseOperatorFixity(OperatorFixity fixity) -> void {
     // its operand.
     if ((prefix ? tokens().HasTrailingWhitespace(*position_)
                 : tokens().HasLeadingWhitespace(*position_))) {
-      CARBON_DIAGNOSTIC(UnaryOperatorHasWhitespace, Error,
-                        "whitespace is not allowed {0} this unary operator",
-                        RelativeLoc);
-      emitter_->Emit(*position_, UnaryOperatorHasWhitespace,
-                     prefix ? RelativeLoc::After : RelativeLoc::Before);
+      CARBON_DIAGNOSTIC(
+          UnaryOperatorHasWhitespace, Error,
+          "whitespace is not allowed {0:after|before} this unary operator",
+          BoolAsSelect);
+      emitter_->Emit(*position_, UnaryOperatorHasWhitespace, prefix);
     } else if (IsLexicallyValidInfixOperator()) {
       // Pre/postfix operators must not satisfy the infix operator rules.
-      CARBON_DIAGNOSTIC(UnaryOperatorRequiresWhitespace, Error,
-                        "whitespace is required {0} this unary operator",
-                        RelativeLoc);
-      emitter_->Emit(*position_, UnaryOperatorRequiresWhitespace,
-                     prefix ? RelativeLoc::Before : RelativeLoc::After);
+      CARBON_DIAGNOSTIC(
+          UnaryOperatorRequiresWhitespace, Error,
+          "whitespace is required {0:before|after} this unary operator",
+          BoolAsSelect);
+      emitter_->Emit(*position_, UnaryOperatorRequiresWhitespace, prefix);
     }
   }
 }

+ 21 - 14
toolchain/parse/handle_brace_expr.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"
 
@@ -22,20 +23,26 @@ auto HandleBraceExpr(Context& context) -> void {
 static auto HandleBraceExprParamError(Context& context,
                                       Context::StateStackEntry state,
                                       State param_finish_state) -> void {
-  bool is_type = param_finish_state == State::BraceExprParamFinishAsType;
-  bool is_value = param_finish_state == State::BraceExprParamFinishAsValue;
-  bool is_unknown = param_finish_state == State::BraceExprParamFinishAsUnknown;
-  CARBON_CHECK(is_type || is_value || is_unknown);
-  CARBON_DIAGNOSTIC(ExpectedStructLiteralField, Error, "expected {0}{1}{2}",
-                    llvm::StringLiteral, llvm::StringLiteral,
-                    llvm::StringLiteral);
-  context.emitter().Emit(
-      *context.position(), ExpectedStructLiteralField,
-      (is_type || is_unknown) ? llvm::StringLiteral("`.field: field_type`")
-                              : llvm::StringLiteral(""),
-      is_unknown ? llvm::StringLiteral(" or ") : llvm::StringLiteral(""),
-      (is_value || is_unknown) ? llvm::StringLiteral("`.field = value`")
-                               : llvm::StringLiteral(""));
+  IntAsSelect mode(0);
+  switch (param_finish_state) {
+    case State::BraceExprParamFinishAsType:
+      mode.value = 0;
+      break;
+    case State::BraceExprParamFinishAsValue:
+      mode.value = 1;
+      break;
+    case State::BraceExprParamFinishAsUnknown:
+      mode.value = 2;
+      break;
+    default:
+      CARBON_FATAL("Unexpected state: {0}", param_finish_state);
+  }
+  CARBON_DIAGNOSTIC(
+      ExpectedStructLiteralField, Error,
+      "expected {0:=0:`.field: field_type`|"
+      "=1:`.field = value`|=2:`.field: field_type` or `.field = value`}",
+      IntAsSelect);
+  context.emitter().Emit(*context.position(), ExpectedStructLiteralField, mode);
 
   state.has_error = true;
   context.PushState(state, param_finish_state);

+ 4 - 4
toolchain/parse/handle_period.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"
 
@@ -36,10 +37,9 @@ static auto HandlePeriodOrArrow(Context& context, NodeKind node_kind,
     return;
   } else {
     CARBON_DIAGNOSTIC(ExpectedIdentifierAfterPeriodOrArrow, Error,
-                      "expected identifier after `{0}`", llvm::StringLiteral);
-    context.emitter().Emit(
-        *context.position(), ExpectedIdentifierAfterPeriodOrArrow,
-        is_arrow ? llvm::StringLiteral("->") : llvm::StringLiteral("."));
+                      "expected identifier after `{0:->|.}`", BoolAsSelect);
+    context.emitter().Emit(*context.position(),
+                           ExpectedIdentifierAfterPeriodOrArrow, is_arrow);
     // If we see a keyword, assume it was intended to be a name.
     // TODO: Should keywords be valid here?
     if (context.PositionKind().is_keyword()) {