Tracking Issue for bound_as_ref

[glittershark]

Feature gate: #![feature(bound_as_ref)]

This is a tracking issue for providing as_ref and as_mut on bound. These methods mirror similar methods on the Option type.

Public API

impl<T> Bound<T> {
  pub fn as_ref(&self) -> Bound<&T>;
  pub fn as_mut(&mut self) -> Bound<&mut T>;
}

Steps / History

• Implementation: Add as_ref and as_mut methods for Bound #80444
• Final commenting period (FCP)
• Stabilization PR

Unresolved Questions

• None yet.

[glittershark]

This has been unstable since Jan 14 and seems relatively uncontroversial, modulo that issue, so I'd love to get the stabilization process kicked off. @dtolnay you reviewed the original PR, any qualms with that?

[dtolnay]

as_ref seems well motivated to me and I would be prepared to propose FCP. Given that RangeBounds revolves around returning Bound<&T>, having an easy way to acquire them from non-borrowed data is compelling.

But does anybody so far have a real world use case for as_mut? Can we delete it?

[glittershark]

But does anybody so far have a real world use case for as_mut? Can we delete it?

it would seem weird to have as_ref and not as_mut, wouldn't it? especially since Option has both. I feel like that violates principle of least surprise.

[dtolnay]

Okay but Option::as_mut is widely used and useful.

[glittershark]

I guess my point is that if Option::as_mut is useful, then Bound::as_mut is reasonably likely to be useful as well, and I don't see a downside to including it - but that latter point is something that maintainers are much more likely to have opinions about than me 😄. Definitely not my hill to die on, though.

[dtolnay]

@rfcbot poll libs-api Delete core::ops::Bound::as_mut because it is useless as far as any of us can tell
@rfcbot poll libs-api Keep core::ops::Bound::as_mut because wherever a as_ref exists, so must as_mut, even if useless

[rfcbot]

Team member @dtolnay has asked teams: T-libs-api, for consensus on:

Delete core::ops::Bound::as_mut because it is useless as far as any of us can tell

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

[rfcbot]

Team member @dtolnay has asked teams: T-libs-api, for consensus on:

Keep core::ops::Bound::as_mut because wherever a as_ref exists, so must as_mut, even if useless

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

[glittershark]

That poll wording seems a little... leading, doesn't it? I think callingas_mut decisively useless isn't quite right - what if you want to call an associated function on the endpoint of a bound that takes &mut self?

[BurntSushi]

@glittershark Could you perhaps elaborate a bit more? Is it possible to show a short code sample?

I think that if as_mut is quite literally useless---in that there really no possible use for it---then I'd be against its inclusion. Principally because I would see its presence as confusing. e.g., "Why is std giving me a routine that has no possible use?"

But if it affords a little extra flexibility or convenience in some cases, then I think i'd be inclined to add it, mostly because my bar for adding as_mut is so low given it is a strong convention.

[Waridley]

what if you want to call an associated function on the endpoint of a bound that takes &mut self?

Then you'd still have to match on the Bound in order to get to the inner &mut T to call that function, right? map_mut seems more applicable in that case. Matching on &mut Bound<T> should still give you &mut T in the match arms.

[glittershark]

Then you'd still have to match on the Bound in order to get to the inner &mut T to call that function, right? map_mut seems more applicable in that case. Matching on &mut Bound<T> should still give you &mut T in the match arms.

yeah, that's fair - it's not often you'd want .as_mut() without immediately being followed by a .map(...)... but isn't that true for Option as well? Basically any time I think about this I net out at preferring API consistency over all else.

[glittershark]

I did have a bit of a lightbulb about this since last time I was engaged in the discussion, which was that perhaps (tell me if I'm wrong) @dtolnay's hesitation about .as_mut() come from an assumption that all Bounds originate at, or are constructed to be returned by, a call to RangeBounds::start_bound or ::end_bound - both of which return Bound<&T>. I on the other hand am storing Bounds directly in lots of places in my codebase, and have code that wants to do things to mutate them relatively frequently. That said, nothing actually calls Bound::as_mut right now, so any code example would be necessarily contrived. The core of my argument is one of principle-of-least-surprise much more than an actual motivating use case.

[Waridley]

I'm also using Bound directly a lot in my code, but since currently all of the types I'm using it with are Copy, I'm always just replacing the Bound itself instead of mutating the inner value anyway. However, I could imagine a scenario where someone were to use Bound<String>, Bound<Vec<u8>>, or some other large [Partial]Ord + !Copy type and map_mut could become useful. However, since map_mut isn't currently part of #86026, maybe as_mut().map(...) could be a solution in that case?

[lopopolo]

I'm implementing my own Range type with Bound and would love to at least have Bound::as_ref to simplify the trait implementation for RangeBounds<usize>:

impl RangeBounds<usize> for Region {
    fn start_bound(&self) -> Bound<&usize> {
        // TODO: Use `self.start.as_ref()` when upstream `std` stabilizes:
        // https://github.com/rust-lang/rust/issues/80996
        match self.start {
            Bound::Included(ref bound) => Bound::Included(bound),
            Bound::Excluded(ref bound) => Bound::Excluded(bound),
            Bound::Unbounded => Bound::Unbounded,
        }
    }

    fn end_bound(&self) -> Bound<&usize> {
        // TODO: Use `self.end.as_ref()` when upstream `std` stabilizes:
        // https://github.com/rust-lang/rust/issues/80996
        match self.end {
            Bound::Included(ref bound) => Bound::Included(bound),
            Bound::Excluded(ref bound) => Bound::Excluded(bound),
            Bound::Unbounded => Bound::Unbounded,
        }
    }
}

[lopopolo]

Would it be possible to split this tracking issue into two features, propose bound_as_ref for stabilization, and defer bound_as_mut for more discussion?

[yaahc]

Basically any time I think about this I net out at preferring API consistency over all else.

Although consistency is a significant design consideration in many situations, I don't believe it is sufficient justification to add APIs on its own. We've updated our API proposal process since this API was added¹, and the template that goes along with it includes a mandatory section for motivating examples. This should hopefully show how important real-world motivating use cases are to us. Otherwise, we'll have a ton of pretty methods that nobody uses and that serve only as clutter in the documentation. I advise sticking with David's original recommendation and stabilizing as_ref while removing as_mut until a compelling use case emerges.

Would it be possible to split this tracking issue into two features, propose bound_as_ref for stabilization, and defer bound_as_mut for more discussion?

I'd just skip straight to a partial stabilization² personally.

Footnotes

1. https://std-dev-guide.rust-lang.org/feature-lifecycle/api-change-proposals.html ↩

2. https://std-dev-guide.rust-lang.org/feature-lifecycle/stabilization.html#partial-stabilizations ↩

[lopopolo]

@yaahc I put up a partial stabilization PR here:

• Partially stabilize bound_as_ref by stabilizing Bound::as_ref #99736