Skip to content

Type inference: Improve base type matching - #22761

Merged
hvitved merged 3 commits into
github:mainfrom
hvitved:type-inference-more-base-matching
Oct 9, 2026
Merged

hvitved merged 3 commits into
github:mainfrom
hvitved:type-inference-more-base-matching

Conversation

@hvitved

@hvitved hvitved commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Before this PR, type information could only propagate backwards through sub typing when using type constraints (because that is the only form supported in Rust):

class Base<B> {
  init(_ b : B) { }
}

class Derived<D> : Base<[D]> {  }

func foo<T1, T2: Base<T1>>(_ value1: T1, _ base: T2) {  }

func bar<T1>(_ value1: T1, _ base: Base<T1>) { }

foo([0], Derived([])) // works: can infer the element type of `[]` to be `Int`
bar([0], Derived([])) // doesn't work: cannot infer the element type of `[]` to be `Int`

This PR closes that gap, which is needed for proper handling of inherited constructors in Swift.

DCA is uneventful for both Rust and Swift.

@github-actions github-actions Bot added Rust Pull requests that update Rust code Unified labels Oct 6, 2026
@hvitved
hvitved force-pushed the type-inference-more-base-matching branch 2 times, most recently from b16635a to 26a8bbc Compare October 7, 2026 12:07
@hvitved
hvitved force-pushed the type-inference-more-base-matching branch from 26a8bbc to 8cb1938 Compare October 7, 2026 13:22
@github-actions github-actions Bot removed the Rust Pull requests that update Rust code label Oct 7, 2026
@hvitved
hvitved requested a balanced review from Copilot October 7, 2026 14:33

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

🔵 Needs a closer look

Shared type-inference semantics affect multiple frontends and warrant human validation despite focused Swift coverage.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Improves nested generic type inference through base-type relationships.

Changes:

  • Matches type parameters across nested base-type paths.
  • Supports contextual inference for empty array literals.
  • Adds Swift generic-inheritance tests and updated expectations.
File Description
unified/​ql/​test/​library-tests/​type-inference/​type-inference.expected Updates generated inference results.
unified/​ql/​test/​library-tests/​type-inference/​generics.swift Adds nested generic inference cases.
unified/​ql/​lib/​codeql/​unified/​internal/​typeinference/​TypeInference.qll Models unknown empty-array element types.
unified/​ql/​lib/​codeql/​unified/​internal/​FacadeAst.qll Exposes array element counts.
shared/​typeinference/​codeql/​typeinference/​internal/​TypeInference.qll Adds nested base-type parameter matching.

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

Comment thread shared/typeinference/codeql/typeinference/internal/TypeInference.qll Outdated
@hvitved
hvitved force-pushed the type-inference-more-base-matching branch from 8cb1938 to 468caa3 Compare October 7, 2026 14:45
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Oct 7, 2026
@hvitved
hvitved marked this pull request as ready for review October 7, 2026 18:42
@hvitved
hvitved requested review from a team as code owners October 7, 2026 18:42
@hvitved
hvitved requested a review from paldepind October 7, 2026 18:42

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

Two questions:

  • Shouldn't we take variance into account? For instance, this type propagation does not make sense for contravariant positions such as the return position? Might it makes sense to have variance encoded on access positions with a type that can be invariant/contravariant/covariant?

  • I wonder if it wouldn't be possible to reuse more of the existing contraint propagation? I haven't thought this through in terms of the QL, but intuitively foo and bar in your example are very similar. Would it not be possible to ensure that they're covered by the same code path and that constraints on parameters are picked up both through type parameters and through direct types that can be subtyped?

@hvitved

hvitved commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author
  • Shouldn't we take variance into account? For instance, this type propagation does not make sense for contravariant positions such as the return position? Might it makes sense to have variance encoded on access positions with a type that can be invariant/contravariant/covariant?

Ideally, yes, but that is already not supported, but we might consider it for the future.

  • I wonder if it wouldn't be possible to reuse more of the existing contraint propagation? I haven't thought this through in terms of the QL, but intuitively foo and bar in your example are very similar. Would it not be possible to ensure that they're covered by the same code path and that constraints on parameters are picked up both through type parameters and through direct types that can be subtyped?

I thought about doing that, but I went for the simpler solution for now.

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

Thanks for the answers. I think we should look into that in the future, but this also LGTM as-is! :)

@hvitved
hvitved merged commit dc7b9bd into github:main Oct 9, 2026
106 of 114 checks passed
@hvitved
hvitved deleted the type-inference-more-base-matching branch October 9, 2026 12:53
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 Unified

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants