-
Notifications
You must be signed in to change notification settings - Fork 163
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
Add initial support for unsized method resolution #1045
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In order to support slices, we end up with an operator overload call of: ``` impl<T, I> Index<I> for [T] where I: SliceIndex<[T]>, { type Output = I::Output; fn index(&self, index: I) -> &I::Output { index.index(self) } } ``` So this means the self in this case is an array[T,capacity] and the index parameter is of type Range<usize>. In order to actually call this method which has a self parameter of [T] we need to be able to 'unsize' the array into a slice. Addresses #849
CohenArthur
approved these changes
Mar 21, 2022
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.
This looks good and is readable, but I guess I am misunderstanding its usage. I thought this could help in allowing the following kind of code to work, but I gues not?
fn takes_slice(a: &[u8]) {}
fn main() {
let slice = &[1u8, 2u8];
takes_slice(slice);
}
bors try |
tryBuild succeeded: |
bors r+ |
Build succeeded:
|
philberty
added a commit
that referenced
this pull request
Apr 9, 2022
This is unfortunatly a mega commit, in testing gccrs against the slice code which is highly generic stress tested our implementation of generics and poked the hole in or lack of support of generic higher ranked trait bounds and more specificily generic associated types. More refactoring is needed to eventually remove the setup_associated_types and replace it entirely with this new setup_associated_types2 which takes into account the trait bound receiver and its predicate. In order to support slices, the code in libcore defines an index lang item ```rust impl<T, I> Index<I> for [T] where I: SliceIndex<[T]>, { type Output = I::Output; fn index(&self, index: I) -> &I::Output { index.index(self) } } ``` This is the entry point where by the self here is a generic slice. So in our case we have: ```rust let a = [1, 2, 3, 4, 5]; let b = &a[1..3]; ``` 'a' is an array and b is our desired slice, so we must remember that from algebraic data type constructor. But our receiver is still an array, so in order to be able to call this index lang item we must 'unsize' our array (see #1045) this allows for method resolution to adjust an array into a FatPtr which is simply a struct containing reference to the array and the capacity (GCC MAX_DOMAIN) of the underlying array data type. So now we are able to infer the substituions for this index fn call to: ``` fn index(&self : [<integer>], index: Range<integer>) -> &I::Output->placeholder ``` The complex piece here is the Higher ranked trait bound: ``` where I: SliceIndex<[T]> ``` So in this method call no generic arguments are specified so we must try and infer the types. So during monomorphization the inference variables need to be recursively propogated into the higher ranked trait bound. So that the higher ranked trait bound looks like: ``` SliceIndex<[<integer>]> // like we seen earlier for the Self type ``` The monomorphization stage also needs to take into account the higher ranked trait bound's type which is 'I' and infered to be: Range<integer>. This is where specialization needs to occur. ```rust unsafe impl<T> SliceIndex<[T]> for Range<usize> { type Output = [T]; unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { unsafe { let a: *const T = slice.as_ptr(); let b: *const T = a.add(self.start); slice_from_raw_parts(b, self.end - self.start) } } fn index(self, slice: &[T]) -> &[T] { unsafe { &*self.get_unchecked(slice) } } } ``` So now we need to compute the constrained type-parameters for this specialized impl block. And in this case is fairly simple: ``` impl<T> SliceIndex<[T]> for Range<usize> vs I: SliceIndex<[<integer>]> and Range<<integer>> ``` Here we need to compute that T is <integer>, which is required since associated type Output is used in our original method call and this is generic which requires us to set it up but both the Self type or the trait bound here in this impl block could be generic so special care needs to be taken to compute this safely. Once the constrained types are computer we can also unify the Self types which specializes our original Range<integer> type into the correct Range<usize> that this trait bound expects. We used a callback here when we reusively pass down the SubstitutionArgumentMappings when any Parameter type is substitued we get a callback to hold a set of mappings in a generic way what generic types are being substituted. From all of this work this stressed our generics implementation to breaking point due to the use of the generic trait bound which was not supported and it also exposed many bugs in our implementation. This is why I feel it is best to keep this a large patch as so much of this patch will cause regressions if we don't keep it together. One of the main changes we have made is how we handle parameters substitution for example we might have a generic such as '&Y' but this gets substituted with Y=T which is a new type parameter. Before we used to directly just change this from &Y to &T which is correct but this looses context from the generic argument bindings. So now we maintain the information that &Y changes to &(Y=T) so that we see Y was substutued with T so that subsequent substitutions or inferences can change Y=?T and correctly map &Y to &(Y=T) to &(Y=?T). The other major piece which was changed during this patch was how we perform the method resolution on higher ranked trait bound calls where we compute the specified bound possible candidates once so that in the case: ``` trait Bar { fn baz(&self) } fn <T:Bar> foo(a: &T) { a.baz() } ``` Here the type parameter T gets derefed to find the specified bound of Bar which contains the method baz. This means that we try calling baz with T vs &T which fails then we try the reference type T again. This results into two useless adjustments of indirection and referencing but GCC optimizes this away. Before this patch we computed the specified bound for each attempt which was wrong. Fixes #849
philberty
added a commit
that referenced
this pull request
Apr 11, 2022
This is unfortunatly a mega commit, in testing gccrs against the slice code which is highly generic stress tested our implementation of generics and poked the hole in or lack of support of generic higher ranked trait bounds and more specificily generic associated types. More refactoring is needed to eventually remove the setup_associated_types and replace it entirely with this new setup_associated_types2 which takes into account the trait bound receiver and its predicate. In order to support slices, the code in libcore defines an index lang item ```rust impl<T, I> Index<I> for [T] where I: SliceIndex<[T]>, { type Output = I::Output; fn index(&self, index: I) -> &I::Output { index.index(self) } } ``` This is the entry point where by the self here is a generic slice. So in our case we have: ```rust let a = [1, 2, 3, 4, 5]; let b = &a[1..3]; ``` 'a' is an array and b is our desired slice, so we must remember that from algebraic data type constructor. But our receiver is still an array, so in order to be able to call this index lang item we must 'unsize' our array (see #1045) this allows for method resolution to adjust an array into a FatPtr which is simply a struct containing reference to the array and the capacity (GCC MAX_DOMAIN) of the underlying array data type. So now we are able to infer the substituions for this index fn call to: ``` fn index(&self : [<integer>], index: Range<integer>) -> &I::Output->placeholder ``` The complex piece here is the Higher ranked trait bound: ``` where I: SliceIndex<[T]> ``` So in this method call no generic arguments are specified so we must try and infer the types. So during monomorphization the inference variables need to be recursively propogated into the higher ranked trait bound. So that the higher ranked trait bound looks like: ``` SliceIndex<[<integer>]> // like we seen earlier for the Self type ``` The monomorphization stage also needs to take into account the higher ranked trait bound's type which is 'I' and infered to be: Range<integer>. This is where specialization needs to occur. ```rust unsafe impl<T> SliceIndex<[T]> for Range<usize> { type Output = [T]; unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { unsafe { let a: *const T = slice.as_ptr(); let b: *const T = a.add(self.start); slice_from_raw_parts(b, self.end - self.start) } } fn index(self, slice: &[T]) -> &[T] { unsafe { &*self.get_unchecked(slice) } } } ``` So now we need to compute the constrained type-parameters for this specialized impl block. And in this case is fairly simple: ``` impl<T> SliceIndex<[T]> for Range<usize> vs I: SliceIndex<[<integer>]> and Range<<integer>> ``` Here we need to compute that T is <integer>, which is required since associated type Output is used in our original method call and this is generic which requires us to set it up but both the Self type or the trait bound here in this impl block could be generic so special care needs to be taken to compute this safely. Once the constrained types are computer we can also unify the Self types which specializes our original Range<integer> type into the correct Range<usize> that this trait bound expects. We used a callback here when we reusively pass down the SubstitutionArgumentMappings when any Parameter type is substitued we get a callback to hold a set of mappings in a generic way what generic types are being substituted. From all of this work this stressed our generics implementation to breaking point due to the use of the generic trait bound which was not supported and it also exposed many bugs in our implementation. This is why I feel it is best to keep this a large patch as so much of this patch will cause regressions if we don't keep it together. One of the main changes we have made is how we handle parameters substitution for example we might have a generic such as '&Y' but this gets substituted with Y=T which is a new type parameter. Before we used to directly just change this from &Y to &T which is correct but this looses context from the generic argument bindings. So now we maintain the information that &Y changes to &(Y=T) so that we see Y was substutued with T so that subsequent substitutions or inferences can change Y=?T and correctly map &Y to &(Y=T) to &(Y=?T). The other major piece which was changed during this patch was how we perform the method resolution on higher ranked trait bound calls where we compute the specified bound possible candidates once so that in the case: ``` trait Bar { fn baz(&self) } fn <T:Bar> foo(a: &T) { a.baz() } ``` Here the type parameter T gets derefed to find the specified bound of Bar which contains the method baz. This means that we try calling baz with T vs &T which fails then we try the reference type T again. This results into two useless adjustments of indirection and referencing but GCC optimizes this away. Before this patch we computed the specified bound for each attempt which was wrong. Fixes #849
philberty
added a commit
that referenced
this pull request
Apr 11, 2022
This is unfortunatly a mega commit, in testing gccrs against the slice code which is highly generic stress tested our implementation of generics and poked the hole in or lack of support of generic higher ranked trait bounds and more specificily generic associated types. More refactoring is needed to eventually remove the setup_associated_types and replace it entirely with this new setup_associated_types2 which takes into account the trait bound receiver and its predicate. In order to support slices, the code in libcore defines an index lang item ```rust impl<T, I> Index<I> for [T] where I: SliceIndex<[T]>, { type Output = I::Output; fn index(&self, index: I) -> &I::Output { index.index(self) } } ``` This is the entry point where by the self here is a generic slice. So in our case we have: ```rust let a = [1, 2, 3, 4, 5]; let b = &a[1..3]; ``` 'a' is an array and b is our desired slice, so we must remember that from algebraic data type constructor. But our receiver is still an array, so in order to be able to call this index lang item we must 'unsize' our array (see #1045) this allows for method resolution to adjust an array into a FatPtr which is simply a struct containing reference to the array and the capacity (GCC MAX_DOMAIN) of the underlying array data type. So now we are able to infer the substituions for this index fn call to: ``` fn index(&self : [<integer>], index: Range<integer>) -> &I::Output->placeholder ``` The complex piece here is the Higher ranked trait bound: ``` where I: SliceIndex<[T]> ``` So in this method call no generic arguments are specified so we must try and infer the types. So during monomorphization the inference variables need to be recursively propogated into the higher ranked trait bound. So that the higher ranked trait bound looks like: ``` SliceIndex<[<integer>]> // like we seen earlier for the Self type ``` The monomorphization stage also needs to take into account the higher ranked trait bound's type which is 'I' and infered to be: Range<integer>. This is where specialization needs to occur. ```rust unsafe impl<T> SliceIndex<[T]> for Range<usize> { type Output = [T]; unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] { unsafe { let a: *const T = slice.as_ptr(); let b: *const T = a.add(self.start); slice_from_raw_parts(b, self.end - self.start) } } fn index(self, slice: &[T]) -> &[T] { unsafe { &*self.get_unchecked(slice) } } } ``` So now we need to compute the constrained type-parameters for this specialized impl block. And in this case is fairly simple: ``` impl<T> SliceIndex<[T]> for Range<usize> vs I: SliceIndex<[<integer>]> and Range<<integer>> ``` Here we need to compute that T is <integer>, which is required since associated type Output is used in our original method call and this is generic which requires us to set it up but both the Self type or the trait bound here in this impl block could be generic so special care needs to be taken to compute this safely. Once the constrained types are computer we can also unify the Self types which specializes our original Range<integer> type into the correct Range<usize> that this trait bound expects. We used a callback here when we reusively pass down the SubstitutionArgumentMappings when any Parameter type is substitued we get a callback to hold a set of mappings in a generic way what generic types are being substituted. From all of this work this stressed our generics implementation to breaking point due to the use of the generic trait bound which was not supported and it also exposed many bugs in our implementation. This is why I feel it is best to keep this a large patch as so much of this patch will cause regressions if we don't keep it together. One of the main changes we have made is how we handle parameters substitution for example we might have a generic such as '&Y' but this gets substituted with Y=T which is a new type parameter. Before we used to directly just change this from &Y to &T which is correct but this looses context from the generic argument bindings. So now we maintain the information that &Y changes to &(Y=T) so that we see Y was substutued with T so that subsequent substitutions or inferences can change Y=?T and correctly map &Y to &(Y=T) to &(Y=?T). The other major piece which was changed during this patch was how we perform the method resolution on higher ranked trait bound calls where we compute the specified bound possible candidates once so that in the case: ``` trait Bar { fn baz(&self) } fn <T:Bar> foo(a: &T) { a.baz() } ``` Here the type parameter T gets derefed to find the specified bound of Bar which contains the method baz. This means that we try calling baz with T vs &T which fails then we try the reference type T again. This results into two useless adjustments of indirection and referencing but GCC optimizes this away. Before this patch we computed the specified bound for each attempt which was wrong. Fixes #849
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to support slices, we end up with an operator overload call of:
So this means the self, in this case, is an array[T,capacity] and the index parameter is of type Range. In order to actually call this method
which has a self parameter of [T] we need to be able to 'unsize' the array
into a slice.
Addresses #849