From 55c257bc9316b26b53f6c6522341b726f63d4e43 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 12 Dec 2024 16:05:33 -0800 Subject: [PATCH] Use a filename without a line number as a cue for autoupdate. (#4677) This is so that diagnostics which lack a location get split file clustering. --- testing/file_test/autoupdate.cpp | 9 +-- testing/file_test/autoupdate_testdata.sh | 3 +- testing/file_test/file_test_base.cpp | 2 +- testing/file_test/file_test_base_test.cpp | 12 ++++ .../file_test/testdata/no_line_number.carbon | 21 ++++++ .../class/syntactic_merge_literal.carbon | 68 +++++++++---------- .../packages/fail_duplicate_api.carbon | 4 +- .../testdata/packages/fail_extension.carbon | 8 +-- 8 files changed, 81 insertions(+), 46 deletions(-) create mode 100644 testing/file_test/testdata/no_line_number.carbon diff --git a/testing/file_test/autoupdate.cpp b/testing/file_test/autoupdate.cpp index 85424c74962c..307753cc2909 100644 --- a/testing/file_test/autoupdate.cpp +++ b/testing/file_test/autoupdate.cpp @@ -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}", @@ -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; @@ -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(); diff --git a/testing/file_test/autoupdate_testdata.sh b/testing/file_test/autoupdate_testdata.sh index c258c803dc63..e3bca94919af 100755 --- a/testing/file_test/autoupdate_testdata.sh +++ b/testing/file_test/autoupdate_testdata.sh @@ -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 "$@" diff --git a/testing/file_test/file_test_base.cpp b/testing/file_test/file_test_base.cpp index fd250422261c..c32ec573d899 100644 --- a/testing/file_test/file_test_base.cpp +++ b/testing/file_test/file_test_base.cpp @@ -277,7 +277,7 @@ auto FileTestBase::GetLineNumberReplacements( -> llvm::SmallVector { return {{.has_file = true, .re = std::make_shared( - llvm::formatv(R"(({0}):(\d+))", llvm::join(filenames, "|"))), + llvm::formatv(R"(({0}):(\d+)?)", llvm::join(filenames, "|"))), .line_formatv = R"({0})"}}; } diff --git a/testing/file_test/file_test_base_test.cpp b/testing/file_test/file_test_base_test.cpp index 6caa1d6cf84a..2660ddb6c534 100644 --- a/testing/file_test/file_test_base_test.cpp +++ b/testing/file_test/file_test_base_test.cpp @@ -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 { + 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 { @@ -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&) { diff --git a/testing/file_test/testdata/no_line_number.carbon b/testing/file_test/testdata/no_line_number.carbon new file mode 100644 index 000000000000..8f911f5b2117 --- /dev/null +++ b/testing/file_test/testdata/no_line_number.carbon @@ -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 diff --git a/toolchain/check/testdata/class/syntactic_merge_literal.carbon b/toolchain/check/testdata/class/syntactic_merge_literal.carbon index af6ad9aa7285..0c7c4e0c67a3 100644 --- a/toolchain/check/testdata/class/syntactic_merge_literal.carbon +++ b/toolchain/check/testdata/class/syntactic_merge_literal.carbon @@ -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 @@ -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)); @@ -213,19 +213,19 @@ 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 [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 [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 -// 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 [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 [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] @@ -233,15 +233,15 @@ class D(b:! C(1_000)) {} // CHECK:STDOUT: %Convert.bound: = bound_method %int_1000, %impl.elem0 [template = constants.%Convert.bound] // CHECK:STDOUT: %Convert.specific_fn: = 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 -// 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 [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 [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] @@ -249,17 +249,17 @@ class D(b:! C(1_000)) {} // CHECK:STDOUT: %Convert.bound: = bound_method %int_1000, %impl.elem0 [template = constants.%Convert.bound] // CHECK:STDOUT: %Convert.specific_fn: = 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 -// 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: @@ -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: @@ -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: diff --git a/toolchain/check/testdata/packages/fail_duplicate_api.carbon b/toolchain/check/testdata/packages/fail_duplicate_api.carbon index d41dd6567424..da19cec46950 100644 --- a/toolchain/check/testdata/packages/fail_duplicate_api.carbon +++ b/toolchain/check/testdata/packages/fail_duplicate_api.carbon @@ -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 diff --git a/toolchain/check/testdata/packages/fail_extension.carbon b/toolchain/check/testdata/packages/fail_extension.carbon index 24aa0d805ef1..400aa6008539 100644 --- a/toolchain/check/testdata/packages/fail_extension.carbon +++ b/toolchain/check/testdata/packages/fail_extension.carbon @@ -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]