Skip to content

Rust: Replace special handling of index expressions in type inference#21698

Open
hvitved wants to merge 2 commits intogithub:mainfrom
hvitved:rust/type-inference-index-expr
Open

Rust: Replace special handling of index expressions in type inference#21698
hvitved wants to merge 2 commits intogithub:mainfrom
hvitved:rust/type-inference-index-expr

Conversation

@hvitved
Copy link
Copy Markdown
Contributor

@hvitved hvitved commented Apr 13, 2026

We have special logic for handling type inference for index expressions foo[bar] where bar is an integer and foo is either an array, a slice, or a vector; in this case the type is whatever the element type is.

Special casing is needed because, even though we correctly see foo[bar] as syntactic sugar for foo.index(bar), we cannot resolve the Output types in the Index implementations for slices, arrays, and vectors. However, instead of having special logic in the type inference library, we can achieve the same effect by adding type-specialized implementations where we can resolve the Output type, which is somewhat cleaner.

DCA is mostly uneventful.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Apr 13, 2026
@hvitved hvitved force-pushed the rust/type-inference-index-expr branch from be02d9a to 40eff65 Compare April 13, 2026 08:30
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Apr 14, 2026
@hvitved hvitved marked this pull request as ready for review April 14, 2026 13:25
@hvitved hvitved requested a review from a team as a code owner April 14, 2026 13:25
Copilot AI review requested due to automatic review settings April 14, 2026 13:25
Copy link
Copy Markdown
Contributor

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 aims to remove bespoke type-inference handling for Rust index expressions (foo[bar]) by instead supplying type-specialized builtin Index impls whose Output types are directly resolvable, allowing the existing method-resolution-based inference to determine element types.

Changes:

  • Add builtin, type-specialized std::ops::Index impls for arrays, slices, and Vec for usize and i32 indices.
  • Remove the dedicated inferIndexExprType special-casing from the type inference library and rely on overload/method resolution instead.
  • Introduce/adjust QL classifications and sibling-impl logic to recognize builtin impls for resolution purposes.
Show a summary per file
File Description
rust/tools/builtins/impls.rs Adds specialized builtin Index impl stubs to make Output resolvable for common indexing targets.
rust/ql/lib/codeql/rust/internal/typeinference/TypeInference.qll Removes the previous index-expression type inference special-case and updates call target handling around builtin impls.
rust/ql/lib/codeql/rust/internal/typeinference/FunctionOverloading.qll Adjusts overload/sibling-impl logic with builtin-aware handling related to Index’s Output.
rust/ql/lib/codeql/rust/frameworks/stdlib/Builtins.qll Adds identification of builtin impls via the new impls.rs builtins file.

Copilot's findings

Comments suppressed due to low confidence (2)

rust/tools/builtins/impls.rs:69

  • These builtins add Index<...> for slices ([T]), but mutable indexing (slice[i] = ...) desugars to IndexMut::index_mut. If the generic std IndexMut impl’s Output remains unresolvable, mutable slice indexing will still miss element type inference. Add IndexMut<i32>/IndexMut<usize> impls for [T] with type Output = T.
    impl<T> Index<i32> for [T] {
        type Output = T;

        fn index(&self, index: i32) -> &Self::Output {
            panic!()
        }
    }

    impl<T> Index<usize> for [T] {
        type Output = T;

        fn index(&self, index: usize) -> &Self::Output {
            panic!()
        }
    }

rust/tools/builtins/impls.rs:85

  • For Vec, only Index<...> is stubbed here. Since vec[i] = ... uses IndexMut::index_mut, consider adding IndexMut<i32>/IndexMut<usize> impls for Vec<T, A> (with type Output = T) so mutable vector indexing can infer the element type as well.
    impl<T, A: Allocator> Index<i32> for Vec<T, A> {
        type Output = T;

        fn index(&self, index: i32) -> &Self::Output {
            panic!()
        }
    }

    impl<T, A: Allocator> Index<usize> for Vec<T, A> {
        type Output = T;

        fn index(&self, index: usize) -> &Self::Output {
            panic!()
        }
    }
  • Files reviewed: 4/4 changed files
  • Comments generated: 1

Comment on lines +39 to +53
impl<T, const N: usize> Index<i32> for [T; N] {
type Output = T;

fn index(&self, index: i32) -> &Self::Output {
panic!()
}
}

impl<T, const N: usize> Index<usize> for [T; N] {
type Output = T;

fn index(&self, index: usize) -> &Self::Output {
panic!()
}
}
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Type inference resolves a[b] to Index::index or IndexMut::index_mut based on whether the index expression is used mutably (e.g. a[b] = ...). These builtins only add Index<...> for arrays, so mutable array indexing may still lack a resolvable Output type. Add corresponding IndexMut<i32>/IndexMut<usize> impls for [T; N] with type Output = T.

This issue also appears in the following locations of the same file:

  • line 55
  • line 71

Copilot uses AI. Check for mistakes.
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.

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