Add try/ref versions to unwrap

[rassvet2]

I extended Unwrap to add one that returns Option, one that returns reference, and a combination of the two, for my use.
It's ready to merge, but I don't think it's well discussed and considered. try_unwrap_<variant>_ref is... little uncool.
So I'll leave it as a draft to solicit opinions.

#[derive(Unwrap)]
enum Number {
    Integer(i64),
    Decimal(f64),
    #[unwrap(ref)] // this affects only `FromString`
    FromString(String),
}

#[derive(Unwrap)]
#[unwrap(ref)] // this affects the entire enum,
               // and adds `unwrap_<name>_ref` and `try_unwrap_<name>_ref`.
enum Maybe<T> {
    Just(T),
    Nothing,
}

fn main() {
    assert_eq!(Maybe::Just(1).unwrap_just(), 1);
    assert_eq!(Maybe::Just(2).try_unwrap_just(), Some(2));
    assert_eq!(Maybe::Nothing.try_unwrap_just(), None);
    assert_eq!(*(&Maybe::Just(42)).unwrap_just_ref(), 42);
}

This relates to #186.
var_a looks better than try_unwrap_var_a_ref. But I think in that case it probably should not be implemented in Unwrap.
Initially, I added it in Unwrap because it was tedious, but maybe it should be split into TryUnwrap, UnwrapRef, and TryUnwrapRef.

Is there anything else I have to do to be merged?

[rassvet2]

There is a known issue with *_ref naming conflicts when an enum has SomeVariant and SomeVariantRef.
I need advice on this as well.

[JelteF]

I didn't look at the code in detail, but a few thoughts on the general design:

1. For completeness this should also add unwrap_xxx_mut versions (unless somehow hard to do)
2. I really don't like the try_unwrap prefix. Some other options I can think of: as_thing, try_into_thing, try_thing. I think I like try_into_thing the best. (I will continue calling it try_unwrap in the rest of this comment.
3. Should try_unwrap return an Error instead of an Option? I think that would probably be better. If so, this issue is quite relevant: Make TryInto use better error type #173
4. If we rename try_unwrap to try_into I feel like we should use something different than the Unwrap derive? Maybe TryIntoVariant

This seems rare enough that i wouldn't consider it a problem:

There is a known issue with *_ref naming conflicts when an enum has SomeVariant and SomeVariantRef.

[JelteF]

To clarify: The most open questions and discussion points for me are around the try_unwrap version of this derive. I think it makes sense to have the unwrap_thing_ref and unwrap_thing_mut without any further discussion on the design. So I think it makes sense to separete these two things in separate PRs.

[rassvet2]

for 1 and 2:

Yes, I will. I like the shortness of as, but it seems misleading. Looking at std, it seems that functions starting with as_ rarely return Option or Result. So I'm going to use try_into.

for 3 and 4:

Makes sense. I just forgot that TryInto returns Result, not Option. I will make it returns the original value as an error for now.
As for try_unwrap, I'm going to delete it temporarily and separate it into a new PR.

[JelteF]

@rassvet2 are you still planning on making your suggested changes?

[rassvet2]

I'm sorry I left this hanging for so long.
I will work on this. It may take about two weeks.

[rassvet2]

It's ready for review. Finally, I decided like:

• try_unwrap_* was renamed to try_into_* and moved to TryIntoVariant (Isn't it a bit long compared to Unwrap?)
• Both Unwrap and TryIntoVariant basically generate owned versions only, but ref and ref_mut can also be generated by specifying them in the attribute.
• try_into_* returns a Result value. This will be Ok if the conversion succeeds, and Err if it fails. Ok contains the converted value, and Err contains the original value.
  This makes it useful when you do not want to lose ownership if conversion fails.

[JelteF]

Overall this looks good! But it needs a rebase with the current master branch. Quite a few things have changed in the file organization of the library since this PR was originally opened.

[rassvet2]

I'm wondering again if I should rename TryIntoVariant. Now I don't feel it's a good name. Since these functions don't convert one enum variant into another, they're not a conversion like std::convert. In that respect, Unwrap was a good name as it "unwraps" an enum variant into a tuple of its fields.
However, I can't think of anything that fits. Do you agree with TryIntoVariant, or not?

[JelteF]

Since these functions don't convert one enum variant into another, they're not a conversion like std::convert

That's a really good point. I think you're right that TryUnwrap is probably the best name. Both Rc and Arc have a try_unwrap method too that does exactly this, so there's also prior art from std on this naming. I'm sorry for the back and forth on the naming.

[JelteF]

Looks pretty good overal, just some small suggestions. Also there's a small merge conflict in the CHANGELOG.md now that #256 is merged which cleaned it up a bit.