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.

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

Support default literals #368

Open
dtolnay opened this issue Jun 10, 2016 · 28 comments
Open

Support default literals #368

dtolnay opened this issue Jun 10, 2016 · 28 comments

Comments

@dtolnay
Copy link
Member

dtolnay commented Jun 10, 2016

#[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
Copy link
Member Author

dtolnay commented Jun 13, 2016

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 dtolnay self-assigned this Jun 13, 2016
@sfackler
Copy link
Contributor

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.

@dtolnay dtolnay removed their assignment Jan 24, 2017
@rekka
Copy link

rekka commented Feb 3, 2017

Would it be possible to support closures? Something like:

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

@skorokithakis
Copy link

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.

@dtolnay
Copy link
Member Author

dtolnay commented Sep 3, 2017

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.

@Fiedzia
Copy link

Fiedzia commented Apr 20, 2018

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

@alexbool
Copy link

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

@richardwhiuk
Copy link

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?

@jhpratt
Copy link

jhpratt commented Jan 30, 2019

@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.

@norru
Copy link

norru commented Feb 20, 2019

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

@jplatte
Copy link
Contributor

jplatte commented Feb 20, 2019

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)

@jhpratt
Copy link

jhpratt commented Feb 20, 2019

@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.

@est31
Copy link
Contributor

est31 commented Feb 23, 2019

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.

@spease
Copy link

spease commented Jul 19, 2019

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)

@est31
Copy link
Contributor

est31 commented Jul 20, 2019

There is now a PR open: #1490

@agausmann
Copy link

agausmann commented Jan 2, 2020

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 #1490 (review), 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.

@jacob-pro
Copy link

jacob-pro commented Jul 6, 2022

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?

@jplatte
Copy link
Contributor

jplatte commented Jul 12, 2022

@jacob-pro See #1490 (review).

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).

@fgimian
Copy link

fgimian commented Sep 14, 2022

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 😄

@cdbennett
Copy link

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!

@Skeletonxf
Copy link

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?

@JohnTheCoolingFan
Copy link

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

@tkr-sh
Copy link

tkr-sh commented Dec 31, 2023

Is there some updates on that ?

@alekspickle
Copy link

alekspickle commented Jan 10, 2024

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

@ollyde
Copy link

ollyde commented Jan 14, 2024

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

@bytedream
Copy link

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

@Lindenk
Copy link

Lindenk commented Jan 31, 2024

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?

@IS2511
Copy link

IS2511 commented Mar 22, 2024

Changes needed to make #[serde(default_value = 0.1)] happen: #1490 (review)

Progress AFAIK (2024-03-22):

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

Successfully merging a pull request may close this issue.