-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Comments
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
|
Seems like an easy route would be |
Would it be possible to support closures? Something like: #[serde(default="||{ 0.1 }")]
pub n: f32 |
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 I would have preferred |
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. |
Any progress on that? Right now I am forced to write functions like |
I agree with @sfackler and think |
Surely In fact, that can replace any of the current usage right? |
@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 I can certainly work on this if it's a feature you and the other maintainers are interested in. |
Just being bitten by this, currently writing a lot of |
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 |
@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. |
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. |
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) |
There is now a PR open: #1490 |
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. |
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 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). |
Just a little thought (in case it's useful). The wonderful clap crate offers a 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 |
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 |
A const generic struct can be used as a partial workaround if you can express the 'literal' with min const generics 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? |
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 |
Is there some updates on that ? |
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 |
@alekspickle awesome! Would be nice to add string and array support 🕺 |
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 |
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? |
Changes needed to make Progress AFAIK (2024-03-22):
|
rust-lang/rfcs#3681 would solve this nicely |
Before that RFC landed, one can use educe |
There is an ambiguity when the field is of function type so we need to figure that out.
The text was updated successfully, but these errors were encountered: