Warn by default when casting a pointer to an integer smaller than usize

[comex]

As it says on the tin.

Rendered

[est31]

I don't like platform dependent warnings. Rust code is supposed to be platform independent, except for things inside cfg blocks. What comes next, a warn for print!("\n") only on windows and mac?

[sfackler]

I think I'd honestly be fine warning when casting a pointer to anything other than a usize or isize.

[petrochenkov]

cc #1052

[oli-obk]

I'd rather throw it into clippy (as suggested in the alternatives) and create an RFC for moving lints from clippy to rustc (#1501).

cc @mcarton @Manishearth @llogiq

[vks]

I think I'd honestly be fine warning when casting a pointer to anything other than a usize or isize.

Will there ever be platforms where a pointer does not fit into 64 bits?

[Manishearth]

Will there ever be platforms where a pointer does not fit into 64 bits?

It's possible, but in that case isize/usize would still be pointer-width.

[vks]

I asked because I want to convert usize to u64 without warnings. That would be possible if there are no pointers larger than that.

[sfackler]

This has come up before in whether a From<usize> implementation for u64 should exist, and our answer then was a tentative "no", I believe.

[nikomatsakis]

I agree with @sfackler that warning when not casting to usize probably makes sense. As for the matter of "platform dependent lints", I still think that the overall strategy of ensuring that rust code is portable to major platforms by default, but providing opt-in lints or other tools for ensuring compliance beyond that makes sense. I'm not sure how much discussion about this idea has been happening lately.

That said, it might be that I'd rather see this change happen as part of a bigger suite of changes addressing integer sizes conversions.

[solson]

@comex Consider expanding the RFC to include casts from function pointers. I think it's ridiculous (and probably a 1.0 rush oversight) that we accept this code without warning:

println!("{}", main as u8); // 192

[keeperofdakeys]

If the official recommendation is that isize and usize should only be used for pointers, and memory sizes, then this lint seems like a rather good idea (even if it's only in clippy for now).

[petrochenkov]

@vks @sfackler @nikomatsakis
rust-lang/rust#37423 exists, by the way, and addresses usize/isize conversions and portability.

[nikomatsakis]

So I think there are two main questions to resolve here:

1. Under what conditions do we add new lints? How do we decide between rustc and clippy? I don't know the answer to this, but I do tend to agree that this lint falls into the "probably a bug" category, as do most (but certainly not all) of the rustc lints.
2. Do we want to make the lint target dependent?

I'm inclined to say: it's ok to add new lints to rustc that fall into the "almost certainly a bug" category. I'd be wary of style lints.

I'm not sure what to think about target dependent lints, but I find the reasoning in the RFC reasonably persuasive. I'd be interested to hear if @aturon thinks that indeed scenarios do apply here (the mechanism described in the current RFC is not obviously applicable, right?).

[nikomatsakis]

@petrochenkov indeed, I'll have to review #37423 and compare I guess. From skimming the comments it seems the sense was "let's figure out how this interacts with scenarios"...maybe we have a better idea of that now?

[Manishearth]

I'm inclined to say: it's ok to add new lints to rustc that fall into the "almost certainly a bug" category. I'd be wary of style lints.

I agree. Clippy is more for things which are less certain or not bugs. We do have a bunch of "almost certainly a bug" lints -- I think they're fine where they are, but would be open to upstreaming too.

[scottmcm]

This seems like another case of the "you have to use as but that might be a bug because widening and truncation aren't distinguished" problem.

Has impl<T> From<*const T> for usize (and similar) been considered? This warning might be more palatable if there was another way that clearly doesn't hit it...

[aturon]

Just to weigh in from the perspective of the portability lint RFC: the whole point of that RFC is to add a platform-specific lint, so this seems fine on that front at least.

If we wanted to aim for maximum alignment with the portability lint, we'd want to warn when e.g. going from usize to u32 on a 32-bit platform, unless you were within a cfg block that implied a 32-bit arch. Or, put differently, this should probably just be part of the portability lint, working as if as in such cases was a normal API marked with cfg.

I'm about to merge the portability lint RFC, and as far as I'm concerned, this RFC is basically an implementation issue for that one. So we might consider punting on this RFC, getting the initial implementation working, and then revisiting this as a PR later (when we have more experience).

[nikomatsakis]

@rfcbot fcp postpone

I agree with @aturon's comment here:

I'm about to merge the portability lint RFC, and as far as I'm concerned, this RFC is basically an implementation issue for that one. So we might consider punting on this RFC, getting the initial implementation working, and then revisiting this as a PR later (when we have more experience).

Let's get the portability lint underway. At that point, I think we can see whether we feel this is a natural consequence thereof (and hence just implement it) or it we want to revisit the RFC in that context.

(Thanks to @comex for a good RFC in any case.)

[rfcbot]

Team member @nikomatsakis has proposed to postpone this. The next step is review by the rest of the tagged teams:

• @aturon
• @eddyb
• @nikomatsakis
• @nrc
• @pnkfelix
• @withoutboats

No concerns currently listed.

Once these reviewers reach consensus, 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!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period is now complete.

[aturon]

Closing as postponed. Thanks again, @comex!