either icon indicating copy to clipboard operation
either copied to clipboard

Either::as_ref method

Open mexus opened this issue 5 years ago • 5 comments

Hi everyone,

I've just hit an issue while trying to replace an impl AsRef with a concrete Either type in a return position, here's an example: https://play.rust-lang.org/?gist=1f4771605ae47ea7e4e10881a994a2dc&version=stable&mode=debug&edition=2015

I believe it happens because Either::as_ref method effectively hides the AsRef::as_ref method :(

Shouldn't it be renamed to something like to_ref so it doesn't overlap with the AsRef trait, or is there any reason behind this specific name? The same goes for as_mut and probably for into_iter as well.

mexus avatar Sep 15 '18 14:09 mexus

I believe it happens because Either::as_ref method effectively hides the AsRef::as_ref method :(

Yes, inherent methods take priority over trait methods.

Shouldn't it be renamed to something like to_ref so it doesn't overlap with the AsRef trait, or is there any reason behind this specific name? The same goes for as_mut

Renaming would be a breaking change, at least, requiring us to bump to either 2.0.

Either::as_ref and as_mut mimic the methods on Option and Result, creating references on the inner types while still remaining an Either. This is unfortunately different than what AsRef and AsMut do, but note that Option and Result don't implement those traits at all! Whereas Either is a weird chimera where both semantics can make sense in different contexts...

and probably for into_iter as well.

You can see in #12 when into_iter was added. This one is also weird as we couldn't implement IntoIterator directly, because we already conditionally implement Iterator, and there's a blanket impl<I> IntoIterator for I where I: Iterator which then applies.

But into_iter doesn't cause the same problem you have here, because the inherent method is a strict superset of the trait method. If the inner types aren't already Iterators, only the inherent method can be called. If they are already Iterators, then the inherent and trait methods actually return the exact same thing!

cuviper avatar Sep 18 '18 00:09 cuviper

Thanks for such a detailed answer! :)

As you've mentioned Either is more of a chimera than a direct analogue of either Iterator or Option or Result or whatever, so I believe an interface should not be simply "taken" from an existing type.. especially an inherent method that hides a trait method :)

To make my long story short, do you agree that it's not a good thing that AsRef trait can't be utilized to its full extent while there is an Either::as_ref method? :) If yes, then I think as a first step a method like by_ref (or whatever) could be introduced to provide the same functionality (Either<A, B> → Either<&A, &B>) and mark the as_ref as deprecated, and when everything's ready for 2.0 the original one might be safely removed. If you don't agree then I'll just go with a workaround, it's not a big deal at all.. well, it's definitely not big enough to start a dispute or anything.

Btw, nice trick with the into_iter! Didn't think of it that far.. tbh I've just briefly looked at all the method names that might clash with any common traits from the stdlib :)

Anyhow, thanks again for the explanation!

mexus avatar Sep 18 '18 20:09 mexus

As you've mentioned Either is more of a chimera than a direct analogue of either Iterator or Option or Result or whatever, so I believe an interface should not be simply "taken" from an existing type..

Option and Result are not direct analogues either, but it's convenient for them to have similar interfaces for functionality that makes sense in both. Either also follows suit, and ideally this is supposed to help user's intuition.

do you agree that it's not a good thing that AsRef trait can't be utilized to its full extent while there is an Either::as_ref method?

I think it's an unfortunate clash, but not that big of a deal. It still works for trait bounds, and you can call it directly in the UFCS style, however ugly that may be. :slightly_smiling_face:

I think it's not helpful to make deprecations and replacements until we're actually planning a 2.0, then we can do it to ease the transition. Until then, it wouldn't help the current conflict, so the deprecation would just add more noise.

cuviper avatar Sep 28 '18 23:09 cuviper

Well, sounds reasonable, but then another question appears: do you think it's worth adding a milestone or a tag or something for a probable 2.0? I have a bad feeling that all the improvement ideas might easily get lost in time, and if I close this issue (since it's more-or-less resolved) at least I will definitely forget about it after some time..

mexus avatar Oct 06 '18 13:10 mexus

I don't know that 2.0 is "probable" any time soon, but I just added a "potential-2.0" label, at least. :)

cuviper avatar Oct 15 '18 18:10 cuviper