Emit warning when using as to directly cast function items to any integral type other than usize

[bstrie]

Imagine you have a u32. You want to check if it's larger than the maximum value of a u16. Of course, you know that the API will give you a u16, so you will need to cast it to a u32.

You write a unit test. 100 million should be above the range of a u16, you think.

let x = 100_000_000;
let y = u16::max as u32;
println!("{}", x > y);

Output:

false

You begin to doubt your computer science education. In a fugue, you abandon all worldly possessions. You move to the woods. You raise chickens now; sometimes, though, you wonder if they're truly the ones raising you. It is a simpler life.

---

u16::max is the UFCS'd form of the default'd max method on the prelude'd Ord trait. To a first approximation, there is no reason to ever directly cast a function item to anything other than a usize. The compiler should emit a warning when attempting to cast a function item to u8, u16, u32, u64, u128, i8, i16, i32, i64, i128, and isize. This would not apply to transitive casts; foo as usize as i8 would not warn.

(If I had realized this existed, I would have renamed u16::MAX back when I had the chance.)

[osa1]

What would be the "lint group" of this lint? Currently we have: "nonstandard_style", "unused", "rust_2018_idioms", "rustdoc". This one doesn't fit into any of these, I think.

Should this be a clippy lint maybe?

[bstrie]

@osa1 , allow me to expand on my reasoning as to why I believe this should be in the compiler, rather than clippy.

Ultimately I believe this to be a flaw in the specification of the type cast operator, as. As documented in the reference (https://doc.rust-lang.org/reference/expressions/operator-expr.html#type-cast-expressions), there are four possible categories of type casts that relate to "addresses":

• "Pointer to address cast": *T where T: Sized as Numeric type [Note: "Numeric type" here is obviously a typo, and is meant to be "Integer type"]
• "Address to pointer cast": Integer type as *V where V: Sized
• "Function item to address cast": Function item as Integer type [Note: this specifically is the cast shown in the example above]
• "Function pointer to address cast": Function pointer as Integer type

It is my opinion that the presence of "Integer type" in all of these "address"-related casts is an error in the language; all instances of "Integer type" should be replaced by "usize".

That said, this would be considered a breaking change if it were to simply be implemented. The only backwards-compatible way of moving forward here is as follows:

1. Make the use of "address" casts that involve types other than usize into a compiler warning.
2. In a future edition of Rust (be that 2021 or further in the future), turn the lint from "warn" to "deny" for users who opt-in to the new edition.

This issue is intended to gauge the willingness of the lang team to make such a change. If people are receptive, I would be willing to close this issue in favor of writing a full RFC for the broader change.

The specific example chosen is deliberately horrific; no Rust expert that I have yet shown it to has immediately understood what's happening (excerpted responses from said Rust experts: "OH", "oof", "AH", "oh wow", "oh goddddd"). If the problem were just "<u16 as Ord>::max and u16::MAX are easily confusable", then that is something that would be most appropriate in clippy rather than in the compiler. However, I argue that the underlying problem is more fundamental.

[osa1]

OK so the long-term goal you have in mind is to restrict pointer to integer casts to usizes. In that case I agree that this check should be in the compiler.

I already implemented the lint. I wasn't sure how to "register" it, but I just realized that we also have built-in lints (like arithmetic_overflow that rejects code like 1i32 << 32) that don't need to be in one of the categories listed in my comment above, so this could be one of those lints. I'll submit a PR later today.

(If anyone could assign this to me that would be helpful)

Regarding disallowing pointer to u32 etc. casts I guess we need to get feedback from the lang team first. Should we mark this as "I-nominated"?

Even if the lang team decides that the change is not desirable perhaps we can still have a warning in the compiler when casting a pointer to a type other than usize. (I'm guessing existing lints like "arithmetic overflow" are also allowed by the spec so maybe that's OK)

[leonardo-m]

The long-term goal should be to nearly deprecate the usage of naked "as" and use it only for special situations. And use u32::from (here u32::from avoids this problem), try_from, and other future safer conversion functions (like a standard function/macro that performs a range test in debug builds and works like as in release builds).

[bstrie]

I agree that there seems to be a vague and unspoken movement that as is less than ideal and that more principled alternatives should be provided and encouraged. However that's a large, disruptive, long-term goal, and I think the way best to move forward while avoiding total design deadlock is through a series of small and gradual improvements such as this one.

[scottmcm]

This makes good sense to me. I can't imagine any situation in which casting a function to an i8 is what you want.

(I might even be tempted to go further and require going through fn(...), like casting references requires going through *const ..., but as you say that's a bigger conversion and shouldn't block a helpful first step like linting this.)

And to add to the fun, this also happens with

let y = u16::max_value as u32;

which is a whole different kind of typo.

[bstrie]

max_value at least is on track to be deprecated (#68490), which should ameliorate that particular unfortunate quirk.

And yes, I considered a proposal to ban both function item and function pointer to address casts, although your suggestion of only banning the function item cast sounds both less disruptive and totally sufficient for preventing confusion. Though even if that were done I would still propose limiting the remaining address casts to usize.

[bstrie]

Here's another really bad case:

One of the allowed as casts is called "enum casting": when you have a C-style enum, you can cast a variant to an integer.

enum Foo {
    A,
    B
}

Foo::A as i32; // works

Now, what happens when we add data to A?

enum Foo {
    A(String),
    B
}

Foo::A as i32; // uh oh!

This still compiles! The compiler is supposed to produce a non-primitive cast error (and it would have if we had done Foo::B as i32), but remember that enum variant constructors are just functions, whose address here is cast to an integer.

[scottmcm]

In addition to a good case for a lint on its own, sounds like another good reason to have an .into_repr() or similar, like the conversation in #81642 (comment).

(Then we could start suggesting that instead of the as, if said trait is implemented, to avoid this possibility.)

[bstrie]

Now that Rust supports explicit discriminants in data-carrying enums in 1.66, this problem is even more acute and easier to stumble into than ever (even the 1.66 release announcement for this feature made it easy for users to stumble into this footgun, see rust-lang/blog.rust-lang.org#1056 ).

[lukas-code]

Clippy has a lint for this, which we could uplift to rustc: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation

[bstrie]

Note that there's currently an RFC to forbid these casts entirely in the next edition: rust-lang/rfcs#3526