Unified: Type inference first version by hvitved · Pull Request #22639 · github/codeql

hvitved · 2026-09-21T12:05:14Z

This PR adds the first version of type inference and type-inference-based call resolution for Unified/Swift. The PR adds a lot of test cases (some of which are copied over from the current Swift implementation, where the annotations have been checked by the Swift compiler), which make up the bulk of the diff of this PR.

Using the shared type inference library, we support already a lot of aspects, for example sub typing and contextual typing (including contextual typing of closures), but there are still a lot of aspects that remain to be handled (a full list of known shortcomings is in the header of TypeInference.qll), for example overloading and pattern matching.

DCA looks great: Percentage of calls with call target increases from 11.5 % to 24.4 % (and as a result more cleartext-logging results) while there is only very little performance impact.

+private import TypeMention
+private import TypeInference
+private import TypeInference::Consistency as Consistency
+import TypeInference::Consistency


+   * parameter.
+   */
+  bindingset[t, arity, i]
+  abstract predicate functionInvokeSignature(Type t, int arity, int i, int j, TypePath path);


+}
+
+/** Provides predicates for debugging the type inference implementation. */
+private module Debug {


+/** Provides predicates for debugging the type inference implementation. */
+private module Debug {
+  AstNode getRelevantNode() {
+    exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |


+/** Provides predicates for debugging the type inference implementation. */
+private module Debug {
+  AstNode getRelevantNode() {
+    exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |


+/** Provides predicates for debugging the type inference implementation. */
+private module Debug {
+  AstNode getRelevantNode() {
+    exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |


Copilot

Copilot review overview

🟡 Changes recommended

Four unresolved moderate issues can produce incorrect inferred types, call targets, and dataflow edges.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Infer correct result types for binary operators · New

What changed in this PR

Introduces initial Swift type inference for the unified CodeQL library and integrates inferred call targets with dataflow.

Changes:

Adds type representations, inference plugins, built-in models, and consistency checks.
Supports generics, tuples, closures, protocols, aliases, fields, and call resolution.
Adds extensive inference and dataflow tests.

File	Review
`unified/tools/builtins/types.swift`	Adds modeled built-in operations.
`unified/ql/test/library-tests/type-inference/type-inference.qlref`	Registers inference tests and postprocessing.
`unified/ql/test/library-tests/type-inference/type-inference.ql`	Defines inference and resolution assertions.
`unified/ql/test/library-tests/type-inference/type_constraints.swift`	Tests generic constraints.
`unified/ql/test/library-tests/type-inference/type_alias.swift`	Tests generic type aliases.
`unified/ql/test/library-tests/type-inference/tuples.swift`	Tests tuple inference.
`unified/ql/test/library-tests/type-inference/protocols.swift`	Tests protocols and conformances.
`unified/ql/test/library-tests/type-inference/pattern_matching.swift`	Tests pattern-bound types.
`unified/ql/test/library-tests/type-inference/overload_resolution.swift`	Tests overload resolution.
`unified/ql/test/library-tests/type-inference/lub.swift`	Tests least-upper-bound inference.
`unified/ql/test/library-tests/type-inference/key_paths.swift`	Covers key-path scenarios.
`unified/ql/test/library-tests/type-inference/generics.swift`	Tests generic declarations and calls.
`unified/ql/test/library-tests/type-inference/fields.swift`	Tests field resolution.
`unified/ql/test/library-tests/type-inference/CONSISTENCY/CfgConsistency.expected`	Updates consistency expectations.
`unified/ql/test/library-tests/type-inference/closures.swift`	Tests closure typing and invocation.
`unified/ql/test/library-tests/type-inference/classes.swift`	Tests classes, inheritance, and methods.
`unified/ql/test/library-tests/type-inference/basics.swift`	Tests foundational inference behavior.
`unified/ql/test/library-tests/dataflow/test.expected`	Updates dataflow expectations.
`unified/ql/test/library-tests/dataflow/calls.swift`	Adds resolved-call flow cases.
`unified/ql/test/library-tests/BasicTest/test.ql`	Filters built-in nodes from tests.
`unified/ql/lib/utils/test/TestUtils.qll`	Adds tuple and callable naming helpers.
`unified/ql/lib/utils/test/ExternalLocationPostProcessing.ql`	Normalizes external type locations.
`unified/ql/lib/qlpack.yml`	Adds the type-inference dependency.
`unified/ql/lib/codeql/unified/internal/typeinference/TypeMention.qll`	Models type mentions and aliases.
`unified/ql/lib/codeql/unified/internal/typeinference/TypeInferencePluginSwift.qll`	Moderate (4 votes): Binary expressions incorrectly inherit the common operand type, including comparison and equality expressions whose result should be `Bool`; result-changing operators must be handled separately without guessing for custom operators.
`unified/ql/lib/codeql/unified/internal/typeinference/TypeInferencePlugin.qll`	Defines language-plugin interfaces.
`unified/ql/lib/codeql/unified/internal/typeinference/TypeInferenceConsistency.qll`	Exposes inference consistency checks.
`unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll`	Moderate (1 vote): Function-valued field invocation remains a target when a direct callable member exists, creating incorrect call and dataflow edges; suppress that branch when a direct target is found.
`unified/ql/lib/codeql/unified/internal/typeinference/TypeAbstraction.qll`	Models generic type abstractions.
`unified/ql/lib/codeql/unified/internal/typeinference/Type.qll`	Moderate (1 vote): `TypeParameterType.getDeclaringItem()` only handles class-like declarations, omitting function and type-alias parameters; return the actual enclosing declaration.
`unified/ql/lib/codeql/unified/internal/FacadeAst.qll`	Adds tuple and closure helpers.
`unified/ql/lib/codeql/unified/internal/ExprPositionsPluginSwift.qll`	Adds Swift type-context handling.
`unified/ql/lib/codeql/unified/internal/ExprPositionsPlugin.qll`	Defines type-context plugin hooks.
`unified/ql/lib/codeql/unified/internal/ExprPositions.qll`	Expands type-context recognition.
`unified/ql/lib/codeql/unified/internal/dataflow/CallGraph.qll`	Moderate (1 vote): Overload-agnostic targets are wired directly into dataflow, permitting flow through unselected same-name overloads; require compatible or unique target filtering.
`unified/ql/consistency-queries/TypeInferenceConsistency.ql`	Registers inference diagnostics.
`shared/typeinference/codeql/typeinference/internal/TypeInference.qll`	Adjusts shared inference propagation.

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

asgerf

Just some observations so far. Will take another look on Monday.

It's quite hard to review the instantiation honestly. It's a collection of plausible-sounding AST class adapters. I have no idea how much of it works.

I'm a little worried about what it would take to support destructuring, as the Declaration interface seems unable to cope with it.

asgerf · 2026-09-25T10:02:31Z

+  or
  expr.getParent() instanceof TypeConstraint
  or
+  exists(Identifier id | id = NameBinding::getStaticBindingTarget(expr) |


getStaticBindingTarget only works if expr is an Identifier. We should also handle the MemberAccessExpr case here.

This happens several times in this PR so it's probably a good time to improve the interface with NameBinding to avoid this footgun. The idea with current interface is that it's purely a relation between Identifiers, ensuring there is never any ambiguity around which AST node you're supposed to give it. We used to get compilation errors if you compared it with Expr, but after loosing the AST schema that's not the case anymore, and clearly it's not as clear as I thought it was.

I'd suggest we expose these two predicates from NameBinding:

getStaticBindingTargetFromIdentifier: A renamed version of getStaticBindingTarget

getStaticBindingTargetFromRef: calls getStaticBindingTargetFromIdentifier(getIdentifierFromRef(x))

I don't know much it matters for this PR so it's OK to defer to another PR if you want.

asgerf · 2026-09-25T10:10:28Z

+) {
+  associatedTypeParameter(base, a, _) and
+  baseRef = c.getABaseType().getType() and
+  base.getNameNode() = getStaticBindingTarget(baseRef) and


Another case of getStaticBindingTarget being limited to identifiers

asgerf · 2026-09-25T11:20:14Z

+        tm = super.getCallee()
+        or
+        tm = super.getCallee().(MemberAccessExpr).getBase()


Won't this have multiple results when referring to a nested class, like A.B where both A and B are classes?

asgerf · 2026-09-25T11:21:31Z

+
+    int getNumberOfArguments() { result = CallExpr.super.getNumberOfArguments() + 1 }
+
+    Expr getArgument(int i) {


Named arguments are not supported? (where the call and callable don't declare them in the same order)

No, not presently.

asgerf · 2026-09-25T11:34:37Z

+    result = getFunctionExprReturnTypePath(getClosureType(c))
+  }
+
+  predicate stepLanguageSpecific(AstNode n1, TypePath prefix1, AstNode n2, TypePath prefix2) {


Does LanguageSpecific here mean "specific to this instantiation of the library", or would you expect this to differ between different languages inside unified?

It means specific to this instantiation of the library. TypeInferencePlugin.inferType provides the hook for language-specific logic.

asgerf · 2026-09-25T11:51:15Z

+  abstract predicate isEnumConstructor(
+    ClassLikeDeclaration enum, ConstructorDeclaration case, Identifier i
+  );


I'm not sure this belongs in the language plugin? What does "enum constructor" actually mean from type inference's perspective?

Something that can be destructured?

The enum is assumed to be a closed sum of its case constructors?

Yes, it will eventually mean something that can be destructured, but right now it means that the return type is not the type of the immediately enclosing class, but instead the class that represents the enum (i.e., Optional.some(0) has type Optional<Int> not some).

asgerf · 2026-09-25T11:55:03Z

+  );
+
+  /** Holds if `field` is an enum field of `enum`. */
+  abstract predicate isEnumField(ClassLikeDeclaration enum, VariableDeclaration field);


Same question as above. What does "enum field" actually mean? Does type inference need to distinguish between enum fields and static fields?

Same reason as below.

asgerf · 2026-09-25T12:00:04Z

+
+class TypeInferencePlugin extends Unit {
+  /** Gets the boolean type which will be assigned to boolean literals. */
+  abstract Type getBoolType();


Perhaps add a TODO comment here about the fact that argument-less predicates in a Unit type can't actually support multiple language plugins, as there is no way to get e.g. the bool type for a specific language.

I'm OK with not solving the problem right now.

asgerf · 2026-09-25T12:06:03Z

+  pragma[nomagic]
+  override FunctionDeclaration getFunctionInvoke(Type t) {
+    result = t.(FunctionType).getClassLikeDeclaration().getAMember() and
+    result.getName() = "invoke"


Suggested change

result.getName() = "invoke"

result.getName() = "callAsFunction"

It's a bit early to try to support callAsFunction in earnest (it's on the language feature backlog), but AFAICT callAsFunction is the correct name to use here.

The name is not really that important, what matters is that the synthetic function exists. But I can change it.

asgerf · 2026-09-25T12:09:37Z

  not exists(cls.getEnclosingClass()) and
  result = cls.getName()
  or
+  result = getStaticBindingTarget(cls.getExtensionTarget()).getValue()


Limited to identifiers

hvitved · 2026-09-25T12:47:55Z

It's quite hard to review the instantiation honestly. It's a collection of plausible-sounding AST class adapters. I have no idea how much of it works.

They should all work; it's only ParenExpr that is none().

I'm a little worried about what it would take to support destructuring, as the Declaration interface seems unable to cope with it.

Destructuring is not expected to work with the Declaration interface as-is. Rust currently has specific logic for this, which I plan to move into the shared library.

github-advanced-security Bot found potential problems Sep 21, 2026

View reviewed changes

github-actions Bot added Rust Pull requests that update Rust code Swift labels Sep 21, 2026

hvitved force-pushed the unified/type-inference branch 2 times, most recently from 9adf294 to bbc8b9a Compare September 21, 2026 12:58

github-actions Bot removed the Swift label Sep 21, 2026

hvitved force-pushed the unified/type-inference branch 4 times, most recently from e71b6f6 to 5886197 Compare September 22, 2026 09:58

github-advanced-security Bot found potential problems Sep 22, 2026

View reviewed changes

hvitved force-pushed the unified/type-inference branch from 5886197 to b2b2531 Compare September 22, 2026 10:04

github-actions Bot added the Swift label Sep 22, 2026

hvitved force-pushed the unified/type-inference branch 2 times, most recently from ab9a603 to 76692f4 Compare September 22, 2026 13:35

github-actions Bot removed the Swift label Sep 22, 2026

hvitved force-pushed the unified/type-inference branch from 76692f4 to c5db7b1 Compare September 22, 2026 14:38

hvitved changed the title ~~Unified: Basic type inference~~ Unified: Type inference first version Sep 22, 2026

github-advanced-security Bot found potential problems Sep 22, 2026

View reviewed changes

Comment thread unified/ql/lib/codeql/unified/internal/typeinference/TypeInference.qll Fixed

hvitved force-pushed the unified/type-inference branch 5 times, most recently from fe9c6eb to ebbac0e Compare September 23, 2026 12:08

github-advanced-security Bot found potential problems Sep 23, 2026

View reviewed changes

github-actions Bot removed the Rust Pull requests that update Rust code label Sep 23, 2026

hvitved force-pushed the unified/type-inference branch from ebbac0e to 228fe9c Compare September 23, 2026 13:46

This was referenced Sep 23, 2026

Unified: Handle GenericTypeExpr in static name binding #22665

Merged

Unified: Add LocalVariable.isImplicitReceiverParameter #22666

Merged

hvitved force-pushed the unified/type-inference branch from 228fe9c to a159ea5 Compare September 24, 2026 08:34

hvitved force-pushed the unified/type-inference branch 2 times, most recently from e58e192 to 8d76018 Compare September 24, 2026 11:03

hvitved requested a balanced review from Copilot September 24, 2026 11:04

Copilot started reviewing on behalf of hvitved September 24, 2026 11:05 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread unified/ql/lib/codeql/unified/internal/typeinference/TypeInferencePluginSwift.qll

hvitved added 2 commits September 24, 2026 16:50

Unified: Extend isInTypeContext

11e19d9

Unified: Add predicates to TupleExpr and FunctionExpr

cf3c54b

hvitved force-pushed the unified/type-inference branch from 8d76018 to 5fd9af6 Compare September 24, 2026 14:52

hvitved added the no-change-note-required This PR does not need a change note label Sep 25, 2026

hvitved marked this pull request as ready for review September 25, 2026 07:57

hvitved requested review from a team as code owners September 25, 2026 07:57

hvitved requested a review from asgerf September 25, 2026 07:59

hvitved force-pushed the unified/type-inference branch from 5fd9af6 to 0c4c11a Compare September 25, 2026 08:27

hvitved added 2 commits September 25, 2026 10:35

Unified: Type inference first version

2b030e2

Unified: Use call graph from type inference library in data flow

fa6d1e4

hvitved force-pushed the unified/type-inference branch from 0c4c11a to fa6d1e4 Compare September 25, 2026 08:36

asgerf reviewed Sep 25, 2026

View reviewed changes

Address review comments

467664b


		int getNumberOfArguments() { result = CallExpr.super.getNumberOfArguments() + 1 }

		Expr getArgument(int i) {

	result.getName() = "invoke"
	result.getName() = "callAsFunction"

Conversation

hvitved commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

asgerf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hvitved commented Sep 25, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hvitved commented Sep 21, 2026 •

edited

Loading