Skip to content

Commit

Permalink
Use core::convert::identity to simplify code generation
Browse files Browse the repository at this point in the history
  • Loading branch information
TedDriggs committed Oct 7, 2024
1 parent 8234b61 commit 58d7cb2
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 57 deletions.
24 changes: 12 additions & 12 deletions core/src/codegen/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::borrow::Cow;

use proc_macro2::TokenStream;
use quote::{quote, quote_spanned, ToTokens, TokenStreamExt};
use syn::{spanned::Spanned, Ident, Path, Type};
use syn::{spanned::Spanned, Ident, Type};

use crate::codegen::{DefaultExpression, PostfixTransform};
use crate::usage::{self, IdentRefSet, IdentSet, UsesTypeParams};
Expand All @@ -22,10 +22,10 @@ pub struct Field<'a> {
/// The type of the field in the input.
pub ty: &'a Type,
pub default_expression: Option<DefaultExpression<'a>>,
/// Initial declaration for `with`; this is used if `with` was a closure,
/// to assign the closure to a local variable.
pub with_initializer: Option<syn::Stmt>,
pub with_path: Cow<'a, Path>,
/// An expression that will be wrapped in a call to [`core::convert::identity`] and
/// then used for converting a provided value into the field value _before_ postfix
/// transforms are called.
pub with_callable: Cow<'a, syn::Expr>,
pub post_transform: Option<&'a PostfixTransform>,
pub skip: bool,
pub multiple: bool,
Expand Down Expand Up @@ -110,10 +110,6 @@ impl<'a> ToTokens for Declaration<'a> {
let mut __flatten: Vec<::darling::ast::NestedMeta> = vec![];
});
}

if let Some(stmt) = &self.0.with_initializer {
stmt.to_tokens(tokens);
}
}
}

Expand Down Expand Up @@ -163,7 +159,7 @@ impl<'a> ToTokens for MatchArm<'a> {

let name_str = &field.name_in_attr;
let ident = field.ident;
let with_path = &field.with_path;
let with_callable = &field.with_callable;
let post_transform = field.post_transform.as_ref();

// Errors include the location of the bad input, so we compute that here.
Expand All @@ -178,15 +174,19 @@ impl<'a> ToTokens for MatchArm<'a> {
quote!(#name_str)
};

// Give darling's generated code the span of the `with_path` so that if the target
// Give darling's generated code the span of the `with_callable` so that if the target
// type doesn't impl FromMeta, darling's immediate user gets a properly-spanned error.
//
// Within the generated code, add the span immediately on extraction failure, so that it's
// as specific as possible.
// The behavior of `with_span` makes this safe to do; if the child applied an
// even-more-specific span, our attempt here will not overwrite that and will only cost
// us one `if` check.
let extractor = quote_spanned!(with_path.span()=>#with_path(__inner)#post_transform.map_err(|e| e.with_span(&__inner).at(#location)));
let extractor = quote_spanned!(with_callable.span()=>
::darling::export::identity::<fn(&::syn::Meta) -> ::darling::Result<_>>(#with_callable)(__inner)
#post_transform
.map_err(|e| e.with_span(&__inner).at(#location))
);

tokens.append_all(if field.multiple {
quote!(
Expand Down
48 changes: 7 additions & 41 deletions core/src/options/input_field.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::borrow::Cow;

use quote::format_ident;
use syn::{parse_quote_spanned, spanned::Spanned};

use crate::codegen;
Expand Down Expand Up @@ -35,8 +34,7 @@ impl InputField {
.map_or_else(|| Cow::Owned(self.ident.to_string()), Cow::Borrowed),
ty: &self.ty,
default_expression: self.as_codegen_default(),
with_initializer: self.with.as_ref().and_then(With::to_closure_declaration),
with_path: self.with.as_ref().map(|w| &w.path).map_or_else(
with_callable: self.with.as_ref().map(|w| &w.call).map_or_else(
|| {
Cow::Owned(
parse_quote_spanned!(self.ty.span()=> ::darling::FromMeta::from_meta),
Expand Down Expand Up @@ -151,7 +149,7 @@ impl ParseAttribute for InputField {
return Err(Error::duplicate_field_path(path).with_span(mi));
}

self.with = Some(With::from_meta(&self.ident, mi)?);
self.with = Some(With::from_meta(mi)?);

if self.flatten.is_present() {
return Err(
Expand Down Expand Up @@ -244,53 +242,21 @@ impl ParseAttribute for InputField {

#[derive(Debug, Clone)]
pub struct With {
/// The path that generated code should use when calling this.
path: syn::Path,
/// If set, the closure that should be assigned to `path` locally.
closure: Option<syn::ExprClosure>,
/// The callable
call: syn::Expr,
}

impl With {
pub fn from_meta(field_name: &syn::Ident, meta: &syn::Meta) -> Result<Self> {
pub fn from_meta(meta: &syn::Meta) -> Result<Self> {
if let syn::Meta::NameValue(nv) = meta {
match &nv.value {
syn::Expr::Path(path) => Ok(Self::from(path.path.clone())),
syn::Expr::Closure(closure) => Ok(Self {
path: format_ident!("__with_closure_for_{}", field_name).into(),
closure: Some(closure.clone()),
syn::Expr::Path(_) | syn::Expr::Closure(_) => Ok(Self {
call: nv.value.clone(),
}),
_ => Err(Error::unexpected_expr_type(&nv.value)),
}
} else {
Err(Error::unsupported_format("non-value"))
}
}

/// Create the statement that declares the closure as a function pointer.
fn to_closure_declaration(&self) -> Option<syn::Stmt> {
self.closure.as_ref().map(|c| {
let path = &self.path;
// An explicit annotation that the input is borrowed is needed here,
// or attempting to pass a closure will fail with an issue about a temporary
// value being dropped while still borrowed in the extractor loop.
//
// Making the parameter type explicit here avoids errors if the closure doesn't
// do enough to make the type clear to the compiler.
//
// The explicit return type is needed, or else using `Ok` and `?` in the closure
// body will produce an error about needing type annotations due to uncertainty
// about the error variant's type. `T` is left undefined so that postfix transforms
// still work as expected
parse_quote_spanned!(c.span()=> let #path: fn(&::syn::Meta) -> ::darling::Result<_> = #c;)
})
}
}

impl From<syn::Path> for With {
fn from(path: syn::Path) -> Self {
Self {
path,
closure: None,
}
}
}
2 changes: 1 addition & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub use darling_core::ToTokens;
/// of the referenced types.
#[doc(hidden)]
pub mod export {
pub use core::convert::From;
pub use core::convert::{identity, From};
pub use core::default::Default;
pub use core::option::Option::{self, None, Some};
pub use core::result::Result::{self, Err, Ok};
Expand Down
6 changes: 3 additions & 3 deletions tests/compile-fail/attrs_with_bad_fn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ note: method defined here
| pub fn handle<T>(&mut self, result: Result<T>) -> Option<T> {
| ^^^^^^
help: try wrapping the expression in `Ok`
|
11 | #[darling(with = Ok(bad_converter))]
| +++ +
|
11 | #[darling(with = Ok(bad_converter))]
| +++ +
21 changes: 21 additions & 0 deletions tests/compile-fail/with_closure_capture.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use darling::{FromDeriveInput, FromMeta};

#[derive(FromDeriveInput)]
#[darling(attributes(demo))]
pub struct Receiver {
example1: String,
#[darling(
// This should fail because `example1` is a local that's been captured
// from the `FromDeriveInput` impl. That's disallowed because exposing
// those internals would make any change to the derived method body a
// potentially-breaking change.
with = |m| Ok(
String::from_meta(m)?.to_uppercase()
+ example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
),
default
)]
example2: String,
}

fn main() {}
30 changes: 30 additions & 0 deletions tests/compile-fail/with_closure_capture.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
error[E0308]: mismatched types
--> tests/compile-fail/with_closure_capture.rs:12:16
|
12 | with = |m| Ok(
| ^ arguments to this function are incorrect
| ________________|
| |
13 | | String::from_meta(m)?.to_uppercase()
14 | | + example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
15 | | ),
| |_________^ expected fn pointer, found closure
|
= note: expected fn pointer `for<'a> fn(&'a syn::Meta) -> Result<String, darling::Error>`
found closure `{closure@$DIR/tests/compile-fail/with_closure_capture.rs:12:16: 12:19}`
note: closures can only be coerced to `fn` types if they do not capture any variables
--> tests/compile-fail/with_closure_capture.rs:14:15
|
14 | + example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
| ^^^^^^^^ `example1` captured here
help: the return type of this call is `{closure@$DIR/tests/compile-fail/with_closure_capture.rs:12:16: 12:19}` due to the type of the argument passed
--> tests/compile-fail/with_closure_capture.rs:12:16
|
12 | with = |m| Ok(
| ________________^
13 | | String::from_meta(m)?.to_uppercase()
14 | | + example1.1.as_ref().map(|s| s.as_str()).unwrap_or("")
15 | | ),
| |_________- this argument influences the return type of `{{root}}`
note: function defined here
--> /rustc/aedd173a2c086e558c2b66d3743b344f977621a7/library/core/src/convert/mod.rs:104:14

0 comments on commit 58d7cb2

Please sign in to comment.