Implement Try and FromResidual<!> for ()

[Jarcho]

Proposal

Problem statement

Using ControlFlow<Self::T> in a trait where most implementors set T = !. (or Result<_, Self::T>) results in a lot of boilerplate for the implementer.

Motivation, use-cases

Visitors with an early return can be modeled as a trait such as the following (used in rustc):

trait Visitor {
    type BreakTy = !;
    fn visit_foo(&mut self, foo: &Foo) -> ControlFlow<Self::BreakTy> {
        walk_foo(self, foo)
    }
    // And more `visit_*` functions
}

fn walk_foo<V: Visitor>(v: &V, foo: &Foo) -> ControlFlow<V::BreakTy> {
    // walk foo using `?` when needed
    Continue(())
}

With non-branching implementations looking like (actual implementations can get worse):

impl Visitor for Bar {
    fn visit_foo(&mut self, foo: &Foo) -> ControlFlow<!> {
        if some_condition() {
            // do something
        } else if something_else() {
            // do something
        } else {
            walk_foo();
        }
        Continue(())
    }
}

Solution sketches

If Try and FromResidual<!> are implemented for (), then the previous example would instead be:

trait Visitor {
    type ResultTy: Try<Output = ()> = ();
    fn visit_foo(&mut self, foo: &Foo) -> Self::ResultTy {
        walk_foo(self, foo)
    }
    // And more `visit_*` functions
}

fn walk_foo<V: Visitor>(v: &V, foo: &Foo) -> V::ResultTy {
    // walk foo using `?` when needed
    V::ResultTy::from_output(())
}

impl Visitor for Bar {
    fn visit_foo(&mut self, foo: &Foo) {
        if some_condition() {
            // do something
        } else if something_else() {
            // do something
        } else {
            walk_foo();
        }
    }
}

Any impl which requires branching can then set ResultTy to ControlFlow<T> (or Result<(), T> if that makes more sense).

---

An alternative would be to use a custom ResultTy trait implemented for both () and ControlFlow<T> (and others if needed). This would allow effectively the same interface, except anything which interacts with generic Visitors wouldn't be able to use the ? operator. This would mainly affect the implementation of walk_* functions which would otherwise make heavy use of it.

Downsides would be allowing foo()????? when foo returns (). This is easily solved with a lint any any use of ? on an expression of the type ().

Links and related work

Came up on zulip while discussing using ControlFlow more.

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

[scottmcm]

(Since I was mentioned in the meeting notes) I could go either way on this. See my previous comments in https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/Use.20.60ControlFlow.60.20in.20HIR.20.60Visitor.60.20compiler-team.23597/near/339419883.

I feel kinda weird about it, like maybe it wants something different, but it also meets all the expected properties, and ()? could be linted about strongly, like how we're about to start linking on let x = returns_unit();, so I don't think it's necessarily unreasonable either.

TLDR I'm happy to follow libs-api's instincts either way on this one 🙂

[m-ou-se]

We discussed this in a recent libs API meeting.

This implementation would allow for quite confusing code. For example, println!("..")?; would be accepted, even though the ? is meaningless. Of course, a lint could address that issue, but it's not clear to us that adding this trait impl will actually result in more consistency rather than just more confusion.

The motivation seems to come down to basically Continue(()) being verbose and perhaps confusing. But that sort of thing seems quite fundamental to Rust, at least today. There are many function bodies that end in Ok(()), which is indeed a bit verbose, but that is not a problem that is solved by adding an extra trait implementation.

The issue of ControlFlow<!> not being as convenient as () is real, but no different than the issue of Result<T, !> not being identical to T. (E.g. having to wrap a result in Ok() even if my function can never fail, just to satisfy a trait that uses a Result type.)

Addressing that issue is perhaps something that can be done at the language level. (Some way to allow implementing a ControlFlow<!>-returning function as a ()-returning function, somehow? And also try {}'s 'ok wrapping' behaviour is relevant.)

[Amanieu]

Closing ACP as per the above comment.