Skip to content

Commit

Permalink
Use a filename without a line number as a cue for autoupdate. (#4677)
Browse files Browse the repository at this point in the history
This is so that diagnostics which lack a location get split file
clustering.
  • Loading branch information
jonmeow authored Dec 13, 2024
1 parent e71fd07 commit 55c257b
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 46 deletions.
9 changes: 5 additions & 4 deletions testing/file_test/autoupdate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@

namespace Carbon::Testing {

// Converts a matched line number to an int, trimming whitespace.
// Converts a matched line number to an int, trimming whitespace. Returns 0 if
// there is no line number, to assist early placement.
static auto ParseLineNumber(absl::string_view matched_line_number) -> int {
llvm::StringRef trimmed = matched_line_number;
trimmed = trimmed.trim();
if (trimmed.empty()) {
return 0;
}
// NOLINTNEXTLINE(google-runtime-int): API requirement.
long long val;
CARBON_CHECK(!llvm::getAsSignedInteger(trimmed, 10, val), "{0}",
Expand All @@ -41,7 +45,6 @@ auto FileTestAutoupdater::CheckLine::RemapLineNumbers(
return;
}

bool found_one = false;
// Use a cursor for the line so that we can't keep matching the same
// content, which may occur when we keep a literal line number.
int line_offset = 0;
Expand All @@ -60,10 +63,8 @@ auto FileTestAutoupdater::CheckLine::RemapLineNumbers(
RE2::PartialMatch(line_cursor, *replacement_->re, &matched_line_number);
}
if (matched_line_number.empty()) {
CARBON_CHECK(found_one, "{0}", line_);
return;
}
found_one = true;

// Update the cursor offset from the match.
line_offset = matched_line_number.begin() - line_.c_str();
Expand Down
3 changes: 2 additions & 1 deletion testing/file_test/autoupdate_testdata.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# Exceptions. See /LICENSE for license information.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

bazel run -c opt --experimental_convenience_symlinks=ignore \
"$(dirname "$0")/../../scripts/run_bazel.py" \
run -c opt --experimental_convenience_symlinks=ignore \
--ui_event_filters=-info,-stdout,-stderr,-finish \
//testing/file_test:file_test_base_test -- --autoupdate "$@"
2 changes: 1 addition & 1 deletion testing/file_test/file_test_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ auto FileTestBase::GetLineNumberReplacements(
-> llvm::SmallVector<LineNumberReplacement> {
return {{.has_file = true,
.re = std::make_shared<RE2>(
llvm::formatv(R"(({0}):(\d+))", llvm::join(filenames, "|"))),
llvm::formatv(R"(({0}):(\d+)?)", llvm::join(filenames, "|"))),
.line_formatv = R"({0})"}};
}

Expand Down
12 changes: 12 additions & 0 deletions testing/file_test/file_test_base_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,17 @@ static auto TestFileOnlyREOneFile(TestParams& params)
return {{.success = true}};
}

// Does printing and returns expected results for no_line_number.carbon.
static auto TestNoLineNumber(TestParams& params)
-> ErrorOr<FileTestBaseTest::RunResult> {
params.stdout << "a.carbon: msg1\n"
"msg2\n"
"b.carbon: msg3\n"
"msg4\n"
"a.carbon: msg5\n";
return {{.success = true}};
}

// Does printing and returns expected results for unattached_multi_file.carbon.
static auto TestUnattachedMultiFile(TestParams& params)
-> ErrorOr<FileTestBaseTest::RunResult> {
Expand Down Expand Up @@ -242,6 +253,7 @@ auto FileTestBaseTest::Run(
.Case("fail_example.carbon", &TestFailExample)
.Case("file_only_re_one_file.carbon", &TestFileOnlyREOneFile)
.Case("file_only_re_multi_file.carbon", &TestFileOnlyREMultiFile)
.Case("no_line_number.carbon", &TestNoLineNumber)
.Case("unattached_multi_file.carbon", &TestUnattachedMultiFile)
.Case("fail_multi_success_overall_fail.carbon",
[&](TestParams&) {
Expand Down
21 changes: 21 additions & 0 deletions testing/file_test/testdata/no_line_number.carbon
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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

// AUTOUPDATE
// TIP: To test this file alone, run:
// TIP: bazel test //testing/file_test:file_test_base_test --test_arg=--file_tests=testing/file_test/testdata/no_line_number.carbon
// TIP: To dump output, run:
// TIP: bazel run //testing/file_test:file_test_base_test -- --dump_output --file_tests=testing/file_test/testdata/no_line_number.carbon
// CHECK:STDOUT: 3 args: `default_args`, `a.carbon`, `b.carbon`

// --- a.carbon
// CHECK:STDOUT: a.carbon: msg1
// CHECK:STDOUT: msg2
aaa

// --- b.carbon
// CHECK:STDOUT: b.carbon: msg3
// CHECK:STDOUT: msg4
// CHECK:STDOUT: a.carbon: msg5
bbb
68 changes: 34 additions & 34 deletions toolchain/check/testdata/class/syntactic_merge_literal.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/class/syntactic_merge_literal.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/class/syntactic_merge_literal.carbon
// CHECK:STDERR: fail_int_mismatch.carbon: error: `Main//default` previously provided by `int_match.carbon` [DuplicateMainApi]
// CHECK:STDERR:

// --- int_match.carbon

Expand All @@ -17,6 +15,8 @@ class D(b:! C(1_000));
class D(b:! C(1_000)) {}

// --- fail_int_mismatch.carbon
// CHECK:STDERR: fail_int_mismatch.carbon: error: `Main//default` previously provided by `int_match.carbon` [DuplicateMainApi]
// CHECK:STDERR:

class C(a:! i32) {}
class D(b:! C(1000));
Expand Down Expand Up @@ -213,53 +213,53 @@ class D(b:! C(1_000)) {}
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %C.decl: %C.type = class_decl @C [template = constants.%C.generic] {
// CHECK:STDOUT: %a.patt.loc2_9.1: %i32 = symbolic_binding_pattern a, 0 [symbolic = %a.patt.loc2_9.2 (constants.%a.patt)]
// CHECK:STDOUT: %a.param_patt: %i32 = value_param_pattern %a.patt.loc2_9.1, runtime_param<invalid> [symbolic = %a.patt.loc2_9.2 (constants.%a.patt)]
// CHECK:STDOUT: %a.patt.loc4_9.1: %i32 = symbolic_binding_pattern a, 0 [symbolic = %a.patt.loc4_9.2 (constants.%a.patt)]
// CHECK:STDOUT: %a.param_patt: %i32 = value_param_pattern %a.patt.loc4_9.1, runtime_param<invalid> [symbolic = %a.patt.loc4_9.2 (constants.%a.patt)]
// CHECK:STDOUT: } {
// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [template = constants.%int_32]
// CHECK:STDOUT: %int.make_type_signed: init type = call constants.%Int(%int_32) [template = constants.%i32]
// CHECK:STDOUT: %.loc2_13.1: type = value_of_initializer %int.make_type_signed [template = constants.%i32]
// CHECK:STDOUT: %.loc2_13.2: type = converted %int.make_type_signed, %.loc2_13.1 [template = constants.%i32]
// CHECK:STDOUT: %.loc4_13.1: type = value_of_initializer %int.make_type_signed [template = constants.%i32]
// CHECK:STDOUT: %.loc4_13.2: type = converted %int.make_type_signed, %.loc4_13.1 [template = constants.%i32]
// CHECK:STDOUT: %a.param: %i32 = value_param runtime_param<invalid>
// CHECK:STDOUT: %a.loc2_9.1: %i32 = bind_symbolic_name a, 0, %a.param [symbolic = %a.loc2_9.2 (constants.%a)]
// CHECK:STDOUT: %a.loc4_9.1: %i32 = bind_symbolic_name a, 0, %a.param [symbolic = %a.loc4_9.2 (constants.%a)]
// CHECK:STDOUT: }
// CHECK:STDOUT: %D.decl: %D.type = class_decl @D [template = constants.%D.generic] {
// CHECK:STDOUT: %b.patt.loc3_9.1: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc3_9.2 (constants.%b.patt)]
// CHECK:STDOUT: %b.param_patt: %C.2 = value_param_pattern %b.patt.loc3_9.1, runtime_param<invalid> [symbolic = %b.patt.loc3_9.2 (constants.%b.patt)]
// CHECK:STDOUT: %b.patt.loc5_9.1: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc5_9.2 (constants.%b.patt)]
// CHECK:STDOUT: %b.param_patt: %C.2 = value_param_pattern %b.patt.loc5_9.1, runtime_param<invalid> [symbolic = %b.patt.loc5_9.2 (constants.%b.patt)]
// CHECK:STDOUT: } {
// CHECK:STDOUT: %C.ref: %C.type = name_ref C, file.%C.decl [template = constants.%C.generic]
// CHECK:STDOUT: %int_1000: Core.IntLiteral = int_value 1000 [template = constants.%int_1000.1]
// CHECK:STDOUT: %impl.elem0: %Convert.type.2 = interface_witness_access constants.%interface.9, element0 [template = constants.%Convert.14]
// CHECK:STDOUT: %Convert.bound: <bound method> = bound_method %int_1000, %impl.elem0 [template = constants.%Convert.bound]
// CHECK:STDOUT: %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(constants.%int_32) [template = constants.%Convert.specific_fn]
// CHECK:STDOUT: %int.convert_checked: init %i32 = call %Convert.specific_fn(%int_1000) [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc3_19.1: %i32 = value_of_initializer %int.convert_checked [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc3_19.2: %i32 = converted %int_1000, %.loc3_19.1 [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc5_19.1: %i32 = value_of_initializer %int.convert_checked [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc5_19.2: %i32 = converted %int_1000, %.loc5_19.1 [template = constants.%int_1000.2]
// CHECK:STDOUT: %C: type = class_type @C, @C(constants.%int_1000.2) [template = constants.%C.2]
// CHECK:STDOUT: %b.param: %C.2 = value_param runtime_param<invalid>
// CHECK:STDOUT: %b.loc3_9.1: %C.2 = bind_symbolic_name b, 0, %b.param [symbolic = %b.loc3_9.2 (constants.%b)]
// CHECK:STDOUT: %b.loc5_9.1: %C.2 = bind_symbolic_name b, 0, %b.param [symbolic = %b.loc5_9.2 (constants.%b)]
// CHECK:STDOUT: }
// CHECK:STDOUT: %.decl: %.type = class_decl @.1 [template = constants.%.generic] {
// CHECK:STDOUT: %b.patt.loc10_9.1: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc10_9.2 (constants.%b.patt)]
// CHECK:STDOUT: %b.param_patt: %C.2 = value_param_pattern %b.patt.loc10_9.1, runtime_param<invalid> [symbolic = %b.patt.loc10_9.2 (constants.%b.patt)]
// CHECK:STDOUT: %b.patt.loc12_9.1: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc12_9.2 (constants.%b.patt)]
// CHECK:STDOUT: %b.param_patt: %C.2 = value_param_pattern %b.patt.loc12_9.1, runtime_param<invalid> [symbolic = %b.patt.loc12_9.2 (constants.%b.patt)]
// CHECK:STDOUT: } {
// CHECK:STDOUT: %C.ref: %C.type = name_ref C, file.%C.decl [template = constants.%C.generic]
// CHECK:STDOUT: %int_1000: Core.IntLiteral = int_value 1000 [template = constants.%int_1000.1]
// CHECK:STDOUT: %impl.elem0: %Convert.type.2 = interface_witness_access constants.%interface.9, element0 [template = constants.%Convert.14]
// CHECK:STDOUT: %Convert.bound: <bound method> = bound_method %int_1000, %impl.elem0 [template = constants.%Convert.bound]
// CHECK:STDOUT: %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(constants.%int_32) [template = constants.%Convert.specific_fn]
// CHECK:STDOUT: %int.convert_checked: init %i32 = call %Convert.specific_fn(%int_1000) [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc10_20.1: %i32 = value_of_initializer %int.convert_checked [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc10_20.2: %i32 = converted %int_1000, %.loc10_20.1 [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc12_20.1: %i32 = value_of_initializer %int.convert_checked [template = constants.%int_1000.2]
// CHECK:STDOUT: %.loc12_20.2: %i32 = converted %int_1000, %.loc12_20.1 [template = constants.%int_1000.2]
// CHECK:STDOUT: %C: type = class_type @C, @C(constants.%int_1000.2) [template = constants.%C.2]
// CHECK:STDOUT: %b.param: %C.2 = value_param runtime_param<invalid>
// CHECK:STDOUT: %b.loc10_9.1: %C.2 = bind_symbolic_name b, 0, %b.param [symbolic = %b.loc10_9.2 (constants.%b)]
// CHECK:STDOUT: %b.loc12_9.1: %C.2 = bind_symbolic_name b, 0, %b.param [symbolic = %b.loc12_9.2 (constants.%b)]
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic class @C(%a.loc2_9.1: %i32) {
// CHECK:STDOUT: %a.loc2_9.2: %i32 = bind_symbolic_name a, 0 [symbolic = %a.loc2_9.2 (constants.%a)]
// CHECK:STDOUT: %a.patt.loc2_9.2: %i32 = symbolic_binding_pattern a, 0 [symbolic = %a.patt.loc2_9.2 (constants.%a.patt)]
// CHECK:STDOUT: generic class @C(%a.loc4_9.1: %i32) {
// CHECK:STDOUT: %a.loc4_9.2: %i32 = bind_symbolic_name a, 0 [symbolic = %a.loc4_9.2 (constants.%a)]
// CHECK:STDOUT: %a.patt.loc4_9.2: %i32 = symbolic_binding_pattern a, 0 [symbolic = %a.patt.loc4_9.2 (constants.%a.patt)]
// CHECK:STDOUT:
// CHECK:STDOUT: !definition:
// CHECK:STDOUT:
Expand All @@ -272,16 +272,16 @@ class D(b:! C(1_000)) {}
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic class @D(%b.loc3_9.1: %C.2) {
// CHECK:STDOUT: %b.loc3_9.2: %C.2 = bind_symbolic_name b, 0 [symbolic = %b.loc3_9.2 (constants.%b)]
// CHECK:STDOUT: %b.patt.loc3_9.2: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc3_9.2 (constants.%b.patt)]
// CHECK:STDOUT: generic class @D(%b.loc5_9.1: %C.2) {
// CHECK:STDOUT: %b.loc5_9.2: %C.2 = bind_symbolic_name b, 0 [symbolic = %b.loc5_9.2 (constants.%b)]
// CHECK:STDOUT: %b.patt.loc5_9.2: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc5_9.2 (constants.%b.patt)]
// CHECK:STDOUT:
// CHECK:STDOUT: class;
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic class @.1(%b.loc10_9.1: %C.2) {
// CHECK:STDOUT: %b.loc10_9.2: %C.2 = bind_symbolic_name b, 0 [symbolic = %b.loc10_9.2 (constants.%b)]
// CHECK:STDOUT: %b.patt.loc10_9.2: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc10_9.2 (constants.%b.patt)]
// CHECK:STDOUT: generic class @.1(%b.loc12_9.1: %C.2) {
// CHECK:STDOUT: %b.loc12_9.2: %C.2 = bind_symbolic_name b, 0 [symbolic = %b.loc12_9.2 (constants.%b)]
// CHECK:STDOUT: %b.patt.loc12_9.2: %C.2 = symbolic_binding_pattern b, 0 [symbolic = %b.patt.loc12_9.2 (constants.%b.patt)]
// CHECK:STDOUT:
// CHECK:STDOUT: !definition:
// CHECK:STDOUT:
Expand All @@ -295,22 +295,22 @@ class D(b:! C(1_000)) {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @C(constants.%a) {
// CHECK:STDOUT: %a.loc2_9.2 => constants.%a
// CHECK:STDOUT: %a.patt.loc2_9.2 => constants.%a
// CHECK:STDOUT: %a.loc4_9.2 => constants.%a
// CHECK:STDOUT: %a.patt.loc4_9.2 => constants.%a
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @C(constants.%int_1000.2) {
// CHECK:STDOUT: %a.loc2_9.2 => constants.%int_1000.2
// CHECK:STDOUT: %a.patt.loc2_9.2 => constants.%int_1000.2
// CHECK:STDOUT: %a.loc4_9.2 => constants.%int_1000.2
// CHECK:STDOUT: %a.patt.loc4_9.2 => constants.%int_1000.2
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @D(constants.%b) {
// CHECK:STDOUT: %b.loc3_9.2 => constants.%b
// CHECK:STDOUT: %b.patt.loc3_9.2 => constants.%b
// CHECK:STDOUT: %b.loc5_9.2 => constants.%b
// CHECK:STDOUT: %b.patt.loc5_9.2 => constants.%b
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @.1(constants.%b) {
// CHECK:STDOUT: %b.loc10_9.2 => constants.%b
// CHECK:STDOUT: %b.patt.loc10_9.2 => constants.%b
// CHECK:STDOUT: %b.loc12_9.2 => constants.%b
// CHECK:STDOUT: %b.patt.loc12_9.2 => constants.%b
// CHECK:STDOUT: }
// CHECK:STDOUT:
4 changes: 2 additions & 2 deletions toolchain/check/testdata/packages/fail_duplicate_api.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/packages/fail_duplicate_api.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/packages/fail_duplicate_api.carbon
// CHECK:STDERR: fail_main2.carbon: error: `Main//default` previously provided by `main1.carbon` [DuplicateMainApi]
// CHECK:STDERR:

// --- main1.carbon

// --- fail_main2.carbon
// CHECK:STDERR: fail_main2.carbon: error: `Main//default` previously provided by `main1.carbon` [DuplicateMainApi]
// CHECK:STDERR:

// --- main_lib1.carbon

Expand Down
8 changes: 4 additions & 4 deletions toolchain/check/testdata/packages/fail_extension.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/packages/fail_extension.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/packages/fail_extension.carbon

// --- fail_main.incorrect
// CHECK:STDERR: fail_main.incorrect: error: file extension of `.carbon` required for api [IncorrectExtension]
// CHECK:STDERR:

// --- fail_main_redundant_with_swapped_ext.impl.carbon
// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: `Main//default` previously provided by `fail_main.incorrect` [DuplicateMainApi]
// CHECK:STDERR:
// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: error: file extension of `.carbon` required for api [IncorrectExtension]
// CHECK:STDERR: fail_main_redundant_with_swapped_ext.impl.carbon: note: file extension of `.impl.carbon` only allowed for `impl` [IncorrectExtensionImplNote]
// CHECK:STDERR:

// --- fail_main.incorrect

// --- fail_main_redundant_with_swapped_ext.impl.carbon

// --- fail_main_lib.incorrect

// CHECK:STDERR: fail_main_lib.incorrect:[[@LINE+4]]:1: error: file extension of `.carbon` required for api [IncorrectExtension]
Expand Down

0 comments on commit 55c257b

Please sign in to comment.