-
Notifications
You must be signed in to change notification settings - Fork 2k
Rust: Replace special handling of index expressions in type inference #21698
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| /// Contains type-specialized versions of | ||
| /// | ||
| /// ``` | ||
| /// impl<T, I, const N: usize> Index<I> for [T; N] | ||
| /// where | ||
| /// [T]: Index<I>, | ||
| /// { | ||
| /// type Output = <[T] as Index<I>>::Output; | ||
| /// ... | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// and | ||
| /// | ||
| /// ``` | ||
| /// impl<T, I> ops::Index<I> for [T] | ||
| /// where | ||
| /// I: SliceIndex<[T]>, | ||
| /// { | ||
| /// type Output = I::Output; | ||
| /// ... | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// and | ||
| /// ``` | ||
| /// impl<T, I: SliceIndex<[T]>, A: Allocator> Index<I> for Vec<T, A> { | ||
| /// type Output = I::Output; | ||
| /// ... | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// (as well as their `IndexMut` counterparts), which the type inference library | ||
| /// cannot currently handle (we fail to resolve the `Output` types). | ||
| mod index_impls { | ||
| use std::alloc::Allocator; | ||
| use std::ops::Index; | ||
|
|
||
| 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> IndexMut<i32> for [T; N] { | ||
| type Output = T; | ||
|
|
||
| fn index_mut(&mut self, index: i32) -> &mut Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| impl<T, const N: usize> Index<usize> for [T; N] { | ||
| type Output = T; | ||
|
|
||
| fn index(&self, index: usize) -> &Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
Comment on lines
+39
to
+61
|
||
|
|
||
| impl<T, const N: usize> IndexMut<usize> for [T; N] { | ||
| type Output = T; | ||
|
|
||
| fn index_mut(&mut self, index: usize) -> &mut Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| impl<T> Index<i32> for [T] { | ||
| type Output = T; | ||
|
|
||
| fn index(&self, index: i32) -> &Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| impl<T> IndexMut<i32> for [T] { | ||
| type Output = T; | ||
|
|
||
| fn index_mut(&mut self, index: i32) -> &mut Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| impl<T> Index<usize> for [T] { | ||
| type Output = T; | ||
|
|
||
| fn index(&self, index: usize) -> &Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| impl<T> IndexMut<usize> for [T] { | ||
| type Output = T; | ||
|
|
||
| fn index_mut(&mut self, index: usize) -> &mut Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| 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> IndexMut<i32> for Vec<T, A> { | ||
| type Output = T; | ||
|
|
||
| fn index_mut(&mut self, index: i32) -> &mut Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| impl<T, A: Allocator> Index<usize> for Vec<T, A> { | ||
| type Output = T; | ||
|
|
||
| fn index(&self, index: usize) -> &Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
|
|
||
| impl<T, A: Allocator> IndexMut<usize> for Vec<T, A> { | ||
| type Output = T; | ||
|
|
||
| fn index_mut(&mut self, index: usize) -> &mut Self::Output { | ||
| panic!() | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see how much of this I understand: we remove the existing approximation of type inference for indexing, and replaces it with a file
impls.rswhich defines pseudo-implementations for the builtin indexing operations. Presumably we include that file (i.e. the contents oftools/builtins) in all Rust databases somehow, so that type inference can see those implementations and make type inferences from them. The piece of CodeQL above ... disambiguates the true builtin and theimpls.rsversions somehow???There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
It is needed to ensure that the type-specialized implementations only apply when the indexer expression is of type
i32orusize; once I get #21703 ready, the above should hopefully no longer be needed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen with older databases that don't have the data from
impls.rsincluded in them? Do we need upgrade / downgrade scripts or can we get away without?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately there is no easy way to write upgrade/downgrade scripts for this kind of change, so there will be a slight loss of type information for upgraded DBs, but I doubt it will matter too much in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The worst case would be inferior type deduction near indexing operations.