Skip to content

Rust: Type inference for non-overloadable operators #19549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 21, 2025

Conversation

paldepind
Copy link
Contributor

This PR adds type inference for all non-overloadable operators in Rust. It turns out that there are only three such operators. && and || (which are not overloadable for short-circuiting reasons) and =.

The assignment operator evaluates to unit, and as we didn't have a unit type I added one.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label May 21, 2025
@paldepind paldepind marked this pull request as ready for review May 21, 2025 11:23
@Copilot Copilot AI review requested due to automatic review settings May 21, 2025 11:23
@paldepind paldepind requested a review from a team as a code owner May 21, 2025 11:23
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

Adds type inference support for Rust’s built-in logical (&&, ||) and assignment (=) operators by introducing a unit type.

  • Implement inference rules for && and || returning bool
  • Infer assignment expressions as () and track LHS types
  • Define TUnit() in the type system and a UnitType class

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 Updated expected results to cover &&, `
rust/ql/test/library-tests/type-inference/main.rs Added operators module with tests for logical and assignment operators
rust/ql/lib/codeql/rust/internal/TypeInference.qll Imported builtins and added inferLogicalOperationType and inferAssignmentOperationType rules
rust/ql/lib/codeql/rust/internal/Type.qll Extended TType with TUnit() and implemented a UnitType class
Comments suppressed due to low confidence (3)

rust/ql/lib/codeql/rust/internal/TypeInference.qll:10

  • Typo in module path: Bultins should be Builtins to correctly import the stdlib Builtins module.
private import codeql.rust.frameworks.stdlib.Bultins as Builtins

rust/ql/lib/codeql/rust/internal/TypeInference.qll:257

  • In typeEquality, the QL class AssignmentExpr is used but elsewhere AssignmentOperation is referenced; use a consistent AST class (likely AssignmentOperation) to ensure coverage.
exists(AssignmentExpr be |

rust/ql/lib/codeql/rust/internal/Type.qll:12

  • [nitpick] Consider adding a brief comment next to TUnit() in the newtype TType declaration to explain its role representing the unit () type.
TUnit() or

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.

LGTM. I have started a DCA run.

@paldepind
Copy link
Contributor Author

DCA is looking good. Thanks for starting it.

@paldepind paldepind merged commit 1828d40 into github:main May 21, 2025
17 checks passed
@paldepind paldepind deleted the rust/type-inference-operators branch May 21, 2025 15:25
@paldepind paldepind restored the rust/type-inference-operators branch May 22, 2025 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants