Skip to content
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.

Sign up for GitHub

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

Jump to bottom

More flexible Selectable derive #2962

Merged
merged 1 commit into from
Nov 22, 2021
Merged

More flexible Selectable derive #2962

merged 1 commit into from
Nov 22, 2021

Conversation

Ten0
Copy link
Member

@Ten0 Ten0 commented Nov 21, 2021

Resolves #2950

Previously, if fields had embed, the Selectable derive supposed that they implemented Selectable<DB> for every DB: Backend.

If that wasn't the case (generic subfield or backend-specific impl) the derive would fail.

This commit fixes this issue by introducing the appropriate additional where bounds.

@Ten0
More flexible Selectable derive
24c7a83 
Resolves diesel-rs#2950

Previously, if fields had `embed`, the `Selectable` derive supposed that
they implemented `Selectable<DB>` for every `DB: Backend`.

If that wasn't the case (generic subfield or backend-specific impl) the
derive would fail.

This commit fixes this issue by introducing the appropriate additional
`where` bounds.
@Ten0 Ten0 requested a review from weiznich November 21, 2021 17:55
@Ten0 Ten0 self-assigned this Nov 21, 2021
@weiznich weiznich requested a review from a team November 21, 2021 20:58
pksunkara
pksunkara reviewed Nov 21, 2021
View reviewed changes
Copy link
Member

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this work when the embed is on a non-generic struct? I don't think rust supports struct: trait syntax in where, right?

@weiznich
Copy link
Member

@pksunkara This does work for non-generic structs. The trick there is that the Selectable trait itself is generic over DB, which means that rustc cannot assume that it is implemented for all types, so putting up a where clause constraint is fine.

@weiznich weiznich merged commit bcd32c8 into diesel-rs:master Nov 22, 2021
@Ten0 Ten0 mentioned this pull request Oct 7, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pksunkara pksunkara pksunkara left review comments

@weiznich weiznich weiznich approved these changes

Assignees

@Ten0 Ten0

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Selectable derive does not work with generic embed sub-fields
3 participants
@Ten0 @weiznich @pksunkara