Skip to content

Commit

Permalink
Push a generic region when handling a where expression. (#4340)
Browse files Browse the repository at this point in the history 
This fixes a crash in the case where a `where` expression appears
outside of any generic.
  • Loading branch information
@zygoloid
zygoloid authored Sep 26, 2024
1 parent ee38363 commit 7f22a28
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 52 deletions.
4 changes: 4 additions & 0 deletions toolchain/check/generic.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,10 @@ auto RebuildGenericEvalBlock(Context& context, SemIR::GenericId generic_id,
return context.inst_block_stack().Pop();
}

auto DiscardGenericDecl(Context& context) -> void {
context.generic_region_stack().Pop();
}

auto FinishGenericDecl(Context& context, SemIR::InstId decl_id)
-> SemIR::GenericId {
auto all_bindings =
Expand Down
5 changes: 5 additions & 0 deletions toolchain/check/generic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ auto StartGenericDecl(Context& context) -> void;
// Start processing a declaration or definition that might be a generic entity.
auto StartGenericDefinition(Context& context) -> void;

// Discard the information about the current generic entity. This should be
// called instead of `FinishGenericDecl` if the corresponding `Generic` object
// would not actually be used, or when recovering from an error.
auto DiscardGenericDecl(Context& context) -> void;

// Finish processing a potentially generic declaration and produce a
// corresponding generic object. Returns SemIR::GenericId::Invalid if this
// declaration is not actually generic.
Expand Down
6 changes: 6 additions & 0 deletions toolchain/check/handle_where.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "toolchain/check/context.h"
#include "toolchain/check/convert.h"
#include "toolchain/check/generic.h"
#include "toolchain/check/handle.h"

namespace Carbon::Check {
Expand All @@ -22,6 +23,8 @@ auto HandleParseNode(Context& context, Parse::WhereOperandId /*node_id*/)
// Introduce a name scope so that we can remove the `.Self` entry we are
// adding to name lookup at the end of the `where` expression.
context.scope_stack().Push();
// Create a generic region containing `.Self` and the constraints.
StartGenericDecl(context);
// Introduce `.Self` as a symbolic binding. Its type is the value of the
// expression to the left of `where`, so `MyInterface` in the example above.
// Because there is no equivalent non-symbolic value, we use `Invalid` as
Expand Down Expand Up @@ -74,6 +77,9 @@ auto HandleParseNode(Context& /*context*/, Parse::RequirementAndId /*node_id*/)
}

auto HandleParseNode(Context& context, Parse::WhereExprId /*node_id*/) -> bool {
// Discard the generic region containing `.Self` and the constraints.
// TODO: Decide if we want to build a `Generic` object for this.
DiscardGenericDecl(context);
// Remove `PeriodSelf` from name lookup, undoing the `Push` done for the
// `WhereOperand`.
context.scope_stack().Pop();
Expand Down
57 changes: 21 additions & 36 deletions toolchain/check/testdata/where_expr/constraints.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %T.patt: %.2 = symbolic_binding_pattern T 0
// CHECK:STDOUT: } {
// CHECK:STDOUT: %I.ref: type = name_ref I, file.%I.decl [template = constants.%.2]
// CHECK:STDOUT: %.Self.1: %.2 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self.ref: %.2 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self: %.2 = bind_symbolic_name .Self 0 [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %.Self.ref: %.2 = name_ref .Self, %.Self [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %Member.ref: %.3 = name_ref Member, @I.%.loc7 [template = constants.%.4]
// CHECK:STDOUT: %.loc11: %.8 = struct_literal ()
// CHECK:STDOUT: %T.param: %.2 = param T, runtime_param<invalid>
Expand All @@ -149,8 +149,8 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %U.patt: %.2 = symbolic_binding_pattern U 0
// CHECK:STDOUT: } {
// CHECK:STDOUT: %I.ref: type = name_ref I, file.%I.decl [template = constants.%.2]
// CHECK:STDOUT: %.Self.1: %.2 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self.ref: %.2 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self: %.2 = bind_symbolic_name .Self 0 [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %.Self.ref: %.2 = name_ref .Self, %.Self [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %.loc13: %.5 = tuple_literal ()
// CHECK:STDOUT: %U.param: %.2 = param U, runtime_param<invalid>
// CHECK:STDOUT: %U.loc13: %.2 = bind_symbolic_name U 0, %U.param [symbolic = %U.1 (constants.%U)]
Expand All @@ -159,8 +159,8 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %V.patt: %.1 = symbolic_binding_pattern V 0
// CHECK:STDOUT: } {
// CHECK:STDOUT: %J.ref: type = name_ref J, file.%J.decl [template = constants.%.1]
// CHECK:STDOUT: %.Self.1: %.1 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.2)]
// CHECK:STDOUT: %.Self.ref: %.1 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self.2)]
// CHECK:STDOUT: %.Self: %.1 = bind_symbolic_name .Self 0 [symbolic = constants.%.Self.2]
// CHECK:STDOUT: %.Self.ref: %.1 = name_ref .Self, %.Self [symbolic = constants.%.Self.2]
// CHECK:STDOUT: %I.ref: type = name_ref I, file.%I.decl [template = constants.%.2]
// CHECK:STDOUT: %V.param: %.1 = param V, runtime_param<invalid>
// CHECK:STDOUT: %V.loc15: %.1 = bind_symbolic_name V 0, %V.param [symbolic = %V.1 (constants.%V)]
Expand All @@ -169,13 +169,13 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %W.patt: %.2 = symbolic_binding_pattern W 0
// CHECK:STDOUT: } {
// CHECK:STDOUT: %I.ref.loc17_12: type = name_ref I, file.%I.decl [template = constants.%.2]
// CHECK:STDOUT: %.Self.1: %.2 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self.ref.loc17_20: %.2 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self: %.2 = bind_symbolic_name .Self 0 [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %.Self.ref.loc17_20: %.2 = name_ref .Self, %.Self [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %Second.ref.loc17_20: %.6 = name_ref Second, @I.%.loc8 [template = constants.%.7]
// CHECK:STDOUT: %I.ref.loc17_34: type = name_ref I, file.%I.decl [template = constants.%.2]
// CHECK:STDOUT: %.Self.ref.loc17_40: %.2 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self.ref.loc17_40: %.2 = name_ref .Self, %.Self [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %Member.ref: %.3 = name_ref Member, @I.%.loc7 [template = constants.%.4]
// CHECK:STDOUT: %.Self.ref.loc17_50: %.2 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %.Self.ref.loc17_50: %.2 = name_ref .Self, %.Self [symbolic = constants.%.Self.1]
// CHECK:STDOUT: %Second.ref.loc17_50: %.6 = name_ref Second, @I.%.loc8 [template = constants.%.7]
// CHECK:STDOUT: %W.param: %.2 = param W, runtime_param<invalid>
// CHECK:STDOUT: %W.loc17: %.2 = bind_symbolic_name W 0, %W.param [symbolic = %W.1 (constants.%W)]
Expand Down Expand Up @@ -206,50 +206,42 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @Equal(%T.loc11: %.2) {
// CHECK:STDOUT: %.Self.2: %.2 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %T.1: %.2 = bind_symbolic_name T 0 [symbolic = %T.1 (constants.%T)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn(%T.loc11: %.2);
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @EqualEqual(%U.loc13: %.2) {
// CHECK:STDOUT: %.Self.2: %.2 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %U.1: %.2 = bind_symbolic_name U 0 [symbolic = %U.1 (constants.%U)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn(%U.loc13: %.2);
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @Impls(%V.loc15: %.1) {
// CHECK:STDOUT: %.Self.2: %.1 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.2)]
// CHECK:STDOUT: %V.1: %.1 = bind_symbolic_name V 0 [symbolic = %V.1 (constants.%V)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn(%V.loc15: %.1);
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @And(%W.loc17: %.2) {
// CHECK:STDOUT: %.Self.2: %.2 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.1)]
// CHECK:STDOUT: %W.1: %.2 = bind_symbolic_name W 0 [symbolic = %W.1 (constants.%W)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn(%W.loc17: %.2);
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @Equal(constants.%T) {
// CHECK:STDOUT: %.Self.2 => constants.%T
// CHECK:STDOUT: %T.1 => constants.%T
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @EqualEqual(constants.%U) {
// CHECK:STDOUT: %.Self.2 => constants.%U
// CHECK:STDOUT: %U.1 => constants.%U
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @Impls(constants.%V) {
// CHECK:STDOUT: %.Self.2 => constants.%V
// CHECK:STDOUT: %V.1 => constants.%V
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @And(constants.%W) {
// CHECK:STDOUT: %.Self.2 => constants.%W
// CHECK:STDOUT: %W.1 => constants.%W
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand All @@ -272,9 +264,9 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %import_ref.1 = import_ref Main//state_constraints, inst+3, unloaded
// CHECK:STDOUT: %import_ref.2: type = import_ref Main//state_constraints, inst+7, loaded [template = constants.%.1]
// CHECK:STDOUT: %import_ref.3 = import_ref Main//state_constraints, inst+32, unloaded
// CHECK:STDOUT: %import_ref.4 = import_ref Main//state_constraints, inst+45, unloaded
// CHECK:STDOUT: %import_ref.5 = import_ref Main//state_constraints, inst+59, unloaded
// CHECK:STDOUT: %import_ref.6 = import_ref Main//state_constraints, inst+77, unloaded
// CHECK:STDOUT: %import_ref.4 = import_ref Main//state_constraints, inst+44, unloaded
// CHECK:STDOUT: %import_ref.5 = import_ref Main//state_constraints, inst+57, unloaded
// CHECK:STDOUT: %import_ref.6 = import_ref Main//state_constraints, inst+74, unloaded
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/operators
Expand Down Expand Up @@ -310,8 +302,8 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %X.patt: %.1 = symbolic_binding_pattern X 0
// CHECK:STDOUT: } {
// CHECK:STDOUT: %I.ref: type = name_ref I, imports.%import_ref.2 [template = constants.%.1]
// CHECK:STDOUT: %.Self.1: %.1 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self)]
// CHECK:STDOUT: %.Self.ref: %.1 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self)]
// CHECK:STDOUT: %.Self: %.1 = bind_symbolic_name .Self 0 [symbolic = constants.%.Self]
// CHECK:STDOUT: %.Self.ref: %.1 = name_ref .Self, %.Self [symbolic = constants.%.Self]
// CHECK:STDOUT: %Member.ref: %.3 = name_ref Member, imports.%import_ref.8 [template = constants.%.4]
// CHECK:STDOUT: %.loc8: i32 = int_literal 2 [template = constants.%.5]
// CHECK:STDOUT: %X.param: %.1 = param X, runtime_param<invalid>
Expand All @@ -328,14 +320,12 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @TypeMismatch(%X.loc8: %.1) {
// CHECK:STDOUT: %.Self.2: %.1 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self)]
// CHECK:STDOUT: %X.1: %.1 = bind_symbolic_name X 0 [symbolic = %X.1 (constants.%X)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn(%X.loc8: %.1);
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @TypeMismatch(constants.%X) {
// CHECK:STDOUT: %.Self.2 => constants.%X
// CHECK:STDOUT: %X.1 => constants.%X
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand All @@ -354,7 +344,6 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %Impls.type: type = fn_type @Impls [template]
// CHECK:STDOUT: %Impls: %Impls.type = struct_value () [template]
// CHECK:STDOUT: %V: %.3 = bind_symbolic_name V 0 [symbolic]
// CHECK:STDOUT: %.Self.1: %.3 = bind_symbolic_name .Self 0 [symbolic]
// CHECK:STDOUT: %ImplicitAs.type: type = generic_interface_type @ImplicitAs [template]
// CHECK:STDOUT: %ImplicitAs: %ImplicitAs.type = struct_value () [template]
// CHECK:STDOUT: %Dest: type = bind_symbolic_name Dest 0 [symbolic]
Expand All @@ -371,7 +360,7 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %.10: type = assoc_entity_type %.9, %Convert.type.2 [template]
// CHECK:STDOUT: %.11: %.10 = assoc_entity element0, imports.%import_ref.12 [template]
// CHECK:STDOUT: %.12: %.7 = assoc_entity element0, imports.%import_ref.13 [symbolic]
// CHECK:STDOUT: %.Self.2: %.3 = bind_symbolic_name .Self 0 [symbolic]
// CHECK:STDOUT: %.Self: %.3 = bind_symbolic_name .Self 0 [symbolic]
// CHECK:STDOUT: %Y: %.3 = bind_symbolic_name Y 0 [symbolic]
// CHECK:STDOUT: %EmptyStruct.type: type = fn_type @EmptyStruct [template]
// CHECK:STDOUT: %EmptyStruct: %EmptyStruct.type = struct_value () [template]
Expand All @@ -383,9 +372,9 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %import_ref.1: type = import_ref Main//state_constraints, inst+3, loaded [template = constants.%.3]
// CHECK:STDOUT: %import_ref.2 = import_ref Main//state_constraints, inst+7, unloaded
// CHECK:STDOUT: %import_ref.3 = import_ref Main//state_constraints, inst+32, unloaded
// CHECK:STDOUT: %import_ref.4 = import_ref Main//state_constraints, inst+45, unloaded
// CHECK:STDOUT: %import_ref.5: %Impls.type = import_ref Main//state_constraints, inst+59, loaded [template = constants.%Impls]
// CHECK:STDOUT: %import_ref.6 = import_ref Main//state_constraints, inst+77, unloaded
// CHECK:STDOUT: %import_ref.4 = import_ref Main//state_constraints, inst+44, unloaded
// CHECK:STDOUT: %import_ref.5: %Impls.type = import_ref Main//state_constraints, inst+57, loaded [template = constants.%Impls]
// CHECK:STDOUT: %import_ref.6 = import_ref Main//state_constraints, inst+74, unloaded
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: .ImplicitAs = %import_ref.8
// CHECK:STDOUT: import Core//prelude
Expand Down Expand Up @@ -432,8 +421,8 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: %Y.patt: %.3 = symbolic_binding_pattern Y 0
// CHECK:STDOUT: } {
// CHECK:STDOUT: %J.ref: type = name_ref J, imports.%import_ref.1 [template = constants.%.3]
// CHECK:STDOUT: %.Self.1: %.3 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.2)]
// CHECK:STDOUT: %.Self.ref: %.3 = name_ref .Self, %.Self.1 [symbolic = %.Self.2 (constants.%.Self.2)]
// CHECK:STDOUT: %.Self: %.3 = bind_symbolic_name .Self 0 [symbolic = constants.%.Self]
// CHECK:STDOUT: %.Self.ref: %.3 = name_ref .Self, %.Self [symbolic = constants.%.Self]
// CHECK:STDOUT: %.loc26: %.1 = struct_literal ()
// CHECK:STDOUT: %Y.param: %.3 = param Y, runtime_param<invalid>
// CHECK:STDOUT: %Y.loc26: %.3 = bind_symbolic_name Y 0, %Y.param [symbolic = %Y.1 (constants.%Y)]
Expand Down Expand Up @@ -492,7 +481,6 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @Impls(constants.%V: %.3) {
// CHECK:STDOUT: %.Self: %.3 = bind_symbolic_name .Self 0 [symbolic = %.Self (constants.%.Self.1)]
// CHECK:STDOUT: %V.2: %.3 = bind_symbolic_name V 0 [symbolic = %V.2 (constants.%V)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn(%V.1: %.3);
Expand All @@ -507,7 +495,6 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @EmptyStruct(%Y.loc26: %.3) {
// CHECK:STDOUT: %.Self.2: %.3 = bind_symbolic_name .Self 0 [symbolic = %.Self.2 (constants.%.Self.2)]
// CHECK:STDOUT: %Y.1: %.3 = bind_symbolic_name Y 0 [symbolic = %Y.1 (constants.%Y)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn(%Y.loc26: %.3);
Expand All @@ -525,7 +512,6 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @Impls(constants.%V) {
// CHECK:STDOUT: %.Self => constants.%V
// CHECK:STDOUT: %V.2 => constants.%V
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand Down Expand Up @@ -560,7 +546,6 @@ fn NotEmptyStruct() {
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @EmptyStruct(constants.%Y) {
// CHECK:STDOUT: %.Self.2 => constants.%Y
// CHECK:STDOUT: %Y.1 => constants.%Y
// CHECK:STDOUT: }
// CHECK:STDOUT:
Loading

0 comments on commit 7f22a28

Please sign in to comment.