Skip to content

Commit 929b5ca

Browse files
jonmeowjonmeow
committed
Remove uses of StringLiteral in format strings.
1 parent 7c22348 commit 929b5ca

File tree

10 files changed

+81
-63
lines changed

10 files changed

+81
-63
lines changed

‎toolchain/check/call.cpp‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@
1919

2020
namespace Carbon::Check {
2121

22+
namespace {
23+
// Entity kinds for the diagnostic. Converted to an int for a select.
24+
enum class EntityKind : uint8_t {
25+
Function,
26+
GenericClass,
27+
GenericInterface,
28+
};
29+
} // namespace
30+
2231
// Resolves the callee expression in a call to a specific callee, or diagnoses
2332
// if no specific callee can be identified. This verifies the arity of the
2433
// callee and determines any compile-time arguments, but doesn't check that the
@@ -31,7 +40,7 @@ namespace Carbon::Check {
3140
// callee is not generic, or `nullopt` if an error has been diagnosed.
3241
static auto ResolveCalleeInCall(Context& context, SemIR::LocId loc_id,
3342
const SemIR::EntityWithParamsBase& entity,
34-
llvm::StringLiteral entity_kind_for_diagnostic,
43+
EntityKind entity_kind_for_diagnostic,
3544
SemIR::GenericId entity_generic_id,
3645
SemIR::SpecificId enclosing_specific_id,
3746
SemIR::InstId self_id,
@@ -43,16 +52,20 @@ static auto ResolveCalleeInCall(Context& context, SemIR::LocId loc_id,
4352
auto params = context.inst_blocks().GetOrEmpty(callee_info.param_refs_id);
4453
if (arg_ids.size() != params.size()) {
4554
CARBON_DIAGNOSTIC(CallArgCountMismatch, Error,
46-
"{0} argument{0:s} passed to {1} expecting "
47-
"{2} argument{2:s}",
48-
IntAsSelect, llvm::StringLiteral, IntAsSelect);
49-
CARBON_DIAGNOSTIC(InCallToEntity, Note, "calling {0} declared here",
50-
llvm::StringLiteral);
55+
"{0} argument{0:s} passed to "
56+
"{1:=0:function|=1:generic class|=2:generic interface}"
57+
" expecting {2} argument{2:s}",
58+
IntAsSelect, IntAsSelect, IntAsSelect);
59+
CARBON_DIAGNOSTIC(
60+
InCallToEntity, Note,
61+
"calling {0:=0:function|=1:generic class|=2:generic interface}"
62+
" declared here",
63+
IntAsSelect);
5164
context.emitter()
5265
.Build(loc_id, CallArgCountMismatch, arg_ids.size(),
53-
entity_kind_for_diagnostic, params.size())
66+
static_cast<int>(entity_kind_for_diagnostic), params.size())
5467
.Note(callee_info.callee_loc, InCallToEntity,
55-
entity_kind_for_diagnostic)
68+
static_cast<int>(entity_kind_for_diagnostic))
5669
.Emit();
5770
return std::nullopt;
5871
}
@@ -80,8 +93,9 @@ static auto PerformCallToGenericClass(Context& context, SemIR::LocId loc_id,
8093
-> SemIR::InstId {
8194
const auto& generic_class = context.classes().Get(class_id);
8295
auto callee_specific_id = ResolveCalleeInCall(
83-
context, loc_id, generic_class, "generic class", generic_class.generic_id,
84-
enclosing_specific_id, /*self_id=*/SemIR::InstId::Invalid, arg_ids);
96+
context, loc_id, generic_class, EntityKind::GenericClass,
97+
generic_class.generic_id, enclosing_specific_id,
98+
/*self_id=*/SemIR::InstId::Invalid, arg_ids);
8599
if (!callee_specific_id) {
86100
return SemIR::InstId::BuiltinError;
87101
}
@@ -99,8 +113,9 @@ static auto PerformCallToGenericInterface(
99113
llvm::ArrayRef<SemIR::InstId> arg_ids) -> SemIR::InstId {
100114
const auto& interface = context.interfaces().Get(interface_id);
101115
auto callee_specific_id = ResolveCalleeInCall(
102-
context, loc_id, interface, "generic interface", interface.generic_id,
103-
enclosing_specific_id, /*self_id=*/SemIR::InstId::Invalid, arg_ids);
116+
context, loc_id, interface, EntityKind::GenericInterface,
117+
interface.generic_id, enclosing_specific_id,
118+
/*self_id=*/SemIR::InstId::Invalid, arg_ids);
104119
if (!callee_specific_id) {
105120
return SemIR::InstId::BuiltinError;
106121
}
@@ -143,7 +158,7 @@ auto PerformCall(Context& context, SemIR::LocId loc_id, SemIR::InstId callee_id,
143158
// If the callee is a generic function, determine the generic argument values
144159
// for the call.
145160
auto callee_specific_id = ResolveCalleeInCall(
146-
context, loc_id, callable, "function", callable.generic_id,
161+
context, loc_id, callable, EntityKind::Function, callable.generic_id,
147162
callee_function.enclosing_specific_id, callee_function.self_id, arg_ids);
148163
if (!callee_specific_id) {
149164
return SemIR::InstId::BuiltinError;

‎toolchain/check/check.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "toolchain/check/sem_ir_diagnostic_converter.h"
2424
#include "toolchain/diagnostics/diagnostic.h"
2525
#include "toolchain/diagnostics/diagnostic_emitter.h"
26+
#include "toolchain/diagnostics/format_providers.h"
2627
#include "toolchain/lex/token_kind.h"
2728
#include "toolchain/parse/node_ids.h"
2829
#include "toolchain/parse/tree.h"
@@ -1189,19 +1190,18 @@ static auto BuildApiMapAndDiagnosePackaging(
11891190
bool is_api_with_impl_ext = !is_impl && filename.ends_with(ImplExt);
11901191
auto want_ext = is_impl ? ImplExt : ApiExt;
11911192
if (is_api_with_impl_ext || !filename.ends_with(want_ext)) {
1192-
CARBON_DIAGNOSTIC(IncorrectExtension, Error,
1193-
"file extension of `{0}` required for `{1}`",
1194-
llvm::StringLiteral, Lex::TokenKind);
1193+
CARBON_DIAGNOSTIC(
1194+
IncorrectExtension, Error,
1195+
"file extension of `{0:.impl|}.carbon` required for {0:`impl`|api}",
1196+
BoolAsSelect);
11951197
auto diag = unit_info.emitter.Build(
11961198
packaging ? packaging->names.node_id : Parse::NodeId::Invalid,
1197-
IncorrectExtension, want_ext,
1198-
is_impl ? Lex::TokenKind::Impl : Lex::TokenKind::Api);
1199+
IncorrectExtension, is_impl);
11991200
if (is_api_with_impl_ext) {
1200-
CARBON_DIAGNOSTIC(IncorrectExtensionImplNote, Note,
1201-
"file extension of `{0}` only allowed for `{1}`",
1202-
llvm::StringLiteral, Lex::TokenKind);
1203-
diag.Note(Parse::NodeId::Invalid, IncorrectExtensionImplNote, ImplExt,
1204-
Lex::TokenKind::Impl);
1201+
CARBON_DIAGNOSTIC(
1202+
IncorrectExtensionImplNote, Note,
1203+
"file extension of `.impl.carbon` only allowed for `impl`");
1204+
diag.Note(Parse::NodeId::Invalid, IncorrectExtensionImplNote);
12051205
}
12061206
diag.Emit();
12071207
}

‎toolchain/check/eval.cpp‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -682,65 +682,65 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIRLoc loc,
682682

683683
bool overflow = false;
684684
llvm::APInt result_val;
685-
llvm::StringLiteral op_str = "<error>";
685+
Lex::TokenKind op_token = Lex::TokenKind::Not;
686686
switch (builtin_kind) {
687687
// Arithmetic.
688688
case SemIR::BuiltinFunctionKind::IntSAdd:
689689
result_val = lhs_val.sadd_ov(rhs_val, overflow);
690-
op_str = "+";
690+
op_token = Lex::TokenKind::Plus;
691691
break;
692692
case SemIR::BuiltinFunctionKind::IntSSub:
693693
result_val = lhs_val.ssub_ov(rhs_val, overflow);
694-
op_str = "-";
694+
op_token = Lex::TokenKind::Minus;
695695
break;
696696
case SemIR::BuiltinFunctionKind::IntSMul:
697697
result_val = lhs_val.smul_ov(rhs_val, overflow);
698-
op_str = "*";
698+
op_token = Lex::TokenKind::Star;
699699
break;
700700
case SemIR::BuiltinFunctionKind::IntSDiv:
701701
result_val = lhs_val.sdiv_ov(rhs_val, overflow);
702-
op_str = "/";
702+
op_token = Lex::TokenKind::Slash;
703703
break;
704704
case SemIR::BuiltinFunctionKind::IntSMod:
705705
result_val = lhs_val.srem(rhs_val);
706706
// LLVM weirdly lacks `srem_ov`, so we work it out for ourselves:
707707
// <signed min> % -1 overflows because <signed min> / -1 overflows.
708708
overflow = lhs_val.isMinSignedValue() && rhs_val.isAllOnes();
709-
op_str = "%";
709+
op_token = Lex::TokenKind::Percent;
710710
break;
711711
case SemIR::BuiltinFunctionKind::IntUAdd:
712712
result_val = lhs_val + rhs_val;
713-
op_str = "+";
713+
op_token = Lex::TokenKind::Plus;
714714
break;
715715
case SemIR::BuiltinFunctionKind::IntUSub:
716716
result_val = lhs_val - rhs_val;
717-
op_str = "-";
717+
op_token = Lex::TokenKind::Minus;
718718
break;
719719
case SemIR::BuiltinFunctionKind::IntUMul:
720720
result_val = lhs_val * rhs_val;
721-
op_str = "*";
721+
op_token = Lex::TokenKind::Star;
722722
break;
723723
case SemIR::BuiltinFunctionKind::IntUDiv:
724724
result_val = lhs_val.udiv(rhs_val);
725-
op_str = "/";
725+
op_token = Lex::TokenKind::Slash;
726726
break;
727727
case SemIR::BuiltinFunctionKind::IntUMod:
728728
result_val = lhs_val.urem(rhs_val);
729-
op_str = "%";
729+
op_token = Lex::TokenKind::Percent;
730730
break;
731731

732732
// Bitwise.
733733
case SemIR::BuiltinFunctionKind::IntAnd:
734734
result_val = lhs_val & rhs_val;
735-
op_str = "&";
735+
op_token = Lex::TokenKind::And;
736736
break;
737737
case SemIR::BuiltinFunctionKind::IntOr:
738738
result_val = lhs_val | rhs_val;
739-
op_str = "|";
739+
op_token = Lex::TokenKind::Pipe;
740740
break;
741741
case SemIR::BuiltinFunctionKind::IntXor:
742742
result_val = lhs_val ^ rhs_val;
743-
op_str = "^";
743+
op_token = Lex::TokenKind::Caret;
744744
break;
745745

746746
// Bit shift.
@@ -777,9 +777,9 @@ static auto PerformBuiltinBinaryIntOp(Context& context, SemIRLoc loc,
777777
if (overflow) {
778778
CARBON_DIAGNOSTIC(CompileTimeIntegerOverflow, Error,
779779
"integer overflow in calculation {0} {1} {2}", TypedInt,
780-
llvm::StringLiteral, TypedInt);
780+
Lex::TokenKind, TypedInt);
781781
context.emitter().Emit(loc, CompileTimeIntegerOverflow,
782-
{.type = lhs.type_id, .value = lhs_val}, op_str,
782+
{.type = lhs.type_id, .value = lhs_val}, op_token,
783783
{.type = rhs.type_id, .value = rhs_val});
784784
}
785785

‎toolchain/check/testdata/packages/fail_extension.carbon‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/packages/fail_extension.carbon
88
// TIP: To dump output, run:
99
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/packages/fail_extension.carbon
10-
// CHECK:STDERR: fail_main.incorrect: error: file extension of `.carbon` required for `api`
10+
// CHECK:STDERR: fail_main.incorrect: error: file extension of `.carbon` required for api
1111
// CHECK:STDERR:
1212
// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: `Main//default` previously provided by `fail_main.incorrect`
1313
// CHECK:STDERR:
14-
// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: file extension of `.carbon` required for `api`
14+
// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: file extension of `.carbon` required for api
1515
// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: note: file extension of `.impl.carbon` only allowed for `impl`
1616
// CHECK:STDERR:
1717

@@ -21,7 +21,7 @@
2121

2222
// --- fail_main_lib.incorrect
2323

24-
// CHECK:STDERR: fail_main_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for `api`
24+
// CHECK:STDERR: fail_main_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for api
2525
// CHECK:STDERR: library "main_lib";
2626
// CHECK:STDERR: ^~~~~~~
2727
// CHECK:STDERR:
@@ -37,7 +37,7 @@ impl library "[[@TEST_NAME]]";
3737

3838
// --- fail_package.incorrect
3939

40-
// CHECK:STDERR: fail_package.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for `api`
40+
// CHECK:STDERR: fail_package.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for api
4141
// CHECK:STDERR: package Package;
4242
// CHECK:STDERR: ^~~~~~~
4343
// CHECK:STDERR:
@@ -53,7 +53,7 @@ impl package Package;
5353

5454
// --- fail_package_lib.incorrect
5555

56-
// CHECK:STDERR: fail_package_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for `api`
56+
// CHECK:STDERR: fail_package_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for api
5757
// CHECK:STDERR: package Package library "package_lib";
5858
// CHECK:STDERR: ^~~~~~~
5959
// CHECK:STDERR:
@@ -69,7 +69,7 @@ impl package Package library "[[@TEST_NAME]]";
6969

7070
// --- fail_swapped_ext.impl.carbon
7171

72-
// CHECK:STDERR: fail_swapped_ext.impl.carbon:[[@LINE+5]]:1: error: file extension of `.carbon` required for `api`
72+
// CHECK:STDERR: fail_swapped_ext.impl.carbon:[[@LINE+5]]:1: error: file extension of `.carbon` required for api
7373
// CHECK:STDERR: package SwappedExt;
7474
// CHECK:STDERR: ^~~~~~~
7575
// CHECK:STDERR: fail_swapped_ext.impl.carbon: note: file extension of `.impl.carbon` only allowed for `impl`

‎toolchain/diagnostics/diagnostic.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,10 @@ struct DiagnosticBase {
119119
explicit constexpr DiagnosticBase(DiagnosticKind kind, DiagnosticLevel level,
120120
llvm::StringLiteral format)
121121
: Kind(kind), Level(level), Format(format) {
122-
static_assert((... && !std::is_same_v<Args, llvm::StringRef>),
123-
"Use std::string or llvm::StringLiteral for diagnostics to "
122+
static_assert((... && !(std::is_same_v<Args, llvm::StringRef> ||
123+
std::is_same_v<Args, llvm::StringLiteral>)),
124+
"For diagnostics, use a format provider (see "
125+
"toolchain/diagnostics/format_providers.h) or std::string to "
124126
"avoid lifetime issues.");
125127
}
126128

‎toolchain/diagnostics/diagnostic_emitter_test.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ TEST_F(DiagnosticEmitterTest, EmitSimpleWarning) {
5555
}
5656

5757
TEST_F(DiagnosticEmitterTest, EmitOneArgDiagnostic) {
58-
CARBON_DIAGNOSTIC(TestDiagnostic, Error, "arg: `{0}`", llvm::StringLiteral);
58+
CARBON_DIAGNOSTIC(TestDiagnostic, Error, "arg: `{0}`", std::string);
5959
EXPECT_CALL(consumer_, HandleDiagnostic(IsSingleDiagnostic(
6060
DiagnosticKind::TestDiagnostic,
6161
DiagnosticLevel::Error, 1, 1, "arg: `str`")));

‎toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ namespace {
1717
using ::Carbon::Testing::IsSingleDiagnostic;
1818
using ::testing::InSequence;
1919

20-
CARBON_DIAGNOSTIC(TestDiagnostic, Error, "{0}", llvm::StringLiteral);
20+
CARBON_DIAGNOSTIC(TestDiagnostic, Error, "M{0}", int);
2121

2222
struct FakeDiagnosticConverter : DiagnosticConverter<DiagnosticLoc> {
2323
auto ConvertLoc(DiagnosticLoc loc, ContextFnT /*context_fn*/) const
@@ -32,12 +32,12 @@ TEST(SortedDiagnosticEmitterTest, SortErrors) {
3232
SortingDiagnosticConsumer sorting_consumer(consumer);
3333
DiagnosticEmitter<DiagnosticLoc> emitter(converter, sorting_consumer);
3434

35-
emitter.Emit({"f", "line", 2, 1}, TestDiagnostic, "M1");
36-
emitter.Emit({"f", "line", 1, 1}, TestDiagnostic, "M2");
37-
emitter.Emit({"f", "line", 1, 3}, TestDiagnostic, "M3");
38-
emitter.Emit({"f", "line", 3, 4}, TestDiagnostic, "M4");
39-
emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, "M5");
40-
emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, "M6");
35+
emitter.Emit({"f", "line", 2, 1}, TestDiagnostic, 1);
36+
emitter.Emit({"f", "line", 1, 1}, TestDiagnostic, 2);
37+
emitter.Emit({"f", "line", 1, 3}, TestDiagnostic, 3);
38+
emitter.Emit({"f", "line", 3, 4}, TestDiagnostic, 4);
39+
emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, 5);
40+
emitter.Emit({"f", "line", 3, 2}, TestDiagnostic, 6);
4141

4242
InSequence s;
4343
EXPECT_CALL(consumer, HandleDiagnostic(IsSingleDiagnostic(

‎toolchain/lex/token_kind.def‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ CARBON_KEYWORD_TOKEN(Abstract, "abstract")
167167
CARBON_KEYWORD_TOKEN(Addr, "addr")
168168
CARBON_TOKEN_WITH_VIRTUAL_NODE(
169169
CARBON_KEYWORD_TOKEN(And, "and"))
170-
CARBON_KEYWORD_TOKEN(Api, "api")
171170
CARBON_KEYWORD_TOKEN(As, "as")
172171
CARBON_KEYWORD_TOKEN(Auto, "auto")
173172
CARBON_KEYWORD_TOKEN(Bool, "bool")

‎toolchain/parse/context.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,9 @@ auto Context::ConsumeAndAddCloseSymbol(Lex::TokenIndex expected_open,
7777
// TODO: Include the location of the matching opening delimiter in the
7878
// diagnostic.
7979
CARBON_DIAGNOSTIC(ExpectedCloseSymbol, Error,
80-
"unexpected tokens before `{0}`", llvm::StringLiteral);
80+
"unexpected tokens before `{0}`", Lex::TokenKind);
8181
emitter_->Emit(*position_, ExpectedCloseSymbol,
82-
open_token_kind.closing_symbol().fixed_spelling());
82+
open_token_kind.closing_symbol());
8383

8484
SkipTo(tokens().GetMatchedClosingToken(expected_open));
8585
AddNode(close_kind, Consume(), /*has_error=*/true);

‎toolchain/parse/handle_binding_pattern.cpp‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Exceptions. See /LICENSE for license information.
33
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
44

5+
#include "toolchain/diagnostics/format_providers.h"
56
#include "toolchain/parse/context.h"
67
#include "toolchain/parse/handle.h"
78

@@ -25,12 +26,13 @@ auto HandleBindingPattern(Context& context) -> void {
2526
}
2627

2728
// Handle an invalid pattern introducer for parameters and variables.
28-
auto on_error = [&](llvm::StringLiteral expected) {
29+
auto on_error = [&](bool expected_name) {
2930
if (!state.has_error) {
3031
CARBON_DIAGNOSTIC(ExpectedBindingPattern, Error,
31-
"expected {0} in binding pattern", llvm::StringLiteral);
32+
"expected {0:name|`:` or `:!`} in binding pattern",
33+
BoolAsSelect);
3234
context.emitter().Emit(*context.position(), ExpectedBindingPattern,
33-
expected);
35+
expected_name);
3436
state.has_error = true;
3537
}
3638
};
@@ -51,7 +53,7 @@ auto HandleBindingPattern(Context& context) -> void {
5153
// Add a placeholder for the name.
5254
context.AddLeafNode(NodeKind::IdentifierName, *context.position(),
5355
/*has_error=*/true);
54-
on_error("name");
56+
on_error(/*expected_name=*/true);
5557
}
5658

5759
if (auto kind = context.PositionKind();
@@ -64,7 +66,7 @@ auto HandleBindingPattern(Context& context) -> void {
6466
context.PushState(state);
6567
context.PushStateForExpr(PrecedenceGroup::ForType());
6668
} else {
67-
on_error("`:` or `:!`");
69+
on_error(/*expected_name=*/false);
6870
// Add a placeholder for the type.
6971
context.AddLeafNode(NodeKind::InvalidParse, *context.position(),
7072
/*has_error=*/true);

0 commit comments

Comments
 (0)