Skip to content

Unified: Track all local name kinds in one pass - #22284

Open
asgerf wants to merge 18 commits into
github:mainfrom
asgerf:unified/local-type-names
Open

Unified: Track all local name kinds in one pass#22284
asgerf wants to merge 18 commits into
github:mainfrom
asgerf:unified/local-type-names

Conversation

@asgerf

@asgerf asgerf commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

In Swift there is a single shared namespace for variables, functions, types, modules, etc. This PR updates the Variables.qll pass to track all names at once, and renames it to LocalNameBinding.qll to account for the fact that this is not only for variables anymore.

Also contains some drive-by AST mapping fixes. Commit-by-commit review recommended.

Why not treat different name kinds separately?

Some languages have distinct namespaces for variables and types, but trying to separate them for Swift leads to some complications. For example, we would have trouble resolving the references to C below, where both references would appear to refer to both the variable and the type -- if neither resolution pass knows about the other name kind, it gets hard to reason about shadowing among them.

class C {}
let C = foo()
C(); // call the lambda returned by foo()

let C = foo()
class C {}
C(); // call the constructor of class C

With that said, statement labels (break/continue targets) actually do have their own namespace, even in Swift, which lets us experiment with various solutions to this. I expect we can just add a namespace column to the local name binding module, e.g. so we get accessCand(AstNode n, string namespace, string name); and treat (namespace, name) as the "effective" name of a local. But I've left this for a future PR.

Uncertain scopes

The old VariableAccess class is now called PotentialLocalNameAccess; in a future we will reintroduce something like VariableAccess once we have the pieces needed to define it properly. The "potential" part is due to the fact that the local resolution might pass through a class scope, in which inherited members take precedence over outer lexical scopes:

let x = 1
class A {}

class B {
  class C : A {} // 'A' refers to 'B.A' if it exists, otherwise the top-level `A`

 func foo() {
    x // Refers to 'self.x' if it exists, otherwise the top-level 'x'
  }
}

Since types mentioned in the base classes of a class can themselves be resolved through inherited names, we can't build the class hierarchy prior to name binding -- it has to be combined (this will be done in a future PR).

@asgerf asgerf added the no-change-note-required This PR does not need a change note label Aug 5, 2026
@asgerf
asgerf force-pushed the unified/local-type-names branch from 9ebfd45 to d94ae9d Compare August 5, 2026 12:28
Comment thread unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll Dismissed
@asgerf
asgerf requested a balanced review from Copilot August 5, 2026 12:40

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.

Pull request overview

Updates Swift name binding to resolve variables, functions, types, modules, and other local names through one unified pass.

Changes:

  • Replaces variable-specific APIs with generalized local-name binding.
  • Adds scope tests for top-level, class, and generic type names.
  • Extracts generic parameters and constraints for nominal Swift types.
Show a summary per file
File Description
unified/ql/test/library-tests/local-name-binding/top_level.swift Tests top-level name resolution.
unified/ql/test/library-tests/local-name-binding/test.swift Adds type-name access coverage.
unified/ql/test/library-tests/local-name-binding/test.ql Updates the test harness for generalized names.
unified/ql/test/library-tests/local-name-binding/test.expected Adds expected query outputs.
unified/ql/test/library-tests/local-name-binding/class_scope.swift Tests class and generic-parameter scopes.
unified/ql/lib/unified.qll Exports the renamed binding module.
unified/ql/lib/codeql/unified/internal/LocalNameBinding.qll Implements generalized local-name binding.
unified/ql/lib/codeql/unified/internal/AstExtra.qll Identifies top-level statements.
unified/extractor/tests/corpus/swift/types/generic-class-parameters-and-constraints.swift Adds generic-class extraction input.
unified/extractor/tests/corpus/swift/types/generic-class-parameters-and-constraints.output Records expected generic-class AST output.
unified/extractor/src/languages/swift/swift.rs Translates nominal-type parameters and constraints.

Review details

Suppressed comments (1)

unified/extractor/src/languages/swift/swift.rs:1168

  • protocolDecl has no genericParameterClause field in swift_node_types.yml; SwiftSyntax represents the angle-bracket list as primaryAssociatedTypeClause. Consequently this optional capture is always empty and protocol primary associated-type names are never represented or bound. Please match primaryAssociatedTypeClause and translate its names with the appropriate associated-type semantics.
                genericParameterClause: (genericParameterClause parameters: _* @params)?
  • Files reviewed: 10/11 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread unified/extractor/src/languages/swift/swift.rs
@asgerf
asgerf force-pushed the unified/local-type-names branch from 4a1e9b8 to aaa2c1c Compare August 5, 2026 13:37
@asgerf
asgerf marked this pull request as ready for review August 5, 2026 13:43
@asgerf
asgerf requested review from a team as code owners August 5, 2026 13:43
@asgerf
asgerf requested a review from hvitved August 5, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants