Skip to content

Conversation

@lokesh-tr
Copy link

@lokesh-tr lokesh-tr commented Dec 22, 2025

⚠️ Under Progress ⚠️

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Nice 😍 Very excited to get my hands on this.

The overall direction looks good to me. A couple things we should ensure:

  • We handle labelled break statements correctly, eg. the following. We should have lookupLabeledStmts to help with this.
LOOP: while true {
  switch x {
  case 1: break LOOP
  default: break
  }
}
  • Double check that all syntax nodes are added correctly, I think subscripts are currently missing but there might be other.

@ahoppen ahoppen requested a review from MAJKFL December 22, 2025 23:43
@ahoppen
Copy link
Member

ahoppen commented Dec 22, 2025

CC @MAJKFL if you’re interested in helping review this and contributing your expertise in name lookup edge cases. 😉

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

Thank you!
Overall, I think this is a great addition to SwiftLexicalLookup.

I know this PR is still in draft, but I left some comments anyway.


extension SyntaxProtocol {

@_spi(Experimental) public func lookupControlStructure() -> Syntax? {
Copy link
Member

@rintaro rintaro Dec 22, 2025

Choose a reason for hiding this comment

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

I don't feel comfortable with having this method on SyntaxProtocol.
Instead, how about making

@_spi(Experimental) public protocol ControlTransferStmtSyntax {
  var controlTransferTarget: Syntax? { get }
}

And make break, continue, return, throw, and fallthrough conform to it?

Comment on lines +29 to +30
case .breakStmt, .continueStmt:
return lookupEnclosingControlStructure(for: [
Copy link
Member

@rintaro rintaro Dec 22, 2025

Choose a reason for hiding this comment

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

Unfortunately, break and continue target is not that simple. You'd need to handle labels e.g. break LOOP.
Also, they can target other non-loop labeled statements , e.g.

OUTER: do {
  for val in values {
    if val.condition {
      break OUTER
    }
  }
}

In the actual compiler, the rule is implemented in swift::findBreakOrContinueStmtTarget

let syntax = Syntax(self)

switch syntax.as(SyntaxEnum.self) {
case .throwStmt, .returnStmt:
Copy link
Member

Choose a reason for hiding this comment

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

The target of throw can be do-catch statements.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, SyntaxProtoco.lookupCatchNode() should be usable for throw case.

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.

3 participants