Array::map annotate with #[must_use]

libs
1

Hi, I frequently call Array::map, and occasionally find that I will map an array, and forget to use the output when the underlying type is Copy. I was thinking it makes sense to annotate Array::map with #[must_use], which would at least throw a warning if the map is not consumed.

I don't know if this is the right forum for asking this question, but if willing I can submit a PR quickly for this.

1 Like
2

This is the right place to ask.

I think this is reasonable, although there are differing opinions on how much stuff to mark #[must_use].

[1]

  1. In this case map couldn't technically be implemented as a &mut self method (it could not change the type if it was), but this may not be immediately obvious to everyone, and there is no meaningful way to use it without using the result. ↩︎

3

Ah great.

I think it's very similar to the case of iterators, where I am doing something like

let a = [....;3];
a.map(|v| v / 0.5); // forgot to reassign to a

Should I wait for some kind of consensus or when is appropriate to submit a PR?

4

The one difference to Iterator::map() is that array::map() is not lazy.

E.g.

let a = [1, 2, 3, 4];
a.map(|i| println!("{i}"));

works and prints four lines, while

let a = [1, 2, 3, 4];
a.into_iter().map(|i| println!("{i}"));

does nothing.

It's not very idiomatic to use array::map() that way, but it's possible and that might be one reason why it isn't marked #[must_use] yet.

2 Likes
5

I think that would be a good reason to add array::for_each, then let map be must_use. However, the map docs also say "[T; N]::map is only necessary if you really need a new array of the same size as the result," which would not be the same situation with for_each.

1 Like
6

I'd rather see:

let _ = a.map(|i| println!("{i}"));

or

drop(a.map(|i| println!("{i}")));

Basically the same argument holds for almost all functions that return something: yes, sometimes you may want to ignore the result, but you can easily do so explicitly and it's more readable than having it dropped silently. So I think more-less, perhaps with a few exceptions, all functions that return something should be marked #[must_use].

In fact I'd prefer it if every expression statement was required to be of type (), and otherwise it would just be a compile error, which would make #[must_use] obsolete.

9 Likes
7

https://doc.rust-lang.org/rustc/lints/listing/allowed-by-default.html#unused-results?

2 Likes
8

I feel pretty strongly against adding Array::for_each, at that point it the array should be directly used in a for loop or converted to an iterator, unless there's something I'm missing.

Certainly there are some situations where every output should be consumed, but I think pragmatically that would be a huge pain. Why I think that Array::map should have #[must_use] is because pragmatically it is not meant to produce side effects (every situation I'm using it in is a transformation of the input, generally vectors in 3D/2D). With both iterators and arrays, it is likely a bug if the output is not consumed, although the actual iterated values won't actually be accessed in the case of iterators.

9

Thanks! I will probably turn this on in my projects.

But I feel that every time I change such a setting from the default I'm creating a new variant of the language, creating a split. When I switch to reading another project I will have to remember that implicit dropping is possible again. So there is value in making certain things enforced as hard errors rather than configurable lints.

10

This lint is "allow" by default because it can be noisy, and may not be an actual problem. For example, calling the remove method of a Vec or HashMap returns the previous value, which you may not care about. Using this lint would require explicitly ignoring or discarding such values.

Maybe methods that are meant to sometimes be ignored, like Vec::remove, should be marked with something like #[may_ignore]. Then the unused_results lint could be promoted to a warning, and in a later edition maybe ignoring a #[must_use] thing could even be an error by default (doing this in another edition would at least avoid breaking crates that aren't maintained anymore).

The only trouble is that many crates would also like to mark some types and/or functions with #[may_ignore] (but probably only after this attribute stabilized), so enabling warnings for unused_results right away could add too much noise.

3 Likes
11

You might be interested in this recent thread:

3 Likes