Jelajahi Sumber

Fix crash when if expressions aren't in a function. (#3373)

Another issue found while trying to make `package` work, lurking in
fuzzer inputs. This leaves TODOs because we probably do want to support
this, it's just non-trivial to fix.
Jon Ross-Perkins 2 tahun lalu
induk
melakukan
84bc8cc4bf

+ 10 - 2
toolchain/check/context.cpp

@@ -379,9 +379,17 @@ auto Context::AddConvergenceBlockWithArgAndPush(
 }
 
 // Add the current code block to the enclosing function.
-auto Context::AddCurrentCodeBlockToFunction() -> void {
+auto Context::AddCurrentCodeBlockToFunction(Parse::Node parse_node) -> void {
   CARBON_CHECK(!inst_block_stack().empty()) << "no current code block";
-  CARBON_CHECK(!return_scope_stack().empty()) << "no current function";
+
+  if (return_scope_stack().empty()) {
+    CARBON_CHECK(parse_node.is_valid())
+        << "No current function, but parse_node not provided";
+    TODO(parse_node,
+         "Control flow expressions are currently only supported inside "
+         "functions.");
+    return;
+  }
 
   if (!inst_block_stack().is_current_block_reachable()) {
     // Don't include unreachable blocks in the function.

+ 5 - 1
toolchain/check/context.h

@@ -158,7 +158,11 @@ class Context {
       std::initializer_list<SemIR::InstId> blocks_and_args) -> SemIR::InstId;
 
   // Add the current code block to the enclosing function.
-  auto AddCurrentCodeBlockToFunction() -> void;
+  // TODO: The parse_node is taken for expressions, which can occur in
+  // non-function contexts. This should be refactored to support non-function
+  // contexts, and parse_node removed.
+  auto AddCurrentCodeBlockToFunction(
+      Parse::Node parse_node = Parse::Node::Invalid) -> void;
 
   // Returns whether the current position in the current block is reachable.
   auto is_current_position_reachable() -> bool;

+ 3 - 3
toolchain/check/handle_if_expression.cpp

@@ -22,7 +22,7 @@ auto HandleIfExpressionIf(Context& context, Parse::Node parse_node) -> bool {
   // Start emitting the `then` block.
   context.inst_block_stack().Pop();
   context.inst_block_stack().Push(then_block_id);
-  context.AddCurrentCodeBlockToFunction();
+  context.AddCurrentCodeBlockToFunction(parse_node);
 
   context.node_stack().Push(if_node, else_block_id);
   return true;
@@ -38,7 +38,7 @@ auto HandleIfExpressionThen(Context& context, Parse::Node parse_node) -> bool {
 
   // Start emitting the `else` block.
   context.inst_block_stack().Push(else_block_id);
-  context.AddCurrentCodeBlockToFunction();
+  context.AddCurrentCodeBlockToFunction(parse_node);
 
   context.node_stack().Push(parse_node, then_value_id);
   return true;
@@ -64,7 +64,7 @@ auto HandleIfExpressionElse(Context& context, Parse::Node parse_node) -> bool {
   // Create a resumption block and branches to it.
   auto chosen_value_id = context.AddConvergenceBlockWithArgAndPush(
       if_node, {else_value_id, then_value_id});
-  context.AddCurrentCodeBlockToFunction();
+  context.AddCurrentCodeBlockToFunction(parse_node);
 
   // Push the result value.
   context.node_stack().Push(else_node, chosen_value_id);

+ 53 - 0
toolchain/check/testdata/if_expression/fail_not_in_function.carbon

@@ -0,0 +1,53 @@
+// 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
+
+// TODO: Should work with compile-time evaluation.
+// CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+9]]:14: ERROR: Semantics TODO: `Control flow expressions are currently only supported inside functions.`.
+// CHECK:STDERR: let x: i32 = if true then 1 else 0;
+// CHECK:STDERR:              ^
+// CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+6]]:22: ERROR: Semantics TODO: `Control flow expressions are currently only supported inside functions.`.
+// CHECK:STDERR: let x: i32 = if true then 1 else 0;
+// CHECK:STDERR:                      ^
+// CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+3]]:29: ERROR: Semantics TODO: `Control flow expressions are currently only supported inside functions.`.
+// CHECK:STDERR: let x: i32 = if true then 1 else 0;
+// CHECK:STDERR:                             ^
+let x: i32 = if true then 1 else 0;
+
+class C {
+  // TODO: Should work with compile-time evaluation.
+  // CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+12]]:10: ERROR: Semantics TODO: `Control flow expressions are currently only supported inside functions.`.
+  // CHECK:STDERR:   var n: if true then i32 else f64;
+  // CHECK:STDERR:          ^
+  // CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+9]]:18: ERROR: Semantics TODO: `Control flow expressions are currently only supported inside functions.`.
+  // CHECK:STDERR:   var n: if true then i32 else f64;
+  // CHECK:STDERR:                  ^
+  // CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+6]]:27: ERROR: Semantics TODO: `Control flow expressions are currently only supported inside functions.`.
+  // CHECK:STDERR:   var n: if true then i32 else f64;
+  // CHECK:STDERR:                           ^
+  // CHECK:STDERR: fail_not_in_function.carbon:[[@LINE+3]]:27: ERROR: Cannot evaluate type expression.
+  // CHECK:STDERR:   var n: if true then i32 else f64;
+  // CHECK:STDERR:                           ^
+  var n: if true then i32 else f64;
+}
+
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc34: type = struct_type {.n: <error>}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file "fail_not_in_function.carbon" {
+// CHECK:STDOUT:   %.loc17: i32 = block_arg <unexpected instblockref 4>
+// CHECK:STDOUT:   %x: i32 = bind_name "x", %.loc17
+// CHECK:STDOUT:   class_declaration @C, ()
+// CHECK:STDOUT:   %C: type = class_type @C
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C {
+// CHECK:STDOUT:   %.loc33: bool = bool_literal true
+// CHECK:STDOUT:   if %.loc33 br !if.expr.then else br !if.expr.else
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .n = <unexpected instref 29>
+// CHECK:STDOUT: }