This is kind of a follow-up to https://github.com/rust-lang/rust/pull/117758 where we added a lint against wide pointer comparisons for being ambiguous and unreliable; well function pointer comparisons are also unreliable. We should IMO follow a similar logic and warn people about it.

`unpredictable_function_pointer_comparisons`

warn-by-default

The unpredictable_function_pointer_comparisons lint checks comparison of function pointer as the operands.

Example

fn foo() {}
let a = foo as fn();

let _ = a == foo;

Explanation

Function pointers comparisons do not produce meaningful result since they are never guaranteed to be unique and could vary between different code generation units. Furthermore different function could have the same address after being merged together.

This PR also uplift the very similar clippy::fn_address_comparisons lint, which only linted on if one of the operand was an ty::FnDef while this PR lints proposes to lint on all ty::FnPtr and ty::FnDef.

@rustbot labels +I-lang-nominated

~~Edit: Blocked on https://github.com/rust-lang/libs-team/issues/323 being accepted and it's follow-up pr~~

Dec 11 '23 17:12 Urgau

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

Dec 11 '23 17:12 rustbot

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

The Miri subtree was changed

cc @rust-lang/miri

Dec 11 '23 17:12 rustbot

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

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

GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=Urgau
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_1d1d0127-6199-4c66-8ecb-fa1c94131316
GITHUB_EVENT_NAME=pull_request
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=4524088bd1a9a71fb896a797535750435a5a6a2d
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_1d1d0127-6199-4c66-8ecb-fa1c94131316
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_1d1d0127-6199-4c66-8ecb-fa1c94131316
GITHUB_TRIGGERING_ACTOR=Urgau
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/118833/merge
GITHUB_WORKFLOW_SHA=4524088bd1a9a71fb896a797535750435a5a6a2d
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
Built container sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Looks like docker image is the same as before, not uploading
https://ci-caches.rust-lang.org/docker/7ebc15c01a233894034d277c8cce4e949f4e7791f66b4727c8fb6e058a0b8171d6152e1441d677cef0653843ceeee469c097b8699b2bb74249e674f6aa1a8813
sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Mon Dec 11 17:24:14 UTC 2023
  network time: Mon, 11 Dec 2023 17:24:14 GMT
  network time: Mon, 11 Dec 2023 17:24:14 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-missing-tools', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: rust.codegen-units-std := 1
---
##[endgroup]
Testing GCC stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished release [optimized] target(s) in 1.38s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --no-default-features --mini-tests --std-tests`
Using system GCC
Using system GCC
[BUILD] mini_core
error: ambiguous wide pointer comparison, the comparison includes metadata which may not be expected
    |
    |
344 |         *self == *other
    |
    |
    = note: `-D ambiguous-wide-pointer-comparisons` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(ambiguous_wide_pointer_comparisons)]`
help: use `std::ptr::addr_eq` or untyped pointers to only compare their addresses
    |
344 |         std::ptr::addr_eq(*self, *other)
help: use explicit `std::ptr::eq` method to compare metadata and addresses
    |
    |
344 |         std::ptr::eq(*self, *other)


error: ambiguous wide pointer comparison, the comparison includes metadata which may not be expected
    |
    |
347 |         *self != *other
    |
    |
help: use `std::ptr::addr_eq` or untyped pointers to only compare their addresses
    |
347 |         !std::ptr::addr_eq(*self, *other)
help: use explicit `std::ptr::eq` method to compare metadata and addresses
    |
    |
347 |         !std::ptr::eq(*self, *other)

error: aborting due to 2 previous errors


Command failed to run: "Command `bash test.sh --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --no-default-features --mini-tests --std-tests` (running in folder `/checkout/compiler/rustc_codegen_gcc`) exited with status Some(1)"



command did not execute successfully: cd "/checkout/compiler/rustc_codegen_gcc" && env -u MAKEFLAGS -u MFLAGS AR_x86_64_unknown_linux_gnu="ar" CARGO_BUILD_INCREMENTAL="false" CARGO_INCREMENTAL="0" CARGO_PROFILE_RELEASE_DEBUG="0" CARGO_PROFILE_RELEASE_DEBUG_ASSERTIONS="true" CARGO_PROFILE_RELEASE_OVERFLOW_CHECKS="true" CARGO_PROFILE_RELEASE_STRIP="false" CARGO_TARGET_DIR="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen" CC_x86_64_unknown_linux_gnu="sccache cc" CFG_COMPILER_HOST_TRIPLE="x86_64-unknown-linux-gnu" CFG_DEFAULT_CODEGEN_BACKEND="llvm" CFG_LIBDIR_RELATIVE="lib" CFG_LLVM_ROOT="/usr/lib/llvm-16/bin/llvm-config" CFG_RELEASE="1.76.0-nightly" CFG_RELEASE_CHANNEL="nightly" CFG_VERSION="1.76.0-nightly (4524088bd 2023-12-11)" CFG_VER_DATE="2023-12-11" CFG_VER_HASH="4524088bd1a9a71fb896a797535750435a5a6a2d" CFLAGS_x86_64_unknown_linux_gnu="-ffunction-sections -fdata-sections -fPIC -m64" CXXFLAGS_x86_64_unknown_linux_gnu="-ffunction-sections -fdata-sections -fPIC -m64" CXX_x86_64_unknown_linux_gnu="sccache c++" LIBC_CHECK_CFG="1" LIBRARY_PATH="/usr/lib/llvm-16/lib" LLVM_CONFIG="/usr/lib/llvm-16/bin/llvm-config" LLVM_LINK_SHARED="1" LLVM_NDEBUG="1" LLVM_STATIC_STDCPP="/usr/lib/gcc/x86_64-linux-gnu/12/libstdc++.a" RANLIB_x86_64_unknown_linux_gnu="ar s" REAL_LIBRARY_PATH_VAR="LD_LIBRARY_PATH" RUSTBUILD_NATIVE_DIR="/checkout/obj/build/x86_64-unknown-linux-gnu/native" RUSTC="/checkout/obj/build/bootstrap/debug/rustc" RUSTC_BOOTSTRAP="1" RUSTC_BREAK_ON_ICE="1" RUSTC_ERROR_METADATA_DST="/checkout/obj/build/tmp/extended-error-metadata" RUSTC_FORCE_UNSTABLE="1" RUSTC_HOST_FLAGS="-Zunstable-options --check-cfg=cfg(bootstrap)" RUSTC_INSTALL_BINDIR="bin" RUSTC_LIBDIR="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib" RUSTC_LINT_FLAGS="-Wrust_2018_idioms -Wunused_lifetimes -Dwarnings" RUSTC_REAL="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" RUSTC_SNAPSHOT="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" RUSTC_SNAPSHOT_LIBDIR="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib" RUSTC_STAGE="1" RUSTC_SYSROOT="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1" RUSTC_VERBOSE="0" RUSTC_VERIFY_LLVM_IR="1" RUSTDOC="/checkout/obj/build/bootstrap/debug/rustdoc" RUSTDOCFLAGS="-Csymbol-mangling-version=v0 -Zunstable-options --check-cfg=cfg(bootstrap,values()) --check-cfg=cfg(parallel_compiler,values()) -Dwarnings -Wrustdoc::invalid_codeblock_attributes --crate-version 1.76.0-nightly\t(4524088bd\t2023-12-11) --cfg=parallel_compiler" RUSTDOC_LIBDIR="/checkout/obj/build/x86_64-unknown-linux-gnu/stage1/lib" RUSTDOC_REAL="/path/to/nowhere/rustdoc/not/required" RUSTFLAGS="-Csymbol-mangling-version=v0 -Zunstable-options --check-cfg=cfg(bootstrap,values()) --check-cfg=cfg(parallel_compiler,values()) -Zmacro-backtrace -Clink-args=-Wl,-z,origin -Clink-args=-Wl,-rpath,$ORIGIN/../lib -Csplit-debuginfo=off -Cllvm-args=-import-instr-limit=10 --cfg=parallel_compiler -Cpanic=abort" RUST_TEST_THREADS="16" TERM="xterm" WINAPI_NO_BUNDLED_LIBRARIES="1" __CARGO_DEFAULT_LIB_METADATA="nightlycodegen" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "run" "--target" "x86_64-unknown-linux-gnu" "--release" "-Zcheck-cfg" "-Zbinary-dep-depinfo" "-j" "16" "--locked" "--color" "always" "--manifest-path" "/checkout/compiler/rustc_codegen_gcc/build_system/Cargo.toml" "--" "test" "--use-system-gcc" "--use-backend" "gcc" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc" "--release" "--no-default-features" "--mini-tests" "--std-tests"

stdout ----

stderr ----

Dec 11 '23 17:12 rust-log-analyzer

The lint name is very long, it would be nice if we could come up with a shorter name. I don't have a suggestion though.

Dec 11 '23 20:12 Noratrieb

The lint name is very long, it would be nice if we could come up with a shorter name. I don't have a suggestion though.

Sure, but I don't think it matter that much, I'm not expecting anyone to write the lint name by hand (since it's warn-by-default, people can just copy/paste in the diagnostic output). We also already have some pretty long lint name, like unknown_or_malformed_diagnostic_attributes. I also think it's better to have a long and descriptive lint name than a short one that is less descriptive.

But if someone comes up with a name as descriptive but shorter I can integrate it, I'm just not sure it's worth anyone time.

Dec 12 '23 08:12 Urgau

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

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

GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=Urgau
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_8431c244-9c51-41fe-a0a8-06f14208f5a1
GITHUB_EVENT_NAME=pull_request
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=ba82a25117dc0939965a62b1f7c176d8975525ab
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_8431c244-9c51-41fe-a0a8-06f14208f5a1
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_8431c244-9c51-41fe-a0a8-06f14208f5a1
GITHUB_TRIGGERING_ACTOR=Urgau
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/118833/merge
GITHUB_WORKFLOW_SHA=ba82a25117dc0939965a62b1f7c176d8975525ab
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
Built container sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Looks like docker image is the same as before, not uploading
https://ci-caches.rust-lang.org/docker/7ebc15c01a233894034d277c8cce4e949f4e7791f66b4727c8fb6e058a0b8171d6152e1441d677cef0653843ceeee469c097b8699b2bb74249e674f6aa1a8813
sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Tue Dec 12 11:40:03 UTC 2023
  network time: Tue, 12 Dec 2023 11:40:03 GMT
  network time: Tue, 12 Dec 2023 11:40:03 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-missing-tools', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: rust.codegen-units-std := 1
---
##[endgroup]
Testing GCC stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished release [optimized] target(s) in 1.43s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --no-default-features --mini-tests --std-tests`
Using system GCC
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc
---
   Compiling rand_xorshift v0.3.0
   Compiling core v0.0.0 (/checkout/library/core)
   Compiling std v0.0.0 (/checkout/library/std)
   Compiling alloc v0.0.0 (/checkout/library/alloc)
error: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique
    |
    |
318 |     assert!(!(p < q));
    |
    |
    = note: even foreign function pointers are not guaranteed to be unique and could vary between different code generation units
    = note: furthermore different functions could have the same address after being merged together
    = note: `-D unpredictable-function-pointer-comparisons` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(unpredictable_function_pointer_comparisons)]`
error: could not compile `core` (test "coretests") due to previous error
warning: build failed, waiting for other jobs to finish...
Build completed unsuccessfully in 0:20:22
  local time: Tue Dec 12 12:00:58 UTC 2023

Dec 12 '23 12:12 rust-log-analyzer

If it's not too much work, could we do a crater run here to find out what kind of places are using this kind of comparison in the wild?

Dec 13 '23 16:12 compiler-errors

If it's not too much work, could we do a crater run here to find out what kind of places are using this kind of comparison in the wild?

Sure, I just pushed a commit making the lint deny-by-default. I will let you do the bors+craterbot commands.

Dec 13 '23 17:12 Urgau

@bors try

Dec 13 '23 17:12 compiler-errors

:hourglass: Trying commit 6c5c5466e6b61e3876711f895111711ffcb06b76 with merge 29596578cb5035fa2b7ac7fbdfea23fab38836e7...

Dec 13 '23 17:12 bors

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

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

GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=Urgau
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_6eae5f5b-3ce0-4519-a9dc-05bfb922a9a6
GITHUB_EVENT_NAME=pull_request
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=531b7d86508d07042af5339e5c0f3064b6fa8ec9
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_6eae5f5b-3ce0-4519-a9dc-05bfb922a9a6
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_6eae5f5b-3ce0-4519-a9dc-05bfb922a9a6
GITHUB_TRIGGERING_ACTOR=Urgau
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/118833/merge
GITHUB_WORKFLOW_SHA=531b7d86508d07042af5339e5c0f3064b6fa8ec9
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
Built container sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Looks like docker image is the same as before, not uploading
https://ci-caches.rust-lang.org/docker/7ebc15c01a233894034d277c8cce4e949f4e7791f66b4727c8fb6e058a0b8171d6152e1441d677cef0653843ceeee469c097b8699b2bb74249e674f6aa1a8813
sha256:9c3c93a371e5aed5c18185b24f130d95d5140dbd72a9b325e7b6b49e521a4faa
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-16]
##[group]Clock drift check
  local time: Wed Dec 13 17:18:37 UTC 2023
  network time: Wed, 13 Dec 2023 17:18:37 GMT
  network time: Wed, 13 Dec 2023 17:18:37 GMT
##[endgroup]
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-16', '--enable-llvm-link-shared', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-missing-tools', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-16/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.thin-lto-import-instr-limit := 10
configure: rust.codegen-units-std := 1
---
##[endgroup]
Testing GCC stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
   Compiling y v0.1.0 (/checkout/compiler/rustc_codegen_gcc/build_system)
    Finished release [optimized] target(s) in 1.43s
     Running `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-codegen/x86_64-unknown-linux-gnu/release/y test --use-system-gcc --use-backend gcc --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc --release --no-default-features --mini-tests --std-tests`
Using system GCC
Using system GCC
[BUILD] example
[AOT] mini_core_hello_world
/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/cg_gcc/mini_core_hello_world
abc
---
.................ii...............................................F....................

failures:

---- compiler/rustc_lint/src/types.rs - types::UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS (line 178) stdout ----
error: function pointer comparisons do not produce meaningful results since their addresses are not guaranteed to be unique
  |
  |
6 | let _ = a == foo;
  |
  = note: the address of the same function can very between different codegen units
  = note: the address of the same function can very between different codegen units
  = note: furthermore, different functions could have the same address after being merged together
  = note: `#[deny(unpredictable_function_pointer_comparisons)]` on by default
error: aborting due to 1 previous error

Couldn't compile the test.


failures:
    compiler/rustc_lint/src/types.rs - types::UNPREDICTABLE_FUNCTION_POINTER_COMPARISONS (line 178)

test result: FAILED. 84 passed; 1 failed; 2 ignored; 0 measured; 0 filtered out; finished in 813.66ms

error: doctest failed, to rerun pass `-p rustc_lint --doc`
  local time: Wed Dec 13 17:46:55 UTC 2023
  network time: Wed, 13 Dec 2023 17:46:56 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

Dec 13 '23 17:12 rust-log-analyzer

:sunny: Try build successful - checks-actions Build commit: 29596578cb5035fa2b7ac7fbdfea23fab38836e7 (29596578cb5035fa2b7ac7fbdfea23fab38836e7)

Dec 13 '23 18:12 bors

@rustbot labels -I-lang-nominated

We discussed this in the T-lang meeting today. There was some support for this, but people were concerned about whether there may be legitimate use cases and what we would be suggesting that those people do instead. For wide pointer comparisons, we suggest that people use ptr::addr_eq even though that has the same problems as comparison with == as it at least makes the intent of the user clear. What would be the comparable thing we would do here?

The consensus was that seeing use cases would help, and that a crater run would help to find those use cases. That's now happening in the comments above. Once the crater run comes back and has been analyzed, please renominate for T-lang discussion.

For the analysis, we're looking to find 1) use cases of this that are correct in the sense that they rely only on the actual semantics, and 2) the prevalence of bugs where people are using this in ways that rely on semantics that do not actually hold.

Dec 13 '23 23:12 traviscross

What would be the comparable thing we would do here?

There's no such thing, comparing Rust functions for equality is in general just inherently meaningless. That doesn't make code using == any less buggy though. If we want to support that usecase we need a fundamental language change, likely removing the LLVM flag which tells the backend that function addresses do not matter.

The flip side of this is that the standard library itself actually relies on such a comparison in the format-args machinery... it's for a special case (a monomorphic function) where the current implementation AFAIK does not generate duplicates (but I don't know how multi-CGU handling works so I am not sure). It's certainly not guaranteed by the language.

Dec 14 '23 07:12 RalfJung

@craterbot check

Dec 14 '23 18:12 compiler-errors

:ok_hand: Experiment pr-118833 created and queued. :robot: Automatically detected try build 29596578cb5035fa2b7ac7fbdfea23fab38836e7 :mag: You can check out the queue and this experiment's details.

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

Dec 14 '23 18:12 craterbot

What would be the comparable thing we would do here?

There's no such thing, comparing Rust functions for equality is in general just inherently meaningless.

One thought expressed in the meeting was as follows:

Comparing functions for equality via pointers may yield false negatives but not false positives. The fact that two functions may compare unequal (based on pointers to them) but in fact do the same thing is a rather fundamental property of not just Rust, but of any language. In general, it's impossible to know whether two different functions may in fact do the same thing.

In this light, maybe it's OK that these comparisons produce false negatives, and maybe there exist valid use cases that only rely on the property that we will not return false positives.

Dec 20 '23 10:12 traviscross

Comparing functions for equality via pointers may yield false negatives but not false positives.

That's not true. There are both false negatives and false positives. That's exactly why I wanted the lint description to be clear about this.

False positives arise when LLVM merges two functions because they optimized to the same code.

Dec 20 '23 12:12 RalfJung

:construction: Experiment pr-118833 is now running

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

Dec 22 '23 08:12 craterbot

:tada: Experiment pr-118833 is completed! :bar_chart: 132 regressed and 2 fixed (399621 total) :newspaper: Open the full report.

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

Dec 24 '23 05:12 craterbot

@craterbot run mode=check crates=https://crater-reports.s3.amazonaws.com/pr-118833/retry-regressed-list.txt p=1

Dec 24 '23 08:12 RalfJung

:rotating_light: Error: failed to parse the command

:sos: If you have any trouble with Crater please ping @rust-lang/infra! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Dec 24 '23 08:12 craterbot

@craterbot run mode=check crates=https://crater-reports.s3.amazonaws.com/pr-118833/retry-regressed-list.txt

Dec 24 '23 08:12 RalfJung

:rotating_light: Error: failed to parse the command

:sos: If you have any trouble with Crater please ping @rust-lang/infra! :information_source: Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Dec 24 '23 08:12 craterbot

@craterbot run mode=check-only crates=https://crater-reports.s3.amazonaws.com/pr-118833/retry-regressed-list.txt p=1

Dec 24 '23 08:12 RalfJung

:ok_hand: Experiment pr-118833-1 created and queued. :robot: Automatically detected try build 29596578cb5035fa2b7ac7fbdfea23fab38836e7 :mag: You can check out the queue and this experiment's details.

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

Dec 24 '23 08:12 craterbot

:construction: Experiment pr-118833-1 is now running

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

Dec 25 '23 17:12 craterbot

:tada: Experiment pr-118833-1 is completed! :bar_chart: 91 regressed and 0 fixed (132 total) :newspaper: Open the full report.

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

Dec 25 '23 19:12 craterbot

Crater Analysis Result

Summary

Crate	True-positive	Not duplicated	In PartialEq	Comments
AntidermisMC.rlox logs	?		✓	VM / Interpreter
BillyLevin.rs-monkes logs	?		✓	VM / Interpreter
BrunoCaste/Lox-Rs logs	?		✓	VM / Interpreter
Danielmelody/Ruschm logs	?		✓	VM / Interpreter
Deen17/aoc2020 logs	✓	✓		fn-ptr != fn-def
HallerPatrick/liva-lang logs	?		✓	VM / Interpreter
KarimIbrahim/monkey_interpreter logs	?		✓	VM / Interpreter
Kiwifuit/event-driven-rust logs	❌			Removing duplicates
MarkRoss470/acpica-rust-bindings logs	❌			fn-ptr == fn-ptr, but fn-ptrs created from unique addresses
cafebabeeee/rust-tutorial logs	✓	✓		tutorial, in println!, fn-ptrs derived from fn-defs
PhilipDaniels/projecteuler logs	✓	✓		fn-ptr == fn-def
ProfessorQu/battleship_website logs	✓	✓		fn-ptr == fn-def(-ish)
RussellSnyder/AOC-rust-day21 logs	✓	✓		fn-ptr != fn-def
ShivaBhattacharjee/Synthia logs	?		✓	VM / Interpreter
Swiftaff/rust-learning-parser-combinators logs	✓	✓		fn-ptr == fn-def
TypstApp-team/typst logs	?		✓	VM / Interpreter
Yiklek/rslua logs	✓	✓		fn-ptr == fn-def
ZachClayburn/CompilerDesign-Project2 logs	✓	✓		fn-ptr == fn-def
adriensamson/aoc logs	❌			filtering on Vec of fn-ptrs
b-r-a-n/pills logs	✓	✓		fn-ptr == fn-def
bgnerdclub/birb logs	❌			filtering on Vec of fn-ptrs
cathangormley/kr-a logs	✓	✓		fn-ptr == fn-def
chrisdean258/terse logs	?		✓	VM / Interpreter
codaishin/project-zyheeda-bevy logs	✓	✓		fn-ptr == fn-def(-ish)
itsaramcc/nes-rs logs	✓	✓		fn-ptr == fn-def

...

Note: I stopped at the nes-rs for now, I may continue later but given the trends I don't think anything new will come.

Highlights

ShivaBhattacharjee/Synthia

code

VM / Interpreter, very similar pattern in many other projects. Unable to determine if it's true-positive or false-positive.

impl PartialEq for Object {
    fn eq(&self, other: &Self) -> bool {
        match (self, other) {
            (Object::Number(a), Object::Number(b)) => a == b,
            (Object::String(a), Object::String(b)) => a == b,
            ...
            (Object::Fn(a, b, c), Object::Fn(d, e, f)) => a == d && b == e && c == f,
            (Object::Inbuilt(a), Object::Inbuilt(b)) => a == b,
            ...
            _ => false,
        }
    }
}

ZachClayburn/CompilerDesign-Project2

logs code

Rely on function NOT being duplicated.

pub fn get_reduction_name(function_pointer: &ReductionOp) -> &'static str {
    match *function_pointer {
        x if x == reduce_value => "reduce_value()",
        x if x == reduce_binary_op => "reduce_binary_op()",
        x if x == reduce_parenthetical => "reduce_parenthetical()",
        x if x == reduce_unary_operator => "reduce_unary_operator()",
        x if x == reduce_program => "reduce_program()",
        x if x == reduce_statement_list => "reduce_statement_list()",
        x if x == reduce_declaration => "reduce_declaration()",
        x if x == reduce_param_spec => "reduce_param_spec()",
        x if x == reduce_return_statement => "reduce_return_statement()",
        x if x == reduce_procedure_declaration => "reduce_procedure_declaration()",
        x if x == reduce_procedure_call => "reduce_procedure_call()",
        _ => panic!("unknown reduction"),
    }
}

HugoBelotDeloro/rlox

logs code

Compare functions pointers derived from the same list.

                *p = p.iter().filter(|&&f2| f2 != f).copied().collect::<Vec<&OpFn>>();

Observation

Some false positives, mainly when filtering fn-ptrs within a vector.
Many true-positives, the vast majority when comparing a fn-ptr with a fn-def.

Requiring that one of the operands is an fn-def may be a good compromise.

Note: I think I remember Josh mentioning a caching pattern that involved fn-ptrs, but I haven't seen any evidence that this pattern is used and affected by this lint.

@rustbot label +I-lang-nominated -S-waiting-on-review +S-waiting-on-team +T-lang

Dec 31 '23 14:12 Urgau

The standard library also uses function pointer comparison to implement Waker::will_wake.

Jan 01 '24 21:01 tmiasko

rust
rust copied to clipboard

Add lint against function pointer comparisons

`unpredictable_function_pointer_comparisons`

Example

Explanation

Crater Analysis Result

Summary

Highlights

ShivaBhattacharjee/Synthia

ZachClayburn/CompilerDesign-Project2

HugoBelotDeloro/rlox

Observation

rust rust copied to clipboard

Add lint against function pointer comparisons

unpredictable_function_pointer_comparisons

Example

Explanation

Crater Analysis Result

Summary

Highlights

ShivaBhattacharjee/Synthia

ZachClayburn/CompilerDesign-Project2

HugoBelotDeloro/rlox

Observation

rust
rust copied to clipboard

`unpredictable_function_pointer_comparisons`