-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
parser: fix match-arm !/? propagation when a match is used as a block value (fix #28000)
#28002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fab69be
0efc15b
439a513
1546c32
b210e50
77a271a
b3b5a13
e3c7894
f46183f
eb4b179
353a90a
23236ba
9225506
7020743
d61a5db
be7a393
6bb7234
58fba14
4095e5f
9c8e75b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -409,8 +409,20 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { | |
| } | ||
|
|
||
| if node.has_update_expr { | ||
| // `[...base, e1, e2]` — array update/spread literal | ||
| // `[...base, e1, e2]` — array update/spread literal. | ||
| // A value `match`/`if` spread operand, e.g. `[...(match x { ... })]`, in a | ||
| // void context (nested in an if-branch) must be checked as an expression so | ||
| // its arms produce values, instead of being typed `void`. | ||
| mut restore_force_value := false | ||
| if c.expected_type == ast.void_type && !c.force_value_match_or_if | ||
| && operand_is_value_match_or_if(node.update_expr) { | ||
| c.force_value_match_or_if = true | ||
| restore_force_value = true | ||
| } | ||
| update_typ := c.expr(mut node.update_expr) | ||
| if restore_force_value { | ||
| c.force_value_match_or_if = false | ||
| } | ||
| // Resolve through type aliases so `type Ints = []int; [...Ints(...)]` | ||
| // is accepted; use final_sym to look past aliases of arrays. | ||
| update_sym := c.table.final_sym(update_typ) | ||
|
|
@@ -507,7 +519,22 @@ fn (mut c Checker) array_init(mut node ast.ArrayInit) ast.Type { | |
| expr_pos) | ||
| continue | ||
| } | ||
| // A value `match`/`if` array element, e.g. `[match x { ... }]`, in a | ||
| // void context (e.g. nested in an if-branch) must be checked as an | ||
| // expression (`is_expr`) so its arms produce values and infer the | ||
| // element type, instead of being lowered as void statements. Signal | ||
| // that via a flag rather than a forced expected type, which would | ||
| // mistype the arms. | ||
| mut restore_array_elem_flag := false | ||
| if c.expected_type == ast.void_type && !c.force_value_match_or_if | ||
| && operand_is_value_match_or_if(expr) { | ||
| c.force_value_match_or_if = true | ||
| restore_array_elem_flag = true | ||
| } | ||
| typ = c.check_expr_option_or_result_call(expr, c.expr(mut expr)) | ||
| if restore_array_elem_flag { | ||
| c.force_value_match_or_if = false | ||
| } | ||
| sym := c.table.sym(expected_value_type) | ||
| if sym.kind == .interface { | ||
| c.type_implements(typ, expected_value_type, expr.pos()) | ||
|
|
@@ -911,7 +938,19 @@ fn (mut c Checker) map_init(mut node ast.MapInit) ast.Type { | |
| map_type = c.expected_type | ||
| } | ||
| if node.has_update_expr { | ||
| // A value `match`/`if` map update operand, e.g. `{ ...(match x { .. }), k: v }`, | ||
| // in a void context must be checked as an expression so its arms produce | ||
| // values, instead of being typed `void` ("non-map type"). | ||
| mut restore_force_value := false | ||
| if map_type == ast.void_type && c.expected_type == ast.void_type | ||
| && !c.force_value_match_or_if && operand_is_value_match_or_if(node.update_expr) { | ||
| c.force_value_match_or_if = true | ||
| restore_force_value = true | ||
| } | ||
| update_type := c.expr(mut node.update_expr) | ||
| if restore_force_value { | ||
| c.force_value_match_or_if = false | ||
| } | ||
| if map_type != ast.void_type { | ||
| if update_type != map_type { | ||
| msg := c.expected_msg(update_type, map_type) | ||
|
|
@@ -934,12 +973,39 @@ fn (mut c Checker) map_init(mut node ast.MapInit) ast.Type { | |
| } else if node.keys.len > 0 { | ||
| // `{'age': 20}` | ||
| mut key_ := node.keys[0] | ||
| // A value `match`/`if` map key, e.g. `{(match x { ... }): v}`, in a void | ||
| // context determines the map key type and must be checked as an | ||
| // expression (like the value below). Same flag mechanism. | ||
| mut restore_key_flag := false | ||
| if c.expected_type == ast.void_type && !c.force_value_match_or_if | ||
| && operand_is_value_match_or_if(key_) { | ||
| c.force_value_match_or_if = true | ||
| restore_key_flag = true | ||
| } | ||
| map_key_type = ast.mktyp(c.expr(mut key_)) | ||
| if restore_key_flag { | ||
| c.force_value_match_or_if = false | ||
| } | ||
| if node.keys[0].is_auto_deref_var() { | ||
| map_key_type = map_key_type.deref() | ||
| } | ||
| mut val_ := node.vals[0] | ||
| // A value `match`/`if` map value, e.g. `{'k': match x { ... }}`, in a | ||
| // void context (e.g. nested in an if-branch) determines the map value | ||
| // type and must be checked as an expression (`is_expr`) so its arms | ||
| // produce values, instead of being lowered as void statements. Signal | ||
| // that via a flag (leaving the expected type void so the arms infer | ||
| // their own type), consumed by `match_expr`/`if_expr`. | ||
| mut restore_container_flag := false | ||
| if c.expected_type == ast.void_type && !c.force_value_match_or_if | ||
| && operand_is_value_match_or_if(val_) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When an inferred map literal is the tail of an assignment-RHS branch and its first key is a match, such as AGENTS.md reference: AGENTS.md:L652-L658 Useful? React with 👍 / 👎.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in be7a393. Investigating this turned up that the first-key case actually fails earlier than the checker: With parsing fixed, I applied the same value-forcing to the first key in |
||
| c.force_value_match_or_if = true | ||
| restore_container_flag = true | ||
| } | ||
| map_val_type = ast.mktyp(c.expr(mut val_)) | ||
| if restore_container_flag { | ||
| c.force_value_match_or_if = false | ||
| } | ||
| if node.vals[0].is_auto_deref_var() { | ||
| map_val_type = map_val_type.deref() | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although the separate unsafe and cast spellings are covered, composing them—for example
i64(unsafe { match value { First { lower_first(value)! } Second { lower_second(value)! } } })in an if-expression branch—still returns false here because this predicate only peelsParExpr.cast_exprtherefore leavesexpected_typeas void;unsafe_exprforwards that void context to the match, which is checked as a statement rather than a value, so valid code is rejected. The v3 predicate has the equivalent unsafe-block gap; recurse through this wrapper in both implementations and add a composition regression.AGENTS.md reference: AGENTS.md:L652-L658
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b210e50. You're right — the predicates only peeled
(...). Recursing throughunsafe { }in both, plus one extra fix the composition surfaced:cast_operand_is_value_match_or_ifnow also recurses throughast.UnsafeExpr, soi64(unsafe { match ... })(in any paren composition) is checked as a value.as_cast_operand_needs_tmp_evaldidn't look throughast.UnsafeExpreither, so(unsafe { match ... }) as Variantemitted invalid C (a temp decl inside the__as_castargument). It now recurses through the wrapper like theParExprcase; this also fixes the direct (non-if-branch) spelling that was already broken.is_value_match_or_if_operandnow looks throughunsafe { }(a.blockwhose value tail is the expression) and a trailingexpr_stmt.Both regression tests now cover
i64(unsafe { match ... })and(unsafe { match ... }) as Circle. Verified: v3 still self-hosts, andcompiler_errors_test(1617 snapshots) + the cast/sumtype/as suites are unchanged.