Skip to content

Revert PR 81473 to resolve (on mainline) issues 81626 and 81658. - #86212

Merged
bors merged 3 commits into
rust-lang:masterfrom
pnkfelix:mainline-targetted-revert-81473-warn-write-only-fields
Jul 22, 2021
Merged

Revert PR 81473 to resolve (on mainline) issues 81626 and 81658.#86212
bors merged 3 commits into
rust-lang:masterfrom
pnkfelix:mainline-targetted-revert-81473-warn-write-only-fields

Conversation

@pnkfelix

Copy link
Copy Markdown
Contributor

This is a nightly-targetted variant of PR #83171

The intent is to just address issue #81658 on all release channels, rather that keep repeatedly reverting PR #83171 on beta.

However, our intent is also to reland PR #83171 after we have addressed issue #81658 , most likely by coupling the re-landing of PR #83171 with an enhancement like PR #83004

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @Mark-Simulacrum

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2021
@pnkfelixpnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 11, 2021
@rust-log-analyzer

This comment has been minimized.

@Mark-Simulacrum

Copy link
Copy Markdown
Member

r=me but looks like there's CI trouble here, and presumably we'll need another backport as this unfortunately didn't land in time for 1.54 branching

@Mark-SimulacrumMark-Simulacrum 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 Jun 21, 2021
@JohnCSimon

Copy link
Copy Markdown

Triage:
@pnkfelix CI trouble here, FYI.

@emilyalbiniemilyalbini added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 17, 2021
@emilyalbini

Copy link
Copy Markdown
Member

Beta-nominated for 1.54, as this will land in 1.55.

@bors

bors commented Jul 18, 2021

Copy link
Copy Markdown
Collaborator

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

@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

Hmm. Do I delete the unused method, or just #[allow(dead_code)] there. I am tempted to do the latter, since it should be less disruptive in case other people have injected uses of it...?

Revert "Add missing brace"
This reverts commit 85ad773.
Revert "Simplify base_expr"
This reverts commit 899aae4.
Revert "Warn write-only fields"
This reverts commit d3c69a4.
I expect these two methods to come back very soon; noise of removing them to satisfy lint seems wrong.
@pnkfelix
pnkfelixforce-pushed the mainline-targetted-revert-81473-warn-write-only-fields branch from 7ae061c to b6e9d06CompareJuly 22, 2021 02:57
@pnkfelix

Copy link
Copy Markdown
ContributorAuthor

@bors r=simulacrum

also, unilaterally approving for beta backport.

@bors

bors commented Jul 22, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit ee2bb1f has been approved by simulacrum

@borsbors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 22, 2021
@pnkfelixpnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jul 22, 2021
@Mark-Simulacrum

Copy link
Copy Markdown
Member

@bors p=1

@Mark-SimulacrumMark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 22, 2021
@bors

bors commented Jul 22, 2021

Copy link
Copy Markdown
Collaborator

⌛ Testing commit ee2bb1f with merge 0271870...

@bors

bors commented Jul 22, 2021

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: simulacrum
Pushing 0271870 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Jul 22, 2021
@bors
bors merged commit 0271870 into rust-lang:masterJul 22, 2021
@rustbotrustbot added this to the 1.55.0 milestone Jul 22, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 23, 2021
…warn-write-only-fields, r=Mark-Simulacrum
Beta targetted revert 81473 warn write only fields
beta backport of PR rust-lang#86212
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`
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`
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`
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`
rust-timer added a commit that referenced this pull request Aug 12, 2025
Rollup merge of #145214 - notJoon:fix/enable-self-assignment, 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 (#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.
- Self-assignment detection was originally implemented in #87129 to address #75356
- The implementation was disabled alongside the revert of #81473's "write-only fields" detection
- #81473 was reverted via #86212 and #83171 due to false positives in write-only field detection (#81626, #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`
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta-acceptedAccepted for backporting to the compiler in the beta channel.merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant 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.

8 participants

@pnkfelix@rust-highfive@rust-log-analyzer@Mark-Simulacrum@JohnCSimon@emilyalbini@bors@rustbot