From e6e61e14ae8ca9278d12eb5420cbcec0b79aad9c Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 31 Jul 2024 09:38:56 -0700 Subject: [PATCH] Fix incorrect `value_id` and location in imported `BindSymbolicName`. (#4178) Instead of updating the `value_id` on the canonical constant `BindSymbolicName` to refer to some particular instance of that constant, create a new instruction, and attach the proper location to it. --- toolchain/check/import_ref.cpp | 11 +++---- .../testdata/class/generic/import.carbon | 29 +++++++++++-------- .../no_prelude/generic_import.carbon | 2 +- .../no_prelude/fail_export_name_params.carbon | 2 +- toolchain/check/testdata/struct/import.carbon | 6 ++-- toolchain/check/testdata/tuples/import.carbon | 6 ++-- 6 files changed, 29 insertions(+), 27 deletions(-) diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index d7e4635fa791..1334d1433b82 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -531,18 +531,15 @@ class ImportRefResolver { break; } case SemIR::BindSymbolicName::Kind: { - // The symbolic name will be created on first reference, so might - // already exist. Update the value in it to refer to the parameter. + // We already imported a constant value for this symbolic binding. + // We can reuse most of it. auto new_bind_inst_id = GetLocalConstantInstId(bind_id); auto new_bind_inst = context_.insts().GetAs( new_bind_inst_id); new_bind_inst.value_id = new_param_id; - // This is not before constant use, but doesn't change the - // constant value of the instruction. - context_.ReplaceInstBeforeConstantUse(new_bind_inst_id, - new_bind_inst); - new_param_id = new_bind_inst_id; + new_param_id = context_.AddInstInNoBlock(AddImportIRInst(bind_id), + new_bind_inst); break; } default: { diff --git a/toolchain/check/testdata/class/generic/import.carbon b/toolchain/check/testdata/class/generic/import.carbon index dffd54bf87a5..7df195a06898 100644 --- a/toolchain/check/testdata/class/generic/import.carbon +++ b/toolchain/check/testdata/class/generic/import.carbon @@ -76,10 +76,15 @@ fn Use() { impl library "foo"; -// CHECK:STDERR: fail_bad_foo.impl.carbon:[[@LINE+5]]:13: ERROR: Redeclaration differs at parameter 1. +// CHECK:STDERR: fail_bad_foo.impl.carbon:[[@LINE+10]]:13: ERROR: Redeclaration differs at parameter 1. // CHECK:STDERR: class Class(U:! type) { // CHECK:STDERR: ^ -// CHECK:STDERR: fail_bad_foo.impl.carbon: Previous declaration's corresponding parameter here. +// CHECK:STDERR: fail_bad_foo.impl.carbon:[[@LINE-5]]:6: In import. +// CHECK:STDERR: impl library "foo"; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: foo.carbon:4:13: Previous declaration's corresponding parameter here. +// CHECK:STDERR: class Class(T:! type); +// CHECK:STDERR: ^ // CHECK:STDERR: class Class(U:! type) { // CHECK:STDERR: fail_bad_foo.impl.carbon:[[@LINE+3]]:10: ERROR: Name `T` not found. @@ -222,7 +227,7 @@ class Class(U:! type) { // CHECK:STDOUT: --- foo.impl.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { -// CHECK:STDOUT: %T: type = bind_symbolic_name T 0, .inst+31 [symbolic] +// CHECK:STDOUT: %T: type = bind_symbolic_name T 0 [symbolic] // CHECK:STDOUT: %Class.type: type = generic_class_type @Class [template] // CHECK:STDOUT: %.1: type = tuple_type () [template] // CHECK:STDOUT: %Class.1: %Class.type = struct_value () [template] @@ -333,7 +338,7 @@ class Class(U:! type) { // CHECK:STDOUT: %CompleteClass.type: type = generic_class_type @CompleteClass [template] // CHECK:STDOUT: %CompleteClass.1: %CompleteClass.type = struct_value () [template] // CHECK:STDOUT: %.2: type = struct_type {.n: i32} [template] -// CHECK:STDOUT: %T: type = bind_symbolic_name T 0, .inst+28 [symbolic] +// CHECK:STDOUT: %T: type = bind_symbolic_name T 0 [symbolic] // CHECK:STDOUT: %CompleteClass.2: type = class_type @CompleteClass, invalid(%T) [symbolic] // CHECK:STDOUT: %CompleteClass.3: type = class_type @CompleteClass, invalid(i32) [template] // CHECK:STDOUT: %.3: type = ptr_type %.2 [template] @@ -432,7 +437,7 @@ class Class(U:! type) { // CHECK:STDOUT: %CompleteClass.type: type = generic_class_type @CompleteClass [template] // CHECK:STDOUT: %CompleteClass.1: %CompleteClass.type = struct_value () [template] // CHECK:STDOUT: %.2: type = struct_type {.n: i32} [template] -// CHECK:STDOUT: %T: type = bind_symbolic_name T 0, .inst+28 [symbolic] +// CHECK:STDOUT: %T: type = bind_symbolic_name T 0 [symbolic] // CHECK:STDOUT: %CompleteClass.2: type = class_type @CompleteClass, invalid(%T) [symbolic] // CHECK:STDOUT: %CompleteClass.3: type = class_type @CompleteClass, invalid(i32) [template] // CHECK:STDOUT: %.3: type = ptr_type %.2 [template] @@ -524,7 +529,7 @@ class Class(U:! type) { // CHECK:STDOUT: %CompleteClass.type: type = generic_class_type @CompleteClass [template] // CHECK:STDOUT: %CompleteClass.1: %CompleteClass.type = struct_value () [template] // CHECK:STDOUT: %.2: type = struct_type {.n: i32} [template] -// CHECK:STDOUT: %T: type = bind_symbolic_name T 0, .inst+19 [symbolic] +// CHECK:STDOUT: %T: type = bind_symbolic_name T 0 [symbolic] // CHECK:STDOUT: %CompleteClass.2: type = class_type @CompleteClass, invalid(%T) [symbolic] // CHECK:STDOUT: %Int32.type: type = fn_type @Int32 [template] // CHECK:STDOUT: %Int32: %Int32.type = struct_value () [template] @@ -612,7 +617,7 @@ class Class(U:! type) { // CHECK:STDOUT: %Class.type: type = generic_class_type @Class [template] // CHECK:STDOUT: %.1: type = tuple_type () [template] // CHECK:STDOUT: %Class.1: %Class.type = struct_value () [template] -// CHECK:STDOUT: %T: type = bind_symbolic_name T 0, .inst+17 [symbolic] +// CHECK:STDOUT: %T: type = bind_symbolic_name T 0 [symbolic] // CHECK:STDOUT: %Class.2: type = class_type @Class, invalid(%T) [symbolic] // CHECK:STDOUT: %.type: type = generic_class_type @.1 [template] // CHECK:STDOUT: %.2: %.type = struct_value () [template] @@ -645,25 +650,25 @@ class Class(U:! type) { // CHECK:STDOUT: %default.import.loc2_6.2 = import // CHECK:STDOUT: %Core.import = import Core // CHECK:STDOUT: %.decl: %.type = class_decl @.1 [template = constants.%.2] { -// CHECK:STDOUT: %U.loc9_13.1: type = param U -// CHECK:STDOUT: %U.loc9_13.2: type = bind_symbolic_name U 0, %U.loc9_13.1 [symbolic = @.1.%U (constants.%U)] +// CHECK:STDOUT: %U.loc14_13.1: type = param U +// CHECK:STDOUT: %U.loc14_13.2: type = bind_symbolic_name U 0, %U.loc14_13.1 [symbolic = @.1.%U (constants.%U)] // CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: class @Class; // CHECK:STDOUT: -// CHECK:STDOUT: generic class @.1(file.%U.loc9_13.2: type) { +// CHECK:STDOUT: generic class @.1(file.%U.loc14_13.2: type) { // CHECK:STDOUT: %U: type = bind_symbolic_name U 0 [symbolic = %U (constants.%U)] // CHECK:STDOUT: // CHECK:STDOUT: !definition: // CHECK:STDOUT: // CHECK:STDOUT: class { // CHECK:STDOUT: %T.ref: = name_ref T, [template = ] -// CHECK:STDOUT: %.loc13: = field_decl x, element0 [template] +// CHECK:STDOUT: %.loc18: = field_decl x, element0 [template] // CHECK:STDOUT: // CHECK:STDOUT: !members: // CHECK:STDOUT: .Self = constants.%.3 -// CHECK:STDOUT: .x = %.loc13 +// CHECK:STDOUT: .x = %.loc18 // CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/interface/no_prelude/generic_import.carbon b/toolchain/check/testdata/interface/no_prelude/generic_import.carbon index b9b5a44d4f49..f8dfa7d9cc3f 100644 --- a/toolchain/check/testdata/interface/no_prelude/generic_import.carbon +++ b/toolchain/check/testdata/interface/no_prelude/generic_import.carbon @@ -98,7 +98,7 @@ impl C as AddWith(C) { // CHECK:STDOUT: %AddWith.type: type = generic_interface_type @AddWith [template] // CHECK:STDOUT: %.2: type = tuple_type () [template] // CHECK:STDOUT: %AddWith: %AddWith.type = struct_value () [template] -// CHECK:STDOUT: %T: type = bind_symbolic_name T 0, .inst+14 [symbolic] +// CHECK:STDOUT: %T: type = bind_symbolic_name T 0 [symbolic] // CHECK:STDOUT: %.3: type = interface_type @AddWith, invalid(%T) [symbolic] // CHECK:STDOUT: %Self: %.3 = bind_symbolic_name Self 1 [symbolic] // CHECK:STDOUT: %.4: type = interface_type @AddWith, invalid(%C) [template] diff --git a/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon b/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon index 2d647faed822..451c3d3b4660 100644 --- a/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon +++ b/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon @@ -82,7 +82,7 @@ export C2(T:! type); // CHECK:STDOUT: %C1.type: type = generic_class_type @C1 [template] // CHECK:STDOUT: %.1: type = tuple_type () [template] // CHECK:STDOUT: %C1.1: %C1.type = struct_value () [template] -// CHECK:STDOUT: %T: type = bind_symbolic_name T 0, .inst+18 [symbolic] +// CHECK:STDOUT: %T: type = bind_symbolic_name T 0 [symbolic] // CHECK:STDOUT: %C1.2: type = class_type @C1, invalid(%T) [symbolic] // CHECK:STDOUT: %C2.type: type = generic_class_type @C2 [template] // CHECK:STDOUT: %C2.1: %C2.type = struct_value () [template] diff --git a/toolchain/check/testdata/struct/import.carbon b/toolchain/check/testdata/struct/import.carbon index bd37ed8dad9e..19ed5422e84e 100644 --- a/toolchain/check/testdata/struct/import.carbon +++ b/toolchain/check/testdata/struct/import.carbon @@ -224,7 +224,7 @@ var c_bad: C({.a = 3, .b = 4}) = F(); // CHECK:STDOUT: %C.1: %C.type = struct_value () [template] // CHECK:STDOUT: %.10: type = struct_type {} [template] // CHECK:STDOUT: %.11: type = struct_type {.a: i32, .b: i32} [template] -// CHECK:STDOUT: %S: %.11 = bind_symbolic_name S 0, .inst+100 [symbolic] +// CHECK:STDOUT: %S: %.11 = bind_symbolic_name S 0 [symbolic] // CHECK:STDOUT: %C.2: type = class_type @C, invalid(%S) [symbolic] // CHECK:STDOUT: %.12: i32 = int_literal 1 [template] // CHECK:STDOUT: %.13: i32 = int_literal 2 [template] @@ -362,7 +362,7 @@ var c_bad: C({.a = 3, .b = 4}) = F(); // CHECK:STDOUT: %C.1: %C.type = struct_value () [template] // CHECK:STDOUT: %.2: type = struct_type {} [template] // CHECK:STDOUT: %.3: type = struct_type {.a: i32, .b: i32} [template] -// CHECK:STDOUT: %S: %.3 = bind_symbolic_name S 0, .inst+21 [symbolic] +// CHECK:STDOUT: %S: %.3 = bind_symbolic_name S 0 [symbolic] // CHECK:STDOUT: %C.2: type = class_type @C, invalid(%S) [symbolic] // CHECK:STDOUT: %.4: i32 = int_literal 1 [template] // CHECK:STDOUT: %.5: i32 = int_literal 2 [template] @@ -443,7 +443,7 @@ var c_bad: C({.a = 3, .b = 4}) = F(); // CHECK:STDOUT: %C.1: %C.type = struct_value () [template] // CHECK:STDOUT: %.2: type = struct_type {} [template] // CHECK:STDOUT: %.3: type = struct_type {.a: i32, .b: i32} [template] -// CHECK:STDOUT: %S: %.3 = bind_symbolic_name S 0, .inst+21 [symbolic] +// CHECK:STDOUT: %S: %.3 = bind_symbolic_name S 0 [symbolic] // CHECK:STDOUT: %C.2: type = class_type @C, invalid(%S) [symbolic] // CHECK:STDOUT: %.4: i32 = int_literal 3 [template] // CHECK:STDOUT: %.5: i32 = int_literal 4 [template] diff --git a/toolchain/check/testdata/tuples/import.carbon b/toolchain/check/testdata/tuples/import.carbon index 44427af86b49..73190a46f35f 100644 --- a/toolchain/check/testdata/tuples/import.carbon +++ b/toolchain/check/testdata/tuples/import.carbon @@ -248,7 +248,7 @@ var c_bad: C((3, 4)) = F(); // CHECK:STDOUT: %C.type: type = generic_class_type @C [template] // CHECK:STDOUT: %C.1: %C.type = struct_value () [template] // CHECK:STDOUT: %.14: type = struct_type {} [template] -// CHECK:STDOUT: %X: %.8 = bind_symbolic_name X 0, .inst+101 [symbolic] +// CHECK:STDOUT: %X: %.8 = bind_symbolic_name X 0 [symbolic] // CHECK:STDOUT: %C.2: type = class_type @C, invalid(%X) [symbolic] // CHECK:STDOUT: %.15: i32 = int_literal 1 [template] // CHECK:STDOUT: %.16: i32 = int_literal 2 [template] @@ -401,7 +401,7 @@ var c_bad: C((3, 4)) = F(); // CHECK:STDOUT: %C.1: %C.type = struct_value () [template] // CHECK:STDOUT: %.2: type = struct_type {} [template] // CHECK:STDOUT: %.3: type = tuple_type (i32, i32) [template] -// CHECK:STDOUT: %X: %.3 = bind_symbolic_name X 0, .inst+17 [symbolic] +// CHECK:STDOUT: %X: %.3 = bind_symbolic_name X 0 [symbolic] // CHECK:STDOUT: %C.2: type = class_type @C, invalid(%X) [symbolic] // CHECK:STDOUT: %.4: i32 = int_literal 1 [template] // CHECK:STDOUT: %.5: i32 = int_literal 2 [template] @@ -484,7 +484,7 @@ var c_bad: C((3, 4)) = F(); // CHECK:STDOUT: %C.1: %C.type = struct_value () [template] // CHECK:STDOUT: %.2: type = struct_type {} [template] // CHECK:STDOUT: %.3: type = tuple_type (i32, i32) [template] -// CHECK:STDOUT: %X: %.3 = bind_symbolic_name X 0, .inst+17 [symbolic] +// CHECK:STDOUT: %X: %.3 = bind_symbolic_name X 0 [symbolic] // CHECK:STDOUT: %C.2: type = class_type @C, invalid(%X) [symbolic] // CHECK:STDOUT: %.4: i32 = int_literal 3 [template] // CHECK:STDOUT: %.5: i32 = int_literal 4 [template]