-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix: Decode and Encode derives (#1031) #2940
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ use syn::punctuated::Punctuated; | |
use syn::token::Comma; | ||
use syn::{ | ||
parse_quote, Arm, Data, DataEnum, DataStruct, DeriveInput, Field, Fields, FieldsNamed, | ||
FieldsUnnamed, Stmt, Variant, | ||
FieldsUnnamed, Stmt, TypeParamBound, Variant, | ||
}; | ||
|
||
pub fn expand_derive_decode(input: &DeriveInput) -> syn::Result<TokenStream> { | ||
|
@@ -265,24 +265,21 @@ fn expand_derive_decode_struct( | |
if cfg!(feature = "postgres") { | ||
let ident = &input.ident; | ||
|
||
// extract type generics | ||
let generics = &input.generics; | ||
let (_, ty_generics, _) = generics.split_for_impl(); | ||
let (_, ty_generics, where_clause) = input.generics.split_for_impl(); | ||
|
||
// add db type for impl generics & where clause | ||
let mut generics = generics.clone(); | ||
generics.params.insert(0, parse_quote!('r)); | ||
|
||
let predicates = &mut generics.make_where_clause().predicates; | ||
|
||
for field in fields { | ||
let ty = &field.ty; | ||
let mut generics = input.generics.clone(); | ||
|
||
predicates.push(parse_quote!(#ty: ::sqlx::decode::Decode<'r, ::sqlx::Postgres>)); | ||
predicates.push(parse_quote!(#ty: ::sqlx::types::Type<::sqlx::Postgres>)); | ||
// add db type for impl generics & where clause | ||
for type_param in &mut generics.type_params_mut() { | ||
type_param.bounds.extend::<[TypeParamBound; 2]>([ | ||
parse_quote!(for<'decode> ::sqlx::decode::Decode<'decode, ::sqlx::Postgres>), | ||
parse_quote!(::sqlx::types::Type<::sqlx::Postgres>), | ||
]); | ||
} | ||
|
||
let (impl_generics, _, where_clause) = generics.split_for_impl(); | ||
generics.params.push(parse_quote!('r)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder what the idea behind this is (I know it existed before already). |
||
|
||
let (impl_generics, _, _) = generics.split_for_impl(); | ||
|
||
let reads = fields.iter().map(|field| -> Stmt { | ||
let id = &field.ident; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ use syn::punctuated::Punctuated; | |
use syn::token::Comma; | ||
use syn::{ | ||
parse_quote, Data, DataEnum, DataStruct, DeriveInput, Expr, Field, Fields, FieldsNamed, | ||
FieldsUnnamed, Lifetime, LifetimeParam, Stmt, Variant, | ||
FieldsUnnamed, Lifetime, LifetimeParam, Stmt, TypeParamBound, Variant, | ||
}; | ||
|
||
pub fn expand_derive_encode(input: &DeriveInput) -> syn::Result<TokenStream> { | ||
|
@@ -205,24 +205,21 @@ fn expand_derive_encode_struct( | |
let ident = &input.ident; | ||
let column_count = fields.len(); | ||
|
||
// extract type generics | ||
let generics = &input.generics; | ||
let (_, ty_generics, _) = generics.split_for_impl(); | ||
let (_, ty_generics, where_clause) = input.generics.split_for_impl(); | ||
|
||
// add db type for impl generics & where clause | ||
let mut generics = generics.clone(); | ||
|
||
let predicates = &mut generics.make_where_clause().predicates; | ||
|
||
for field in fields { | ||
let ty = &field.ty; | ||
let mut generics = input.generics.clone(); | ||
|
||
predicates | ||
.push(parse_quote!(#ty: for<'q> ::sqlx::encode::Encode<'q, ::sqlx::Postgres>)); | ||
predicates.push(parse_quote!(#ty: ::sqlx::types::Type<::sqlx::Postgres>)); | ||
// add db type for impl generics & where clause | ||
for type_param in &mut generics.type_params_mut() { | ||
type_param.bounds.extend::<[TypeParamBound; 2]>([ | ||
parse_quote!(for<'encode> ::sqlx::encode::Encode<'encode, ::sqlx::Postgres>), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
parse_quote!(::sqlx::types::Type<::sqlx::Postgres>), | ||
]); | ||
} | ||
|
||
let (impl_generics, _, where_clause) = generics.split_for_impl(); | ||
generics.params.push(parse_quote!('q)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. |
||
|
||
let (impl_generics, _, _) = generics.split_for_impl(); | ||
|
||
let writes = fields.iter().map(|field| -> Stmt { | ||
let id = &field.ident; | ||
|
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.
Why is this now a higher-order bound instead of
Decode<'r, _>
like before?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.
honestly, i wrote this so long ago, i don't recall any of the reasoning behind my changes. happy to change this though
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.
If it works, I think
'r
without thefor<>
makes more sense. If not, I'd be curious why.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.
It may have been to get around an "implementation is not general enough" error. People complain about those from time to time.
This change is what I was thinking about in this comment BTW #2940
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.
Which "this comment"? GitHub doesn't highlight anything for me when clicking that link.
And I'm pretty sure the
for<'decode>
makes the impl less general and can't possibly fix any errors.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.
The comment I @-mention'd you in above.
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.
Ahh, so you agree with keeping the
'r
if possible, right?