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

There is no way to set/unset a Flag without losing its span information #315

Open
LikeLakers2 opened this issue Dec 4, 2024 · 7 comments

Comments

@LikeLakers2
Copy link

LikeLakers2 commented Dec 4, 2024

Hi! I was doing some derive-macro practice over at https://github.com/LikeLakers2/michis_rand_distr_derive.

In the midst of this, I found myself wanting to set a Flag value - but found there was no way to do so without losing its span information.

In my specific case, I have two options that can be applied to enum variants - a weight (how likely that variant is to be selected) and a skip option (if specified, the variant will never be selected). What I wanted to do was, if the weight was a zero-like value, erase the weight value and enable the skip flag. Unfortunately, I found there was no way to do this with Flag alone.

I did find a solution, SpannedValue<Flag>, which allows me to recreate the Flag value without (technically) losing the span information. However, this feels hacky, as now I'm storing the Span twice - and more importantly, I am forced to add #[darling(default)] to that field, because SpannedValue<Flag> does not inherit Flag's property of being inherently optional.

I'm hoping something can be done to handle this.

(P.S. If you're curious about the relevant code: https://github.com/LikeLakers2/michis_rand_distr_derive/blob/98200c0ed81ddace432787e4fe4214db6085049e/src/standard_distribution/derive_variant.rs#L19-L27)

@TedDriggs
Copy link
Owner

Instead of modifying the skip field in check_and_correct if the weight evaluates to 0, could is_skipped check whether the flag is present or the weight is zero?

It looks like you are checking skip.is_present elsewhere in the code, but could instead call is_skipped in those places?

@LikeLakers2
Copy link
Author

Hmm... that would probably work. Thank you for the suggestion.

That said, I still wonder if it may be useful to allow (un)setting a Flag. What do you think?

@TedDriggs
Copy link
Owner

That can be achieved by setting the field to be Flag::default(). Why would you need a method to do that?

@TedDriggs
Copy link
Owner

TedDriggs commented Jan 18, 2025 via email

@LikeLakers2
Copy link
Author

LikeLakers2 commented Jan 18, 2025

@TedDriggs Sorry, let me clarify.

While your suggestion to change how is_skipped is implemented would work for me... I still think it would be useful for Flag to have methods to set/unset whether it's present, without losing the span information.

Flag::default() won't work for this because it won't preserve the span info. My original solution, SpannedValue<Flag> would work - but then we're duplicating the span info.

I would much prefer a method on Flag, let's say set_present(bool), which sets whether the Flag is considered present - without changing the span info.

I hope that helps you understand.

@TedDriggs
Copy link
Owner

The struct literally cannot be unset while preserving its span; internally it's just Option<Span>.

@LikeLakers2
Copy link
Author

I'm aware that it's just an Option<Span> internally. I get why it's like that.

But I also don't care how it works internally - I'm a user of darling, and all I care is that it works as I expect.

If implementing methods to set/unset a Flag while preserving its span info requires changing how it works internally (maybe to a (bool, Span)), then so be it.

If you don't want to do that, then just say so.

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

No branches or pull requests

2 participants