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
Projects
None yet
Development

Successfully merging this pull request may close these issues.