Skip to content

Unwrap parenthesized expressions in unified Swift extraction - #22552

Open
aschackmull with Copilot wants to merge 6 commits into
mainfrom
copilot/fix-unified-tuple-expr-bug
Open

Unwrap parenthesized expressions in unified Swift extraction#22552
aschackmull with Copilot wants to merge 6 commits into
mainfrom
copilot/fix-unified-tuple-expr-bug

Conversation

Copilot AI commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Parenthesized Swift conditions such as while (x > 0) were extracted as opaque tuple expressions, omitting the contained comparison.

  • Tuple expression mapping
    • Elide unlabelled single-element tupleExpr nodes as grouping expressions.
    • Retain tuple extraction for other tuple forms.
while (x > 0) { ... }

now extracts the condition as the x > 0 binary expression.

Copilot AI and others added 5 commits September 11, 2026 09:28
Co-authored-by: aschackmull <28296824+aschackmull@users.noreply.github.com>
Co-authored-by: aschackmull <28296824+aschackmull@users.noreply.github.com>
Co-authored-by: aschackmull <28296824+aschackmull@users.noreply.github.com>
Co-authored-by: aschackmull <28296824+aschackmull@users.noreply.github.com>
Co-authored-by: aschackmull <28296824+aschackmull@users.noreply.github.com>
Comment on lines +286 to +287
elements: (labeledExpr label: _? @label expression: @e)
elements: _* @rest)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick question on the recursive translation and semantics of sequential rule-macro evaluation: As far as I've understood, the single-@ on label and rest mean "do recursive translation". Is this work wasted if the where clause fails and we proceed to the next tupleExpr rule below (which also specifies recursive translation), or is the recursive translation guaranteed to only be done once? To put it differently, would this pair of rules be more efficient if they used @@ for label and rest?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the purposes of the guard, all @-captures behave as if they are @@-captures. That is, guard evaluation happens before any automatic translation takes place.

(This makes the syntax a bit awkward, but the alternative would be to force the user to explicitly translate things that could have been auto-translated, and this seemed like the worse option to me.)

@aschackmull
aschackmull marked this pull request as ready for review September 11, 2026 12:18
@aschackmull
aschackmull requested a review from a team as a code owner September 11, 2026 12:18
Copilot AI balanced review requested due to automatic review settings September 11, 2026 12:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The unwrapping guard incorrectly matches trailing-comma tuples such as (x,).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates Swift extraction to unwrap parenthesized grouping expressions while preserving tuple structure.

Changes:

  • Adds single-element tuple-expression unwrapping.
  • Preserves multi-element tuple mappings.
  • Updates Swift extraction corpus expectations.
File summaries
File Description
unified/extractor/tests/corpus/swift/operators/parenthesised-expression.output Updates grouped expression extraction.
unified/extractor/tests/corpus/swift/control-flow/switch-expression-pattern.output Verifies structured tuple patterns.
unified/extractor/tests/corpus/swift/collections/tuple-literal.output Verifies tuple literal extraction.
unified/extractor/src/languages/swift/swift.rs Implements tuple-expression mapping and unwrapping.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread unified/extractor/src/languages/swift/swift.rs Outdated
tausbn
tausbn previously approved these changes Sep 11, 2026

@tausbn tausbn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! 👍
(Edit: Spoke too soon -- Copilot raised a valid issue.)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants