Stream: t-lang

Topic: MustUse


nikomatsakis (May 11 2020 at 20:07, on Zulip):

Hey @Jonas Schievink and @varkor -- could I maybe ask you all a question about MustUse

nikomatsakis (May 11 2020 at 20:08, on Zulip):

We've had a lot of conversation about it lately, and a few PRs:

nikomatsakis (May 11 2020 at 20:09, on Zulip):

The latter had some crater run results, and I think the results were a bit mixed. i.e., it caught some bugs, but also some amount of "false positives".

nikomatsakis (May 11 2020 at 20:10, on Zulip):

At the same time, particularly around async, this is a helpful lint to catch missing await, with at least a few use cases:

have been raised.

nikomatsakis (May 11 2020 at 20:11, on Zulip):

I feel like it'd be nice to get a bit more data -- for example, I'm curious about a proposal similar to #62262 except that we say that a type SomeType<T0..Tn> is must use if

(but we don't look at the fields of SomeType, as #62262 did)

Did we ever try that?

nikomatsakis (May 11 2020 at 20:11, on Zulip):

Anyway I was wondering whether either of you was motivated to try and pursue a change here. I think it would probably involve an RFC, although it depends on the magnitude of the change we land on.

Lokathor (May 11 2020 at 20:34, on Zulip):

So I think that being able to have types be Must Use mostly applies to closures and places where you can't mark the code's output as being must_use? I mean like Felix said in the meeting today, Must Use is really a property of code, not of data? Or, I guess "it should be", even if that's not true in Rust today. It seems like having Must Use on data types is just a shortcut to having Must Use on every function that returns that particular type of data. And then all this difficulty over having it be a trait or recursive or anything else is basically being caused by the fact that Must Use "isn't supposed" to be on data types in the first place?

I guess i'm not trying to favor any particular proposal at all, just asking for clarification on how you see things.

Josh Triplett (May 11 2020 at 20:42, on Zulip):

@Lokathor It's not just "you can't", it's also "you don't want to go to every function and mark it".

Josh Triplett (May 11 2020 at 20:42, on Zulip):

For instance, Result being must_use is useful.

simulacrum (May 11 2020 at 20:45, on Zulip):

I would even go so far as to say that if Result was not must_use, we would be in a significantly worse place wrt to "Rust ensures you check errors" today -- it's more than just useful.

Lokathor (May 11 2020 at 20:46, on Zulip):

I am in this case ignoring the grunt work of marking each function as must_use individually, yes.

Lokathor (May 11 2020 at 20:48, on Zulip):

But that's exactly my point really, making Result must_use as a type is just a shortcut for the actual practical effect that people want, which is that code that returns Result values should generally be must_use

simulacrum (May 11 2020 at 20:49, on Zulip):

Yes, that's largely true I think.

Lokathor (May 11 2020 at 20:52, on Zulip):

Well if we take that as the point of must_use being on a type, having a must_use field make your own type automatically become must_use by default and then adding a field attribute that you can place on a field for "this counts as used, stay quiet about it" if you need to opt out seems like the simplest system to understand as a user.

Jonas Schievink (May 11 2020 at 21:13, on Zulip):

I don't really think I have the time to pursue an RFC for this (I didn't realize one would be necessary, as I just intended to slightly improve diagnostics without any plans for stabilizing MustUse)

varkor (May 11 2020 at 21:15, on Zulip):

nikomatsakis said:

I feel like it'd be nice to get a bit more data -- for example, I'm curious about a proposal similar to #62262 except that we say that a type SomeType<T0..Tn> is must use if

(but we don't look at the fields of SomeType, as #62262 did)

Did we ever try that?

We didn't try that, but this seems quite ad hoc to me. Why should the behaviour of SomeType<Foo> be different to the same type declaration with Foo inlined?

varkor (May 11 2020 at 21:16, on Zulip):

I appreciate it's pragmatically motivated, but this seems special-cased enough that it's sure to be too limited.

varkor (May 11 2020 at 21:16, on Zulip):

A trait to me seems like the best approach, though there are tricky design decisions. Maybe we could write up something detailing the issues with the various approaches?

Asa Zeren (May 11 2020 at 21:28, on Zulip):

If we decide to use a trait (which I'm not 100% sold on), then perhaps the way that #[must_use] on functions work is by wrapping the result in a type MustUse<T> that impls MustUse and Deref<T>

Lokathor (May 11 2020 at 21:29, on Zulip):

I can't seem to see an RFC where must_use was added for types, just the one where it was added for functions.

Particularly, I'd like to see if there was long ago discussion about types other than Result being must_use. And more particularly, a quick ripgrep of just the files i have on this machine shows i've put must_use on at least 675 functions in my own projects, though I don't have all of my projects cloned on to this machine all at once, so it's probably higher than that.

nikomatsakis (May 11 2020 at 21:41, on Zulip):

varkor said:

I appreciate it's pragmatically motivated, but this seems special-cased enough that it's sure to be too limited.

A few thoughts:

I guess that your crater run likely contains all the relevant data, since I think it is a superset of what I proposed, right?

varkor (May 11 2020 at 22:41, on Zulip):

I guess that your crater run likely contains all the relevant data, since I think it is a superset of what I proposed, right?

Yes, that's true.

varkor (May 11 2020 at 22:42, on Zulip):

Though maybe it was long ago enough that it won't have picked up so many of the async-related cases.

scottmcm (May 13 2020 at 22:31, on Zulip):

I don't know if the parallel is useful here, but there's also been discussion of "automatic" [must_use] for functions. The idea being that a heuristic could catch the vast majority of places where it's be helpful (perhaps using a Freeze-like auto trait). If functions can sometimes be must-use based on their arguments, it seems plausible that types could also be must-use based on their arguments (if in a rather different way).

(Especially if there was an opt-out form of must_use so said heuristic could have some, though certainly very very few, false positives.)

nikomatsakis (May 14 2020 at 15:51, on Zulip):

Side note that, early on in Rust, there was a debate about whether we should make all values "must use", so that any unused result not of type () would be warned

nikomatsakis (May 14 2020 at 15:51, on Zulip):

We settled on the #[must_use] lint, for better or worse...

nikomatsakis (May 14 2020 at 15:52, on Zulip):

@scottmcm is the idea basically to catch things that don't appear to have side-effects and consider them "must use"? Sounds tricky to me, but maybe.

simulacrum (May 14 2020 at 15:54, on Zulip):

yeah, in particular basically we've recently been applying must_use to "all" functions that are something like fn(T, ...) -> T in std, particularly the integer and float inherent methods

oli (May 14 2020 at 15:57, on Zulip):

One obvious flag for side effect free functions are const fn without &mut or UnsafeCell arguments

simulacrum (May 14 2020 at 15:58, on Zulip):

yeah, something like that

simulacrum (May 14 2020 at 15:58, on Zulip):

it's unclear to me that we gain much from avoiding the must_use annotations, though otoh it's kinda similar to Result

Hanna Kruppe (May 14 2020 at 16:08, on Zulip):

oli said:

One obvious flag for side effect free functions are const fn without &mut or UnsafeCell arguments

However, this isn't 100% fool-proof, since const fns can panic. For example, Result::unwrap should be const once that feature is stable, and it's sometimes called primarily to panic on errors even if the Ok payload isn't relevant.

simulacrum (May 14 2020 at 16:11, on Zulip):

yeah you get into "pure" functions I guess :)

oli (May 14 2020 at 16:12, on Zulip):

ugh

oli (May 14 2020 at 16:12, on Zulip):

yea, I keep forgetting panicking counts as a side effect

scottmcm (May 14 2020 at 17:22, on Zulip):

On unwrap: one could argue that assert_matches!(x, Ok(_)); is better as a statement than x.unwrap();, or similar. (Or that it's weird to care that it's ok without looking at the non-() value anyway.)

It's not unusual for must_use functions to possibly be able to panic. For example, split_off can panic but is still must_use.

Hanna Kruppe (May 14 2020 at 17:27, on Zulip):

I just wanted to point out that this approach to inferring must_use has false positives, I'm not taking any position about how important those are.

Hanna Kruppe (May 14 2020 at 17:31, on Zulip):

(But indeed, split_off and many other functions are different from unwrap in that I've never seen a program call split_off etc. just to get its panic behavior.)

Hanna Kruppe (May 14 2020 at 17:36, on Zulip):

FWIW https://github.com/rust-lang/rfcs/pull/2450

Hanna Kruppe (May 14 2020 at 17:36, on Zulip):

(truly, there is nothing new under the sun)

Last update: Jun 05 2020 at 23:10UTC