Add a FRC about implicit numeric widening

[scottmcm]

r? @joshtriplett

[rustbot]

Error: The feature assign is not enabled in this repository.
To enable it add its section in the triagebot.toml in the root of the repository.

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #triagebot on Zulip.

[Mark-Simulacrum]

@rfcbot fcp merge lang

[rust-rfcbot]

Team member @Mark-Simulacrum has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[cramertj]

I appreciate the desire for users to consider overflow and think about whether upcasting early is the right choice.

However, I don't think it's inherently more problematic to add two u16s that are then used as a u32 (or, more commonly, as a usize) than it is to add two u16s values in the first place.

It's possibly useful to consider what changes we'd like to make if Rust allowed this coercion today. Personally, I don't think this meets the bar for a warn-by-default-lint: false positives would be extremely common. I would expect we'd instead suggest that this should be a clippy lint, one which could be a bit "fuzzier" and capture more information about the context in which the upcasting occurs (for example, is the value a result of an addition or multiplication, or some other operation which could result in an undesirable overflow?).

I think nearly every Rust user has at some point encountered my_vec[x as usize], and in some codebases it's littered everywhere. This is an extremely common papercut that I hear from users coming from other languages (C, C++, Python, Java, Kotlin, TypeScript, etc.) who are surprised at the level of unnecessary strictness and verbosity. It also has a tendency to show up in unsafe code, where the additional verbosity it adds to unsafe indexing makes it harder to verify the correctness of subtle algorithms.

Personally, I think implicit numeric widening + a thoughtful and comprehensive clippy lint is the correct balance.

[scottmcm]

I think nearly every Rust user has at some point encountered my_vec[x as usize], and in some codebases it's littered everywhere.

I agree that that's a problem, but it's not obvious to me that widening is the answer.

What if we had something like rustc's IndexVec in alloc? Then if you want to index by u32 everywhere, you'd use BikeshedVec<u32, T> instead of Vec<T>, and you wouldn't need implicit conversions "everywhere" because of course that's the type you're using to index, and indeed if you're ever indexing with usize something weird happened and you get an error about it. That'd let BikeshedVec<u32, T> be smaller too, which would be good for cache usage.

Or even without that, over an edition (because of inference changes) we could just allow indexing with any integer type but still not do widening.

(And widening to usize also adds all kinds of annoying complications around whether u32 can widen to usize and whether we add target-specific type system rules like that.)

[tmandry]

The strongest version of this argument to me goes like this. A user might not realize that the following code has a risk of overflow:

// Defined elsewhere, with a less obvious name
struct HasSmallOffset {
    offset: i8,
}

let sm1: HasSmallOffset;
let sm2: HasSmallOffset;
unsafe {
    some_ptr
        .offset(sm1.offset + sm2.offset)
        .write(value);
}

In this case it is clear that the argument type to offset() is unlikely to wrap for normal offset values, but for the offset field this is not so. Requiring a cast forces the user to consider this fact and put the cast in the right place.

The issue though is that detecting this problematic case happens downstream of the actual problematic operation, at the coercion site instead of at the add, and as a result it is very much an overapproximation. This leads to cases where the need to cast is purely a nuisance, like foo[sm1.offset as usize].

I think the places where the current behavior is useful would be better served by a custom integer type that requires more explicit syntax for operations that risk overflow. There is always an opportunity for the user to consider whether they ought to use an integer type like this. Usually multiple, e.g. in this case when writing the HasSmallOffset struct and when performing a narrowing cast from an isize to i8.

A family of types could be provided by the standard library that don't implement traits like Add<Self> but require explicit wrapping methods. Separately, we should consider a feature that allows assigning integer literals to custom integer types to make them more convenient.