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

Remove `expression_impls!` in favor of a derive #1453

Merged
merged 1 commit into from Jan 7, 2018

Conversation

Projects
None yet
2 participants
@sgrif
Copy link
Member

sgrif commented Jan 6, 2018

The nullable ToSql impl feels funky here,
but I've left it there to keep parity with the old macro.
Similar to FromSqlRow, there is an undocumented foreign_derive flag.
Any external crates attempting to use that flag would actually fail to
compile,
since impl AsExpression<Nullable<Type>, DB> for NotLocal is a
coherence violation.

This derive does not work for types which need to be generic on the SQL
type, so Option, arrays, and ranges can't use it.

@sgrif sgrif requested a review from diesel-rs/reviewers Jan 6, 2018

/// - `impl AsExpression<SqlType> for &'a &'b YourType`
/// - `impl AsExpression<Nullable<SqlType>> for &'a &'b YourType`
///
/// If your type is unsized,

This comment has been minimized.

@Eijebong

Eijebong Jan 7, 2018

Member

That's a weird way to put that. Did rustfmt do that ?

@@ -62,6 +66,11 @@ fn str_value_of_attr<'a>(attr: &'a Attribute, name: &str) -> &'a str {
str_value_of_meta_item(&attr.value, name)
}

pub fn ty_value_of_attr<'a>(attr: &'a Attribute, name: &str) -> Ty {
parse::ty(str_value_of_attr(attr, name))
.expect(&format!("#[{}] did not contian a valid Rust type", name))

This comment has been minimized.

@Eijebong

Eijebong Jan 7, 2018

Member

contain

Remove `expression_impls!` in favor of a derive
The nullable `ToSql` impl feels funky here,
but I've left it there to keep parity with the old macro.
Similar to `FromSqlRow`, there is an undocumented `foreign_derive` flag.
Any external crates attempting to use that flag would actually fail to
compile,
since `impl AsExpression<Nullable<Type>, DB> for NotLocal` is a
coherence violation.

This derive does not work for types which need to be generic on the SQL
type, so `Option`, arrays, and ranges can't use it.

@sgrif sgrif force-pushed the sg-derive-as-expression branch from 1403eab to c6ad5ea Jan 7, 2018

@sgrif sgrif merged commit c6ad5ea into master Jan 7, 2018

@sgrif sgrif deleted the sg-derive-as-expression branch Jan 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment