-
Notifications
You must be signed in to change notification settings - Fork 165
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
Rewrite our unconstrained type-param error checking #1030
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
…onRef This means TypeBoundPredicate will now be able to inherit all behaviours of normal generics so we do not duplicate the work in handling generics it will also allow us to more easily check for unconstrained type parameters on traits.
The TraitReference wrapper is a class that allows us to work with traits in a query manar. This patch allows us to keep track of the substitution mappings that are defined on the trait. This will always be non-empty since traits always contain an implicit Self type parameter so there is special handling in how we perform monomorphization.
This will allow us to reuse our generic substitions code to manage generic traits and their substitions better. It will unify the handling in one path so we get the same error handling.
This patch removes our old method of checking for unconstrained type parameters which only worked for the basic cases such as: impl<T> Foo { } But checking for unconstrained types is more complex, we need to handle covariant types such as: impl<T> *T { } Or struct foo<X,Y>(X,Y); impl<T> foo<&T,*T> {} This rewrites the algorithm to take advantage of our substition abstractions and HirIds so we can map the ids of the type-params to be constrained and look at the trait-references used-arguments when the generics are applied (or they may be empty) and then do the same for any used arguments on an algebraic data type. Fixes #1019
CohenArthur
approved these changes
Mar 17, 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.
LGTM!
bors r+ |
Build succeeded: |
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.
This is a series of patches that were all required to fix this issue. We
now take advantage of our substitutions abstractions and traits
so that our TypeBoundPredicate's which form the basis of our HRTB code
I think this class is almost akin to rustc existential-trait-references. This now
reuses the same code path to give us the same error checking for generics
as we get with ADT's, functions etc.
With this refactoring in place we can then reuse the abstractions to map the
ID's from the used arguments in the type-bound-predicate, the impl block type
substation mappings and the self type itself.
There are quite a few cases to handle and our testsuite picked up all the regressions
so no behaviour of our existing test-cases have changed now. See each commit for
more detailed information.
Fixes #1019
Addresses #849