Skip to content

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Mar 30, 2024

t-lang nomination comment

This PR aims at implementing a lint for implicit autoref of raw pointer dereference, it is based on #103735 with suggestion and improvements from #103735 (comment).

The goal is to catch cases like this, where the user probably doesn't realise it just created a reference.

pub struct Test { data: [u8], } pub fn test_len(t: *const Test) -> usize { unsafe { (*t).data.len() } // this calls <[T]>::len(&self) }

Since #103735 already went 2 times through T-lang, where they T-lang ended-up asking for a more restricted version (which is what this PR does), I would prefer this PR to be reviewed first before re-nominating it for T-lang.


Compared to the PR it is as based on, this PR adds 3 restrictions on the outer most expression, which must either be:

  1. A deref followed by any non-deref place projection (that intermediate deref will typically be auto-inserted)
  2. A method call annotated with #[rustc_no_implicit_refs].
  3. A deref followed by a addr_of! or addr_of_mut!. See bottom of post for details.

There are several points that are not 100% clear to me when implementing the modifications:

  • "4. Any number of automatically inserted deref/derefmut calls." I as never able to trigger this. Am I missing something? Fixed
  • Are "index" and "field" enough?

cc @JakobDegen @WaffleLapkin
r? @RalfJung

try-job: dist-various-1
try-job: dist-various-2

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 30, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 30, 2024

The Miri subtree was changed

cc @rust-lang/miri

@RalfJung
Copy link
Member

Sorry, I can't take on more reviews currently.
r? compiler
(or feel free to pick someone specific who's suited)

@rustbot rustbot assigned fmease and unassigned RalfJung Mar 30, 2024
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as outdated.

@Urgau Urgau force-pushed the dangerous_implicit_autorefs branch from 2b3fe45 to 57f6416 Compare May 14, 2024 17:22
@rust-log-analyzer

This comment has been minimized.

@Urgau Urgau force-pushed the dangerous_implicit_autorefs branch from 57f6416 to 824c1f5 Compare May 14, 2024 18:04
@bors

This comment was marked as outdated.

@Urgau Urgau force-pushed the dangerous_implicit_autorefs branch from 824c1f5 to c2d6e62 Compare May 23, 2024 18:49
@bors

This comment was marked as outdated.

@Dylan-DPC
Copy link
Member

@Urgau if you can rebase the latest conflicts we can push this forward and maybe get it reviewed by another reviewer

@Dylan-DPC Dylan-DPC added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 12, 2024
@Urgau Urgau force-pushed the dangerous_implicit_autorefs branch from c2d6e62 to 78288af Compare August 12, 2024 15:49
@Urgau
Copy link
Member Author

Urgau commented Aug 12, 2024

@Dylan-DPC rebased.

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 12, 2024
@Urgau Urgau force-pushed the dangerous_implicit_autorefs branch from 78288af to d060615 Compare October 9, 2024 13:58
@rust-log-analyzer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a932eb3): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
1.8% [0.3%, 3.6%] 6
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.8% [0.3%, 3.6%] 6

Max RSS (memory usage)

Results (primary 0.4%, secondary -0.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.2% [0.5%, 2.3%] 4
Regressions ❌
(secondary)
2.8% [2.8%, 2.8%] 1
Improvements ✅
(primary)
-0.6% [-0.7%, -0.5%] 3
Improvements ✅
(secondary)
-2.2% [-3.2%, -1.2%] 2
All ❌✅ (primary) 0.4% [-0.7%, 2.3%] 7

Cycles

Results (primary 1.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.1% [0.5%, 2.7%] 4
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.5% [-0.6%, -0.5%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.0% [-0.6%, 2.7%] 7

Binary size

Results (primary 0.0%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.0% [0.0%, 0.0%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.0% [0.0%, 0.0%] 1

Bootstrap: 761.588s -> 761.378s (-0.03%)
Artifact size: 365.21 MiB -> 365.18 MiB (-0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Apr 28, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 28, 2025
perf: delay checking of `#[rustc_no_implicit_autorefs]` in autoref lint Try to address the regression seen in rust-lang#123239 (comment) by delaying the checking of `#[rustc_no_implicit_autorefs]` on method call.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 29, 2025
…mann,traviscross Implement a lint for implicit autoref of raw pointer dereference - take 2 *[t-lang nomination comment](rust-lang/rust#123239 (comment) This PR aims at implementing a lint for implicit autoref of raw pointer dereference, it is based on #103735 with suggestion and improvements from rust-lang/rust#103735 (comment). The goal is to catch cases like this, where the user probably doesn't realise it just created a reference. ```rust pub struct Test { data: [u8], } pub fn test_len(t: *const Test) -> usize { unsafe { (*t).data.len() } // this calls <[T]>::len(&self) } ``` Since #103735 already went 2 times through T-lang, where they T-lang ended-up asking for a more restricted version (which is what this PR does), I would prefer this PR to be reviewed first before re-nominating it for T-lang. ---- Compared to the PR it is as based on, this PR adds 3 restrictions on the outer most expression, which must either be: 1. A deref followed by any non-deref place projection (that intermediate deref will typically be auto-inserted) 2. A method call annotated with `#[rustc_no_implicit_refs]`. 3. A deref followed by a `addr_of!` or `addr_of_mut!`. See bottom of post for details. There are several points that are not 100% clear to me when implementing the modifications: - ~~"4. Any number of automatically inserted deref/derefmut calls." I as never able to trigger this. Am I missing something?~~ Fixed - Are "index" and "field" enough? ---- cc `@JakobDegen` `@WaffleLapkin` r? `@RalfJung` try-job: dist-various-1 try-job: dist-various-2
@rylev
Copy link
Member

rylev commented Apr 29, 2025

The perf regression is reversed in #140406

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Apr 29, 2025
github-merge-queue bot pushed a commit to bevyengine/bevy that referenced this pull request Apr 30, 2025
# Objective - new nightly lint make CI fail ## Solution - Follow the lint: rust-lang/rust#123239
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request May 1, 2025
…mann,traviscross Implement a lint for implicit autoref of raw pointer dereference - take 2 *[t-lang nomination comment](rust-lang/rust#123239 (comment) This PR aims at implementing a lint for implicit autoref of raw pointer dereference, it is based on #103735 with suggestion and improvements from rust-lang/rust#103735 (comment). The goal is to catch cases like this, where the user probably doesn't realise it just created a reference. ```rust pub struct Test { data: [u8], } pub fn test_len(t: *const Test) -> usize { unsafe { (*t).data.len() } // this calls <[T]>::len(&self) } ``` Since #103735 already went 2 times through T-lang, where they T-lang ended-up asking for a more restricted version (which is what this PR does), I would prefer this PR to be reviewed first before re-nominating it for T-lang. ---- Compared to the PR it is as based on, this PR adds 3 restrictions on the outer most expression, which must either be: 1. A deref followed by any non-deref place projection (that intermediate deref will typically be auto-inserted) 2. A method call annotated with `#[rustc_no_implicit_refs]`. 3. A deref followed by a `addr_of!` or `addr_of_mut!`. See bottom of post for details. There are several points that are not 100% clear to me when implementing the modifications: - ~~"4. Any number of automatically inserted deref/derefmut calls." I as never able to trigger this. Am I missing something?~~ Fixed - Are "index" and "field" enough? ---- cc `@JakobDegen` `@WaffleLapkin` r? `@RalfJung` try-job: dist-various-1 try-job: dist-various-2
VlaDexa added a commit to VlaDexa/rust that referenced this pull request May 2, 2025
perf: delay checking of `#[rustc_no_implicit_autorefs]` in autoref lint Try to address the regression seen in rust-lang#123239 (comment) by delaying the checking of `#[rustc_no_implicit_autorefs]` on method call.
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2025
perf: delay checking of `#[rustc_no_implicit_autorefs]` in autoref lint Try to address the regression seen in rust-lang#123239 (comment) by delaying the checking of `#[rustc_no_implicit_autorefs]` on method call.
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request May 3, 2025
perf: delay checking of `#[rustc_no_implicit_autorefs]` in autoref lint Try to address the regression seen in rust-lang/rust#123239 (comment) by delaying the checking of `#[rustc_no_implicit_autorefs]` on method call.
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request May 5, 2025
perf: delay checking of `#[rustc_no_implicit_autorefs]` in autoref lint Try to address the regression seen in rust-lang/rust#123239 (comment) by delaying the checking of `#[rustc_no_implicit_autorefs]` on method call.
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request May 5, 2025
perf: delay checking of `#[rustc_no_implicit_autorefs]` in autoref lint Try to address the regression seen in rust-lang/rust#123239 (comment) by delaying the checking of `#[rustc_no_implicit_autorefs]` on method call.
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request May 9, 2025
…=jdonszelmann,traviscross Implement a lint for implicit autoref of raw pointer dereference - take 2 *[t-lang nomination comment](rust-lang#123239 (comment) This PR aims at implementing a lint for implicit autoref of raw pointer dereference, it is based on rust-lang#103735 with suggestion and improvements from rust-lang#103735 (comment). The goal is to catch cases like this, where the user probably doesn't realise it just created a reference. ```rust pub struct Test { data: [u8], } pub fn test_len(t: *const Test) -> usize { unsafe { (*t).data.len() } // this calls <[T]>::len(&self) } ``` Since rust-lang#103735 already went 2 times through T-lang, where they T-lang ended-up asking for a more restricted version (which is what this PR does), I would prefer this PR to be reviewed first before re-nominating it for T-lang. ---- Compared to the PR it is as based on, this PR adds 3 restrictions on the outer most expression, which must either be: 1. A deref followed by any non-deref place projection (that intermediate deref will typically be auto-inserted) 2. A method call annotated with `#[rustc_no_implicit_refs]`. 3. A deref followed by a `addr_of!` or `addr_of_mut!`. See bottom of post for details. There are several points that are not 100% clear to me when implementing the modifications: - ~~"4. Any number of automatically inserted deref/derefmut calls." I as never able to trigger this. Am I missing something?~~ Fixed - Are "index" and "field" enough? ---- cc `@JakobDegen` `@WaffleLapkin` r? `@RalfJung` try-job: dist-various-1 try-job: dist-various-2
andrewzhurov pushed a commit to andrewzhurov/bevy that referenced this pull request May 17, 2025
# Objective - new nightly lint make CI fail ## Solution - Follow the lint: rust-lang/rust#123239
@jieyouxu jieyouxu added the L-dangerous_implicit_autorefs Lint: dangerous_implicit_autorefs label May 30, 2025
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 5, 2025
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 7, 2025
…refs, r=traviscross Make the `dangerous_implicit_autorefs` lint deny-by-default I intended for the `dangerous_implicit_autorefs` lint to be deny-by-default, the [T-lang nomination comment](rust-lang#123239 (comment)) even clearly mentioned deny-by-default, but somehow I and other missed that it is only warn-by-default. I think the lint should still be deny-by-default as the implicit aliasing requirements can be quite dangerous. In any-case, opening this PR for T-lang awareness. `@rustbot` label +I-lang-nominated +T-lang r? `@traviscross`
rust-timer added a commit that referenced this pull request Jun 8, 2025
Rollup merge of #141661 - Urgau:deny-dangerous_implicit_autorefs, r=traviscross Make the `dangerous_implicit_autorefs` lint deny-by-default I intended for the `dangerous_implicit_autorefs` lint to be deny-by-default, the [T-lang nomination comment](#123239 (comment)) even clearly mentioned deny-by-default, but somehow I and other missed that it is only warn-by-default. I think the lint should still be deny-by-default as the implicit aliasing requirements can be quite dangerous. In any-case, opening this PR for T-lang awareness. `@rustbot` label +I-lang-nominated +T-lang r? `@traviscross`
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Jun 9, 2025
…raviscross Make the `dangerous_implicit_autorefs` lint deny-by-default I intended for the `dangerous_implicit_autorefs` lint to be deny-by-default, the [T-lang nomination comment](rust-lang/rust#123239 (comment)) even clearly mentioned deny-by-default, but somehow I and other missed that it is only warn-by-default. I think the lint should still be deny-by-default as the implicit aliasing requirements can be quite dangerous. In any-case, opening this PR for T-lang awareness. `@rustbot` label +I-lang-nominated +T-lang r? `@traviscross`
@jieyouxu jieyouxu mentioned this pull request Jun 22, 2025
github-merge-queue bot pushed a commit to getsentry/relay that referenced this pull request Jun 27, 2025
wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Jun 29, 2025
Pkgsrc changes: * Adjust patches to adapt to upstream changes and new versions. * associated checksums Upstream changes relative to 1.87.0: Version 1.88.0 (2025-06-26) ========================== Language -------- - [Stabilize `#![feature(let_chains)]` in the 2024 edition.] (rust-lang/rust#132833) This feature allows `&&`-chaining `let` statements inside `if` and `while`, allowing intermixture with boolean expressions. The patterns inside the `let` sub-expressions can be irrefutable or refutable. - [Stabilize `#![feature(naked_functions)]`.] (rust-lang/rust#134213) Naked functions allow writing functions with no compiler-generated epilogue and prologue, allowing full control over the generated assembly for a particular function. - [Stabilize `#![feature(cfg_boolean_literals)]`.] (rust-lang/rust#138632) This allows using boolean literals as `cfg` predicates, e.g. `#[cfg(true)]` and `#[cfg(false)]`. - [Fully de-stabilize the `#[bench]` attribute] (rust-lang/rust#134273). Usage of `#[bench]` without `#![feature(custom_test_frameworks)]` already triggered a deny-by-default future-incompatibility lint since Rust 1.77, but will now become a hard error. - [Add warn-by-default `dangerous_implicit_autorefs` lint against implicit autoref of raw pointer dereference.] (rust-lang/rust#123239) The lint [will be bumped to deny-by-default] (rust-lang/rust#141661) in the next version of Rust. - [Add `invalid_null_arguments` lint to prevent invalid usage of null pointers.] (rust-lang/rust#119220) This lint is uplifted from `clippy::invalid_null_ptr_usage`. - [Change trait impl candidate preference for builtin impls and trivial where-clauses.] (rust-lang/rust#138176) - [Check types of generic const parameter defaults] (rust-lang/rust#139646) Compiler -------- - [Stabilize `-Cdwarf-version` for selecting the version of DWARF debug information to generate.] (rust-lang/rust#136926) Platform Support ---------------- - [Demote `i686-pc-windows-gnu` to Tier 2.] (https://blog.rust-lang.org/2025/05/26/demoting-i686-pc-windows-gnu/) Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support. [platform-support-doc]: https://doc.rust-lang.org/rustc/platform-support.html Libraries --------- - [Remove backticks from `#[should_panic]` test failure message.] (rust-lang/rust#136160) - [Guarantee that `[T; N]::from_fn` is generated in order of increasing indices.] (rust-lang/rust#139099), for those passing it a stateful closure. - [The libtest flag `--nocapture` is deprecated in favor of the more consistent `--no-capture` flag.] (rust-lang/rust#139224) - [Guarantee that `{float}::NAN` is a quiet NaN.] (rust-lang/rust#139483) Stabilized APIs --------------- - [`Cell::update`] (https://doc.rust-lang.org/stable/std/cell/struct.Cell.html#method.update) - [`impl Default for *const T`] (https://doc.rust-lang.org/nightly/std/primitive.pointer.html#impl-Default-for-*const+T) - [`impl Default for *mut T`] (https://doc.rust-lang.org/nightly/std/primitive.pointer.html#impl-Default-for-*mut+T) - [`HashMap::extract_if`] (https://doc.rust-lang.org/stable/std/collections/struct.HashMap.html#method.extract_if) - [`HashSet::extract_if`] (https://doc.rust-lang.org/stable/std/collections/struct.HashSet.html#method.extract_if) - [`proc_macro::Span::line`] (https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.line) - [`proc_macro::Span::column`] (https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.column) - [`proc_macro::Span::start`] (https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.start) - [`proc_macro::Span::end`] (https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.end) - [`proc_macro::Span::file`] (https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.file) - [`proc_macro::Span::local_file`] (https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.local_file) These previously stable APIs are now stable in const contexts: - [`NonNull<T>::replace`] (https://doc.rust-lang.org/stable/std/ptr/struct.NonNull.html#method.replace) - [`<*mut T>::replace`] (https://doc.rust-lang.org/stable/std/primitive.pointer.html#method.replace) - [`std::ptr::swap_nonoverlapping`] (rust-lang/rust#137280) - [`Cell::{replace, get, get_mut, from_mut, as_slice_of_cells}`] (rust-lang/rust#137928) Cargo ----- - [Stabilize automatic garbage collection.] (rust-lang/cargo#14287) - [use `zlib-rs` for gzip compression in rust code] (rust-lang/cargo#15417) Rustdoc ----- - [Doctests can be ignored based on target names using `ignore-*` attributes.] (rust-lang/rust#137096) - [Stabilize the `--test-runtool` and `--test-runtool-arg` CLI options to specify a program (like qemu) and its arguments to run a doctest.] (rust-lang/rust#137096) Compatibility Notes ------------------- - [Finish changing the internal representation of pasted tokens] (rust-lang/rust#124141). Certain invalid declarative macros that were previously accepted in obscure circumstances are now correctly rejected by the compiler. Use of a `tt` fragment specifier can often fix these macros. - [Fully de-stabilize the `#[bench]` attribute] (rust-lang/rust#134273). Usage of `#[bench]` without `#![feature(custom_test_frameworks)]` already triggered a deny-by-default future-incompatibility lint since Rust 1.77, but will now become a hard error. - [Fix borrow checking some always-true patterns.] (rust-lang/rust#139042) The borrow checker was overly permissive in some cases, allowing programs that shouldn't have compiled. - [Update the minimum external LLVM to 19.] (rust-lang/rust#139275) - [Make it a hard error to use a vector type with a non-Rust ABI without enabling the required target feature.] (rust-lang/rust#139309)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) CI-spurious-fail-mingw CI spurious failure: target env mingw CI-spurious-fail-rust-lld-crash CI spurious failure: `rust-lld` crashing / SIGSEGV / 0xc0000374 heap corruption disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. L-dangerous_implicit_autorefs Lint: dangerous_implicit_autorefs merged-by-bors This PR was explicitly merged by bors. O-SGX Target: SGX perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team