Implement new lint for detecting buggy pointer-to-int casts

[osa1]

Fixes #81686

[rust-highfive]

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

[bors]

☔ The latest upstream changes (presumably #79078) made this pull request unmergeable. Please resolve the merge conflicts.

[varkor]

@osa1: are you waiting for this to be reviewed, or is it not ready yet? (I assumed you were still working on it because it was marked as a draft, but now I realise this might not be the case.)

[osa1]

Hi @varkor -- sorry for late reply. This is ready for reviews.

Note that the lint is not enabled by default, I'm guessing we want to enable it by default before shipping, right?

I realize I didn't add a test -- I'll do that now.

[varkor]

Thanks, this looks pretty good so far! I just have a few comments about wording, to try to make it a little clearer for users.

[varkor]

Note that the lint is not enabled by default, I'm guessing we want to enable it by default before shipping, right?

I think it would be reasonable to make this Warn by default. We'll also need to get the lang team to sign off on the new lint; I'll ping them once this is ready.

[osa1]

Hm, it seems like the check is not quite right. This program doesn't generate a warning:

fn main() {
    fn test() {}
    let x: u16 = ({
        test
    }) as u16;
}

[osa1]

Hm, it seems like the check is not quite right. This program doesn't generate a warning:

Figured it out, update coming.

[osa1]

@varkor should be ready now.

[osa1]

Ping @varkor

[varkor]

I think this is mostly looking good now. Could you address the one comment, and squash your commits (or tidy them up a bit, e.g. there shouldn't be fixup or merge commits)?

[osa1]

@varkor all done.

[osa1]

I made the lint deny-by-default, for the crater run.

[bjorn3]

@bors try

[bors]

⌛ Trying commit 81df07d with merge 7b3565da729099dbe807466fcd118fb2815986e0...

[rust-log-analyzer]

The job x86_64-gnu-llvm-10 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

.................................................................................................... 900/11735
.................................................................................................... 1000/11735
.................................................................................................... 1100/11735
...................................i................................................................ 1200/11735
...........................F........F............................................................... 1300/11735
.............................................iiii.ii.i.............i................................ 1500/11735
.................................................................................................... 1600/11735
............................................................i....................................... 1700/11735
.................................................................................................... 1800/11735
---
.................................................................................................... 9400/11735
.................................................................................................... 9500/11735
............................................................................i......i................ 9600/11735
.................................................................................................... 9700/11735
......................iiiiiii..iiiiii.i............................................................. 9800/11735
.................................................................................................... 10000/11735
.................................................................................................... 10100/11735
.................................................................................................... 10200/11735
.................................................................................................... 10300/11735
---
3    |
4 LL |         main as u32

-    |         ^^^^^^^^^^^
+    |         ^^^^^^^^^^^ help: to cast to `u32`, cast to `usize` first: `main as usize as u32`
-    = note: see issue #51910 <https://github.com/rust-lang/rust/issues/51910> for more information
-    = help: add `#![feature(const_raw_ptr_to_usize_cast)]` to the crate attributes to enable
-    = help: add `#![feature(const_raw_ptr_to_usize_cast)]` to the crate attributes to enable
+    = note: `#[deny(invalid_ptr_to_int_cast)]` on by default
+    = help: pointers should only be cast to `usize` or `u64`
- error[E0658]: casting pointers to integers in constants is unstable
+ error: casting pointer to `u32`
11   --> $DIR/cast-ptr-to-int-const.rs:9:9
12    |
12    |
13 LL |         &Y as *const u32 as u32
-    |         ^^^^^^^^^^^^^^^^^^^^^^^
-    |         ^^^^^^^^^^^^^^^^^^^^^^^
+    |         ^^^^^^^^^^^^^^^^^^^^^^^ help: to cast to `u32`, cast to `usize` first: `&Y as *const u32 as usize as u32`
-    = note: see issue #51910 <https://github.com/rust-lang/rust/issues/51910> for more information
-    = help: add `#![feature(const_raw_ptr_to_usize_cast)]` to the crate attributes to enable
-    = help: add `#![feature(const_raw_ptr_to_usize_cast)]` to the crate attributes to enable
+    = help: pointers should only be cast to `usize` or `u64`
Some tests failed in compiletest suite=ui mode=ui host=x86_64-unknown-linux-gnu target=x86_64-unknown-linux-gnu
19 error: aborting due to 2 previous errors
20 


- For more information about this error, try `rustc --explain E0658`.
22 


The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/cast/cast-ptr-to-int-const/cast-ptr-to-int-const.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args cast/cast-ptr-to-int-const.rs`
error: 1 errors occurred comparing output.
status: exit status: 1
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/cast/cast-ptr-to-int-const.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/cast/cast-ptr-to-int-const" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/cast/cast-ptr-to-int-const/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error: casting pointer to `u32`
  --> /checkout/src/test/ui/cast/cast-ptr-to-int-const.rs:5:9
   |
LL |         main as u32 //~ ERROR casting pointers to integers in constants is unstable
   |         ^^^^^^^^^^^ help: to cast to `u32`, cast to `usize` first: `main as usize as u32`
   |
   = note: `#[deny(invalid_ptr_to_int_cast)]` on by default
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `u32`
  --> /checkout/src/test/ui/cast/cast-ptr-to-int-const.rs:9:9
   |
   |
LL |         &Y as *const u32 as u32 //~ ERROR is unstable
   |         ^^^^^^^^^^^^^^^^^^^^^^^ help: to cast to `u32`, cast to `usize` first: `&Y as *const u32 as usize as u32`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: aborting due to 2 previous errors


------------------------------------------
------------------------------------------


---- [ui] ui/cast/cast-rfc0401.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/cast/cast-rfc0401.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/cast/cast-rfc0401/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/cast/cast-rfc0401/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error: casting pointer to `i16`
  --> /checkout/src/test/ui/cast/cast-rfc0401.rs:165:20
   |
LL |         assert_eq!(foo as i16, foo as usize as i16);
   |                    ^^^^^^^^^^ help: to cast to `i16`, cast to `usize` first: `foo as usize as i16`
   |
   = note: `#[deny(invalid_ptr_to_int_cast)]` on by default
   = help: pointers should only be cast to `usize` or `u64`
error: aborting due to previous error


------------------------------------------
------------------------------------------


---- [ui] ui/consts/const-eval/const_prop_errors.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/consts/const-eval/const_prop_errors.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/consts/const-eval/const_prop_errors" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/consts/const-eval/const_prop_errors/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error: casting pointer to `u32`
  --> /checkout/src/test/ui/consts/const-eval/const_prop_errors.rs:10:9
   |
LL |         bar::<T> as u32
   |         ^^^^^^^^^^^^^^^ help: to cast to `u32`, cast to `usize` first: `bar::<T> as usize as u32`
   |
   = note: `#[deny(invalid_ptr_to_int_cast)]` on by default
   = help: pointers should only be cast to `usize` or `u64`
error: aborting due to previous error


------------------------------------------
------------------------------------------


---- [ui] ui/issues/issue-40883.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/issues/issue-40883.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-40883/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-40883/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error: casting pointer to `isize`
  --> /checkout/src/test/ui/issues/issue-40883.rs:79:9
   |
LL |         (&mut stack_var as *mut _ as isize) -
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: to cast to `isize`, cast to `usize` first: `&mut stack_var as *mut _ as usize as isize`
   |
   = note: `#[deny(invalid_ptr_to_int_cast)]` on by default
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `isize`
  --> /checkout/src/test/ui/issues/issue-40883.rs:80:13
   |
   |
LL |             (before_ptr as isize)) as usize;
   |             ^^^^^^^^^^^^^^^^^^^^^ help: to cast to `isize`, cast to `usize` first: `before_ptr as usize as isize`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: aborting due to 2 previous errors


------------------------------------------
------------------------------------------


---- [ui] ui/mir/mir_misc_casts.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/mir/mir_misc_casts.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/mir/mir_misc_casts/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/mir/mir_misc_casts/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error: casting pointer to `isize`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:10:14
   |
LL |     let c1 = f as isize;
   |              ^^^^^^^^^^ help: to cast to `isize`, cast to `usize` first: `f as usize as isize`
   |
   = note: `#[deny(invalid_ptr_to_int_cast)]` on by default
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i8`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:12:14
   |
   |
LL |     let c3 = f as i8;
   |              ^^^^^^^ help: to cast to `i8`, cast to `usize` first: `f as usize as i8`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i16`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:13:14
   |
   |
LL |     let c4 = f as i16;
   |              ^^^^^^^^ help: to cast to `i16`, cast to `usize` first: `f as usize as i16`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i32`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:14:14
   |
   |
LL |     let c5 = f as i32;
   |              ^^^^^^^^ help: to cast to `i32`, cast to `usize` first: `f as usize as i32`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i64`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:15:14
   |
   |
LL |     let c6 = f as i64;
   |              ^^^^^^^^ help: to cast to `i64`, cast to `usize` first: `f as usize as i64`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `u8`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:16:14
   |
LL |     let c7 = f as u8;
LL |     let c7 = f as u8;
   |              ^^^^^^^ help: to cast to `u8`, cast to `usize` first: `f as usize as u8`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `u16`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:17:14
   |
LL |     let c8 = f as u16;
LL |     let c8 = f as u16;
   |              ^^^^^^^^ help: to cast to `u16`, cast to `usize` first: `f as usize as u16`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `u32`
  --> /checkout/src/test/ui/mir/mir_misc_casts.rs:18:14
   |
LL |     let c9 = f as u32;
LL |     let c9 = f as u32;
   |              ^^^^^^^^ help: to cast to `u32`, cast to `usize` first: `f as usize as u32`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: aborting due to 8 previous errors


------------------------------------------
------------------------------------------


---- [ui] ui/supported-cast.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/supported-cast.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/supported-cast/a" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/supported-cast/auxiliary"
------------------------------------------

------------------------------------------
stderr:
stderr:
------------------------------------------
error: casting pointer to `isize`
  --> /checkout/src/test/ui/supported-cast.rs:5:20
   |
LL |   println!("{:?}", f as isize);
   |                    ^^^^^^^^^^ help: to cast to `isize`, cast to `usize` first: `f as usize as isize`
   |
   = note: `#[deny(invalid_ptr_to_int_cast)]` on by default
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i8`
  --> /checkout/src/test/ui/supported-cast.rs:7:20
   |
   |
LL |   println!("{:?}", f as i8);
   |                    ^^^^^^^ help: to cast to `i8`, cast to `usize` first: `f as usize as i8`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i16`
  --> /checkout/src/test/ui/supported-cast.rs:8:20
   |
   |
LL |   println!("{:?}", f as i16);
   |                    ^^^^^^^^ help: to cast to `i16`, cast to `usize` first: `f as usize as i16`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i32`
  --> /checkout/src/test/ui/supported-cast.rs:9:20
   |
   |
LL |   println!("{:?}", f as i32);
   |                    ^^^^^^^^ help: to cast to `i32`, cast to `usize` first: `f as usize as i32`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `i64`
  --> /checkout/src/test/ui/supported-cast.rs:10:20
   |
   |
LL |   println!("{:?}", f as i64);
   |                    ^^^^^^^^ help: to cast to `i64`, cast to `usize` first: `f as usize as i64`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `u8`
  --> /checkout/src/test/ui/supported-cast.rs:11:20
   |
   |
LL |   println!("{:?}", f as u8);
   |                    ^^^^^^^ help: to cast to `u8`, cast to `usize` first: `f as usize as u8`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `u16`
  --> /checkout/src/test/ui/supported-cast.rs:12:20
   |
   |
LL |   println!("{:?}", f as u16);
   |                    ^^^^^^^^ help: to cast to `u16`, cast to `usize` first: `f as usize as u16`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: casting pointer to `u32`
  --> /checkout/src/test/ui/supported-cast.rs:13:20
   |
   |
LL |   println!("{:?}", f as u32);
   |                    ^^^^^^^^ help: to cast to `u32`, cast to `usize` first: `f as usize as u32`
   |
   = help: pointers should only be cast to `usize` or `u64`
error: aborting due to 8 previous errors


------------------------------------------
---
test result: FAILED. 11633 passed; 6 failed; 96 ignored; 0 measured; 0 filtered out; finished in 140.24s



command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "ui" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-10/bin/FileCheck" "--nodejs" "/usr/bin/node" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "10.0.0" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets amdgpu amdgpuasmparser amdgpucodegen amdgpudesc amdgpudisassembler amdgpuinfo amdgpuutils analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter bpf bpfasmparser bpfcodegen bpfdesc bpfdisassembler bpfinfo cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker engine executionengine frontendopenmp fuzzmutate globalisel hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interpreter ipo irreader jitlink lanai lanaiasmparser lanaicodegen lanaidesc lanaidisassembler lanaiinfo libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcerror orcjit passes perfjitevents powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo riscvutils runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info x86utils xcore xcorecodegen xcoredesc xcoredisassembler xcoreinfo xray" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test --exclude src/tools/tidy
Build completed unsuccessfully in 0:14:02

[osa1]

Can I skip the compiler tests before the crater run somehow? I don't want to update all these tests just to run crater with the lint is denied by default.

[bjorn3]

The failed job is one of the PR CI jobs. Try builds still succeed if tests are failing as far as I know.

[bors]

☀️ Try build successful - checks-actions
Build commit: 7b3565da729099dbe807466fcd118fb2815986e0 (7b3565da729099dbe807466fcd118fb2815986e0)

[bjorn3]

@craterbot check

[craterbot]

👌 Experiment pr-81789 created and queued.
🤖 Automatically detected try build 7b3565da729099dbe807466fcd118fb2815986e0
⚠️ Try build based on commit 836c317, but latest commit is 81df07d. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[osa1]

3 days and the job is still in the queue? 😲

[varkor]

@osa1: crater takes a while to complete (it has to compile all the crates on cargo.io), and the queue is quite busy at the moment.

[craterbot]

🚧 Experiment pr-81789 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-81789 is completed!
📊 239 regressed and 6 fixed (154723 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[cramertj]

@rust-lang/lang discussed this again recently. We felt fairly hesitant about adding this lint.

When we discussed previously, there was consensus that we did not want to lint on pointer -> u64 casts. However, at that time we didn't properly understand the main problem this lint was aiming to solve. After reviewing the original issue, it seems like the lint introduced in this PR doesn't solve the general problem described in that issue.

There are a number of related issues occurring:

1. I meant to call a function and cast the result but I cast a function pointer instead. Example: u16::max as u32 when I wanted u16::max() as u32.
2. I meant reference a constant and cast it but I cast a similarly named function pointer instead. Example: u16::max as u32 when I wanted u16::MAX as u32.
3. I meant to dereference a pointer and cast the result but I cast the pointer instead. Example: ptr as u32 when I wanted *ptr as u32.
4. I accidentally wrote non-portable code by casting a pointer to u32. Example: ptr as u32 when I wanted to store pointers in an integer, possibly ptr as u64.

As far as I can tell, it seems like the primary issue described in #81686 is (1) and maybe (2). This PR seems to primarily address (4), with the side effect that some (but not all) instances of 1-3 are also detected.

It feels like the lint in this PR as-implemented is likely to introduce a number of false positives (I really did want to store that pointer in a u32), yet a number of the cases in the original bug are not detected (e.g. u16::max as u128).

Personally, I feel like this issue is another reason why we should offer convenient and ergonomic replacements for as (e.g. my_u8.upcast():u64, my_u64.truncate():u8), at which point we could guide users away from as entirely, possibly even removing it on an edition boundary.

[osa1]

I think #81686 describes (2), not (1). Quoting the original issue:

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

However, what the issue describes may not matter too much, we may still want to provide a lint for casting function items/pointers to integer types when it will obviously overflow. We already do this for some other cases, for example:

fn main() {
    let x: u32 = u32::MAX + 1;
}

generates

error: this arithmetic operation will overflow
 --> src/main.rs:2:18
  |
2 |     let x: u32 = u32::MAX + 1;
  |                  ^^^^^^^^^^^^ attempt to compute `u32::MAX + 1_u32`, which would overflow
  |
  = note: `#[deny(arithmetic_overflow)]` on by default

It seems inconsistent to not do the same check when casting a pointer to a smaller integer type.

It feels like the lint in this PR as-implemented is likely to introduce a number of false positives (I really did want to store that pointer in a u32)

Could you say more about your use case? Is this on a 32-bit platform?

yet a number of the cases in the original bug are not detected (e.g. u16::max as u128).

We could detect this case as well. It was originally detected, but removed per @joshtriplett's request: #81789 (comment)

[osa1]

As I say above, I think this lint makes sense regardless what you think #81686 is about, but if the language team disagrees then please feel free to close this PR, or let me know and I will close it.

[scottmcm]

We discussed this again in the @rust-lang/lang meeting this week. We didn't come to a clear consensus, though, so what I'm posting here is a proposal for a direction to take this, not anything final or signed-off.

I propose that we move forward with this lint, but remove the "Fixes #81686" from the OP. That would mean that this lint becomes just about general pointer truncation, and another PR would target that issue more specifically.

That means that this lint will not solve the isize::max as usize problem. I would propose that that be addressed by an additional PR that would focus on casts from function voldemort ZSTs specifically, where we can lint more aggressively than we can on pointers. (We certainly don't want to lint on as usize on a normal pointer -- although maybe we could move that way in the future after some hypothetical libs additions that could be used instead to be specific in intent.)

I think that also implies a few changes here. For example, the crater run shows a bunch of errors from casting to isize in order to subtract, which seems to me like they probably don't meet the threshold for a rustc lint.

[nikomatsakis]

I'm curious about this statement by @cramertj:

It feels like the lint in this PR as-implemented is likely to introduce a number of false positives (I really did want to store that pointer in a u32)

This seems like the important question! I can imagine this comes up because of portability (e.g., when targeting 32-bit systems), but I'm not sure if there are other times when people really do want to store pointers into a u32 (which of course remains an option, but more awkward).

[Mark-Simulacrum]

We discussed this in the last lang team meeting.

We noted that:

• We'd like to have a concrete example of a real-world bug that the lint in this PR (not described on the issue) is fixing
  • We've eliminated a lot of cases where the lint would fire already (u64/i64, isize/usize...)
• In terms of portability, if writing code targeting only 32-bit systems, it may be beneficial to have an allow-by-default lint catching additional cases.
  • We're pretty sure we don't want a lint that fires only on some targets, because that's confusing and hard to manage for users.
• We would like to have these lint(s) recommend a "better path", and suspect that better path is explicit to/from_bits methods, at least to start - a thread has been kicked off on Zulip to discuss those.
  • That would let us be more aggressive about deprecating parts of as casts, in favor of methods which are more explicit about exactly what the user wanted.

We didn't discuss next steps too concretely, but it feels like it might be good for someone to propose a PR adding to and from_bits to *const T, *mut T, and perhaps fn(...) and the zero-sized function types. For the first two some of the discussion in the lang meeting indicated we may want to restrict to just 'thin' pointers to start (via <T as Pointee>::Metadata = (). For the latter that starts to touch on Niko's proposal in rust-lang/lang-team#23.

r? @joshtriplett for now as you've been driving this the most I think

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting, as well as last week's @rust-lang/libs meeting.

In @rust-lang/lang, our consensus was that we'd love to see as used much less, and we'd be happy to lint against it in cases where we have some specific function to recommend that better expresses the intent and isn't as X as Y. And if we get to the point where most functions of as are served by functions that better express intent, we could consider changing the behavior of as in a new edition (e.g. making it only a shorthand for a subset of type changes, like widening).

In @rust-lang/libs, our consensus was that we'd be happy to take proposals for functions that serve the function of as (e.g. truncate for integer narrowing, as_bits to turn a pointer into an integer, to_signed or to_unsigned for signedness changes only). We'd love to see a comprehensive effort in this regard, but we'd be happy to take individual proposals for such functions, as long as we feel they'd fit into a coherent overall design.

@rust-lang/libs also expressed the sentiment that as is a very convenient shorthand, especially since it doesn't require writing turbofish (as u8 seems lexically simpler than truncate::<u8>()), but overall still seemed amenable to the idea of providing functions that better express intent.

We're going to close this PR, as we don't think we're going to accept this precisely as written. We'd be thrilled to see functions proposed to libs, and then corresponding lints proposed to lang to use those functions rather than as. (This lint in particular could suggest the use of as_bits.)

[joshtriplett]

To clarify: we'd be happy to see some version of this lint, and we think there's value in catching issues like this; we'd be happy to see this reopened and updated when there are methods that it can suggest.