Implement Termination for Option<()>

[GuillaumeGomez]

After a question about this here, I decided to open this PR. However, is this how we should implement it? Should we consider None as a failure (because that's how I implemented it)?

Anyway, I think having this PR open is a good place to discuss about it (and closing it if we don't want it of course).

cc @ollie27 @rust-lang/compiler

[rust-highfive]

r? @withoutboats

(rust_highfive has picked a reviewer for you, use r? to override)

[jonas-schievink]

This seems like more of a libs/lang issue than compiler, no?

[ollie27]

Should we consider None as a failure

The fact that that is even a question suggests that we shouldn't add this impl. See also rust-lang/rfcs#1937 (comment).

[cramertj]

IMO it's non-obvious what fn main() -> Option<()> means.

[Centril]

This seems like a matter primarily for the language team.

[GuillaumeGomez]

Oh yes it is, sorry!

@cramertj: Absolutely. I first thought than in any case, we should just return 0 and here we go. Then I decided to go for an "error" in case of None. It'd make a rustdoc test automatic implementation a bit better to have it. But if not, we'll revert the add of this feature. I just prefer to check our options first.

[llogiq]

@cramertj we have Option<_> results in checked_* arithmetic methods, in many iterator methods, in from_str and in the patterns API. In all of those cases, None would denote the path out. I don't see why using None as the failure case is even questioned, given the automatic error conversion of the Try trait.

[cramertj]

I'm not confused by None being the "failure"-esque case, I'm confused by what we'd expect returning "None" from main to do. What status code would you expect that to return, and why? Would you expect an error message to be printed? It feels poorly-motivated to me.

[cramertj]

why only () rather than anything implementing Termination?

[llogiq]

Would that flatten the Option? I'm afraid this could lead to too clever code like fn main -> Option<Option<Result<(), _>>> { .. }.

[llogiq]

I'd expect None returning the canonical failure code, which is 1 on most unixoid systems. And since we have no suitable error message to print, I'd expect no error message.

[GuillaumeGomez]

@llogiq So the current code (with the update) is how you would do it? That's convenient. :)

What the others are thinking about it?

[scottmcm]

We had a conversation about specific impls and such in the first stabilization issue, #48453, and the RFC (https://github.com/rust-lang/rfcs/blob/master/text/1937-ques-in-main.md) had Result but not Option. Is there new information that makes Option a good candidate for a main function?

I, of course, agree with @ollie27's comment 😄

Additionally, I don't want Option<impl Termination>, the same way I didn't want Result<impl Termination, _> (which I in fact explicitly removed in #48497).

And since we have no suitable error message to print,

I consider that a good reason to not implement Termination here.

[nikomatsakis]

We discussed this in the @rust-lang/lang team meeting today, though notably @scottmcm was not present. The consensus was that we were "not opposed" (some mildly in favor, some neutral, some mildly opposed but not enough to block).

Speaking personally, I found the idea of returning Option<()> in main uncompelling, but using it in tests made a lot of sense to me. And in that setting, it seems natural that having an "early return" from foo? ought to be considered a test failure (else you would write assert!(x.is_none()) or something).

Ultimately, we decided we'd rather defer the final details to the @rust-lang/libs team. If someone from that team wants to move to FCP, it would be approved by the lang team (modulo the fact that not everyone was present). But if they have doubts and would prefer to wait, that seems ok.

[dtolnay]

I would prefer not to do this. I agree with Niko that in main this is not compelling. Regarding tests I would add:

using it in tests made a lot of sense to me. And in that setting, it seems natural that having an "early return" from foo? ought to be considered a test failure (else you would write assert!(x.is_none()) or something).

Nope, just use unwrap in tests. Then you even get to find out which step failed. Writing Some(()) at the bottom of the test to mean success is not great.

The main justifiable use case I see for this impl is in doc tests with a hidden function signature. These allow the doc test to be rendered by rustdoc with ? the way the user might see it in their own code:

/// ```
/// # fn main() -> Option<()> {
/// let x = f()?;
/// let y = g(x)?;
/// # Some(())
/// # }
/// ```

But for these it seems fine to recommend something like:

/// ```
/// # try {
/// let x = f()?;
/// let y = g(x)?;
/// # }.unwrap()
/// ```

[llogiq]

@dtolnay in current nightly, you can just add Some(()) and use ? and rustdoc will insert the missing bits even without Termination (because that's not available in core and we want to be able to test no_std code). So I guess I'm with Niko in not finding a compelling use for Option-Termination.

[QuietMisdreavus]

I'm late to the party, but if the libs team decides to close this, i would prefer #61279 be reverted. I left a longer comment over there: #61279 (comment)

[GuillaumeGomez]

I'm late to the party, but if the libs team decides to close this, i would prefer #61279 be reverted.

👍

[Centril]

I'm late to the party, but if the libs team decides to close this, i would prefer #61279 be reverted.

👍

@QuietMisdreavus @GuillaumeGomez Should we revert #61279 until such time that an affirmative decision to support Termination for Option<()> is made?

[GuillaumeGomez]

Fine by me.

[Dylan-DPC-zz]

Closing this based on #61360 (comment).

Let's terminate the termination
- Guillaume Gomez

[QuietMisdreavus]

My comment was more of an "if this gets closed" - i was not calling for its closure myself, just deferring to libs team, which was pinged up-thread as the responsible party.