fix standalone comptime diagnostics#27683
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4c92716e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if c.fn_level == 0 { | ||
| c.error('`\$dbg` can only be used inside functions', node.pos) |
There was a problem hiding this comment.
In single-file script mode (pref.is_script is true for .v/.vv inputs), a leading $dbg; reaches the checker as a top-level ast.DebuggerStmt because top_stmt() returns p.dbg_stmt() before other_stmts() has created the synthetic main; this new fn_level == 0 check rejects it. If any ordinary script statement precedes it, the same $dbg is parsed inside the generated main and is accepted, so scripts whose first statement is $dbg are treated inconsistently and lose a valid debugging entry point. Route that top-level $dbg through script wrapping, or account for script mode, before emitting this error.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf9248787a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.should_check_unused_exprs(.dollar) && !p.expr_can_be_unused_stmt(expr) { | ||
| return p.error_with_pos('expression evaluated but not used', expr.pos()) |
There was a problem hiding this comment.
$embed_file branch results
This diagnostic now fires for every statement that starts with $embed_file, including cases where the call is the value of an expression block rather than an unused statement. For example, data := if cond { $embed_file('a') } else { $embed_file('b') } reaches this helper for the branch statement and, after parsing the call, the next token is }; parse_multi_expr deliberately skips the unused-expression check in that situation so the checker can use the last expression as the branch result. Gate this path the same way, otherwise valid if/match expression branches returning embedded data are rejected during parsing.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73b27f4842
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.should_check_unused_exprs(.dollar) && (is_top_level || p.tok.kind !in [.comment, .rcbr]) | ||
| && !p.expr_can_be_unused_stmt(expr) { | ||
| return p.error_with_pos('expression evaluated but not used', expr.pos()) |
There was a problem hiding this comment.
$embed_file before any block close
When a standalone $embed_file(...) is the final statement of an ordinary block, e.g. fn main() { { $embed_file(@FILE) } }, this guard skips the new unused-expression diagnostic just because the next token is }. That } exemption is only safe for expression branches such as if/match; in normal blocks it lets the value-producing comptime call proceed past the parser, leaving the same later-stage failure this change is trying to avoid.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5ef3061f5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.should_check_unused_exprs(.dollar) && (is_top_level || !p.is_expr_branch_result_end()) | ||
| && !p.expr_can_be_unused_stmt(expr) { |
There was a problem hiding this comment.
$embed_file in statement-only branches
When $embed_file is the last statement of an if/match used only as a statement, e.g. fn main() { if cond { $embed_file(@FILE) } }, this exemption still treats the closing } as an expression-branch result because inside_if_expr and inside_match_body are set while parsing both expression and statement forms. The new unused-expression diagnostic is skipped in those statement-only branches, so the standalone comptime call still reaches later stages and embeds discarded data even though the equivalent final statement in a normal block now errors; gate this on the enclosing branch actually producing an expression value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d94668b843
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn (p &Parser) is_expr_branch_result_end() bool { | ||
| return p.tok.kind == .rcbr && (p.inside_expr_branch || p.inside_or_expr) |
There was a problem hiding this comment.
When an if/match expression branch is being parsed, this check treats any following } as the end of the value-producing branch. That state is not reset for nested statement blocks parsed inside the branch, so code like x := if cond { { $embed_file(@FILE) } 0 } else { 1 } skips the new unused-expression error for the inner block’s discarded $embed_file and still reaches later stages. The branch-result exemption needs to distinguish the branch’s own closing brace from braces of nested blocks/defer/unsafe bodies.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34af8af092
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.should_check_unused_exprs(.dollar) && (is_top_level || !p.is_expr_branch_result_end()) | ||
| && !p.expr_can_be_unused_stmt(expr) { |
There was a problem hiding this comment.
$embed_file in inferred comptime branch values
When a value-producing branch ends with a comptime $if/$match, that inner comptime expression is parsed through stmt() with is_expr false even though the outer branch uses the final statement as its value. This new check therefore rejects valid compositions such as embedded := if cond { $if linux { $embed_file('a') } $else { $embed_file('b') } } else { $embed_file('c') }, while the equivalent direct assignment to $if remains accepted. The exemption needs to account for final statements whose value is inferred by the enclosing expression branch, not only branches parsed with inside_expr_branch already set.
Useful? React with 👍 / 👎.
| fn (p &Parser) is_expr_branch_result_end() bool { | ||
| return p.tok.kind == .rcbr | ||
| && ((p.inside_expr_branch && p.opened_scopes == p.expr_branch_scope_depth) | ||
| || p.inside_or_expr) |
There was a problem hiding this comment.
Because inside_or_expr stays true for every nested block parsed inside an or block, this treats any following } as a value-producing or-block result. In a value-producing handler like value := maybe_int() or { { $embed_file(@FILE) } 0 }, the $embed_file is the final statement of an ordinary inner block and its value is discarded, but the new unused-expression diagnostic is skipped and the same bad standalone embed reaches later stages. Track the or-block scope depth, as the if/match branch code does, so only the actual or-block result is exempted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7127cb98d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.should_check_unused_exprs(.dollar) && (is_top_level || !p.is_expr_branch_result_end()) | ||
| && !p.expr_can_be_unused_stmt(expr) { | ||
| return p.error_with_pos('expression evaluated but not used', expr.pos()) |
There was a problem hiding this comment.
When a direct $embed_file(...) is the final value of an if/match expression branch or value-producing or {} block and it has an inline/trailing comment before the closing brace, p.expr(0) leaves p.tok on .comment; is_expr_branch_result_end() only accepts .rcbr, so this new check emits expression evaluated but not used. The old parse_multi_expr path explicitly allowed .comment as an end marker, so valid branches such as data := if ok { $embed_file('a') // fallback } else { ... } are now rejected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4437cf5497
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if c.fn_level == 0 { | ||
| c.error('`\$dbg` can only be used inside functions', node.pos) |
There was a problem hiding this comment.
$dbg in leading script $if through main
When a script starts with a comptime branch such as $if linux { $dbg; }, top_stmt() leaves that $if at top level because comptime_if_expr_contains_top_stmt() does not classify ast.DebuggerStmt as an ordinary script statement. This new fn_level == 0 check then rejects the debugger statement even though a leading bare $dbg; is now wrapped into the generated main; handle debugger statements in leading comptime branches the same way before emitting this error.
Useful? React with 👍 / 👎.
| if !(p.is_current_expr_branch_scope() || p.is_current_or_expr_scope()) { | ||
| return false | ||
| } | ||
| if p.tok.kind == .rcbr { | ||
| return true |
There was a problem hiding this comment.
or {} embed exemptions on used values
For a statement-only or {} block, inside_or_expr is still set, so a final $embed_file followed by the handler's } skips the unused-expression diagnostic. For example, if maybe_file() ?embed_file.EmbedFileData is called as maybe_file() or { $embed_file(@FILE) } with the call result discarded, this treats the embed as a value-producing handler result and allows embedding data that is never used; only exempt the handler's closing brace when the enclosing or {} value is actually consumed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3737eaac21
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !p.is_current_expr_branch_scope() { | ||
| return false |
There was a problem hiding this comment.
This only recognizes inferred comptime values when the enclosing scope is an if/match expression branch. In a value-producing or {} fallback, e.g. file := maybe_file() or { $if linux { $embed_file('a') } $else { $embed_file('b') } }, the handler value is consumed, but this returns false, so the nested $if/$match branches are parsed as statement-only and their final $embed_file is rejected as unused. Treat the current used or-block scope like expression branches before applying the unused-expression diagnostic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6efe50c434
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn (p &Parser) or_block_value_is_used() bool { | ||
| return p.expecting_value || ((p.is_current_expr_branch_scope() | ||
| || (p.is_current_or_expr_scope() && p.or_expr_is_used)) | ||
| && p.current_or_block_is_followed_by_result_end()) |
There was a problem hiding this comment.
In value-producing if/match branches, the final statement is still parsed via expr_no_value, so p.expecting_value is false while an or {} handler is parsed. This heuristic only marks the handler as used when its closing brace is immediately followed by the branch }, which rejects valid branch results where the unwrapped value feeds a larger expression, e.g. same := if ok { maybe_file() or { $embed_file('v.png') } == fallback } else { false }; the $embed_file fallback is consumed by the comparison but now gets the new unused-expression diagnostic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0aefe2177d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (is_top_level || p.tok.kind !in [.comment, .rcbr]) | ||
| && node !in [ast.CallExpr, ast.PostfixExpr, ast.ComptimeCall, ast.SelectorExpr, ast.DumpExpr] { | ||
| && !p.expr_can_be_unused_stmt(node) { |
There was a problem hiding this comment.
When a standalone embed is wrapped in parentheses, for example fn main() { ($embed_file(@FILE)) }, parsing reaches parse_multi_expr as an ast.ParExpr. Because the next token is the block's }, this guard skips the unused-expression check before expr_can_be_unused_stmt can reject it, so the value-producing embed still reaches later stages instead of the new diagnostic; ordinary blocks and statement-only branches should not use the branch-result } exemption.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69c9dc9ef0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn (p &Parser) or_block_value_is_used() bool { | ||
| return p.expecting_value || p.current_or_block_is_followed_by_expr_continuation() | ||
| || ((p.is_current_expr_branch_scope() || (p.is_current_or_expr_scope() | ||
| && p.or_expr_is_used)) && p.current_or_block_is_followed_by_result_end()) |
There was a problem hiding this comment.
When a value-producing branch returns multiple values, an or {} expression can be one element of the branch result, e.g. a, b := if ok { maybe_file() or { $embed_file('v.png') }, 1 } else { ... }. In that path the branch is parsed with expecting_value == false, and the token after the handler is ,, so this helper marks the handler as unused because comma is neither a precedence continuation nor the branch-closing }. The final $embed_file in the fallback then gets the new unused-expression diagnostic even though the unwrapped value is consumed as part of the branch result.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4987c21e5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if c.fn_level == 0 { | ||
| c.error('`\$dbg` can only be used inside functions', node.pos) |
There was a problem hiding this comment.
When a script starts with a comptime $match such as $match @OS { 'linux' { $dbg; } $else {} }, top_stmt() returns the match as a top-level ExprStmt rather than entering the synthetic main path used for the new bare $dbg and leading $if cases. The debugger branch is therefore checked with fn_level == 0, so this new guard rejects an otherwise valid script debugging entry point before later script statements are wrapped into main; classify script-bearing $match branches the same way before applying this error.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d4e756608
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.pref.is_script && !p.pref.is_test { | ||
| return p.other_stmts(ast.empty_stmt) |
There was a problem hiding this comment.
When a script starts with a top-level $if/$match branch containing $dbg, parse_block_no_scope() recursively calls top_stmt() while parsing that branch because the parser is still in top-level comptime mode. This new branch then starts other_stmts() from inside the comptime branch, whose loop consumes until EOF rather than stopping at the branch's }, corrupting parsing of cases like $if linux { $dbg; } println('after'). Only the outer file-level $dbg should enter script wrapping; nested comptime-branch $dbg should be returned to the outer $if/$match so the whole comptime statement can be wrapped once.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c69955d12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn (p &Parser) or_block_value_is_used() bool { | ||
| if p.expecting_value || p.current_or_block_is_followed_by_expr_continuation() { |
There was a problem hiding this comment.
When an unused or {} expression is wrapped in parentheses, expr_no_value() enters the parens but check_expr(.lpar) reparses the inner expression with p.expr(0), so p.expecting_value is true here even though the parenthesized expression is only a statement. That marks the handler as value-producing, so (maybe_file() or { $embed_file(@FILE) }) still skips the new unused-expression diagnostic and embeds discarded data; the used-value check needs to account for whether the enclosing parenthesized expression is actually consumed.
Useful? React with 👍 / 👎.
| is_expr := p.expecting_value || p.prev_tok.kind.is_assign() | ||
| node = p.if_expr(false, is_expr) |
There was a problem hiding this comment.
When a normal if is the final value of another value-producing branch, it is parsed through expr_no_value(), so this condition leaves the nested if with is_expr == false. Its own branches are then parsed without value-result scope, causing valid code like data := if ok { if other { $embed_file('a') } else { $embed_file('b') } } else { $embed_file('c') } to raise the new unused $embed_file diagnostic even though the nested if is the outer branch result; include the current value-result branch context when deciding is_expr.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d210acfa4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn (p &Parser) expr_value_is_used() bool { | ||
| return p.expecting_value || p.prev_tok.kind.is_assign() || p.is_current_value_result_scope() |
There was a problem hiding this comment.
Because this treats the current value-result scope as enough to make any nested expression used, every statement parsed at the top of a value-producing branch or used or {} block inherits expression context even when expr_no_value() is parsing a non-final statement. For example, x := if ok { if other { $embed_file('a') } else { $embed_file('b') }; $embed_file('c') } else { ... } accepts and embeds the first nested if's branch values although that nested if result is discarded, bypassing the unused-$embed_file diagnostic. The value-scope shortcut needs to be limited to the actual final result statement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6f9120c5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return false | ||
| } | ||
| next_kind := p.relative_token(next_offset).kind | ||
| return next_kind in [.comma, .rcbr] |
There was a problem hiding this comment.
When the final expression of a value-producing branch starts with another if/match whose result is consumed by an operator, e.g. same := if ok { if other { $embed_file('v.png') } else { fallback } == fallback } else { false }, the token after the nested branch is ==. This helper only treats , or } as evidence that the nested branch value is used, so the nested if is parsed with is_expr=false and its $embed_file branch hits the new unused-expression diagnostic even though the value feeds the comparison. Include expression-continuation tokens here, as the or {} path now does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07dd0c925e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fn (p &Parser) or_block_value_is_used() bool { | ||
| if p.parenthesized_expr_is_used | ||
| && (p.expecting_value || p.current_or_block_is_followed_by_expr_continuation()) { | ||
| return true |
There was a problem hiding this comment.
This marks an or {} fallback as value-producing whenever an operator follows the handler, even if the whole expression is only a discarded statement. For a final ordinary statement such as maybe_file() or { $embed_file(@FILE) } == fallback, the handler is treated as used because of ==, and then the parser is at the enclosing } so the generic unused-expression check is skipped; the embedded data is still unused. Please only apply the continuation exemption when the enclosing expression value is actually consumed, such as an assignment or value-producing branch result.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1a155dae6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if brace_depth == 0 { | ||
| next_offset := p.next_non_comment_token_offset(offset + 1) | ||
| next_tok := p.relative_token(next_offset) | ||
| if next_tok.kind == .key_else { | ||
| offset = next_offset | ||
| } else if next_tok.kind == .dollar | ||
| && p.relative_token(next_offset + 1).kind == .key_else { | ||
| offset = next_offset + 1 | ||
| } else { | ||
| return next_offset |
There was a problem hiding this comment.
When a nested if is the final value of an outer expression branch, this lookahead treats the first balanced brace pair as the end of the whole if. If the condition itself contains an or {} fallback, e.g. data := if outer { if maybe_ok() or { false } { $embed_file('v.png') } else { fallback } } else { fallback }, the scan returns the token after the fallback ({ for the real body), so current_expr_is_value_result_stmt() marks the nested if as statement-only and the valid $embed_file branch value is rejected as unused. The scan needs to distinguish condition-side braces from the branch bodies before deciding whether the nested if result is consumed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd03bf7eda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .lcbr { | ||
| found_lcbr = true | ||
| brace_depth++ |
There was a problem hiding this comment.
When a nested match is the final value of an expression branch and its condition unwraps with or {}, e.g. embedded := if ok { match maybe_int() or { 0 } { 0 { $embed_file('v.png') } else { fallback } } } else { fallback }, this scanner treats the fallback block's {} as the match body because every .lcbr sets found_lcbr. It returns the token after the fallback ({ for the real match body), so current_expr_is_value_result_stmt() returns false and match_expr(..., is_expr: false) rejects the branch $embed_file values as unused even though the match result is consumed. Fresh evidence: the final patch only added key_orelse skipping to the if lookahead, while this match lookahead still has no such skip.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba8acf41fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .lcbr { | ||
| brace_depth++ | ||
| } |
There was a problem hiding this comment.
When a nested if is the final value of an expression branch, this lookahead treats the first balanced {} after if as the branch body. If the condition itself contains a struct/map literal, e.g. data := if outer { if Flag{ok: true}.ok { $embed_file('v.png') } else { fallback } } else { fallback }, the scan stops after the literal, expr_value_is_used() returns false, and the $embed_file branch value is rejected as unused even though the nested if result is consumed. Fresh evidence: this version skips or {} condition blocks but still counts other condition-side braces as branch bodies.
Useful? React with 👍 / 👎.
| .lcbr { | ||
| found_lcbr = true | ||
| brace_depth++ |
There was a problem hiding this comment.
This match lookahead has the same condition-side brace problem: it marks the first { as the match body, so a final branch value like match Flag{kind: 0}.kind { 0 { $embed_file('v.png') } else { fallback } } is classified as statement-only after the struct literal closes. The $embed_file values in the match arms then hit the new unused-expression diagnostic despite the nested match being the outer branch result; locate the real match body rather than treating literal braces in the condition as branch braces.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d94c7488d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| opens_literal := tok.is_next_to(prev_tok) | ||
| || prev_tok.kind in [.key_if, .key_match, .lpar, .lsbr, .comma] | ||
| || prev_tok.kind.precedence() > 0 |
There was a problem hiding this comment.
When the final nested if/match value has a condition that calls an anonymous function, e.g. if fn () ?bool { return true }() or { false } { $embed_file(@FILE) } else { fallback }, this lookahead treats the function body { ... } as the nested branch body because the brace is after the return type and does not satisfy opens_literal. It then decides the nested branch result is unused, so valid $embed_file values in the real branches are rejected with expression evaluated but not used; anonymous function calls in if conditions are supported elsewhere in the repo, so the condition-side body needs to be skipped too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15440239da
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if next_kind == .lcbr { | ||
| return end_offset | ||
| } | ||
| if p.current_brace_opens_anon_fn_body(start_offset) && next_kind == .lpar { | ||
| return end_offset |
There was a problem hiding this comment.
When the final value of an outer expression branch is an if/match whose condition itself contains an if or match expression, this helper does not recognize the condition-side branch braces because the first inner } is followed by else/another match arm rather than the real body {. For example, data := if outer { if (if cond { true } else { false }) { $embed_file(@FILE) } else { fallback } } else { fallback } is valid, but the lookahead treats the inner condition branch as the nested if body, expr_value_is_used() returns false, and the $embed_file branch value is rejected as unused.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9f25595e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| || prev_tok.kind in [.key_if, .key_match, .lpar, .lsbr, .comma] | ||
| || prev_tok.kind.precedence() > 0 |
There was a problem hiding this comment.
When a nested if/match condition contains an unsafe { ... } expression followed by an operator, such as if unsafe { C.foo() } == 0 { ... }, this helper does not classify the unsafe block as condition-side. The scan then returns the == after the unsafe block and current_expr_is_value_result_stmt() treats the nested branch as value-producing even when it is a discarded non-final statement, so { if unsafe { true } == true { $embed_file(@FILE) } else { fallback } 0 } bypasses the new unused-$embed_file diagnostic in the discarded branch.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45adb3dcf1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return p.parenthesized_expr_is_used && p.is_current_value_result_scope() | ||
| && (next_kind in [.comma, .rcbr] || next_kind.precedence() > 0) |
There was a problem hiding this comment.
When an or {} fallback is parsed in an expression statement whose value is otherwise unused, this returns false unless the outer scope is a branch result or already expects a value. That rejects valid side-effecting postfix chains such as maybe_file() or { $embed_file(@FILE) }.to_string(): next_kind is .dot, so the $embed_file statement in the fallback is reported as “expression evaluated but not used” even though the fallback object is immediately used as the receiver of the following call. Please treat postfix continuations (., (, [, etc., including after parentheses) as value use before applying the new unused-embed diagnostic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a87b898111
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.should_check_unused_exprs(.dollar) && (is_top_level || !p.is_expr_branch_result_end()) | ||
| && !p.expr_can_be_unused_stmt(expr) { | ||
| return p.error_with_pos('expression evaluated but not used', expr.pos()) |
There was a problem hiding this comment.
When $embed_file is the sole expression inside an unsafe { ... } expression that is itself used as a branch or or {} result, this check runs while unsafe_stmt() has opened its inner scope, so is_expr_branch_result_end() cannot see the enclosing value-producing branch and reports expression evaluated but not used. For example, embedded := if ok { unsafe { $embed_file(@FILE) } } else { fallback } should be parsed like other branch values, but the new diagnostic rejects it before unsafe_stmt() can wrap the expression as an UnsafeExpr.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2da5ba4155
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn (p &Parser) unsafe_stmt_value_is_used() bool { | ||
| if p.expecting_value || p.prev_tok.kind.is_assign() { |
There was a problem hiding this comment.
When unsafe { ... } is parsed as an ordinary statement, stmt() calls unsafe_stmt() directly rather than through expr_no_value(), so p.expecting_value is still the parser's ambient default true. This branch therefore marks discarded blocks like fn main() { unsafe { $embed_file(@FILE) } } as value-producing, and the inner $embed_file reaches is_current_unsafe_expr_result_end() and skips the new unused-expression diagnostic. Gate this on an actual assignment/result context instead of the default expecting_value state.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
2e70147 to
b1a6be5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1a6be5b08
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ast.DumpExpr { | ||
| return p.expr_contains_embed_file_value(expr.expr) | ||
| } | ||
| else {} |
There was a problem hiding this comment.
When a final discarded wrapper consumes a spawned call result, such as '' == (spawn load($embed_file(@FILE).path)).wait(), this traversal reaches an ast.SpawnExpr under the selector/call chain and falls through here. parse_multi_expr() then preserves the closing-brace exemption because it sees no embedded-file value, so the file is still embedded just to compute the discarded comparison; scan SpawnExpr.call_expr (and GoExpr.call_expr for coroutine builds) like ordinary calls.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
b1a6be5 to
9e1a306
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e1a306157
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ast.GoExpr { | ||
| return p.expr_contains_embed_file_value(ast.Expr(expr.call_expr)) | ||
| } | ||
| else {} |
There was a problem hiding this comment.
When a discarded final expression hides $embed_file inside an ORM query expression, this traversal falls through for ast.SqlExpr, so has_embed_file_value stays false and the closing-} exemption suppresses the new diagnostic. For example, in a function body like 0 == (sql db { select from User limit $embed_file(@FILE).len } or { []User{} }).len, the file is embedded only to compute a discarded value; scan the SQL expression fields such as db_expr, where_expr, order_expr, limit_expr, offset_expr, joins, and or_expr before returning false.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
9e1a306 to
9f0e2c7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f0e2c7c8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
| } | ||
| else {} |
There was a problem hiding this comment.
When the embedded value is hidden inside a variadic array-decompose argument of a discarded final expression, e.g. fn collect(xs ...string) string { return '' } fn f(){ '' == collect(...[$embed_file(@FILE).path]) }, the CallExpr argument scan reaches ast.ArrayDecompose and falls through here. That leaves has_embed_file_value false, so parse_multi_expr() keeps the closing-} exemption and the file is embedded even though the comparison result is discarded; please recurse into ArrayDecompose.expr before deciding the wrapper has no $embed_file value.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
9f0e2c7 to
1c3c938
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c3c938836
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| return true | ||
| } | ||
| return tok.kind in [.comma, .rcbr] |
There was a problem hiding this comment.
or blocks as value continuations
When a parenthesized branch result is followed by or {}, for example (if ok { ?embed_file.EmbedFileData($embed_file(@FILE)) } else { none }) or { fallback } as the final value of an outer branch, the parenthesized expression is consumed by the or block. Because this continuation helper only treats commas/closing braces as value continuations here, parenthesized_expr_is_used remains false and the nested if/match branches are parsed as statement-only, so valid code using $embed_file as the branch value is rejected as “expression evaluated but not used”.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
1c3c938 to
19bdfe8
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19bdfe8503
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if opens_literal && next_kind == .lcbr { | ||
| return end_offset | ||
| } | ||
| if p.current_brace_opens_anon_fn_body(start_offset) && next_kind in [.lpar, .rpar, .comma] { |
There was a problem hiding this comment.
] when skipping anonymous function bodies
When a nested branch result has an anonymous function inside an array literal in its condition, for example if accepts([fn () bool { return true }]) { $embed_file(@FILE) } else { fallback }, the lookahead reaches the anonymous function body but this whitelist only skips it when the following token is (, ), or ,. Since the next token is ], the helper treats the function body as the real if body and decides the inner if is not value-producing, so the valid $embed_file branch is parsed as statement-only and rejected as unused; closing delimiters that can follow anonymous functions in literals need to be skipped too.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
19bdfe8 to
3856da2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3856da2138
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ast.ForStmt { | ||
| return p.stmts_contain_embed_file_value(stmt.stmts) | ||
| } |
There was a problem hiding this comment.
When a discarded final wrapper invokes an anonymous function whose return value is controlled by a loop condition, e.g. '' == fn () string { for $embed_file(@FILE).len > 0 { return 'x' }; return '' }(), this matcher only recurses into ForStmt.stmts. That leaves has_embed_file_value false, so the closing-} exemption still suppresses the new unused-expression diagnostic even though the embed only affects a discarded value; please scan ForStmt.cond as well, and the analogous header expressions for for in/C-style loops just below.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
3856da2 to
059a1ae
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 059a1ae249
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.expr_contains_embed_file_value(expr.len_expr) | ||
| || p.expr_contains_embed_file_value(expr.cap_expr) | ||
| || p.expr_contains_embed_file_value(expr.init_expr) | ||
| || p.expr_contains_embed_file_value(expr.update_expr) { |
There was a problem hiding this comment.
When a discarded final array initializer uses a comptime type expression, e.g. fn f() { []typeof([$embed_file(@FILE)]).element_type{} }, the checker resolves elem_type_expr and visits the typeof operand, but this traversal only checks len/cap/init/update/value expressions. has_embed_file_value stays false, so parse_multi_expr() keeps the closing-} exemption and the file is still embedded for an unused value; include elem_type_expr in the scan (and the analogous StructInit.typ_expr path below).
Useful? React with 👍 / 👎.
| } | ||
| } | ||
| } | ||
| else {} |
There was a problem hiding this comment.
When orm.QueryData is available and a dynamic ORM query-data literal is the final discarded expression, e.g. fn f() { { name == $embed_file(@FILE).path } }, this default treats ast.SqlQueryDataExpr as embed-free. The checker still descends into query-data leaf RHSs and $if branch conditions/items, so the file is embedded while parse_multi_expr() keeps the final-} exemption; recurse through SqlQueryDataExpr.items before falling through.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
059a1ae to
ee933de
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee933de03c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .dot { | ||
| field_offset := p.next_non_comment_token_offset(offset + 1) | ||
| if p.relative_token(field_offset).kind == .eof { | ||
| return -1 | ||
| } | ||
| offset = p.next_non_comment_token_offset(field_offset + 1) | ||
| } |
There was a problem hiding this comment.
When the continuation after an expression-branch result is a selector or index that immediately propagates with ?/!, this scanner stops on the propagation token and result_token_is_value_continuation() returns false. For example, in a final branch value like maybe_wrapper() or { Wrapper{ file: $embed_file(@FILE) } }.file?, the or-block result is consumed by the selector propagation, but or_block_value_is_used() treats it as unused and the valid $embed_file fallback is rejected as expression evaluated but not used; skip post-selector/index propagation tokens before deciding the postfix chain has no value continuation.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
ee933de to
6ef2ada
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ef2ada5e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for branch in expr.branches { | ||
| if p.expr_contains_embed_file_value(branch.cond) | ||
| || p.stmts_contain_embed_file_value(branch.stmts) { | ||
| return true |
There was a problem hiding this comment.
$if branches as evaluated
When a discarded final expression contains a value-producing compile-time $if, this loop scans every branch before the checker applies the $if condition. For example, fn f() { 0 == ($if missing ? { $embed_file(@FILE).len } $else { 0 }) } compiled without -d missing has no evaluated/embedded file, but has_embed_file_value becomes true and parse_multi_expr() reports expression evaluated but not used. Inactive conditional-compilation branches can contain otherwise invalid code, so this diagnostic needs to respect the selected branch or avoid treating skipped branches as embeds.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
6ef2ada to
99e0530
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99e053023f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return p.expecting_consumed_value() || p.prev_tok.kind.is_assign() | ||
| || p.current_expr_is_value_result_stmt() | ||
| } |
There was a problem hiding this comment.
In contexts like fn load(ok bool) embed_file.EmbedFileData { fallback := $embed_file(@FILE); return if ok { $embed_file(@FILE) } else { fallback } }, return_stmt() parses the RHS with inside_assign_rhs = true/expr_list(true), but this helper returns false because it only recognizes assignments, consumed parentheses/call args, or branch-result scopes. The returned if/match is then parsed as statement-only, so the $embed_file branch is rejected as expression evaluated but not used even though its value is the function return; include return/RHS value contexts before deciding the branch is not a value.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
99e0530 to
7df8472
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7df8472738
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.expr_contains_embed_file_value(expr.cond) { | ||
| return true | ||
| } | ||
| for branch in expr.branches { |
There was a problem hiding this comment.
$match branches
When the discarded wrapper contains a value $match, this loop treats every branch as evaluated. For example, 0 == ($match 'ok' { 'skip' { $embed_file(@FILE).len } $else { 0 } }) should only evaluate the $else branch, but the parser still sees the skipped branch and emits the new unused-expression diagnostic. Please make the embed scan respect the selected $match branch, like the $if path above does.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
7df8472 to
759c365
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 759c36599e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if cname == 'threads' { | ||
| return comptime_if_embed_scan_bool_value(p.table.gostmts > 0) |
There was a problem hiding this comment.
$if threads as unknown during embed scan
When a discarded $if threads expression is parsed before the first spawn/go in the file, p.table.gostmts is still 0 here, so the embed scan treats the threads branch as disabled and leaves has_embed_file_value false. After the rest of the file is parsed, checker/cgen use the final gostmts value, so a later spawn makes that branch active and $embed_file can be evaluated without the new unused-expression diagnostic; this scan should not decide threads from partial parser state.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
759c365 to
cf76848
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf76848d03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if has_unknown_case && p.stmts_contain_embed_file_value(branch.stmts) { | ||
| return true |
There was a problem hiding this comment.
When a value $match case label is a const identifier, the checker resolves it to a string/int/bool literal, but this path treats it as an unknown case and scans that branch body anyway. Fresh evidence: the current helper now skips inactive literal branches, but const skip = 'skip'; 0 == ($match 'ok' { skip { $embed_file(@FILE).len } $else { 0 } }) still gets flagged as expression evaluated but not used even though the skip branch is not selected; resolve literal const identifiers here or defer the embed decision instead of treating these unknown cases as evaluated.
Useful? React with 👍 / 👎.
medvednikov
force-pushed
the
agent/fix-dbg-embed-file-invalid-c
branch
from
cf76848 to
cfeb3e0
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfeb3e0e11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if p.should_check_unused_exprs(.dollar) && (is_top_level || !is_expr_branch_result_end) | ||
| && !p.expr_can_be_unused_stmt(expr) { |
There was a problem hiding this comment.
When $embed_file appears as a standalone statement inside a disabled comptime branch, e.g. $if missing ? { $embed_file(@FILE) }, this parser-side check runs while the branch is being parsed, before the checker can skip inactive comptime branches. Because statement branches are not expression-result scopes, is_expr_branch_result_end is false and the parser reports expression evaluated but not used for code that should not be evaluated on that target/flag, regressing valid platform- or define-gated embeds.
Useful? React with 👍 / 👎.
1 participant
What changed
$embed_file(...)expressions, including parenthesized forms, parenthesizedif/matchresult forms, selectors rooted in$embed_file(...), and discarded field selections after embed-producingor {}/lockfallbacks, with the existingexpression evaluated but not usedparser diagnostic instead of letting discarded embedded data reach later stages.$embed_file(...)under value-only wrappers such as infix expressions, containers, casts, selectors, string interpolations, attachedor {}fallbacks, lock bodies, and nested value-result branches, instead of only checking expressions rooted directly at$embed_file(...).$embed_file(...)values when they are actually consumed as final values ofif/matchexpression branches, usedor {}fallbacks, single-expressionunsafe { ... }results, and value-producinglock/rlockexpressions.unsafe {}result context to nested value-producingif/matchexpressions andor {}fallbacks inside that unsafe block.or {}handlers, unused unsafe statements, unused lock statements, discarded selector-only postfix chains, newline-separated pseudo-calls, and non-final nested branch statements still reject discarded$embed_file(...)values.if/matchresults consumed by outer operators, parenthesized nestedif/matchresults consumed by postfix calls, and unused call statements whose receiver is a parenthesized branch value.lock/rlockresult values used as postfix call receivers, e.g.(rlock s { ... }).to_string().or {}blocks, braced literals, braced literals passed as call arguments, called anonymous function bodies, nested branch expressions,unsafe {}blocks, andlock/rlockbodies while locating the real body of nestedif/matchresult expressions.$dbg;through generatedmain, including when it appears inside an initial comptime$ifor$matchbranch, while still rejecting debugger statements outside functions in non-script contexts.$embed_filediagnostics, valid branch/or/unsafe/lock result values, nested condition lookahead cases, nested value constructs inside used unsafe results, condition literal argument delimiters, parenthesized unused branch expressions, selectors rooted in embeds, discarded field selections after embed fallbacks, wrapped selector fallbacks, wrapped lock selectors, parenthesized lock postfix calls, parenthesized nested branch postfix calls, unused parenthesized branch receiver calls, newline-separated selector/call lookalikes, infix expressions and string interpolations hiding discarded embeds, and leading script debugger wrapping.Root cause
The parser previously allowed direct
$...expression statements too broadly, so a discarded$embed_file(...)could survive parsing and fail later. The new diagnostic needs to distinguish real value-result contexts from any arbitrary following}. The parser now tracks the exact scopes where a final expression value is consumed, includingif/matchbranches, usedor {}handlers, used single-expressionunsafe {}bodies, and usedlock/rlockbodies. Nested scopes only inherit that state when their expression is the actual result expression.expr_no_value()reparses parenthesized contents withp.expr(0), soexpecting_valuealone is not reliable inside an unused parenthesized statement. Value-use detection now also respects the current parenthesized-used state and detects postfix receiver calls after the closing parenthesis, while still preserving real consumed contexts such as call arguments. Selector statements remain generally allowed, but selectors whose receiver contains discarded embedded data are rejected unless the selector value is actually consumed.Postfix handling for embed-producing
or {}/lock/unsaferesults must distinguish selector-only chains from call chains. A plain.pathstill discards the value, while.to_string()or.field.method()consumes the value as a call receiver; the parser now only suppresses the inner embed diagnostic for postfix chains that reach a same-line call, matchingexpr_with_left()call parsing. The same postfix-call continuation logic is used when deciding whether a parenthesized nestedif/matchexpression is the current branch result or the receiver of an otherwise allowed call statement.Discarded expression detection needs to look through all value-producing wrappers that can hide an embed, not just the root node. String interpolations store their payload expressions and dynamic format expressions separately, so the embed search now descends into
StringInterLiteral.exprs,fwidth_exprs, andprecision_exprs. It also descends into call/selector receiver and argument subexpressions plus attachedor {}blocks, so a consumed fallback cannot disappear inside a larger discarded wrapper. Lock bodies are scanned directly during parsing rather than relying on checker-populatedLockExpr.is_expr.A used single-expression
unsafe {}body is itself a value-result scope. Without including that scope in the shared value-result helper, nestedif/matchexpressions andor {}fallbacks inside the unsafe body were parsed as statement-only even though their value becomes the unsafe expression result.Condition lookahead for nested
if/matchresult expressions must skip braced constructs that belong to the condition, including braced literals used as call arguments andlock/rlockresult bodies. Otherwise those condition-side braces are mistaken for the nested branch body and valid final$embed_file(...)branch values are parsed as statement-only. Parenthesized lock results also need postfix-call handling through closing parentheses, because a closing)before.to_string()still means the value is consumed as a receiver.The checker also accepted top-level
ast.DebuggerStmt, which let cgen emit debugger code outside a function. Script parsing now wraps leading script debugger statements, including initial comptime branches containing them, into the syntheticmainbefore checking.Fixes #27681.
Fixes #27682.
Validation
Latest follow-up:
./v -g -keepc -o ./vnew cmd/v./vnew -check vlib/v/tests/debugger_reserved_arg_name_test.v$if linux { #include <stdint.h>; $dbg } $else { #include <stdint.h>; $dbg }compiled with./vnew -o ... main.v.$match @OS { 'linux' { #include <stdint.h>; $dbg } $else { #include <stdint.h>; $dbg } }compiled with./vnew -o ... main.v.git diff --check -- vlib/v/parser/parser.v vlib/v/tests/debugger_reserved_arg_name_test.vgit diff --cached --check./vnew fmt -w vlib/v/parser/parser.v vlib/v/tests/debugger_reserved_arg_name_test.v, but compilingcmd/tools/vfmt.vcurrently fails on existingvlib/v/parser/fn.venum inference errors.Earlier checks on this PR included targeted compiler-error fixtures, embed-file tests, debugger tests, repeated formatting, rebuilds, and staged diff checks. Broad
./vnew -silent test vlib/v/was started earlier and then stopped/skipped per request.