Pre-RFC: `#[must_use]` on a Result's Ok type

[josh]

Rust 1.27 added the ability to annotate a function as #[must_use], which results in a lint if you don’t use the return value. However, if a function returns Result<T, E>, the existing #[must_use] annotation on Result itself will only force the caller to look at the Result somehow, which ? will do; that doesn’t stop the caller from then ignoring the T. I’d like to propose an extension to this syntax that ensures the Ok type gets used:

fn f() -> Result<#[must_use] T, E> { ... }

f(); // lint warning, must use the Result
f()?; // Also a lint warning, must use the T

Does this seem reasonable?

Does this syntax seem reasonable, or would some other syntax make more sense? (Note that this uses the “attributes on generic type parameters” syntax also introduced in Rust 1.27.)

[gbutler]

What about Option<T>? What about Result<Option<T>,E>? What about Option<Result<T,E>> (weird)? What about MyResultLikeStruct<T,U,E>???

It would seem odd to special-case Result<T,E> and not have these and similar cases also be must_use. What might that look like? How difficult would that be? Would it be possible?

If you couldn’t make them all work, I’d feel the justification for Result<T,E> would be a little weak.

[josh]

This isn’t special-casing Result<T, E>; there’s no fundamental reason the syntax wouldn’t allow putting #[must_use] on an arbitrary type parameter. The main reason I bring it up for Result specifically is precisely because it’s common to write func()? and that would use the Result but not the T.

[gbutler]

How does the compiler know how to use the T? It has so have special knowledge to know that it must be called as unwrap or match Some(T), no? How would that translate to other things that are similar like the straw-man I gave of, MyResultLike< #[must_use]T,U,E>

[josh]

Obviously there’s a limit to how far the compiler can track this. I’m not necessarily expecting full dataflow analysis here. Just as you can write let _ = func() and that’s (by design) enough to silence a #[must_use] on func, I’m primarily looking to catch func()?;, and for that matter func().unwrap(); or func().unwrap_or(...);. I can live with best-effort here; the goal is to catch a common erroneous pattern.

[gbutler]

Would a [#must_use] applied T mean that any enum variant in the returned enum that encapsulates a T must be matched against? I’m having difficulty tracking this through. Probably need to go to bed.

[scottmcm]

A related issue I just opened today:

Issue: It would be nice if JoinHandle<T> were must-use for T != ()

I’m a bit afraid of trying to expand things with must_use without a better model of what it’s supposed to actually do. For example, today

    4.clone(); // Warns
    (4.clone(),); // No warning

As to the specific solution proposal, I’m unsure what things are supposed to be affected by it. For example, does it make .ok() give you an Option<#[must_use] T>?

[birkenfeld]

Surely at some point it becomes only the caller’s choice if a return value is used or not? I understand special-casing Result, to encourage error handling, but this goes a lot further in the direction of “pure functions” and the associated minefield…

[josh]

Right now, #[must_use] on a function can be suppressed by writing something like let _ = func(). You could suppress this with any number of patterns, too. This isn’t intended to flag explicitly ignoring a return value, it’s intended to flag accidentally ignoring a return value.

And I’d only expect to see this attribute used on functions where it makes no sense to ignore the result. For instance, if you have a “modify and return modified copy” function, that doesn’t modify in place, then it makes no sense to ignore the result; you might as well not call the function at all.

[Ixrec]

Do we have any way of excluding Result<(), Error> from this?

[josh]

This is for annotating a specific function’s return value. If you have a function returning Result<(), Error>, don’t annotate the () with #[must_use].

If you’re concerned about a generic function being run with a type parameter of (), note that #[must_use] already handles that case as you might expect: a function returning () (including via generic parameter) will always consider the () “used” even if you don’t do anything with it, and you’ll never get an unused_must_use lint about it.

[Riateche]

Looks like there are only a few cases where must_use on a generic type parameter would make sense. For example, what fn f() -> Vec<#[must_use] T> would mean?

If your primary concern is the f()?; case, it makes sense to only handle functions that return types that implement Carrier. I think it should be possible to make it work like this:

#[must_use_ok]
fn f() -> Result<T, E> { ... }

Then the compiler will issue a warning to both f(); and f()?;. It doesn’t have to care what the exact structure of the return type is. If f()'s return type does not implement Carrier, adding #[must_use_ok] to it will result in an error. (The must_use_ok name is not great, though.)

There is also the f().unwrap(); case that isn’t backed by a trait. While it’s possible to add special handling for Result and Option here, it’s hard to imagine a way to handle this for any custom type. Imagine a type like this:

enum MyResult<T> {
    Ok(T),
    Err,
}
struct MyAccessWrapper<T>(T);
impl<T> MyResult<T> {
    fn my_unwrap(self) -> MyAccessWrapper<T> {
        match self {
            MyResult::Ok(v) => MyAccessWrapper(v),
            MyResult::Err => panic!("err")
        }
    }
}

Even if you declare your function to return MyResult<#[must_use] T>, the compiler doesn’t have a way to handle the f().my_unwrap(); case. The name of the unwrap function isn’t unwrap and its return type is not T, so it’s not clear in general that the return value of my_unwrap() must be used in this case.

[josh]

This seems plausible to me. It doesn’t special-case Result specifically, and it should work for any implementation of Try (or whatever we end up calling it when stabilized).

[system]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.