Skip to content

Commit

Permalink
Add a test and check-support for positional params with a return type (
Browse files Browse the repository at this point in the history
…#4899)

Functions with positional parameters omit any implicit or explicit
parameter lists. This causes them to not have a pattern block, which
crashes if there is a return type that needs to add to the pattern
block.

Add a test covering this and handle it by having the ReturnTypeId
handler peek at the node stack and conditionally add the missing pattern
block. To do so it looks to see if the previous node is a
`IdentifierNameNotBeforeParams` which implies it was not expecting a
pattern (since there are no params) and thus the pattern block was not
added to the stack.

Note that lambdas also allow functions to omit an identifier, which will
need a pattern block on the stack for implicit parameters, explicit
parameters or a return type, without seeing any IdentifierName-like
parse nodes. To handle this, we will need to look for additional nodes
in the future and add the missing pattern block to the stack - possibly
for the FunctionInitializer, but the parse support needs to be created
for lambdas first.
  • Loading branch information
danakj authored Feb 6, 2025
1 parent 7eee9a3 commit 7d6cd3d
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 1 deletion.
14 changes: 14 additions & 0 deletions toolchain/check/handle_function.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,20 @@ auto HandleParseNode(Context& context, Parse::ReturnTypeId node_id) -> bool {
// Propagate the type expression.
auto [type_node_id, type_inst_id] = context.node_stack().PopExprWithNodeId();
auto type_id = ExprAsType(context, type_node_id, type_inst_id).type_id;

// If the previous node was `IdentifierNameBeforeParams`, then it would have
// caused these entries to be pushed to the pattern stacks. But it's possible
// to have a fn declaration without any parameters, in which case we find
// `IdentifierNameNotBeforeParams` on the node stack. Then these entries are
// not on the pattern stacks yet. They are only needed in that case if we have
// a return type, which we now know that we do.
if (context.node_stack().PeekNodeKind() ==
Parse::NodeKind::IdentifierNameNotBeforeParams) {
context.pattern_block_stack().Push();
context.full_pattern_stack().PushFullPattern(
FullPatternStack::Kind::ExplicitParamList);
}

auto return_slot_pattern_id =
context.AddPatternInst<SemIR::ReturnSlotPattern>(
node_id, {.type_id = type_id, .type_inst_id = type_inst_id});
Expand Down
3 changes: 2 additions & 1 deletion toolchain/check/name_component.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ auto PopNameComponent(Context& context, SemIR::InstId return_slot_pattern_id)
auto call_params_id = SemIR::InstBlockId::None;
auto pattern_block_id = SemIR::InstBlockId::None;
if (param_patterns_id->has_value() ||
implicit_param_patterns_id->has_value()) {
implicit_param_patterns_id->has_value() ||
return_slot_pattern_id.has_value()) {
call_params_id =
CalleePatternMatch(context, *implicit_param_patterns_id,
*param_patterns_id, return_slot_pattern_id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ library "[[@TEST_NAME]]";
fn A {
}

// --- fail_todo_return_type.carbon

library "[[@TEST_NAME]]";
// CHECK:STDERR: fail_todo_return_type.carbon:[[@LINE+4]]:1: error: semantics TODO: `function with positional parameters` [SemanticsTodo]
// CHECK:STDERR: fn A -> ();
// CHECK:STDERR: ^~~~~~~~~~~
// CHECK:STDERR:
fn A -> ();

// --- fail_todo_arrow_body.carbon

// TODO: We don't have parsing support for this yet.
Expand Down Expand Up @@ -94,6 +103,31 @@ fn A {
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_todo_return_type.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %empty_tuple.type: type = tuple_type () [template]
// CHECK:STDOUT: %A.type: type = fn_type @A [template]
// CHECK:STDOUT: %A: %A.type = struct_value () [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .A = %A.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %A.decl: %A.type = fn_decl @A [template = constants.%A] {
// CHECK:STDOUT: %return.patt: %empty_tuple.type = return_slot_pattern
// CHECK:STDOUT: %return.param_patt: %empty_tuple.type = out_param_pattern %return.patt, runtime_param0
// CHECK:STDOUT: } {
// CHECK:STDOUT: %.loc7_10.1: %empty_tuple.type = tuple_literal ()
// CHECK:STDOUT: %.loc7_10.2: type = converted %.loc7_10.1, constants.%empty_tuple.type [template = constants.%empty_tuple.type]
// CHECK:STDOUT: %return.param: ref %empty_tuple.type = out_param runtime_param0
// CHECK:STDOUT: %return: ref %empty_tuple.type = return_slot %return.param
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @A() -> %empty_tuple.type;
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_todo_arrow_body.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: file {}
Expand Down

0 comments on commit 7d6cd3d

Please # to comment.