Skip to content

Unified: Type inference for nested types - #22866

Open
hvitved wants to merge 4 commits into
github:mainfrom
hvitved:unified/type-inference-nested-types
Open

hvitved wants to merge 4 commits into
github:mainfrom
hvitved:unified/type-inference-nested-types

Conversation

@hvitved

@hvitved hvitved commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

This PR adds support for nested types in type inference. Since nested types can interact with both generics and sub typing, we need a way to represent outer types in addition to the nested type itself:

class Outer<TOuter> {
    class Inner<TInner> {
    }
}

class SubOuter<TSubOuter> : Outer<TSubOuter?>

For example, we need to distinguish Outer<Int>.Inner<Int> from Outer<Bool>.Inner<Int>, as well as from SubOuter<Int>.Inner<Int>.

We do this by adding a pseudo type parameter to each nested type, which will hold type information about the outer type. For example, Outer<Int>.Inner<String> is represented as

TypePath Type
"" Inner
"TInner" String
"Inner_Parent" Outer
"Inner_Parent.TOuter" Int

where Inner_Parent is the special type parameter belonging to the nested class Inner.

Things can get further complicated when type aliases are used, which prompted a change to the existing TypeMention logic: Instead of resolving aliases directly in the getTypeAt relation, we split resolution logic into different phases, where the initial getUnexpandedTypeAt does not expand aliases, but instead treat them as ordinary types. Ultimately, in getTypeAt, aliases are fully expanded, and type inference still does not need to worry about them.

DCA is fine.

@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch from 2c69161 to b14b51a Compare October 21, 2026 13:57
@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch 6 times, most recently from 03284cb to 0455baf Compare October 22, 2026 09:30
@hvitved
hvitved requested a max review from Copilot October 22, 2026 09:30

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.

🟡 Changes recommended

Implicit argument handling and parent-scope checks still produce incorrect or incomplete nested-type inference.

2 open issues
What changed in this PR

Extends Unified’s type inference to represent nested types and preserve their enclosing generic arguments, with Swift regression coverage.

Changes:

  • Adds parent-type parameters and qualified type names.
  • Updates type mentions and enum construction to propagate parent-type paths.
  • Adds nested-type tests and updates inference expectations.
File Description
unified/​ql/​test/​library-tests/​type-inference/​type-inference.expected Updates expected inference results.
unified/​ql/​test/​library-tests/​type-inference/​nested_types.swift Adds nested-generic, self-type, and extension coverage.
unified/​ql/​test/​library-tests/​type-inference/​generics.swift Removes a resolved MISSING annotation.
unified/​ql/​test/​library-tests/​type-inference/​classes.swift Updates the inline expected nested-type name.
unified/​ql/​lib/​codeql/​unified/​internal/​typeinference/​TypeMention.qll Adds nested-type qualification and implicit argument handling.
unified/​ql/​lib/​codeql/​unified/​internal/​typeinference/​TypeInferencePluginSwift.qll Removes Swift-specific enum plugin hooks.
unified/​ql/​lib/​codeql/​unified/​internal/​typeinference/​TypeInferencePlugin.qll Removes enum-specific hooks and wrappers.
unified/​ql/​lib/​codeql/​unified/​internal/​typeinference/​TypeInference.qll Integrates parent paths into constructor inference.
unified/​ql/​lib/​codeql/​unified/​internal/​typeinference/​Type.qll Introduces parent-type parameters and qualified names.
unified/​ql/​lib/​codeql/​unified/​internal/​AstExtra.qll Adds shared enum relationships and enum-field modeling.

Review effort: Max

@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch from 0455baf to e55b569 Compare October 22, 2026 13:31
@hvitved
hvitved requested a max review from Copilot October 22, 2026 13:32

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.

🔵 Needs a closer look

Unresolved generic-inference regressions and changes to recursive type paths require fixes and human validation.

3 open issues
2 resolved since last review

Review effort: Max

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.

🔵 Needs a closer look

Recursive generic substitution and constructor propagation have unresolved correctness concerns and need expert validation.

2 open issues
3 resolved since last review

Review effort: Max

@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch 4 times, most recently from 606fac6 to fee52f9 Compare October 23, 2026 09:51
@hvitved
hvitved requested a max review from Copilot October 23, 2026 09:51

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.

@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch from fee52f9 to a014cb6 Compare October 23, 2026 12:24
@hvitved
hvitved requested a max review from Copilot October 23, 2026 12:25

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.

@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch 3 times, most recently from 9a2da2f to f27b5df Compare October 29, 2026 19:32
@github-actions github-actions Bot added the Rust Pull requests that update Rust code label Oct 29, 2026
@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch 2 times, most recently from 5ffdc62 to 16e479a Compare October 30, 2026 12:23
@github-actions github-actions Bot removed the Rust Pull requests that update Rust code label Oct 30, 2026
@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch from 16e479a to d259816 Compare October 30, 2026 12:34
@hvitved
hvitved requested a max review from Copilot October 30, 2026 12:51

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.

🟡 Changes recommended

Nested alias handling still loses qualifier information in forwarded references, generic arguments, and tuple elements, causing incorrect outer-type inference for qualified alias uses.

2 open issues
1 resolved since last review

Review effort: Max

@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch 2 times, most recently from af6d3b3 to cd773cb Compare October 30, 2026 14:17
@hvitved
hvitved requested a max review from Copilot October 30, 2026 14:31

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.

🟡 Changes recommended

The enum-field inference rule misses generic arguments beneath parent paths, preventing nested enum initializers from acquiring outer type arguments from their annotations.

1 open issue
2 resolved since last review

Review effort: Max

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.

🟡 Changes recommended

The QLDoc references a nonexistent parameter, and test annotations contain internal issue URLs that external contributors cannot access.

2 open issues
1 resolved since last review

Review effort: Max

@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch from 12f09e4 to 6475b06 Compare October 31, 2026 09:58
@hvitved
hvitved force-pushed the unified/type-inference-nested-types branch from 6475b06 to 6ca07df Compare November 2, 2026 08:48
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Nov 2, 2026
@hvitved
hvitved marked this pull request as ready for review November 2, 2026 11:08
private import codeql.unified.internal.ExprPositions

/**
* Holds if `alias` is an alias at `pathToNested` for nested type `nested` with

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.

The docs here are not that easy to follow. What does it means to be an alias "at" a given type path? Would it make sense to say that it contains an alias for nested at pathToNested?

Independently of that, I'd suggest adding type parameters on the Parent class in the example. At least for me it was easier to see the "why" behind this after seeing the example for getExpandedTypeAt where there are type parameters in play.

Lastly, I would have expected this logic to build on UnqualifiedMemberAccess and its getAccessingClass predicate, in order to treat unqualified type mentions in general as it they were qualified. For example, Nested in Parent<T> would be seen as Parent<T>.Nested. I'm not sure why aliases are special-cased to this degree, since unqualified type mentions can appear in many other places. Why is all this not just an extension of getTypeAt using UnqualifiedMemberAccess.getAccessingClass to populate chains of ParentTypeParameters leading to the accessing class?

@hvitved hvitved Nov 4, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The docs here are not that easy to follow. What does it means to be an alias "at" a given type path? Would it make sense to say that it contains an alias for nested at pathToNested?

I will try to make it clearer.

Independently of that, I'd suggest adding type parameters on the Parent class in the example. At least for me it was easier to see the "why" behind this after seeing the example for getExpandedTypeAt where there are type parameters in play.

Will do.

Lastly, I would have expected this logic to build on UnqualifiedMemberAccess and its getAccessingClass predicate, in order to treat unqualified type mentions in general as it they were qualified. For example, Nested in Parent<T> would be seen as Parent<T>.Nested. I'm not sure why aliases are special-cased to this degree, since unqualified type mentions can appear in many other places. Why is all this not just an extension of getTypeAt using UnqualifiedMemberAccess.getAccessingClass to populate chains of ParentTypeParameters leading to the accessing class?

I should perhaps use different terminology than qualified then: For example, if we add

typealias Unbound = Holder.InnerAlias

to the class Outer in the test example, then the RHS is qualified, but it doesn't tell us the parent type (in contrast, the existing Fixed alias already present in the test does pin the parent type). I'll add this test example.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have pushed an update. In the end I went with "orphaned" instead of "unqualified".

parent = nested.getEnclosingClassResolved() and
alias.getEnclosingClassResolved+() = parent and
pathParents = TypePath::singleton(TParentTypeParameterType(alias)) and
encl = alias.getEnclosingClassResolved()

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.

What happens when nested comes from a base class? Then the alias and its target don't have a common enclosing class.

class Base<T> {
  struct Nested { var x: T }
}
class Sub : Base<String> {
  typealias Element = Nested
}

let x: Sub.Element // --> Base<String>.Nested

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Though, instead of resolving Sub.Element to Base<String>.Nested it should resolve to Sub.Nested (and it would then be the job of sub typing to be able to match T in Base with String, which requires variance support).

Comment on lines +102 to +103
this.getSimpleExpandedTypeAt(prefix
.appendInverse(any(TypePath cons | cons.isCons(tp, suffix)))) and

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.

This is very hard to follow. Could we rewrite this to put the recursive call first with a variable as its argument, and bind prefix based on this?

This branch has not been deployed

No deployments
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.

4 participants