Просмотр исходного кода

Don't substitute into the targeted instructions of an associated constant. (#4342)

When an instruction makes an absolute reference to another instruction,
such as when `assoc_const` refers to the declaration of the associated
constant in an interface, substitution into that instruction should not
substitute into the referenced instruction.

Mark the corresponding `InstId` fields in the typed instructions as
being absolute by giving them a distinct ID type that `Subst` doesn't
substitute into. This formation of unnecessarily complicated SemIR that
could in some cases lead to a CHECK failure when printing formatted
SemIR because the same instruction ends up in multiple scopes.
Richard Smith 1 год назад
Родитель
Сommit
42bda1e38f

+ 164 - 0
toolchain/check/testdata/interface/no_prelude/assoc_const_in_generic.carbon

@@ -0,0 +1,164 @@
+// 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 //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/interface/no_prelude/assoc_const_in_generic.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interface/no_prelude/assoc_const_in_generic.carbon
+
+interface I(T:! type) {
+  fn F(U:! type) -> U;
+}
+
+fn G(T:! type) {
+  // This should not result in a `fn_decl` instruction being added to the eval
+  // block for the generic G. This used to crash when printing formatted SemIR
+  // because the same instruction ended up in multiple scopes.
+  I(T).F;
+}
+
+fn H() {
+  G({});
+}
+
+// CHECK:STDOUT: --- assoc_const_in_generic.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %T: type = bind_symbolic_name T 0 [symbolic]
+// CHECK:STDOUT:   %I.type: type = generic_interface_type @I [template]
+// CHECK:STDOUT:   %.1: type = tuple_type () [template]
+// CHECK:STDOUT:   %I: %I.type = struct_value () [template]
+// CHECK:STDOUT:   %.2: type = interface_type @I, @I(%T) [symbolic]
+// CHECK:STDOUT:   %Self: %.2 = bind_symbolic_name Self 1 [symbolic]
+// CHECK:STDOUT:   %U: type = bind_symbolic_name U 2 [symbolic]
+// CHECK:STDOUT:   %F.type: type = fn_type @F, @I(%T) [symbolic]
+// CHECK:STDOUT:   %F: %F.type = struct_value () [symbolic]
+// CHECK:STDOUT:   %.3: type = assoc_entity_type %.2, %F.type [symbolic]
+// CHECK:STDOUT:   %.4: %.3 = assoc_entity element0, @I.%F.decl [symbolic]
+// CHECK:STDOUT:   %G.type: type = fn_type @G [template]
+// CHECK:STDOUT:   %G: %G.type = struct_value () [template]
+// CHECK:STDOUT:   %H.type: type = fn_type @H [template]
+// CHECK:STDOUT:   %H: %H.type = struct_value () [template]
+// CHECK:STDOUT:   %.5: type = struct_type {} [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .I = %I.decl
+// CHECK:STDOUT:     .G = %G.decl
+// CHECK:STDOUT:     .H = %H.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %I.decl: %I.type = interface_decl @I [template = constants.%I] {
+// CHECK:STDOUT:     %T.patt: type = symbolic_binding_pattern T 0
+// CHECK:STDOUT:   } {
+// CHECK:STDOUT:     %T.param: type = param T, runtime_param<invalid>
+// CHECK:STDOUT:     %T.loc11: type = bind_symbolic_name T 0, %T.param [symbolic = %T.1 (constants.%T)]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %G.decl: %G.type = fn_decl @G [template = constants.%G] {
+// CHECK:STDOUT:     %T.patt: type = symbolic_binding_pattern T 0
+// CHECK:STDOUT:   } {
+// CHECK:STDOUT:     %T.param: type = param T, runtime_param<invalid>
+// CHECK:STDOUT:     %T.loc15: type = bind_symbolic_name T 0, %T.param [symbolic = %T.1 (constants.%T)]
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %H.decl: %H.type = fn_decl @H [template = constants.%H] {} {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: generic interface @I(%T.loc11: type) {
+// CHECK:STDOUT:   %T.1: type = bind_symbolic_name T 0 [symbolic = %T.1 (constants.%T)]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %.1: type = interface_type @I, @I(%T.1) [symbolic = %.1 (constants.%.2)]
+// CHECK:STDOUT:   %Self.2: %.2 = bind_symbolic_name Self 1 [symbolic = %Self.2 (constants.%Self)]
+// CHECK:STDOUT:   %F.type: type = fn_type @F, @I(%T.1) [symbolic = %F.type (constants.%F.type)]
+// CHECK:STDOUT:   %F: @I.%F.type (%F.type) = struct_value () [symbolic = %F (constants.%F)]
+// CHECK:STDOUT:   %.2: type = assoc_entity_type @I.%.1 (%.2), @I.%F.type (%F.type) [symbolic = %.2 (constants.%.3)]
+// CHECK:STDOUT:   %.3: @I.%.2 (%.3) = assoc_entity element0, %F.decl [symbolic = %.3 (constants.%.4)]
+// CHECK:STDOUT:
+// CHECK:STDOUT:   interface {
+// CHECK:STDOUT:     %Self.1: @I.%.1 (%.2) = bind_symbolic_name Self 1 [symbolic = %Self.2 (constants.%Self)]
+// CHECK:STDOUT:     %F.decl: @I.%F.type (%F.type) = fn_decl @F [symbolic = @I.%F (constants.%F)] {
+// CHECK:STDOUT:       %U.patt: type = symbolic_binding_pattern U 2
+// CHECK:STDOUT:     } {
+// CHECK:STDOUT:       %U.param: type = param U, runtime_param<invalid>
+// CHECK:STDOUT:       %U.loc12: type = bind_symbolic_name U 2, %U.param [symbolic = %U.1 (constants.%U)]
+// CHECK:STDOUT:       %U.ref: type = name_ref U, %U.loc12 [symbolic = %U.1 (constants.%U)]
+// CHECK:STDOUT:       %return: ref @F.%U.1 (%U) = var <return slot>
+// CHECK:STDOUT:     }
+// CHECK:STDOUT:     %.loc12: @I.%.2 (%.3) = assoc_entity element0, %F.decl [symbolic = %.3 (constants.%.4)]
+// CHECK:STDOUT:
+// CHECK:STDOUT:   !members:
+// CHECK:STDOUT:     .Self = %Self.1
+// CHECK:STDOUT:     .F = %.loc12
+// CHECK:STDOUT:     witness = (%F.decl)
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: generic fn @F(@I.%T.loc11: type, @I.%Self.1: @I.%.1 (%.2), %U.loc12: type) {
+// CHECK:STDOUT:   %U.1: type = bind_symbolic_name U 2 [symbolic = %U.1 (constants.%U)]
+// CHECK:STDOUT:
+// CHECK:STDOUT:   fn(%U.loc12: type) -> @F.%U.1 (%U);
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: generic fn @G(%T.loc15: type) {
+// CHECK:STDOUT:   %T.1: type = bind_symbolic_name T 0 [symbolic = %T.1 (constants.%T)]
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %.1: type = interface_type @I, @I(%T.1) [symbolic = %.1 (constants.%.2)]
+// CHECK:STDOUT:   %F.type: type = fn_type @F, @I(%T.1) [symbolic = %F.type (constants.%F.type)]
+// CHECK:STDOUT:   %.2: type = assoc_entity_type @G.%.1 (%.2), @G.%F.type (%F.type) [symbolic = %.2 (constants.%.3)]
+// CHECK:STDOUT:   %.3: @G.%.2 (%.3) = assoc_entity element0, @I.%F.decl [symbolic = %.3 (constants.%.4)]
+// CHECK:STDOUT:
+// CHECK:STDOUT:   fn(%T.loc15: type) {
+// CHECK:STDOUT:   !entry:
+// CHECK:STDOUT:     %I.ref: %I.type = name_ref I, file.%I.decl [template = constants.%I]
+// CHECK:STDOUT:     %T.ref: type = name_ref T, %T.loc15 [symbolic = %T.1 (constants.%T)]
+// CHECK:STDOUT:     %.loc19_4: type = interface_type @I, @I(constants.%T) [symbolic = %.1 (constants.%.2)]
+// CHECK:STDOUT:     %.loc19_7: @G.%.2 (%.3) = specific_constant @I.%.loc12, @I(constants.%T) [symbolic = %.3 (constants.%.4)]
+// CHECK:STDOUT:     %F.ref: @G.%.2 (%.3) = name_ref F, %.loc19_7 [symbolic = %.3 (constants.%.4)]
+// CHECK:STDOUT:     return
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @H() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %G.ref: %G.type = name_ref G, file.%G.decl [template = constants.%G]
+// CHECK:STDOUT:   %.loc23_6: %.5 = struct_literal ()
+// CHECK:STDOUT:   %.loc23_4: type = converted %.loc23_6, constants.%.5 [template = constants.%.5]
+// CHECK:STDOUT:   %G.call: init %.1 = call %G.ref()
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: specific @I(constants.%T) {
+// CHECK:STDOUT:   %T.1 => constants.%T
+// CHECK:STDOUT:
+// CHECK:STDOUT: !definition:
+// CHECK:STDOUT:   %.1 => constants.%.2
+// CHECK:STDOUT:   %Self.2 => constants.%Self
+// CHECK:STDOUT:   %F.type => constants.%F.type
+// CHECK:STDOUT:   %F => constants.%F
+// CHECK:STDOUT:   %.2 => constants.%.3
+// CHECK:STDOUT:   %.3 => constants.%.4
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: specific @F(constants.%T, constants.%Self, constants.%U) {
+// CHECK:STDOUT:   %U.1 => constants.%U
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: specific @I(@I.%T.1) {
+// CHECK:STDOUT:   %T.1 => constants.%T
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: specific @G(constants.%T) {
+// CHECK:STDOUT:   %T.1 => constants.%T
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: specific @I(@G.%T.1) {
+// CHECK:STDOUT:   %T.1 => constants.%T
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: specific @G(constants.%.5) {
+// CHECK:STDOUT:   %T.1 => constants.%.5
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

+ 4 - 0
toolchain/sem_ir/formatter.cpp

@@ -1030,6 +1030,10 @@ class FormatterImpl {
     out_ << inst_namer_->GetNameFor(scope_, id);
   }
 
+  auto FormatName(AbsoluteInstId id) -> void {
+    FormatName(static_cast<InstId>(id));
+  }
+
   auto FormatName(SpecificId id) -> void {
     const auto& specific = sem_ir_.specifics().Get(id);
     FormatName(specific.generic_id);

+ 5 - 4
toolchain/sem_ir/id_kind.h

@@ -120,10 +120,11 @@ using IdKind = TypeEnum<
     // From base/value_store.h.
     IntId, RealId, FloatId, StringLiteralValueId,
     // From sem_ir/id.h.
-    InstId, ConstantId, EntityNameId, CompileTimeBindIndex, RuntimeParamIndex,
-    FunctionId, ClassId, InterfaceId, ImplId, GenericId, SpecificId, ImportIRId,
-    ImportIRInstId, LocId, BoolValue, IntKind, NameId, NameScopeId, InstBlockId,
-    TypeId, TypeBlockId, ElementIndex, LibraryNameId, FloatKind>;
+    InstId, AbsoluteInstId, ConstantId, EntityNameId, CompileTimeBindIndex,
+    RuntimeParamIndex, FunctionId, ClassId, InterfaceId, ImplId, GenericId,
+    SpecificId, ImportIRId, ImportIRInstId, LocId, BoolValue, IntKind, NameId,
+    NameScopeId, InstBlockId, TypeId, TypeBlockId, ElementIndex, LibraryNameId,
+    FloatKind>;
 
 }  // namespace Carbon::SemIR
 

+ 19 - 0
toolchain/sem_ir/ids.h

@@ -86,6 +86,25 @@ constexpr InstId InstId::Invalid = InstId(InvalidIndex);
       InstId::ForBuiltin(BuiltinInstKind::Name);
 #include "toolchain/sem_ir/builtin_inst_kind.def"
 
+// An ID of an instruction that is referenced absolutely by another instruction.
+// This should only be used as the type of a field within a typed instruction
+// class.
+//
+// When a typed instruction has a field of this type, that field represents an
+// absolute reference to another instruction that typically resides in a
+// different entity. This behaves in most respects like an InstId field, but
+// substitution into the typed instruction leaves the field unchanged rather
+// than substituting into it.
+class AbsoluteInstId : public InstId {
+ public:
+  // Support implicit conversion from InstId so that InstId and AbsoluteInstId
+  // have the same interface.
+  // NOLINTNEXTLINE(google-explicit-constructor)
+  constexpr AbsoluteInstId(InstId inst_id) : InstId(inst_id) {}
+
+  using InstId::InstId;
+};
+
 // The package namespace will be the instruction after builtins.
 constexpr InstId InstId::PackageNamespace = InstId(BuiltinInstKind::ValidCount);
 

+ 3 - 3
toolchain/sem_ir/typed_insts.h

@@ -188,7 +188,7 @@ struct AssociatedEntity {
   // The type of the associated entity. This is an AssociatedEntityType.
   TypeId type_id;
   ElementIndex index;
-  InstId decl_id;
+  AbsoluteInstId decl_id;
 };
 
 // The type of an expression that names an associated entity, such as
@@ -810,7 +810,7 @@ struct Namespace {
   NameScopeId name_scope_id;
   // If the namespace was produced by an `import` line, the associated line for
   // diagnostics.
-  InstId import_id;
+  AbsoluteInstId import_id;
 };
 
 // A parameter for a function or other parameterized block.
@@ -904,7 +904,7 @@ struct SpecificConstant {
       {.ir_name = "specific_constant", .is_lowered = false});
 
   TypeId type_id;
-  InstId inst_id;
+  AbsoluteInstId inst_id;
   SpecificId specific_id;
 };