Skip to content

Conversation

paldepind
Copy link
Contributor

@paldepind paldepind commented Sep 10, 2025

Adds certain type information for struct expressions.

DCA seems fine.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Sep 10, 2025
@paldepind paldepind force-pushed the rust/certain-struct-expr branch from 3da56ad to eea7006 Compare September 11, 2025 07:54
@paldepind paldepind marked this pull request as ready for review September 11, 2025 09:57
@paldepind paldepind requested a review from a team as a code owner September 11, 2025 09:57
@Copilot Copilot AI review requested due to automatic review settings September 11, 2025 09:57
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds type inference for struct expressions in Rust's type inference system. The changes improve the certainty of type information for struct expressions by allowing the system to infer types directly from struct constructors.

Key changes:

  • Added type inference capability for struct expressions that resolves types from their paths
  • Enhanced struct expression handling to provide type arguments from inferred types
  • Updated test expectations to reflect improved certainty levels for struct-related type inference

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
rust/ql/test/library-tests/type-inference/type-inference.expected Test expectation updates reflecting removed type information entries for struct expressions
rust/ql/test/library-tests/type-inference/pattern_matching.rs Test comment updates changing from type= to certainType= annotations for variables
rust/ql/test/library-tests/type-inference/main.rs Test comment updates changing from type= to certainType= annotations for struct-related variables
rust/ql/lib/codeql/rust/internal/TypeInference.qll Core implementation adding struct expression type inference functionality

exists(TypePath suffix |
suffix.isCons(TTypeParamTypeParameter(apos.asTypeParam()), path) and
result = CertainTypeInference::inferCertainType(this, suffix)
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Implementing this should prune some predicates thanks to the not exists(getTypeArgument(a, target, tp, _)) checks in the shared library. Using inferCertainType will include both explicit types in the path to the struct as well as other certain ways to infer the type parameter.

| dereference.rs:122:23:122:29 | &... | &T.&T | dereference.rs:99:5:100:21 | Key |
| dereference.rs:122:24:122:29 | Key {...} | | dereference.rs:99:5:100:21 | Key |
| dereference.rs:122:24:122:29 | Key {...} | | file://:0:0:0:0 | & |
| dereference.rs:122:24:122:29 | Key {...} | &T | dereference.rs:99:5:100:21 | Key |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These and the removed types below are spurious, so this is an improvement.

@paldepind paldepind added the no-change-note-required This PR does not need a change note label Sep 11, 2025
hvitved
hvitved previously approved these changes Sep 11, 2025
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Would it make sense to do this for struct patterns on this PR as well?

@paldepind
Copy link
Contributor Author

Would it make sense to do this for struct patterns on this PR as well?

Yes, that makes sense. I've done that and kicked off another DCA run.

@paldepind paldepind merged commit fd5b5ba into github:main Sep 12, 2025
19 checks passed
@paldepind paldepind deleted the rust/certain-struct-expr branch September 12, 2025 08:28
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 Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants