From 1131c451b5838094082687df86d7c1435d3ad2d4 Mon Sep 17 00:00:00 2001 From: jonmeow Date: Wed, 16 Oct 2024 13:37:33 -0700 Subject: [PATCH] Addressing comments --- toolchain/check/convert.cpp | 9 +- toolchain/check/eval.cpp | 4 +- toolchain/check/handle_binding_pattern.cpp | 16 +-- toolchain/check/merge.cpp | 94 +++++++--------- toolchain/diagnostics/BUILD | 1 + toolchain/diagnostics/format_providers.cpp | 92 ++++++++++++++++ toolchain/diagnostics/format_providers.h | 101 ++++-------------- .../diagnostics/format_providers_test.cpp | 78 ++++++++------ toolchain/parse/context.cpp | 13 ++- toolchain/parse/handle_brace_expr.cpp | 4 +- toolchain/parse/handle_period.cpp | 5 +- 11 files changed, 219 insertions(+), 198 deletions(-) create mode 100644 toolchain/diagnostics/format_providers.cpp diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 33185a85b3cf7..f705504159a3f 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -416,9 +416,9 @@ static auto ConvertStructToStructOrClass(Context& context, StructInitElementCountMismatch, Error, "cannot initialize {0:class|struct} with {1} field(s) from struct " "with {2} field(s).", - FormatBool, size_t, size_t); + BoolAsSelect, size_t, size_t); context.emitter().Emit(value_loc_id, StructInitElementCountMismatch, - {.value = ToClass}, dest_elem_fields.size(), + ToClass, dest_elem_fields.size(), src_elem_fields.size()); return SemIR::InstId::BuiltinError; } @@ -1163,9 +1163,8 @@ static auto ConvertSelf(Context& context, SemIR::LocId call_loc_id, CARBON_DIAGNOSTIC(InCallToFunctionSelf, Note, "initializing `{0:addr self|self}` parameter of " "method declared here", - FormatBool); - builder.Note(self_param_id, InCallToFunctionSelf, - {.value = addr_pattern}); + BoolAsSelect); + builder.Note(self_param_id, InCallToFunctionSelf, addr_pattern); }); return CallerPatternMatch(context, callee_specific_id, self_param_id, diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index 7b0d06afbcfdd..ac85dce6144b3 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -751,11 +751,11 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIRLoc loc, CARBON_DIAGNOSTIC( CompileTimeShiftOutOfRange, Error, "shift distance not in range [0, {0}) in {1} {2:<<|>>} {3}", - unsigned, TypedInt, FormatBool, TypedInt); + unsigned, TypedInt, BoolAsSelect, TypedInt); context.emitter().Emit( loc, CompileTimeShiftOutOfRange, lhs_val.getBitWidth(), {.type = lhs.type_id, .value = lhs_val}, - {.value = builtin_kind == SemIR::BuiltinFunctionKind::IntLeftShift}, + 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; diff --git a/toolchain/check/handle_binding_pattern.cpp b/toolchain/check/handle_binding_pattern.cpp index 90b579669bb73..afa51f0a6a4d5 100644 --- a/toolchain/check/handle_binding_pattern.cpp +++ b/toolchain/check/handle_binding_pattern.cpp @@ -106,18 +106,18 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id, [&] { CARBON_DIAGNOSTIC(IncompleteTypeInVarDecl, Error, "{0:field|variable} has incomplete type {1}", - FormatBool, SemIR::TypeId); - return context.emitter().Build( - type_node, IncompleteTypeInVarDecl, - {.value = parent_class_decl.has_value()}, cast_type_id); + BoolAsSelect, SemIR::TypeId); + return context.emitter().Build(type_node, IncompleteTypeInVarDecl, + parent_class_decl.has_value(), + cast_type_id); }, [&] { CARBON_DIAGNOSTIC(AbstractTypeInVarDecl, Error, "{0:field|variable} has abstract type {1}", - FormatBool, SemIR::TypeId); - return context.emitter().Build( - type_node, AbstractTypeInVarDecl, - {.value = parent_class_decl.has_value()}, cast_type_id); + 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, diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index 2c87388204e6f..515014c0d00fe 100644 --- a/toolchain/check/merge.cpp +++ b/toolchain/check/merge.cpp @@ -12,29 +12,6 @@ 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 { - 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. @@ -217,7 +194,7 @@ 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, ParamKind param_kind, +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, @@ -231,15 +208,16 @@ static auto CheckRedeclParam(Context& context, ParamKind param_kind, return; } CARBON_DIAGNOSTIC(RedeclParamDiffers, Error, - "redeclaration differs at {0}parameter {1}", ParamKind, - int32_t); - CARBON_DIAGNOSTIC(RedeclParamPrevious, Note, - "previous declaration's corresponding {0}parameter here", - ParamKind); + "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_kind, + .Build(new_param_pattern_id, RedeclParamDiffers, is_implicit_param, param_index + 1) - .Note(prev_param_pattern_id, RedeclParamPrevious, param_kind) + .Note(prev_param_pattern_id, RedeclParamPrevious, is_implicit_param) .Emit(); }; @@ -291,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, - ParamKind param_kind, + bool is_implicit_param, SemIR::SpecificId prev_specific_id, bool diagnose) -> bool { // This will often occur for empty params. @@ -304,18 +282,19 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc, if (!diagnose) { return false; } - CARBON_DIAGNOSTIC( - RedeclParamListDiffers, Error, - "redeclaration differs because of {1:'|missing '}{0}parameter list", - ParamKind, FormatBool); + CARBON_DIAGNOSTIC(RedeclParamListDiffers, Error, + "redeclaration differs because of " + "{1:'|missing '}{0:implicit |}parameter list", + BoolAsSelect, BoolAsSelect); CARBON_DIAGNOSTIC(RedeclParamListPrevious, Note, - "previously declared {1:with|without} {0}parameter list", - ParamKind, FormatBool); + "previously declared " + "{1:with|without} {0:implicit |}parameter list", + BoolAsSelect, BoolAsSelect); context.emitter() - .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()}) + .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; } @@ -332,23 +311,25 @@ static auto CheckRedeclParams(Context& context, SemIRLoc new_decl_loc, } CARBON_DIAGNOSTIC( RedeclParamCountDiffers, Error, - "redeclaration differs because of {0}parameter count of {1}", ParamKind, - int32_t); - CARBON_DIAGNOSTIC(RedeclParamCountPrevious, Note, - "previously declared with {0}parameter count of {1}", - ParamKind, 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_kind, + .Build(new_decl_loc, RedeclParamCountDiffers, is_implicit_param, new_param_pattern_ids.size()) - .Note(prev_decl_loc, RedeclParamCountPrevious, param_kind, + .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_kind, index, new_param_pattern_id, - prev_param_pattern_id, prev_specific_id, diagnose)) { + if (!CheckRedeclParam(context, is_implicit_param, index, + new_param_pattern_id, prev_param_pattern_id, + prev_specific_id, diagnose)) { return false; } } @@ -434,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, - ParamKind::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, - ParamKind::Explicit, prev_specific_id, diagnose)) { + /*is_implicit_param=*/false, prev_specific_id, + diagnose)) { return false; } if (check_syntax && diff --git a/toolchain/diagnostics/BUILD b/toolchain/diagnostics/BUILD index cdc22885930f5..55e2195ca6b22 100644 --- a/toolchain/diagnostics/BUILD +++ b/toolchain/diagnostics/BUILD @@ -59,6 +59,7 @@ cc_library( cc_library( name = "format_providers", + srcs = ["format_providers.cpp"], hdrs = ["format_providers.h"], deps = [ "//common:check", diff --git a/toolchain/diagnostics/format_providers.cpp b/toolchain/diagnostics/format_providers.cpp new file mode 100644 index 0000000000000..70500798404a3 --- /dev/null +++ b/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::format( + const Carbon::BoolAsSelect& wrapper, raw_ostream& out, StringRef style) + -> void { + if (style.empty()) { + llvm::format_provider::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::format( + const Carbon::IntAsSelect& wrapper, raw_ostream& out, StringRef style) + -> void { + if (style.empty()) { + llvm::format_provider::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); +} diff --git a/toolchain/diagnostics/format_providers.h b/toolchain/diagnostics/format_providers.h index 933d2995cb0b8..97d8af0ca755b 100644 --- a/toolchain/diagnostics/format_providers.h +++ b/toolchain/diagnostics/format_providers.h @@ -5,14 +5,13 @@ #ifndef CARBON_TOOLCHAIN_DIAGNOSTICS_FORMAT_PROVIDERS_H_ #define CARBON_TOOLCHAIN_DIAGNOSTICS_FORMAT_PROVIDERS_H_ -#include "common/check.h" #include "common/ostream.h" -#include "llvm/ADT/StringExtras.h" -#include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/FormatVariadicDetails.h" namespace Carbon { -// Selects a formatv string based on the value. +// 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 `|`. // @@ -22,12 +21,15 @@ namespace Carbon { // 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 FormatBool { - public: +struct BoolAsSelect { + // NOLINTNEXTLINE(google-explicit-constructor) + BoolAsSelect(bool value) : value(value) {} + bool value; }; -// Selects a formatv string based on the 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 `:`. @@ -48,88 +50,27 @@ struct FormatBool { // 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 FormatInt { - public: +struct IntAsSelect { + // NOLINTNEXTLINE(google-explicit-constructor) + IntAsSelect(int value) : value(value) {} + int value; }; } // namespace Carbon -// See FormatBool. +// See BoolAsSelect. template <> -struct llvm::format_provider { - static void format(const Carbon::FormatBool& wrapper, raw_ostream& out, - StringRef style) { - // 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, - "FormatBool requires a `|` separating true and false results: `{0}`", - style); - if (wrapper.value) { - out << style.take_front(sep); - } else { - out << style.drop_front(sep + 1); - } - } +struct llvm::format_provider { + static auto format(const Carbon::BoolAsSelect& wrapper, raw_ostream& out, + StringRef style) -> void; }; -// See FormatInt. +// See IntAsSelect. template <> -struct llvm::format_provider { - static void format(const Carbon::FormatInt& wrapper, raw_ostream& out, - StringRef style) { - // 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, - "FormatInt 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(), - "FormatInt 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), - "FormatInt has invalid value in comparison: `{0}`", style); - if (value == wrapper.value) { - out << output_string; - return; - } - } else { - CARBON_FATAL("FormatInt has unrecognized comparison: `{0}`", style); - } - } - - CARBON_FATAL("FormatInt doesn't handle `{0}`: `{1}`", wrapper.value, style); - } +struct llvm::format_provider { + static auto format(const Carbon::IntAsSelect& wrapper, raw_ostream& out, + StringRef style) -> void; }; #endif // CARBON_TOOLCHAIN_DIAGNOSTICS_FORMAT_PROVIDERS_H_ diff --git a/toolchain/diagnostics/format_providers_test.cpp b/toolchain/diagnostics/format_providers_test.cpp index c7c423d1e3de2..0d6cf68eecf34 100644 --- a/toolchain/diagnostics/format_providers_test.cpp +++ b/toolchain/diagnostics/format_providers_test.cpp @@ -7,71 +7,79 @@ #include #include +#include "llvm/Support/FormatVariadic.h" + namespace Carbon { namespace { using ::testing::Eq; -TEST(FormatBool, Cases) { +TEST(BoolAsSelect, Cases) { constexpr char Format[] = "{0:a|b}"; - EXPECT_THAT(llvm::formatv(Format, FormatBool{.value = true}).str(), Eq("a")); - EXPECT_THAT(llvm::formatv(Format, FormatBool{.value = false}).str(), Eq("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(FormatBool, Spaces) { +TEST(BoolAsSelect, Spaces) { constexpr char Format[] = "{0: a | b }"; - EXPECT_THAT(llvm::formatv(Format, FormatBool{.value = true}).str(), Eq("a ")); - EXPECT_THAT(llvm::formatv(Format, FormatBool{.value = false}).str(), - Eq(" b")); + EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(true)).str(), Eq("a ")); + EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(false)).str(), Eq(" b")); } -TEST(FormatBool, QuotedSpaces) { +TEST(BoolAsSelect, QuotedSpaces) { constexpr char Format[] = "{0:' a | b '}"; - EXPECT_THAT(llvm::formatv(Format, FormatBool{.value = true}).str(), - Eq(" a ")); - EXPECT_THAT(llvm::formatv(Format, FormatBool{.value = false}).str(), - Eq(" b ")); + EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(true)).str(), Eq(" a ")); + EXPECT_THAT(llvm::formatv(Format, BoolAsSelect(false)).str(), Eq(" b ")); } -TEST(FormatInt, OnlyDefault) { +TEST(IntAsSelect, OnlyDefault) { constexpr char Format[] = "{0::default}"; - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 0}).str(), - Eq("default")); + EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("default")); } -TEST(FormatInt, OneEquals) { +TEST(IntAsSelect, OneEquals) { constexpr char Format[] = "{0:=0:zero}"; - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 0}).str(), Eq("zero")); + EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("zero")); } -TEST(FormatInt, TwoEquals) { +TEST(IntAsSelect, TwoEquals) { constexpr char Format[] = "{0:=0:zero|=1:one}"; - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 0}).str(), Eq("zero")); - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 1}).str(), Eq("one")); + EXPECT_THAT(llvm::formatv(Format, IntAsSelect(0)).str(), Eq("zero")); + EXPECT_THAT(llvm::formatv(Format, IntAsSelect(1)).str(), Eq("one")); } -TEST(FormatInt, TwoEqualsAndDefault) { +TEST(IntAsSelect, TwoEqualsAndDefault) { constexpr char Format[] = "{0:=0:zero|=1:one|:default}"; - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 0}).str(), Eq("zero")); - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 1}).str(), Eq("one")); - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 2}).str(), - Eq("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(FormatInt, Spaces) { +TEST(IntAsSelect, Spaces) { constexpr char Format[] = "{0:=0: zero |=1: one |: default }"; - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 0}).str(), Eq(" zero ")); - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 1}).str(), Eq(" one ")); - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 2}).str(), - Eq(" 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(FormatInt, QuotedSpaces) { +TEST(IntAsSelect, QuotedSpaces) { constexpr char Format[] = "{0:'=0: zero |=1: one |: default '}"; - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 0}).str(), Eq(" zero ")); - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 1}).str(), Eq(" one ")); - EXPECT_THAT(llvm::formatv(Format, FormatInt{.value = 2}).str(), - Eq(" 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 diff --git a/toolchain/parse/context.cpp b/toolchain/parse/context.cpp index 6de8dcae62461..ca5cfe21ce85e 100644 --- a/toolchain/parse/context.cpp +++ b/toolchain/parse/context.cpp @@ -281,8 +281,8 @@ auto Context::DiagnoseOperatorFixity(OperatorFixity fixity) -> void { CARBON_DIAGNOSTIC(BinaryOperatorRequiresWhitespace, Error, "whitespace missing {0:=-1:before|=0:around|=1:after} " "binary operator", - FormatInt); - FormatInt pos{.value = 0}; + IntAsSelect); + IntAsSelect pos(0); if (tokens().HasLeadingWhitespace(*position_)) { pos.value = 1; } else if (tokens().HasTrailingWhitespace(*position_)) { @@ -300,16 +300,15 @@ auto Context::DiagnoseOperatorFixity(OperatorFixity fixity) -> void { CARBON_DIAGNOSTIC( UnaryOperatorHasWhitespace, Error, "whitespace is not allowed {0:after|before} this unary operator", - FormatBool); - emitter_->Emit(*position_, UnaryOperatorHasWhitespace, {.value = prefix}); + 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:before|after} this unary operator", - FormatBool); - emitter_->Emit(*position_, UnaryOperatorRequiresWhitespace, - {.value = prefix}); + BoolAsSelect); + emitter_->Emit(*position_, UnaryOperatorRequiresWhitespace, prefix); } } } diff --git a/toolchain/parse/handle_brace_expr.cpp b/toolchain/parse/handle_brace_expr.cpp index 54ddee04e0f6f..3956ab2caa9e9 100644 --- a/toolchain/parse/handle_brace_expr.cpp +++ b/toolchain/parse/handle_brace_expr.cpp @@ -23,7 +23,7 @@ auto HandleBraceExpr(Context& context) -> void { static auto HandleBraceExprParamError(Context& context, Context::StateStackEntry state, State param_finish_state) -> void { - FormatInt mode; + IntAsSelect mode(0); switch (param_finish_state) { case State::BraceExprParamFinishAsType: mode.value = 0; @@ -41,7 +41,7 @@ static auto HandleBraceExprParamError(Context& context, ExpectedStructLiteralField, Error, "expected {0:=0:`.field: field_type`|" "=1:`.field = value`|=2:`.field: field_type` or `.field = value`}", - FormatInt); + IntAsSelect); context.emitter().Emit(*context.position(), ExpectedStructLiteralField, mode); state.has_error = true; diff --git a/toolchain/parse/handle_period.cpp b/toolchain/parse/handle_period.cpp index 28fab52888d90..b29f5b82b7437 100644 --- a/toolchain/parse/handle_period.cpp +++ b/toolchain/parse/handle_period.cpp @@ -37,10 +37,9 @@ static auto HandlePeriodOrArrow(Context& context, NodeKind node_kind, return; } else { CARBON_DIAGNOSTIC(ExpectedIdentifierAfterPeriodOrArrow, Error, - "expected identifier after `{0:->|.}`", FormatBool); + "expected identifier after `{0:->|.}`", BoolAsSelect); context.emitter().Emit(*context.position(), - ExpectedIdentifierAfterPeriodOrArrow, - {.value = is_arrow}); + 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()) {