Skip to content

Commit

Permalink
Narrower check for disallowed compile time bindings.
Browse files Browse the repository at this point in the history 
We now allow `let` bindings in all scopes except `class` and `impl`, which are
known to be problematic. This also switches to GetCurrentScopeAs instead of
PeekInstId, to better encapsulate handling of invalid scope inst IDs.
This fixes a fuzzer-found crash when a compile-time binding occurs inside a
block (which is represented by an invalid inst ID).

This partially reverts carbon-language#4338, but is consistent with @jonmeow's comments on
that PR.
  • Loading branch information
@geoffromer
geoffromer committed Oct 7, 2024
1 parent 55d8edc commit 7f462bb
Show file tree
Hide file tree
Showing 5 changed files with 414 additions and 86 deletions.
8 changes: 3 additions & 5 deletions toolchain/check/handle_binding_pattern.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,9 @@ auto HandleParseNode(Context& context,
bool is_generic = true;
if (context.decl_introducer_state_stack().innermost().kind ==
Lex::TokenKind::Let) {
auto scope_inst = context.insts().Get(context.scope_stack().PeekInstId());
if (!scope_inst.Is<SemIR::InterfaceDecl>() &&
!scope_inst.Is<SemIR::FunctionDecl>()) {
context.TODO(node_id,
"`let` compile time binding outside function or interface");
if (context.GetCurrentScopeAs<SemIR::ClassDecl>() ||
context.GetCurrentScopeAs<SemIR::ImplDecl>()) {
context.TODO(node_id, "`let` compile time binding in class or impl");
is_generic = false;
}
}
Expand Down
198 changes: 177 additions & 21 deletions toolchain/check/testdata/let/compile_time_bindings.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ library "[[@TEST_NAME]]";

class C {
fn F() -> () { return x; }
// CHECK:STDERR: fail_let_after.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding outside function or interface`
// CHECK:STDERR: fail_let_after.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding in class or impl`
// CHECK:STDERR: let x:! () = ();
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR:
Expand All @@ -30,7 +30,7 @@ class C {
library "[[@TEST_NAME]]";

class C {
// CHECK:STDERR: fail_let_before.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding outside function or interface`
// CHECK:STDERR: fail_let_before.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding in class or impl`
// CHECK:STDERR: let x:! () = ();
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR:
Expand All @@ -43,7 +43,7 @@ class C {
library "[[@TEST_NAME]]";

class C {
// CHECK:STDERR: fail_multiple_lets.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding outside function or interface`
// CHECK:STDERR: fail_multiple_lets.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding in class or impl`
// CHECK:STDERR: let a:! () = ();
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR:
Expand All @@ -56,7 +56,7 @@ class C {
var c1: ((), ()) = c;
var d1: ((), (), ()) = d;
}
// CHECK:STDERR: fail_multiple_lets.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding outside function or interface`
// CHECK:STDERR: fail_multiple_lets.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding in class or impl`
// CHECK:STDERR: let d:! ((), (), ()) = ((), (), ());
// CHECK:STDERR: ^~~~~~~~~~~~~~~~
// CHECK:STDERR:
Expand All @@ -69,7 +69,7 @@ library "[[@TEST_NAME]]";

class C {
fn F() -> () { return x; }
// CHECK:STDERR: fail_invalid_let_after.carbon:[[@LINE+8]]:7: error: semantics TODO: ``let` compile time binding outside function or interface`
// CHECK:STDERR: fail_invalid_let_after.carbon:[[@LINE+8]]:7: error: semantics TODO: ``let` compile time binding in class or impl`
// CHECK:STDERR: let x:! ();
// CHECK:STDERR: ^~~~~~
// CHECK:STDERR:
Expand All @@ -89,6 +89,18 @@ fn F() -> i32 {
return Zero;
}

// --- use_in_block.carbon

library "[[@TEST_NAME]]";

fn F() -> i32 {
if (true) {
let Zero:! i32 = 0;
return Zero;
}
return 1;
}

// --- fail_return_in_interface.carbon

library "[[@TEST_NAME]]";
Expand All @@ -110,7 +122,7 @@ interface I {
library "[[@TEST_NAME]]";

class I {
// CHECK:STDERR: fail_return_in_class.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding outside function or interface`
// CHECK:STDERR: fail_return_in_class.carbon:[[@LINE+4]]:7: error: semantics TODO: ``let` compile time binding in class or impl`
// CHECK:STDERR: let T:! type = i32;
// CHECK:STDERR: ^~~~~~~~
// CHECK:STDERR:
Expand All @@ -122,20 +134,26 @@ class I {
fn F() -> T;
}

// --- fail_return_in_package_scope.carbon
// --- return_in_package_scope.carbon

library "[[@TEST_NAME]]";

// CHECK:STDERR: fail_return_in_package_scope.carbon:[[@LINE+4]]:5: error: semantics TODO: ``let` compile time binding outside function or interface`
// CHECK:STDERR: let T:! type = i32;
// CHECK:STDERR: ^~~~~~~~
// CHECK:STDERR:
let T:! type = i32;
// CHECK:STDERR: fail_return_in_package_scope.carbon:[[@LINE+3]]:11: error: cannot evaluate type expression
// CHECK:STDERR: fn F() -> T;
// CHECK:STDERR: ^
fn F() -> T;

// --- fail_use_in_impl.carbon

library "[[@TEST_NAME]]";

interface Empty {}

impl i32 as Empty {
// CHECK:STDERR: fail_use_in_impl.carbon:[[@LINE+3]]:7: error: semantics TODO: ``let` compile time binding in class or impl`
// CHECK:STDERR: let Zero:! i32 = 0;
// CHECK:STDERR: ^~~~~~~~~~
let Zero:! i32 = 0;
}

// CHECK:STDOUT: --- fail_let_after.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
Expand Down Expand Up @@ -560,6 +578,70 @@ fn F() -> T;
// CHECK:STDOUT: return %Zero.ref
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- use_in_block.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Int32.type: type = fn_type @Int32 [template]
// CHECK:STDOUT: %.1: type = tuple_type () [template]
// CHECK:STDOUT: %Int32: %Int32.type = struct_value () [template]
// CHECK:STDOUT: %F.type: type = fn_type @F [template]
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: %.2: bool = bool_literal true [template]
// CHECK:STDOUT: %.3: i32 = int_literal 0 [template]
// CHECK:STDOUT: %Zero: i32 = bind_symbolic_name Zero, 0 [symbolic]
// CHECK:STDOUT: %.4: i32 = int_literal 1 [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: .Int32 = %import_ref
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/operators
// CHECK:STDOUT: import Core//prelude/types
// CHECK:STDOUT: import Core//prelude/operators/arithmetic
// CHECK:STDOUT: import Core//prelude/operators/as
// CHECK:STDOUT: import Core//prelude/operators/bitwise
// CHECK:STDOUT: import Core//prelude/operators/comparison
// CHECK:STDOUT: import Core//prelude/types/bool
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref: %Int32.type = import_ref Core//prelude/types, inst+4, loaded [template = constants.%Int32]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .F = %F.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {
// CHECK:STDOUT: %int.make_type_32.loc4: init type = call constants.%Int32() [template = i32]
// CHECK:STDOUT: %.loc4_11.1: type = value_of_initializer %int.make_type_32.loc4 [template = i32]
// CHECK:STDOUT: %.loc4_11.2: type = converted %int.make_type_32.loc4, %.loc4_11.1 [template = i32]
// CHECK:STDOUT: %return: ref i32 = var <return slot>
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Int32() -> type = "int.make_type_32";
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F() -> i32 {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %.loc5: bool = bool_literal true [template = constants.%.2]
// CHECK:STDOUT: if %.loc5 br !if.then else br !if.else
// CHECK:STDOUT:
// CHECK:STDOUT: !if.then:
// CHECK:STDOUT: %int.make_type_32.loc6: init type = call constants.%Int32() [template = i32]
// CHECK:STDOUT: %.loc6_16.1: type = value_of_initializer %int.make_type_32.loc6 [template = i32]
// CHECK:STDOUT: %.loc6_16.2: type = converted %int.make_type_32.loc6, %.loc6_16.1 [template = i32]
// CHECK:STDOUT: %.loc6_22: i32 = int_literal 0 [template = constants.%.3]
// CHECK:STDOUT: %Zero: i32 = bind_symbolic_name Zero, 0, %.loc6_22 [symbolic = constants.%Zero]
// CHECK:STDOUT: %Zero.ref: i32 = name_ref Zero, %Zero [symbolic = constants.%Zero]
// CHECK:STDOUT: return %Zero.ref
// CHECK:STDOUT:
// CHECK:STDOUT: !if.else:
// CHECK:STDOUT: %.loc9: i32 = int_literal 1 [template = constants.%.4]
// CHECK:STDOUT: return %.loc9
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_return_in_interface.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
Expand Down Expand Up @@ -771,12 +853,13 @@ fn F() -> T;
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F() -> <error>;
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_return_in_package_scope.carbon
// CHECK:STDOUT: --- return_in_package_scope.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Int32.type: type = fn_type @Int32 [template]
// CHECK:STDOUT: %.1: type = tuple_type () [template]
// CHECK:STDOUT: %Int32: %Int32.type = struct_value () [template]
// CHECK:STDOUT: %T: type = bind_symbolic_name T, 0 [symbolic]
// CHECK:STDOUT: %F.type: type = fn_type @F [template]
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: }
Expand Down Expand Up @@ -804,21 +887,94 @@ fn F() -> T;
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {
// CHECK:STDOUT: %T.ref: type = name_ref T, @__global_init.%T
// CHECK:STDOUT: %return: ref <error> = var <return slot>
// CHECK:STDOUT: %T.ref: type = name_ref T, @__global_init.%T [symbolic = %T (constants.%T)]
// CHECK:STDOUT: %return: ref @F.%T (%T) = var <return slot>
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Int32() -> type = "int.make_type_32";
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F() -> <error>;
// CHECK:STDOUT: generic fn @F(@__global_init.%T: type) {
// CHECK:STDOUT: %T: type = bind_symbolic_name T, 0 [symbolic = %T (constants.%T)]
// CHECK:STDOUT:
// CHECK:STDOUT: fn() -> @F.%T (%T);
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @__global_init() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %int.make_type_32: init type = call constants.%Int32() [template = i32]
// CHECK:STDOUT: %.loc8_19.1: type = value_of_initializer %int.make_type_32 [template = i32]
// CHECK:STDOUT: %.loc8_19.2: type = converted %int.make_type_32, %.loc8_19.1 [template = i32]
// CHECK:STDOUT: %T: type = bind_name T, %.loc8_19.2
// CHECK:STDOUT: %.loc4_19.1: type = value_of_initializer %int.make_type_32 [template = i32]
// CHECK:STDOUT: %.loc4_19.2: type = converted %int.make_type_32, %.loc4_19.1 [template = i32]
// CHECK:STDOUT: %T: type = bind_symbolic_name T, 0, %.loc4_19.2 [symbolic = constants.%T]
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @F(constants.%T) {
// CHECK:STDOUT: %T => constants.%T
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_use_in_impl.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Empty.type: type = interface_type @Empty [template]
// CHECK:STDOUT: %Self: %Empty.type = bind_symbolic_name Self, 0 [symbolic]
// CHECK:STDOUT: %Int32.type: type = fn_type @Int32 [template]
// CHECK:STDOUT: %.1: type = tuple_type () [template]
// CHECK:STDOUT: %Int32: %Int32.type = struct_value () [template]
// CHECK:STDOUT: %.2: i32 = int_literal 0 [template]
// CHECK:STDOUT: %.3: <witness> = interface_witness () [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: .Int32 = %import_ref
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/operators
// CHECK:STDOUT: import Core//prelude/types
// CHECK:STDOUT: import Core//prelude/operators/arithmetic
// CHECK:STDOUT: import Core//prelude/operators/as
// CHECK:STDOUT: import Core//prelude/operators/bitwise
// CHECK:STDOUT: import Core//prelude/operators/comparison
// CHECK:STDOUT: import Core//prelude/types/bool
// CHECK:STDOUT: }
// CHECK:STDOUT: %import_ref: %Int32.type = import_ref Core//prelude/types, inst+4, loaded [template = constants.%Int32]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Empty = %Empty.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Empty.decl: type = interface_decl @Empty [template = constants.%Empty.type] {} {}
// CHECK:STDOUT: impl_decl @impl [template] {} {
// CHECK:STDOUT: %int.make_type_32.loc6: init type = call constants.%Int32() [template = i32]
// CHECK:STDOUT: %.loc6_6.1: type = value_of_initializer %int.make_type_32.loc6 [template = i32]
// CHECK:STDOUT: %.loc6_6.2: type = converted %int.make_type_32.loc6, %.loc6_6.1 [template = i32]
// CHECK:STDOUT: %Empty.ref: type = name_ref Empty, file.%Empty.decl [template = constants.%Empty.type]
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: interface @Empty {
// CHECK:STDOUT: %Self: %Empty.type = bind_symbolic_name Self, 0 [symbolic = constants.%Self]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = %Self
// CHECK:STDOUT: witness = ()
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: impl @impl: %.loc6_6.2 as %Empty.ref {
// CHECK:STDOUT: %int.make_type_32.loc10: init type = call constants.%Int32() [template = i32]
// CHECK:STDOUT: %.loc10_14.1: type = value_of_initializer %int.make_type_32.loc10 [template = i32]
// CHECK:STDOUT: %.loc10_14.2: type = converted %int.make_type_32.loc10, %.loc10_14.1 [template = i32]
// CHECK:STDOUT: %.loc10_20: i32 = int_literal 0 [template = constants.%.2]
// CHECK:STDOUT: %Zero: i32 = bind_name Zero, %.loc10_20
// CHECK:STDOUT: %.loc6_19: <witness> = interface_witness () [template = constants.%.3]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Zero = %Zero
// CHECK:STDOUT: witness = %.loc6_19
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Int32() -> type = "int.make_type_32";
// CHECK:STDOUT:
Loading

0 comments on commit 7f462bb

Please sign in to comment.