Fix leaking bounds #24
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
We'll want this in a moment. No functional chnage.
* Make the internal wrapper type have *all* the parent's generics, and contain a PhantomData, so we can: * Put the bounds on the Debug impl for the internal wrapper type. * Rename the internal wrapper type to avoid name clashes with users' types. * Rename the formatter variable to avoid name clashes with users's values.
Rather than trying to filter the generic parameters for ones used in types, state the bounds in terms of the actual field types. This is precisely correct. It avoids lossage in situations where the field type's implementation of the trait has bounds which depend in a less-than-straightforward way on its generics. There is still an issue with implementing traits which have supertraits. We ought to apply a `Self: Super` bound, but we don't yet.
When we are implementing traits which have supertraits, add bounds of the form `where Self: Super` for each supertrait. While we're touching every call site, abolish the unused parameters. (I have future plans that will want `params` in into_where_predicates_by_generic_parameters_check_types, but not in create_where_predicates_from_generic_parameters_check_types, so leave that one there for now.)
Use the new precise bounds function.
Use the new precise bounds function, rather than the ident-hunter. Apply the Clone supertrait explicitly. This disposes of the oddity mentioned in the comment, as will be demonstrated in a test.
Use the new precise bounds function.
This is no longer used, now that everything uses the precise bounds function.
Nothing calls this. And, nothing should, because it would give wrong answers. Rename create_where_predicates_from_generic_parameters to create_where_predicates_from_all_generic_parameters. This will be useful if we implement a `bound(...)` syntax that allows requesting the "apply a bound for each generic parameter, regardless" mode. If we don't want to implement such a mode, we should delete this function, but let's not entangle this already large bugfix MR with questions about future features and UI.
Replace modification of `ast.generics` with cloning, and then modification. (the Into impl already does things this way.) Every one of the 18 hunks is identical. Fixes magiclen#23
This just invites bugs.
Fails when magiclen#23 is unfixed, passes now.
|
Hi. Sorry, and I don't mean to pressure you, but: Do you expect to review and maybe merge my MRs any time soon? We (Arti) are still on educe 0.4.x, because of these issues. |
|
@magiclen are you available to help with this? may you share rights to merge things to other devs? |
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.
Fixes #23.
The branch for this is on top of my MR #19, which fixes other problems with trait bounds, #17.