Skip to content

Unified: Extract built-in Swift types - #22593

Open
hvitved wants to merge 1 commit into
github:mainfrom
hvitved:unified/builtins
Open

hvitved wants to merge 1 commit into
github:mainfrom
hvitved:unified/builtins

Conversation

@hvitved

@hvitved hvitved commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Needed for type inference.

DCA shows a big improvement for Percentage of statically resolvable names (from 56.3 % to 69.6 %).

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.

Copilot review overview

🟡 Changes recommended

Mutating the supplied file list breaks read-only inputs and malformed trailing-line handling.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 1 Low severity

Open (2)
What changed in this PR

Adds synthetic Swift builtin declarations so Unified name binding can resolve standard types while excluding them from source-only analyses.

Changes:

  • Packages and extracts Swift builtin type stubs.
  • Adds source-origin filtering for AST, CFG, and tests.
  • Updates name-binding expectations for builtin types.
File Description
unified/​tools/​builtins/​types.swift Defines synthetic Swift builtin types.
unified/​tools/​builtins/​BUILD.bazel Packages builtin stubs.
unified/​tools/​BUILD.bazel Adjusts nested tools packaging.
unified/​BUILD.bazel Combines extractor, scripts, and builtins under tools.
unified/​extractor/​src/​extractor.rs Adds builtins to extraction inputs.
unified/​ql/​lib/​codeql/​files/​FileSystem.qll Distinguishes source-root files.
unified/​ql/​lib/​codeql/​unified/​internal/​Builtins.qll Identifies builtin declarations.
unified/​ql/​lib/​codeql/​unified/​internal/​FacadeAst.qll Exposes AST source-origin filtering.
unified/​ql/​lib/​codeql/​unified/​internal/​ControlFlowGraph.qll Excludes external callables from CFG construction.
unified/​ql/​test/​library-tests/​BasicTest/​test.ql Filters synthetic identifiers.
unified/​ql/​test/​library-tests/​comments/​comments.ql Filters synthetic comments.
unified/​ql/​test/​library-tests/​definitions/​test.swift Tests Optional resolution.
unified/​ql/​test/​library-tests/​static-name-binding/​explicit-instance-field-access.swift Tests Int resolution.

💡 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/extractor.rs Outdated
Comment thread unified/ql/lib/codeql/unified/internal/Builtins.qll Outdated
@hvitved
hvitved requested a balanced review from Copilot September 17, 2026 13:33
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Sep 17, 2026

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.

Copilot review overview

🟡 Changes recommended

Persistent scratch-file creation breaks repeated indexing, and builtin classification and API exposure are incomplete.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
Resolved since last review (2)

Comment thread unified/extractor/src/extractor.rs
Comment thread unified/ql/lib/codeql/unified/internal/Builtins.qll
Comment thread unified/ql/lib/codeql/unified/internal/Builtins.qll
@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Sep 17, 2026
@hvitved
hvitved marked this pull request as ready for review September 17, 2026 13:45
@hvitved
hvitved requested review from a team as code owners September 17, 2026 13:45
@github-actions github-actions Bot removed the Rust Pull requests that update Rust code label Sep 17, 2026

@asgerf asgerf 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.

👍

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