From 660d62ecc1f528ffbc16dff628ef10af625d9a94 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 28 Mar 2025 16:41:40 -0700 Subject: [PATCH] Preserve source locations in imported eval blocks (#5213) Don't lose track of where the instructions in an eval block are across import. --- toolchain/check/import_ref.cpp | 14 ++++++++-- .../testdata/basics/type_literals.carbon | 16 ++++++------ .../function/generic/resolve_used.carbon | 26 +++++++++---------- toolchain/testing/BUILD | 1 + toolchain/testing/file_test.cpp | 9 +++++++ 5 files changed, 43 insertions(+), 23 deletions(-) diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index a34e2eb2b753..d6bf14e39c01 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -3108,8 +3108,18 @@ static auto ResolveLocalEvalBlock(ImportRefResolver& resolver, } auto inst_ids = ResolveLocalInstBlockContents(resolver, import_block_id); - return RebuildGenericEvalBlock(resolver.local_context(), generic_id, region, - inst_ids); + auto eval_block_id = RebuildGenericEvalBlock(resolver.local_context(), + generic_id, region, inst_ids); + + // Set the locations of the instructions in the inst block to match those of + // the imported instructions. + for (auto [import_inst_id, local_inst_id] : + llvm::zip(resolver.import_inst_blocks().Get(import_block_id), + resolver.local_inst_blocks().Get(eval_block_id))) { + auto import_ir_inst_id = AddImportIRInst(resolver, import_inst_id); + resolver.local_insts().SetLocId(local_inst_id, import_ir_inst_id); + } + return eval_block_id; } // Fills in the remaining information in a partially-imported generic. diff --git a/toolchain/check/testdata/basics/type_literals.carbon b/toolchain/check/testdata/basics/type_literals.carbon index 01505223832b..9402419346fe 100644 --- a/toolchain/check/testdata/basics/type_literals.carbon +++ b/toolchain/check/testdata/basics/type_literals.carbon @@ -28,14 +28,14 @@ var test_i0: i0; // CHECK:STDERR: ^~ // CHECK:STDERR: var test_i1: i1; -// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+4]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.Int(15)` instead [IntWidthNotMultipleOf8] +// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+7]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.Int(15)` instead [IntWidthNotMultipleOf8] // CHECK:STDERR: var test_i15: i15; // CHECK:STDERR: ^~~ // CHECK:STDERR: +// CHECK:STDERR: {{.*}}/prelude/types/int.carbon:13:9: error: integer type width of 1000000000 is greater than the maximum supported width of 8388608 [IntWidthTooLarge] +// CHECK:STDERR: adapt MakeInt(N); +// CHECK:STDERR: ^~~~~~~~~~ var test_i15: i15; -// CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+7]]:23: error: integer type width of 1000000000 is greater than the maximum supported width of 8388608 [IntWidthTooLarge] -// CHECK:STDERR: var test_i1000000000: i1000000000; -// CHECK:STDERR: ^~~~~~~~~~~ // CHECK:STDERR: fail_iN_bad_width.carbon:[[@LINE+4]]:23: note: in `i1000000000` used here [ResolvingSpecificHere] // CHECK:STDERR: var test_i1000000000: i1000000000; // CHECK:STDERR: ^~~~~~~~~~~ @@ -72,14 +72,14 @@ var test_u0: u0; // CHECK:STDERR: ^~ // CHECK:STDERR: var test_u1: u1; -// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+4]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.UInt(15)` instead [IntWidthNotMultipleOf8] +// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+7]]:15: error: bit width of integer type literal must be a multiple of 8; use `Core.UInt(15)` instead [IntWidthNotMultipleOf8] // CHECK:STDERR: var test_u15: u15; // CHECK:STDERR: ^~~ // CHECK:STDERR: +// CHECK:STDERR: {{.*}}/prelude/types/uint.carbon:14:9: error: integer type width of 1000000000 is greater than the maximum supported width of 8388608 [IntWidthTooLarge] +// CHECK:STDERR: adapt MakeUInt(N); +// CHECK:STDERR: ^~~~~~~~~~~ var test_u15: u15; -// CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+7]]:23: error: integer type width of 1000000000 is greater than the maximum supported width of 8388608 [IntWidthTooLarge] -// CHECK:STDERR: var test_u1000000000: u1000000000; -// CHECK:STDERR: ^~~~~~~~~~~ // CHECK:STDERR: fail_uN_bad_width.carbon:[[@LINE+4]]:23: note: in `u1000000000` used here [ResolvingSpecificHere] // CHECK:STDERR: var test_u1000000000: u1000000000; // CHECK:STDERR: ^~~~~~~~~~~ diff --git a/toolchain/check/testdata/function/generic/resolve_used.carbon b/toolchain/check/testdata/function/generic/resolve_used.carbon index e04b33fc73b2..6de29e43de92 100644 --- a/toolchain/check/testdata/function/generic/resolve_used.carbon +++ b/toolchain/check/testdata/function/generic/resolve_used.carbon @@ -7,6 +7,9 @@ // TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/function/generic/resolve_used.carbon // TIP: To dump output, run: // TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/function/generic/resolve_used.carbon +// CHECK:STDERR: {{.*}}/prelude/types/int.carbon:13:9: error: integer type width of 0 is not positive [IntWidthNotPositive] +// CHECK:STDERR: adapt MakeInt(N); +// CHECK:STDERR: ^~~~~~~~~~ // --- fail_todo_call_monomorphization_error.carbon @@ -17,9 +20,6 @@ fn ErrorIfNIsZero(N:! Core.IntLiteral()) { // ensuring we produce an error when doing so. Notionally this error is // produced as a result of instantiating the `Core.Int` template, although // that's not how we currently model `Core.Int`. - // CHECK:STDERR: fail_todo_call_monomorphization_error.carbon:[[@LINE+6]]:10: error: integer type width of 0 is not positive [IntWidthNotPositive] - // CHECK:STDERR: var v: Core.Int(N); - // CHECK:STDERR: ^~~~~~~~~~~ // CHECK:STDERR: fail_todo_call_monomorphization_error.carbon:[[@LINE+3]]:10: note: in `i0` used here [ResolvingSpecificHere] // CHECK:STDERR: var v: Core.Int(N); // CHECK:STDERR: ^~~~~~~~~~~ @@ -94,23 +94,23 @@ fn CallNegative() { // CHECK:STDOUT: %N.patt.loc4_19.2: Core.IntLiteral = symbolic_binding_pattern N, 0 [symbolic = %N.patt.loc4_19.2 (constants.%N.patt)] // CHECK:STDOUT: // CHECK:STDOUT: !definition: -// CHECK:STDOUT: %Int.loc15_20.2: type = class_type @Int, @Int(%N.loc4_19.2) [symbolic = %Int.loc15_20.2 (constants.%Int)] -// CHECK:STDOUT: %require_complete: = require_complete_type @ErrorIfNIsZero.%Int.loc15_20.2 (%Int) [symbolic = %require_complete (constants.%require_complete.b4f)] +// CHECK:STDOUT: %Int.loc12_20.2: type = class_type @Int, @Int(%N.loc4_19.2) [symbolic = %Int.loc12_20.2 (constants.%Int)] +// CHECK:STDOUT: %require_complete: = require_complete_type @ErrorIfNIsZero.%Int.loc12_20.2 (%Int) [symbolic = %require_complete (constants.%require_complete.b4f)] // CHECK:STDOUT: // CHECK:STDOUT: fn(%N.patt.loc4_19.1: Core.IntLiteral) { // CHECK:STDOUT: !entry: // CHECK:STDOUT: name_binding_decl { -// CHECK:STDOUT: %v.patt: @ErrorIfNIsZero.%Int.loc15_20.2 (%Int) = binding_pattern v -// CHECK:STDOUT: %.loc15_3: @ErrorIfNIsZero.%Int.loc15_20.2 (%Int) = var_pattern %v.patt +// CHECK:STDOUT: %v.patt: @ErrorIfNIsZero.%Int.loc12_20.2 (%Int) = binding_pattern v +// CHECK:STDOUT: %.loc12_3: @ErrorIfNIsZero.%Int.loc12_20.2 (%Int) = var_pattern %v.patt // CHECK:STDOUT: } -// CHECK:STDOUT: %v.var: ref @ErrorIfNIsZero.%Int.loc15_20.2 (%Int) = var v -// CHECK:STDOUT: %.loc15_20: type = splice_block %Int.loc15_20.1 [symbolic = %Int.loc15_20.2 (constants.%Int)] { -// CHECK:STDOUT: %Core.ref.loc15: = name_ref Core, imports.%Core [concrete = imports.%Core] +// CHECK:STDOUT: %v.var: ref @ErrorIfNIsZero.%Int.loc12_20.2 (%Int) = var v +// CHECK:STDOUT: %.loc12_20: type = splice_block %Int.loc12_20.1 [symbolic = %Int.loc12_20.2 (constants.%Int)] { +// CHECK:STDOUT: %Core.ref.loc12: = name_ref Core, imports.%Core [concrete = imports.%Core] // CHECK:STDOUT: %Int.ref: %Int.type = name_ref Int, imports.%Core.Int [concrete = constants.%Int.generic] // CHECK:STDOUT: %N.ref: Core.IntLiteral = name_ref N, %N.loc4_19.1 [symbolic = %N.loc4_19.2 (constants.%N)] -// CHECK:STDOUT: %Int.loc15_20.1: type = class_type @Int, @Int(constants.%N) [symbolic = %Int.loc15_20.2 (constants.%Int)] +// CHECK:STDOUT: %Int.loc12_20.1: type = class_type @Int, @Int(constants.%N) [symbolic = %Int.loc12_20.2 (constants.%Int)] // CHECK:STDOUT: } -// CHECK:STDOUT: %v: ref @ErrorIfNIsZero.%Int.loc15_20.2 (%Int) = bind_name v, %v.var +// CHECK:STDOUT: %v: ref @ErrorIfNIsZero.%Int.loc12_20.2 (%Int) = bind_name v, %v.var // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: } @@ -134,7 +134,7 @@ fn CallNegative() { // CHECK:STDOUT: %N.patt.loc4_19.2 => constants.%int_0 // CHECK:STDOUT: // CHECK:STDOUT: !definition: -// CHECK:STDOUT: %Int.loc15_20.2 => constants.%i0 +// CHECK:STDOUT: %Int.loc12_20.2 => constants.%i0 // CHECK:STDOUT: %require_complete => constants.%complete_type.d94 // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/testing/BUILD b/toolchain/testing/BUILD index fd00065b4d4c..59416877010b 100644 --- a/toolchain/testing/BUILD +++ b/toolchain/testing/BUILD @@ -68,6 +68,7 @@ file_test( "//common:error", "//testing/file_test:file_test_base", "//toolchain/driver", + "@abseil-cpp//absl/strings", "@llvm-project//llvm:Support", ], ) diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index aa8a2da01448..ae03ff1b5f61 100644 --- a/toolchain/testing/file_test.cpp +++ b/toolchain/testing/file_test.cpp @@ -9,6 +9,7 @@ #include #include +#include "absl/strings/str_replace.h" #include "common/error.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" @@ -230,6 +231,14 @@ auto ToolchainFileTest::DoExtraCheckReplacements(std::string& check_line) const // The column happens to be right for FileStart, but the line is wrong. static RE2 file_token_re(R"((FileEnd.*column: |FileStart.*line: )( *\d+))"); RE2::Replace(&check_line, file_token_re, R"(\1{{ *\\d+}})"); + } else if (component_ == "check") { + // The path to the core package appears in some check diagnostics, and will + // differ between testing environments, so don't test it. + // TODO: Consider adding a content keyword to name the core package, and + // replace with that instead. Alternatively, consider adding the core + // package to the VFS with a fixed name. + absl::StrReplaceAll({{installation_.core_package(), "{{.*}}"}}, + &check_line); } else { FileTestBase::DoExtraCheckReplacements(check_line); }