serde icon indicating copy to clipboard operation
serde copied to clipboard

Support default literals

Open dtolnay opened this issue 8 years ago • 30 comments

#[serde(default="0.1")]
pub n: f32

There is an ambiguity when the field is of function type so we need to figure that out.

dtolnay avatar Jun 10 '16 16:06 dtolnay

This needs to be fleshed out a lot more before it can be implemented so please chime in if you have ideas for how this should work.

I am glad we implemented #[serde(default="...")] and it has been widely useful but backward compatibility is going to be tricky. To start brainstorming, here are a few possible alternatives:

  • Another level of nesting
    • #[serde(default(expr="0.1"))] - we insert the expression verbatim into the generated code
    • This is flexible but clunky.
  • Look for parentheses to denote an expression
    • #[serde(default="(0.1)")]
    • This is hacky and doesn't provide a path forward for deprecating the current syntax.
  • Use a different attribute name
    • #[serde(missing="0.1")], #[serde(default_expr="0.1")]
    • But default is such a perfect fit.
  • Treat as an expression if it fails to parse as a path
    • #[serde(default="0.1")]
    • This is ambiguous in many cases, for example enums: #[serde(default="Color::Yellow")]

dtolnay avatar Jun 13 '16 03:06 dtolnay

Seems like an easy route would be #[serde(default_value = "0.1")]. It's a bit more wordy but would allow us to get the functionality out there, and we can always deprecate default_value if a better approach comes around.

sfackler avatar Aug 12 '16 20:08 sfackler

Would it be possible to support closures? Something like:

#[serde(default="||{ 0.1 }")]
pub n: f32

rekka avatar Feb 03 '17 04:02 rekka

The closure route would be the most logical for me (and the first thing I tried when exploring how to do what I wanted). However, I think default_expr would be by far the clearest.

I would have preferred default=3 to be the literal mode and default_func the function-based approach, but eh, can't do much about that now.

skorokithakis avatar Jul 28 '17 09:07 skorokithakis

As reported in #1031, unit variants would be nice to support as well. Rust no longer enforces that the value in an attribute is a literal.

dtolnay avatar Sep 03 '17 18:09 dtolnay

Any progress on that? Right now I am forced to write functions like fn one() -> u32 { 1 } and it really looks silly

Fiedzia avatar Apr 20 '18 09:04 Fiedzia

I agree with @sfackler and think #[serde(default_value = "0.1")] is a way to go

alexbool avatar May 20 '18 09:05 alexbool

Surely #[serde(default_value = 0.1)] is better - and then any expression can be provided?

In fact, that can replace any of the current usage right?

richardwhiuk avatar Jan 24 '19 21:01 richardwhiuk

@dtolnay Would a PR be accepted for this? Using proc macros, it shouldn't be terribly difficult to generate a function (which can have an #[inline] hint) and do something along the lines of #[serde(default = #fn_name)] in the expansion.

I can certainly work on this if it's a feature you and the other maintainers are interested in.

jhpratt avatar Jan 30 '19 20:01 jhpratt

Just being bitten by this, currently writing a lot of fn one() -> u32 { 1 } which is very clunky.

norru avatar Feb 20 '19 10:02 norru

I don't know how to drive this forward, but I think a very small change to the compiler (or some other trick I have yet to find) could enable the existing attribute to work with values too: https://internals.rust-lang.org/t/implementing-traits-for-function-types/5888 (the playground code linked in that thread uses specialization but IIRC there was a way around that)

jplatte avatar Feb 20 '19 11:02 jplatte

@dtolnay Would a PR be accepted that resolves this? I asked a few weeks ago and received no answer. I don't think there's any question people want this.

jhpratt avatar Feb 20 '19 16:02 jhpratt

This crate could also give inspiration: https://github.com/idanarye/rust-smart-default

It allows specifying literals as well as code for the default value.

est31 avatar Feb 23 '19 06:02 est31

How about using closure notation? Eg just like you’d unwrap_or_else(some_void_func) or unwrap_or_else(||1.0f64)

This would be backwards compatible, consistent with existing Rust syntax, and even similar to the syntax you’d use for providing a “default” value to an option-wrapped-type.

I’d also consider dropping the quotes, that has always confused me, but I don’t know if there’s some parse thing that makes them necessary.

Considering how closures are generally used, it’s expected that the compiler will be good at optimizing them so you might actually be able to drop it in as an actual closure (so it could capture variables from the enclosing scope)

spease avatar Jul 19 '19 21:07 spease

There is now a PR open: #1490

est31 avatar Jul 20 '19 06:07 est31

I'd love to help get this feature completed; though I don't have much experience developing proc macros I've started studying the relevant parts of serde_derive and it doesn't seem too bad.

Regarding the concerns made in https://github.com/serde-rs/serde/pull/1490#pullrequestreview-288464564, while I agree that it may be nicer to be able to use paths and expressions, that shouldn't be a blocker for this particular feature. A MVP just with support for literals would still be incredibly useful.

agausmann avatar Jan 02 '20 03:01 agausmann

What is the status on this issue - its been open for a long time, is it likely to be implemented anytime soon? It feels like a rough edge on an otherwise brilliant crate and ecosystem.

I can offer my help, although I'm not particularly knowlegable about derive macros. Is there currently any minimum design / idea for how this should be implemented, that you would accept PRs for?

jacob-pro avatar Jul 06 '22 21:07 jacob-pro

@jacob-pro See https://github.com/serde-rs/serde/pull/1490#pullrequestreview-288464564.

However, this is a big refactoring and I have seen many PRs to serde be ignored entirely by @dtolnay, who seems to be the only person willing to maintain serde at the same time. I would be happy to implement the plan outlines in that comment myself, but only after having some reassurance that it would actually get looked at (preferably in the form of getting my existing PRs reviewed).

jplatte avatar Jul 12 '22 20:07 jplatte

Just a little thought (in case it's useful). The wonderful clap crate offers a default_value and default_value_t where default_value_t is a typed value.

e.g.

...
#[derive(Clone, Debug, ValueEnum)]
pub enum Platform {
    All,
    PC,
    Mac,
}

...
#[derive(Parser, Debug)]
#[clap(author, version, about, long_about = None)]
pub struct Cli {
    #[clap(short, long, value_enum, default_value_t = Platform::from_current_os())]
    pub platform: Platform,
...

I'm still new to Rust, but after using clap to build my CLI, my natural inclination was to try default_t in Serde 😄

fgimian avatar Sep 14 '22 23:09 fgimian

Just a little thought (in case it's useful). The wonderful clap crate offers a default_value and default_value_t where default_value_t is a typed value. ... I'm still new to Rust, but after using clap to build my CLI, my natural inclination was to try default_t in Serde 😄

I was just doing the exact same thing! I was searching the Serde docs, feeling sure that I had written some Serde code a couple weeks ago that used default_value_t. I felt like I must be losing my mind, when your comment reminded me that it was Clap, not Serde, I had written that code for!

cdbennett avatar Dec 29 '22 21:12 cdbennett

A const generic struct can be used as a partial workaround if you can express the 'literal' with min const generics

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=da7e2027b942882165fc4e781a94d290

It took me quite a bit of playing around with this to get the syntax to work so might be worth adding to the docs if this issue is going to be open for a long time?

Skeletonxf avatar Jun 06 '23 21:06 Skeletonxf

Using @Skeletonxf's solution, I managed to make a smaller workaround using const generics on functions. This supports all integers and types that can be converted from integers (floats, for example, but limited to only integers): https://github.com/JohnTheCoolingFan/factorio-lib-rs/blob/02137bb3f231f63a3cdef32add5511d92352e062/src/util/defaults.rs

JohnTheCoolingFan avatar Jul 09 '23 10:07 JohnTheCoolingFan

Is there some updates on that ?

tkr-sh avatar Dec 31 '23 17:12 tkr-sh

FYI I made a solution with const generics available as a crate. Basically it's @JohnTheCoolingFan's solution, just a made it prettier with macro + paste crate. It's also no_std compatible as serde itself

upd: I also added @bytedream's solution to it avaliable under the feature, since it's a lot more powerful way to deal with strings and slices

olekspickle avatar Jan 10 '24 12:01 olekspickle

@alekspickle awesome! Would be nice to add string and array support 🕺

ollyde avatar Jan 14 '24 16:01 ollyde

I also made a tiny crate for this issue a while ago. It works a bit different than the implementation of @alekspickle; tldr you can specify any expression as default and it will be expanded to a regular function which just returns the expression

bytedream avatar Jan 18 '24 21:01 bytedream

This seems to be a wildly requested feature, is there any update on whether or not it's on the roadmap or if a PR would be accepted?

Lindenk avatar Jan 31 '24 20:01 Lindenk

Changes needed to make #[serde(default_value = 0.1)] happen: https://github.com/serde-rs/serde/pull/1490#pullrequestreview-288464564

Progress AFAIK (2024-03-22):

  • [x] 1. Parse attribute values not only as string literals.
    • Done! PR: #2406
    • Related (should be rejected probably): #1704
  • [ ] 2. Add non string literal support for existing attributes where it makes sense.
    • No progress or PRs yet. (Assuming serde.rs web docs are up to date)
    • Related: #2525
    • Related: #2209
    • Candidate for related: #2161
  • [ ] 3. Add new default_value attribute that accepts expression as value.
    • No open PRs yet.
    • Outdated: #1490

IS2511 avatar Mar 22 '24 16:03 IS2511