-
Notifications
You must be signed in to change notification settings - Fork 214
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
feat: Derive Eq & Default #5667
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
jfecher
commented
Aug 1, 2024
jfecher
commented
Aug 1, 2024
jfecher
commented
Aug 1, 2024
vezenovm
approved these changes
Aug 1, 2024
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.
Lets goo!
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 2, 2024
# Description ## Problem\* Resolves an issue alluded to in #5667 where we still could not derive generic types ## Summary\* This PR allows us to derive generic types by allowing a new "resolved generic" type in the UnresolvedGeneric enum. The issue before was that when we splice `Type`s into quoted snippets we lower them as a special token kind which preserves the original `Type` so that we don't have to worry about re-resolving it if it is a struct type that is not imported into the caller's scope, etc. This also means though that any type variables in the original type are preserved. In the following snippet: ```rs // Lets say this is `MyType<T>` let my_type: Type = ...; quote { impl<T> Foo for $my_type { ... } } ``` The impl will introduce a fresh generic `T` into scope, but the `T` in `MyType<T>` will still be the old `T` with a different TypeVariableId. To fix this there were two options: - Add another function to lower types in a way they'd need to be reparsed, and would thus pick up new type variables but be susceptible to "name not in scope" errors mentioned earlier. - Somehow get the `T` in `impl<T>` to refer to the same `T` in `MyType<T>`. I chose the later approach since it is much simpler from a user's perspective and leads to fewer footguns over forgetting to convert the type into a `Quoted` before splicing it into other code. This way, users just need to ensure the generics inserted as part of the impl generics come from the type which is what is typically done anyway. This means that the original snippet above will actually still fail since it still introduces a new `T`. To fix it, we have to get the T from the type's generics: ```rs let generics = my_struct.generics().map(|g| quote { $g }).join(quote {,}); let my_type = my_struct.as_type(); quote { impl<$generics> Foo for $my_type { ... } } ``` This is what `derive` does already since you don't know the names of the generics ahead of time in practice. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Michael J Klein <[email protected]>
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.
Description
Problem*
Resolves #4594
First traits to derive in the stdlib!
Summary*
Lets us derive the
Eq
andDefault
traits for almost any type.I anticipate there will be issues with deriving generic types. I'll fix this in another PR.
Additional Context
Since derive is in the stdlib and we can derive traits now, I'm arbitrarily marking the epic for metaprogramming as completed. There are still bugs to fix and many helper functions we can add but the bulk of the work and infrastructure is all in.
Documentation*
Check one:
PR Checklist*
cargo fmt
on default settings.