From 059ee7b9ed0a57da4ca24abe87b7a05aeee8049c Mon Sep 17 00:00:00 2001 From: David Blaikie Date: Wed, 7 Oct 2026 23:05:55 +0000 Subject: [PATCH] Parse `-> ref T` as a primitive return form (#7911) Previously `-> ref T` was parsed as a type expression whose outermost node is a `ref` prefix operator (the same one used for `F(ref x)` arguments), and `ReturnExprAsForm` recognized the resulting `RefTagExpr` and turned it into a `RefForm`. Per p5434, a return form that begins with `val`, `ref`, or `var` is not a type expression. Parse a leading `ref` after `->` as a `RefPrimitiveForm` node wrapping the type expression, reusing the node produced by `form(ref T)`, and check it as a form. This prepares for supporting `-> val T` and `-> var T` in the same way. The only SemIR change is that the `ref_tag` instruction is no longer emitted, and the return slot pattern refers to a `type_component_of` the form, matching `->? form(ref T)`. --------- Co-authored-by: Nicholas Bishop --- toolchain/check/convert.cpp | 52 +++++---------- toolchain/check/convert.h | 12 ++-- toolchain/check/handle_function.cpp | 6 +- .../testdata/function/declaration/ref.carbon | 24 ++++++- toolchain/parse/handle_function.cpp | 12 +++- toolchain/parse/state.def | 22 ++++++- .../testdata/function/declaration.carbon | 65 +++++++++++++++++++ 7 files changed, 143 insertions(+), 50 deletions(-) diff --git a/toolchain/check/convert.cpp b/toolchain/check/convert.cpp index 361c3ef60e22..2a36945bd770 100644 --- a/toolchain/check/convert.cpp +++ b/toolchain/check/convert.cpp @@ -2785,44 +2785,22 @@ auto FormExprAsForm(Context& context, SemIR::LocId loc_id, auto ReturnExprAsForm(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id) -> Context::FormExpr { - auto form_inst_id = SemIR::InstId::None; - auto type_inst_id = SemIR::InstId::None; - if (auto ref_tag = context.insts().TryGetAs(value_id)) { - type_inst_id = ConvertToValueOfType(context, loc_id, ref_tag->expr_id, - SemIR::TypeType::TypeId); - if (type_inst_id == SemIR::ErrorInst::InstId) { - return Context::FormExpr::Error; - } - if (!context.constant_values().Get(type_inst_id).is_constant()) { - DiagnoseTypeExprEvaluationFailure(context, - SemIR::LocId(ref_tag->expr_id)); - return Context::FormExpr::Error; - } - form_inst_id = AddInst( - context, - SemIR::LocIdAndInst::RuntimeVerified( - context.sem_ir(), loc_id, - SemIR::RefForm{.type_id = SemIR::FormType::TypeId, - .type_component_inst_id = - context.types().GetAsTypeInstId(type_inst_id)})); - } else { - type_inst_id = ConvertToValueOfType(context, loc_id, value_id, - SemIR::TypeType::TypeId); - if (type_inst_id == SemIR::ErrorInst::InstId) { - return Context::FormExpr::Error; - } - if (!context.constant_values().Get(type_inst_id).is_constant()) { - DiagnoseTypeExprEvaluationFailure(context, loc_id); - return Context::FormExpr::Error; - } - form_inst_id = AddInst( - context, SemIR::LocIdAndInst::RuntimeVerified( - context.sem_ir(), loc_id, - SemIR::InitForm{ - .type_id = SemIR::FormType::TypeId, - .type_component_inst_id = - context.types().GetAsTypeInstId(type_inst_id)})); + auto type_inst_id = + ConvertToValueOfType(context, loc_id, value_id, SemIR::TypeType::TypeId); + if (type_inst_id == SemIR::ErrorInst::InstId) { + return Context::FormExpr::Error; } + if (!context.constant_values().Get(type_inst_id).is_constant()) { + DiagnoseTypeExprEvaluationFailure(context, loc_id); + return Context::FormExpr::Error; + } + auto form_inst_id = AddInst( + context, + SemIR::LocIdAndInst::RuntimeVerified( + context.sem_ir(), loc_id, + SemIR::InitForm{.type_id = SemIR::FormType::TypeId, + .type_component_inst_id = + context.types().GetAsTypeInstId(type_inst_id)})); auto type_const_id = context.constant_values().Get(type_inst_id); CARBON_CHECK(type_const_id.is_constant()); diff --git a/toolchain/check/convert.h b/toolchain/check/convert.h index 8dbb84079821..02519a79cc2d 100644 --- a/toolchain/check/convert.h +++ b/toolchain/check/convert.h @@ -247,17 +247,17 @@ auto ExprAsType(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id, // Converts an expression in a form position for use as a form. // -// Note that the right-hand side of a `->` return type declaration is not -// a form position for this purpose, because it uses a special syntax to specify -// forms. `ReturnExprAsForm` should be used instead in that case. +// Note that the right-hand side of a `->` return type declaration is normally +// a type expression, not a form, and should be handled by `ReturnExprAsForm`. +// It's only a form if it begins with a primitive form keyword such as `ref`. // // `diagnose` has the same effect as in `ExprAsType`. auto FormExprAsForm(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id) -> Context::FormExpr; -// Evaluates an expression in the return-type position (following `->`, not -// `->?`) for use as a form, following the special-case language rules for -// evaluating an expression in that position. +// Evaluates a type expression in the return-type position (following `->`, not +// `->?`, and not beginning with a primitive form keyword such as `ref`) for use +// as a form. The result is an initializing form of that type. auto ReturnExprAsForm(Context& context, SemIR::LocId loc_id, SemIR::InstId value_id) -> Context::FormExpr; diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 5f17a17c0e9f..fb86f2829cd1 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -55,7 +55,11 @@ static auto HandleReturnDecl(Context& context, Parse::AnyReturnDeclId node_id) -> bool { auto [expr_node_id, expr_inst_id] = context.node_stack().PopExprWithNodeId(); Context::FormExpr form_expr = [&] { - if (context.parse_tree().node_kind(node_id) == Parse::ReturnTypeId::Kind) { + // `-> T` provides a type expression, whereas `-> ref T` and `->? F` + // provide a form. + if (context.parse_tree().node_kind(node_id) == Parse::ReturnTypeId::Kind && + context.parse_tree().node_kind(expr_node_id) != + Parse::RefPrimitiveFormId::Kind) { return ReturnExprAsForm(context, expr_node_id, expr_inst_id); } else { return FormExprAsForm(context, expr_node_id, expr_inst_id); diff --git a/toolchain/check/testdata/function/declaration/ref.carbon b/toolchain/check/testdata/function/declaration/ref.carbon index 3507f568c3ac..4b7fb7cc1edf 100644 --- a/toolchain/check/testdata/function/declaration/ref.carbon +++ b/toolchain/check/testdata/function/declaration/ref.carbon @@ -21,6 +21,24 @@ fn G() { let unused ref x: i32 = F(); } +// --- ref_return_parenthesized_type.carbon + +library "[[@TEST_NAME]]"; + +fn F() -> ref (i32); + +// --- fail_todo_ref_return_parenthesized.carbon + +library "[[@TEST_NAME]]"; + +// TODO: Per p5434, a return form starting with `(` is a compound form, which +// isn't supported yet. +// CHECK:STDERR: fail_todo_ref_return_parenthesized.carbon:[[@LINE+4]]:12: error: `ref` tag is not an argument to a `ref` parameter [RefTagNoRefParam] +// CHECK:STDERR: fn F() -> (ref i32); +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: +fn F() -> (ref i32); + // CHECK:STDOUT: --- ref_return.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { @@ -58,11 +76,11 @@ fn G() { // CHECK:STDOUT: %Core.import = import Core // CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [concrete = constants.%F] { // CHECK:STDOUT: %.loc4_11.1: %pattern_type.6b6 = ref_return_pattern [concrete = constants.%.ae0] -// CHECK:STDOUT: %return.patt: %pattern_type.6b6 = return_slot_pattern %.loc4_11.1, %i32 [concrete = constants.%return.patt] +// CHECK:STDOUT: %return.patt: %pattern_type.6b6 = return_slot_pattern %.loc4_11.1, %.loc4_11.3 [concrete = constants.%return.patt] // CHECK:STDOUT: } { // CHECK:STDOUT: %i32: type = type_literal constants.%i32 [concrete = constants.%i32] -// CHECK:STDOUT: %.loc4_11.2: type = ref_tag %i32 -// CHECK:STDOUT: %.loc4_11.3: Core.Form = ref_form %i32 [concrete = constants.%.512] +// CHECK:STDOUT: %.loc4_11.2: Core.Form = ref_form %i32 [concrete = constants.%.512] +// CHECK:STDOUT: %.loc4_11.3: type = type_component_of %.loc4_11.2 [concrete = constants.%i32] // CHECK:STDOUT: %.loc4_11.4: ref %i32 = ref_return // CHECK:STDOUT: %return: ref %i32 = return_slot %.loc4_11.4 // CHECK:STDOUT: } diff --git a/toolchain/parse/handle_function.cpp b/toolchain/parse/handle_function.cpp index 004945530423..a3aadaa1017b 100644 --- a/toolchain/parse/handle_function.cpp +++ b/toolchain/parse/handle_function.cpp @@ -24,7 +24,7 @@ auto HandleFunctionAfterParams(Context& context) -> void { if (context.PositionIs(Lex::TokenKind::MinusGreater)) { context.PushState(StateKind::FunctionReturnTypeFinish); context.ConsumeAndDiscard(); - context.PushStateForExpr(PrecedenceGroup::ForType()); + context.PushState(StateKind::ReturnTypeOrFormExpr); } else if (context.PositionIs(Lex::TokenKind::MinusGreaterQuestion)) { context.PushState(StateKind::FunctionReturnFormFinish); context.ConsumeAndDiscard(); @@ -32,6 +32,16 @@ auto HandleFunctionAfterParams(Context& context) -> void { } } +auto HandleReturnTypeOrFormExpr(Context& context) -> void { + context.PopAndDiscardState(); + + // `-> ref T` specifies a primitive return form. + if (context.PositionIs(Lex::TokenKind::Ref)) { + context.PushState(StateKind::PrimitiveFormFinish, context.Consume()); + } + context.PushStateForExpr(PrecedenceGroup::ForType()); +} + auto HandleFunctionReturnTypeFinish(Context& context) -> void { auto state = context.PopState(); diff --git a/toolchain/parse/state.def b/toolchain/parse/state.def index b813f07a10ec..31d49121b33c 100644 --- a/toolchain/parse/state.def +++ b/toolchain/parse/state.def @@ -815,8 +815,9 @@ CARBON_PARSE_STATE(LambdaAfterImplicitParams) // // fn <...> -> <...> // ^ -// 1. FunctionReturnTypeFinish -// 2. LambdaBody +// 1. ReturnTypeOrFormExpr (Reused) +// 2. FunctionReturnTypeFinish +// 3. LambdaBody // // fn <...> ->? <...> // ^ @@ -863,6 +864,10 @@ CARBON_PARSE_STATE(LambdaBodyFinish) // fn F(<...>) -> <...> // ^~ // 1. Expr +// +// fn F(...) -> ... +// ^~ +// 1. ReturnTypeOrFormExpr // 2. FunctionReturnTypeFinish // 3. FunctionSignatureFinish // @@ -877,6 +882,19 @@ CARBON_PARSE_STATE(LambdaBodyFinish) // 1. FunctionSignatureFinish CARBON_PARSE_STATE(FunctionAfterParams) +// Handles the expression following a `->` in a function or lambda, which may +// begin with `ref` to specify a primitive return form. +// +// -> ref expr ... +// ^~~ +// 1. Expr +// 2. PrimitiveFormFinish +// +// -> expr ... +// ^ +// 1. Expr +CARBON_PARSE_STATE(ReturnTypeOrFormExpr) + // Finishes a function return type. // // fn F(<...>) -> expr <...> diff --git a/toolchain/parse/testdata/function/declaration.carbon b/toolchain/parse/testdata/function/declaration.carbon index b5ca491d4c9f..db3f5a2b6db2 100644 --- a/toolchain/parse/testdata/function/declaration.carbon +++ b/toolchain/parse/testdata/function/declaration.carbon @@ -183,6 +183,20 @@ fn FormInVar(var x:? Form); fn SimpleReturnForm() ->? Form; fn ComplexReturnForm() ->? X.Y(Z); +// --- ref_return.carbon + +fn RefReturn() -> ref u32; +fn ComplexRefReturn() -> ref X.Y(Z); +let a: auto = fn -> ref u32 => 0; + +// --- fail_ref_return_missing_type.carbon + +// CHECK:STDERR: fail_ref_return_missing_type.carbon:[[@LINE+4]]:22: error: expected expression [ExpectedExpr] +// CHECK:STDERR: fn RefReturn() -> ref; +// CHECK:STDERR: ^ +// CHECK:STDERR: +fn RefReturn() -> ref; + // CHECK:STDOUT: - filename: basic.carbon // CHECK:STDOUT: ╭─FileStart '' // CHECK:STDOUT: │ ╭─FunctionIntroducer 'fn' @@ -505,3 +519,54 @@ fn ComplexReturnForm() ->? X.Y(Z); // CHECK:STDOUT: ├─FunctionDecl ';' // CHECK:STDOUT: ├─FileEnd '' // CHECK:STDOUT: (root) +// CHECK:STDOUT: - filename: ref_return.carbon +// CHECK:STDOUT: ╭─FileStart '' +// CHECK:STDOUT: │ ╭─FunctionIntroducer 'fn' +// CHECK:STDOUT: │ ├─IdentifierNameMaybeBeforeSignature 'RefReturn' +// CHECK:STDOUT: │ │ ╭─ExplicitParamListStart '(' +// CHECK:STDOUT: │ ├─ExplicitParamList ')' +// CHECK:STDOUT: │ │ ╭─UnsignedIntTypeLiteral 'u32' +// CHECK:STDOUT: │ │ ╭─RefPrimitiveForm 'ref' +// CHECK:STDOUT: │ ├─ReturnType '->' +// CHECK:STDOUT: ├─FunctionDecl ';' +// CHECK:STDOUT: │ ╭─FunctionIntroducer 'fn' +// CHECK:STDOUT: │ ├─IdentifierNameMaybeBeforeSignature 'ComplexRefReturn' +// CHECK:STDOUT: │ │ ╭─ExplicitParamListStart '(' +// CHECK:STDOUT: │ ├─ExplicitParamList ')' +// CHECK:STDOUT: │ │ ╭─IdentifierNameExpr 'X' +// CHECK:STDOUT: │ │ ├─IdentifierNameNotBeforeSignature 'Y' +// CHECK:STDOUT: │ │ ╭─MemberAccessExpr '.' +// CHECK:STDOUT: │ │ ╭─CallExprStart '(' +// CHECK:STDOUT: │ │ ├─IdentifierNameExpr 'Z' +// CHECK:STDOUT: │ │ ╭─CallExpr ')' +// CHECK:STDOUT: │ │ ╭─RefPrimitiveForm 'ref' +// CHECK:STDOUT: │ ├─ReturnType '->' +// CHECK:STDOUT: ├─FunctionDecl ';' +// CHECK:STDOUT: │ ╭─LetIntroducer 'let' +// CHECK:STDOUT: │ │ ╭─IdentifierNameNotBeforeSignature 'a' +// CHECK:STDOUT: │ │ ├─BindingPatternTypeStart ':' +// CHECK:STDOUT: │ │ ├─AutoTypeLiteral 'auto' +// CHECK:STDOUT: │ ├─LetBindingPattern ':' +// CHECK:STDOUT: │ ├─LetInitializer '=' +// CHECK:STDOUT: │ │ ╭─LambdaIntroducer 'fn' +// CHECK:STDOUT: │ │ │ ╭─UnsignedIntTypeLiteral 'u32' +// CHECK:STDOUT: │ │ │ ╭─PrefixOperatorRef 'ref' +// CHECK:STDOUT: │ │ ├─ReturnType '->' +// CHECK:STDOUT: │ │ ╭─LambdaDefinitionStart '=>' +// CHECK:STDOUT: │ │ ├─IntLiteral '0' +// CHECK:STDOUT: │ ├─Lambda 'fn' +// CHECK:STDOUT: ├─LetDecl ';' +// CHECK:STDOUT: ├─FileEnd '' +// CHECK:STDOUT: (root) +// CHECK:STDOUT: - filename: fail_ref_return_missing_type.carbon +// CHECK:STDOUT: ╭─FileStart '' +// CHECK:STDOUT: │ ╭─FunctionIntroducer 'fn' +// CHECK:STDOUT: │ ├─IdentifierNameMaybeBeforeSignature 'RefReturn' +// CHECK:STDOUT: │ │ ╭─ExplicitParamListStart '(' +// CHECK:STDOUT: │ ├─ExplicitParamList ')' +// CHECK:STDOUT: │ │ ╭─InvalidParse ';' has_error +// CHECK:STDOUT: │ │ ╭─RefPrimitiveForm 'ref' has_error +// CHECK:STDOUT: │ ├─ReturnType '->' +// CHECK:STDOUT: ├─FunctionDecl ';' +// CHECK:STDOUT: ├─FileEnd '' +// CHECK:STDOUT: (root)