RFC for unused const fn results

[est31]

Proposes a lint on side effect free const fn invocations that are not used by the program.

Rendered

An example would be:

const fn add_one(v: u32) -> u32 {
    v + 1
}
fn foo() {
    add_one(2); // WARNING unused result of const fn
}

Thanks go to @Centril and @rkruppe who reviewed the drafts.

[oli-obk]

This needs to be enforced recursively, as otherwise a simple wrapper type around a Drop type would suffice to trick the analysis. Maybe we should just start out with copy types?

[est31]

We might already need a recursive check for UnsafeCell. Or does UnsafeCell inhibit Copy?

[oli-obk]

No, Cell is Copy and contains an UnsafeCell.

[scottmcm]

Is this what TyS::is_freeze is for?

[fstirlitz]

Which of the two criteria does this example fall under? Remember () is inhabited.

[est31]

"have all its generic types known" and "have the types of all input parameters contain no mutability". I re-started the numbering for the second group of criteria, but this seems to be confusing, so I might change that.

[fstirlitz]

IIRC 'dead code' is synonymous with 'unreachable code'. The invocations in your examples aren't unreachable; it's just that their results are discarded (and since they are free of side effects, they can be optimised away).

[WiSaGaN]

I feel positive in general for this. Several points below:

• Should we add a way to suppress this warning for a specific function invocation? I can't think of a situation that I really need this, but there may be.

• Besides, when defining a const fn can we attach attribute of allow_unused to it to suppress this warning in general for that function?

• Should any thing be done when one add a must_use attribute to a const fn?

[scottmcm]

This sounds great, @est31. I'd much rather piggy-back the lint on the already-happening constification (like slice len just happened, for example) than have an additional must-use-ification.

[joshtriplett]

This sentence seems awkwardly phrased, with a long middle clause and no commas. Consider rephrasing it for clarity.

[hanna-kruppe]

[I've put off commenting here long enough, it's becoming clear I'll never get the time and energy to write something that's as careful and nuanced as this topic deserves. So I'll just give it my best. Apologies if that winds up being curt or heavy-handed.]

Like @scottmcm I am quite excited about the prospect of side-stepping the must_use-ification and getting better lints for free. However, after some reflection I am not sure whether that actually works, since const fns can diverge and that divergence can be desirable and a sufficient reason for calling. Divergence is discussed in the RFC, but hand-waved away by focusing on the not-obviously-useful case of infinite or long-running loops. Much more important, and harder to explain away, are panics, especially conditional ones (which can't be visible in the function signature).

For example, consider an assert!-like function¹ that returns () but panics if some condition isn't true. Complaining about the return value being unused would be pointless, the interesting case is when it doesn't return. To pre-empt proposals to treat () specially, also consider Option::expect or Result::unwrap, which generally return non-unit values and can be useful as an assertion that something is Ok/Some even when that return value is unused. Two examples of that are the docs of Thread::join and I/O functions such as Seek::seek (which can be part of const fn code because Cursor<&[u8]> and other in-memory types implement it).

One can argue whether all of those are good coding style, but even if not, it makes clear that the proposed lint has serious false positives. Furthermore, these false positives are inherent in the approach and can't easily be fixed. Panics are an important tool in Rust, and as const fns encompass more and more of Rust, treating const fns without considering panics is as hopeless as approaching general Rust code without considering panics.

¹ assert!() being a macro complicates the matter somewhat, but there's no shortage of functions which do basically the same job.

---

Now, to preempt some objections:

• yes, unconditional panics are often visible from the function signature (returning -> !), but that's not very interesting (though it does probably prevent false positives on panic!() and the like)
• yes, we could and probably should special case () return values (though it would be special casing and hence icky -- I don't think all ZSTs should get this treatment) that would help with some but not all cases
• yes, this is a bit of an edge case, but we have a policy of very few false positives for rustc lints.

[oli-obk]

The RFC OP states that the goal is to "lint on side effect free const fn invocations that are not used by the program". The motivation then clarifies this as

The Rust language already has formalized side effect freedom for functions through the const fn language subset.

Which is true right now, but as @rkruppe correctly asserted, won't be so in the future. Panicking is a side-effect, but still fine for const fn, because panicking inside a const fn aborts the compilation in case it happens at compile-time.

While const fn and side-effect-free functions have a large overlap, they are not the same set.

In rust-lang/rust#51570 I am writing an analysis that figures out whether a call to a const fn should be promotable. Promotability is a subset of side-effect-freedom, because only trivial functions are allowed (that do not do any actual computations, but only destruct/construct objects)

Thus, I suggest we use the same analysis to decide which function calls to lint.

[est31]

The period of my contributions to Rust upstream has reached an end. Thus I'm unable to continue my work on this. I still think something like this is a great addition. I urge anyone interested in this change to adopt and continue it from here on. Thanks.

[Aaron1011]

I'm interested in adopting this RFC. What is the first step that I should take to do so?

[Centril]

@Aaron1011 Make a new branch of this one in your own fork; you should also try to address @rkruppe's and @oli-obk's comments / concerns in a new RFC.