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

fix: Decode and Encode derives (#1031) #2940

Merged
merged 1 commit into from
May 31, 2024
+24 −30
Merged

fix: Decode and Encode derives (#1031) #2940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 12 additions & 15 deletions sqlx-macros-core/src/derives/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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>),
Copy link
Contributor

@jplatte jplatte Mar 22, 2024 •
edited

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?

Copy link
Contributor Author

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

Copy link
Contributor

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 the for<> makes more sense. If not, I'd be curious why.

Copy link
Collaborator

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

Copy link
Contributor

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.

Copy link
Collaborator

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.

Copy link
Contributor

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?

parse_quote!(::sqlx::types::Type<::sqlx::Postgres>),
]);
}

let (impl_generics, _, where_clause) = generics.split_for_impl();
generics.params.push(parse_quote!('r));
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
Expand Down
27 changes: 12 additions & 15 deletions sqlx-macros-core/src/derives/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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>),
Copy link
Contributor

Choose a reason for hiding this comment

The 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));
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
Expand Down