Skip to content

fix: re-enable self-assignment #145214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

notJoon
Copy link
Contributor

@notJoon notJoon commented Aug 10, 2025

Description

Re-enables the self-assignment detection that was previously disabled due to unrelated regressions. The fix detects useless assignments like x = x and foo.field = foo.field.

History

The original regressions (#81626, #81658) were specifically about false positives in write-only field detection, not self-assignment detection. Belows are brief history for the rule that I understand.

This PR only re-enables the self-assignment checks, which are orthogonal to the problematic write-only field analysis.

Changes

  • Removed #[allow(dead_code)] from compiler/rustc_passes/src/dead.rs file
    • handle_assign and
    • check_for_self_assign
  • Added ExprKind::Assign handling in visit_expr to call both methods
  • Updated test expectations in tests/ui/lint/dead-code/self-assign.rs

@rustbot
Copy link
Collaborator

rustbot commented Aug 10, 2025

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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. labels Aug 10, 2025
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 11, 2025

📌 Commit ba350ff has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 11, 2025
fmease added a commit to fmease/rust that referenced this pull request Aug 11, 2025
…, r=petrochenkov

fix: re-enable self-assignment

## Description

Re-enables the self-assignment detection that was previously disabled due to unrelated regressions. The fix detects useless assignments like `x = x` and `foo.field = foo.field`.

## History

The original regressions (rust-lang#81626, rust-lang#81658) were specifically about false positives in write-only field detection, not self-assignment detection. Belows are brief history for the rule that I understand.

- Self-assignment detection was originally implemented in rust-lang#87129 to address rust-lang#75356
- The implementation was disabled alongside the revert of rust-lang#81473's "write-only fields" detection
- rust-lang#81473 was reverted via rust-lang#86212 and rust-lang#83171 due to false positives in write-only field detection (rust-lang#81626, rust-lang#81658)
- The self-assignment detection feature got removed, even though it wasn't the reason for the problems

This PR only re-enables the self-assignment checks, which are orthogonal to the problematic write-only field analysis.

## Changes
- Removed `#[allow(dead_code)]` from `compiler/rustc_passes/src/dead.rs` file
    - `handle_assign` and
    - `check_for_self_assign`
- Added `ExprKind::Assign` handling in `visit_expr` to call both methods
- Updated test expectations in `tests/ui/lint/dead-code/self-assign.rs`
fmease added a commit to fmease/rust that referenced this pull request Aug 12, 2025
…, r=petrochenkov

fix: re-enable self-assignment

## Description

Re-enables the self-assignment detection that was previously disabled due to unrelated regressions. The fix detects useless assignments like `x = x` and `foo.field = foo.field`.

## History

The original regressions (rust-lang#81626, rust-lang#81658) were specifically about false positives in write-only field detection, not self-assignment detection. Belows are brief history for the rule that I understand.

- Self-assignment detection was originally implemented in rust-lang#87129 to address rust-lang#75356
- The implementation was disabled alongside the revert of rust-lang#81473's "write-only fields" detection
- rust-lang#81473 was reverted via rust-lang#86212 and rust-lang#83171 due to false positives in write-only field detection (rust-lang#81626, rust-lang#81658)
- The self-assignment detection feature got removed, even though it wasn't the reason for the problems

This PR only re-enables the self-assignment checks, which are orthogonal to the problematic write-only field analysis.

## Changes
- Removed `#[allow(dead_code)]` from `compiler/rustc_passes/src/dead.rs` file
    - `handle_assign` and
    - `check_for_self_assign`
- Added `ExprKind::Assign` handling in `visit_expr` to call both methods
- Updated test expectations in `tests/ui/lint/dead-code/self-assign.rs`
fmease added a commit to fmease/rust that referenced this pull request Aug 12, 2025
…, r=petrochenkov

fix: re-enable self-assignment

## Description

Re-enables the self-assignment detection that was previously disabled due to unrelated regressions. The fix detects useless assignments like `x = x` and `foo.field = foo.field`.

## History

The original regressions (rust-lang#81626, rust-lang#81658) were specifically about false positives in write-only field detection, not self-assignment detection. Belows are brief history for the rule that I understand.

- Self-assignment detection was originally implemented in rust-lang#87129 to address rust-lang#75356
- The implementation was disabled alongside the revert of rust-lang#81473's "write-only fields" detection
- rust-lang#81473 was reverted via rust-lang#86212 and rust-lang#83171 due to false positives in write-only field detection (rust-lang#81626, rust-lang#81658)
- The self-assignment detection feature got removed, even though it wasn't the reason for the problems

This PR only re-enables the self-assignment checks, which are orthogonal to the problematic write-only field analysis.

## Changes
- Removed `#[allow(dead_code)]` from `compiler/rustc_passes/src/dead.rs` file
    - `handle_assign` and
    - `check_for_self_assign`
- Added `ExprKind::Assign` handling in `visit_expr` to call both methods
- Updated test expectations in `tests/ui/lint/dead-code/self-assign.rs`
fmease added a commit to fmease/rust that referenced this pull request Aug 12, 2025
…, r=petrochenkov

fix: re-enable self-assignment

## Description

Re-enables the self-assignment detection that was previously disabled due to unrelated regressions. The fix detects useless assignments like `x = x` and `foo.field = foo.field`.

## History

The original regressions (rust-lang#81626, rust-lang#81658) were specifically about false positives in write-only field detection, not self-assignment detection. Belows are brief history for the rule that I understand.

- Self-assignment detection was originally implemented in rust-lang#87129 to address rust-lang#75356
- The implementation was disabled alongside the revert of rust-lang#81473's "write-only fields" detection
- rust-lang#81473 was reverted via rust-lang#86212 and rust-lang#83171 due to false positives in write-only field detection (rust-lang#81626, rust-lang#81658)
- The self-assignment detection feature got removed, even though it wasn't the reason for the problems

This PR only re-enables the self-assignment checks, which are orthogonal to the problematic write-only field analysis.

## Changes
- Removed `#[allow(dead_code)]` from `compiler/rustc_passes/src/dead.rs` file
    - `handle_assign` and
    - `check_for_self_assign`
- Added `ExprKind::Assign` handling in `visit_expr` to call both methods
- Updated test expectations in `tests/ui/lint/dead-code/self-assign.rs`
bors added a commit that referenced this pull request Aug 12, 2025
Rollup of 15 pull requests

Successful merges:

 - #131477 (Apple: Always pass SDK root when linking with `cc`, and pass it via `SDKROOT` env var)
 - #139806 (std: sys: pal: uefi: Overhaul Time)
 - #144386 (Extract TraitImplHeader in AST/HIR)
 - #144542 (Stabilize `sse4a` and `tbm` target features)
 - #144921 (Don't emit `rustdoc::broken_intra_doc_links` for GitHub-flavored Markdown admonitions like `[!NOTE]`)
 - #145155 (Port `#[allow_internal_unsafe]` to the new attribute system (attempt 2))
 - #145214 (fix: re-enable self-assignment)
 - #145216 (rustdoc: correct negative-to-implicit discriminant display)
 - #145238 (Tweak invalid builtin attribute output)
 - #145249 (Rename entered trace span variables from `_span` to  `_trace`)
 - #145251 (Support using #[unstable_feature_bound] on trait)
 - #145253 (Document compiler and stdlib in stage1 in `pr-check-2` CI job)
 - #145260 (Make explicit guarantees about `Vec`’s allocator)
 - #145263 (Update books)
 - #145273 (Account for new `assert!` desugaring in `!condition` suggestion)

r? `@ghost`
`@rustbot` modify labels: rollup
fmease added a commit to fmease/rust that referenced this pull request Aug 12, 2025
…, r=petrochenkov

fix: re-enable self-assignment

## Description

Re-enables the self-assignment detection that was previously disabled due to unrelated regressions. The fix detects useless assignments like `x = x` and `foo.field = foo.field`.

## History

The original regressions (rust-lang#81626, rust-lang#81658) were specifically about false positives in write-only field detection, not self-assignment detection. Belows are brief history for the rule that I understand.

- Self-assignment detection was originally implemented in rust-lang#87129 to address rust-lang#75356
- The implementation was disabled alongside the revert of rust-lang#81473's "write-only fields" detection
- rust-lang#81473 was reverted via rust-lang#86212 and rust-lang#83171 due to false positives in write-only field detection (rust-lang#81626, rust-lang#81658)
- The self-assignment detection feature got removed, even though it wasn't the reason for the problems

This PR only re-enables the self-assignment checks, which are orthogonal to the problematic write-only field analysis.

## Changes
- Removed `#[allow(dead_code)]` from `compiler/rustc_passes/src/dead.rs` file
    - `handle_assign` and
    - `check_for_self_assign`
- Added `ExprKind::Assign` handling in `visit_expr` to call both methods
- Updated test expectations in `tests/ui/lint/dead-code/self-assign.rs`
bors added a commit that referenced this pull request Aug 12, 2025
Rollup of 14 pull requests

Successful merges:

 - #131477 (Apple: Always pass SDK root when linking with `cc`, and pass it via `SDKROOT` env var)
 - #139806 (std: sys: pal: uefi: Overhaul Time)
 - #144210 (std: thread: Return error if setting thread stack size fails)
 - #144386 (Extract TraitImplHeader in AST/HIR)
 - #144921 (Don't emit `rustdoc::broken_intra_doc_links` for GitHub-flavored Markdown admonitions like `[!NOTE]`)
 - #145155 (Port `#[allow_internal_unsafe]` to the new attribute system (attempt 2))
 - #145214 (fix: re-enable self-assignment)
 - #145216 (rustdoc: correct negative-to-implicit discriminant display)
 - #145238 (Tweak invalid builtin attribute output)
 - #145249 (Rename entered trace span variables from `_span` to  `_trace`)
 - #145251 (Support using #[unstable_feature_bound] on trait)
 - #145253 (Document compiler and stdlib in stage1 in `pr-check-2` CI job)
 - #145263 (Update books)
 - #145273 (Account for new `assert!` desugaring in `!condition` suggestion)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Aug 12, 2025
…, r=petrochenkov

fix: re-enable self-assignment

## Description

Re-enables the self-assignment detection that was previously disabled due to unrelated regressions. The fix detects useless assignments like `x = x` and `foo.field = foo.field`.

## History

The original regressions (rust-lang#81626, rust-lang#81658) were specifically about false positives in write-only field detection, not self-assignment detection. Belows are brief history for the rule that I understand.

- Self-assignment detection was originally implemented in rust-lang#87129 to address rust-lang#75356
- The implementation was disabled alongside the revert of rust-lang#81473's "write-only fields" detection
- rust-lang#81473 was reverted via rust-lang#86212 and rust-lang#83171 due to false positives in write-only field detection (rust-lang#81626, rust-lang#81658)
- The self-assignment detection feature got removed, even though it wasn't the reason for the problems

This PR only re-enables the self-assignment checks, which are orthogonal to the problematic write-only field analysis.

## Changes
- Removed `#[allow(dead_code)]` from `compiler/rustc_passes/src/dead.rs` file
    - `handle_assign` and
    - `check_for_self_assign`
- Added `ExprKind::Assign` handling in `visit_expr` to call both methods
- Updated test expectations in `tests/ui/lint/dead-code/self-assign.rs`
bors added a commit that referenced this pull request Aug 12, 2025
Rollup of 17 pull requests

Successful merges:

 - #131477 (Apple: Always pass SDK root when linking with `cc`, and pass it via `SDKROOT` env var)
 - #139806 (std: sys: pal: uefi: Overhaul Time)
 - #144210 (std: thread: Return error if setting thread stack size fails)
 - #144386 (Extract TraitImplHeader in AST/HIR)
 - #144921 (Don't emit `rustdoc::broken_intra_doc_links` for GitHub-flavored Markdown admonitions like `[!NOTE]`)
 - #145155 (Port `#[allow_internal_unsafe]` to the new attribute system (attempt 2))
 - #145214 (fix: re-enable self-assignment)
 - #145216 (rustdoc: correct negative-to-implicit discriminant display)
 - #145238 (Tweak invalid builtin attribute output)
 - #145249 (Rename entered trace span variables from `_span` to  `_trace`)
 - #145251 (Support using #[unstable_feature_bound] on trait)
 - #145253 (Document compiler and stdlib in stage1 in `pr-check-2` CI job)
 - #145260 (Make explicit guarantees about `Vec`’s allocator)
 - #145263 (Update books)
 - #145273 (Account for new `assert!` desugaring in `!condition` suggestion)
 - #145283 (Make I-miscompile imply I-prioritize)
 - #145291 (bootstrap: Only warn about `rust.debug-assertions` if downloading rustc)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants