From 96964ee534c20e4d6550e92e60e2f6cb72d4e110 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Wed, 16 Oct 2024 15:46:15 -0700 Subject: [PATCH] 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. --- toolchain/check/BUILD | 2 + toolchain/check/convert.cpp | 42 +++++---- toolchain/check/eval.cpp | 20 ++-- toolchain/check/handle_binding_pattern.cpp | 25 +++-- toolchain/check/merge.cpp | 78 ++++++++-------- toolchain/diagnostics/BUILD | 26 ++++++ toolchain/diagnostics/format_providers.cpp | 92 +++++++++++++++++++ toolchain/diagnostics/format_providers.h | 76 +++++++++++++++ .../diagnostics/format_providers_test.cpp | 86 +++++++++++++++++ toolchain/parse/BUILD | 2 + toolchain/parse/context.cpp | 69 +++++--------- toolchain/parse/handle_brace_expr.cpp | 35 ++++--- toolchain/parse/handle_period.cpp | 8 +- 13 files changed, 416 insertions(+), 145 deletions(-) create mode 100644 toolchain/diagnostics/format_providers.cpp create mode 100644 toolchain/diagnostics/format_providers.h create mode 100644 toolchain/diagnostics/format_providers_test.cpp diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index d2ba70df69515..cae21cdec0c37 100644 --- a/toolchain/check/BUILD +++ b/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", diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index feebea99e645a..f705504159a3f 100644 --- a/toolchain/check/convert.cpp +++ b/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 || + std::is_same_v); + constexpr bool ToClass = + std::is_same_v; + 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( - 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( - 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(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, diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index 1934d36b5b268..ac85dce6144b3 100644 --- a/toolchain/check/eval.cpp +++ b/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; } diff --git a/toolchain/check/handle_binding_pattern.cpp b/toolchain/check/handle_binding_pattern.cpp index 2253c094274ad..afa51f0a6a4d5 100644 --- a/toolchain/check/handle_binding_pattern.cpp +++ b/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, diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index e6c3ca188150f..515014c0d00fe 100644 --- a/toolchain/check/merge.cpp +++ b/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 && diff --git a/toolchain/diagnostics/BUILD b/toolchain/diagnostics/BUILD index 32ee2f31ea580..55e2195ca6b22 100644 --- a/toolchain/diagnostics/BUILD +++ b/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"], 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 new file mode 100644 index 0000000000000..97d8af0ca755b --- /dev/null +++ b/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 `:`. +// +// Supported selectors are: +// - `=`: 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 { + static auto format(const Carbon::BoolAsSelect& wrapper, raw_ostream& out, + StringRef style) -> void; +}; + +// See IntAsSelect. +template <> +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 new file mode 100644 index 0000000000000..0d6cf68eecf34 --- /dev/null +++ b/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 +#include + +#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 diff --git a/toolchain/parse/BUILD b/toolchain/parse/BUILD index 338e2a29009b5..5f7ba0327fee1 100644 --- a/toolchain/parse/BUILD +++ b/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", ], diff --git a/toolchain/parse/context.cpp b/toolchain/parse/context.cpp index 0daead8f9a26f..ca5cfe21ce85e 100644 --- a/toolchain/parse/context.cpp +++ b/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 { - 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); } } } diff --git a/toolchain/parse/handle_brace_expr.cpp b/toolchain/parse/handle_brace_expr.cpp index 42a0829a78b5f..3956ab2caa9e9 100644 --- a/toolchain/parse/handle_brace_expr.cpp +++ b/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); diff --git a/toolchain/parse/handle_period.cpp b/toolchain/parse/handle_period.cpp index 1e4d7500ceea4..b29f5b82b7437 100644 --- a/toolchain/parse/handle_period.cpp +++ b/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()) {