Skip to content

Warn write-only fields #81473

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

Merged
merged 3 commits into from
Jan 30, 2021
Merged

Warn write-only fields #81473

merged 3 commits into from
Jan 30, 2021

Conversation

sanxiyn
Copy link
Member

@sanxiyn sanxiyn commented Jan 28, 2021

cc @Boscop's example in #49256.

@rust-highfive
Copy link
Contributor

r? @oli-obk

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 28, 2021
Co-authored-by: Oli Scherer <[email protected]>
@rust-log-analyzer

This comment has been minimized.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 28, 2021

@bors r+ squash

@bors
Copy link
Collaborator

bors commented Jan 28, 2021

📌 Commit 85ad773 has been approved by oli-obk

@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 Jan 28, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 30, 2021
Rollup of 16 pull requests

Successful merges:

 - rust-lang#79023 (Add `core::stream::Stream`)
 - rust-lang#80562 (Consider Scalar to be a bool only if its unsigned)
 - rust-lang#80886 (Stabilize raw ref macros)
 - rust-lang#80959 (Stabilize `unsigned_abs`)
 - rust-lang#81291 (Support FRU pattern with `[feature(capture_disjoint_fields)]`)
 - rust-lang#81409 (Slight simplification of chars().count())
 - rust-lang#81468 (cfg(version): treat nightlies as complete)
 - rust-lang#81473 (Warn write-only fields)
 - rust-lang#81495 (rustdoc: Remove unnecessary optional)
 - rust-lang#81499 (Updated Vec::splice documentation)
 - rust-lang#81501 (update rustfmt to v1.4.34)
 - rust-lang#81505 (`fn cold_path` doesn't need to be pub)
 - rust-lang#81512 (Add missing variants in match binding)
 - rust-lang#81515 (Fix typo in pat.rs)
 - rust-lang#81519 (Don't print error output from rustup when detecting default build triple)
 - rust-lang#81520 (Don't clone LLVM submodule when download-ci-llvm is set)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 774ba83 into rust-lang:master Jan 30, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 30, 2021
@sanxiyn sanxiyn deleted the write-only-field branch January 30, 2021 14:16
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 20, 2021
Consider auto derefs before warning about write only fields

Changes from rust-lang#81473 extended the dead code lint with an ability to detect
fields that are written to but never read from. The implementation skips
over fields on the left hand side of an assignment, without marking them
as live.

A field access might involve an automatic dereference and de-facto read
the field. Conservatively mark expressions with deref adjustments as
live to avoid generating false positive warnings.

Closes rust-lang#81626.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 22, 2021
Consider auto derefs before warning about write only fields

Changes from rust-lang#81473 extended the dead code lint with an ability to detect
fields that are written to but never read from. The implementation skips
over fields on the left hand side of an assignment, without marking them
as live.

A field access might involve an automatic dereference and de-facto read
the field. Conservatively mark expressions with deref adjustments as
live to avoid generating false positive warnings.

Closes rust-lang#81626.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 23, 2021
Consider auto derefs before warning about write only fields

Changes from rust-lang#81473 extended the dead code lint with an ability to detect
fields that are written to but never read from. The implementation skips
over fields on the left hand side of an assignment, without marking them
as live.

A field access might involve an automatic dereference and de-facto read
the field. Conservatively mark expressions with deref adjustments as
live to avoid generating false positive warnings.

Closes rust-lang#81626.
@pnkfelix
Copy link
Member

We discussed this PR in the compiler triage meeting.

In short #81473 injected some issues, namely #81626 and #81658, that have leaked into the beta release.

We decided in the compiler triage meeting to revert #81473 on the 1.51 beta branch. We'll leave it in place on nightly 1.52 branch, since we expect #81658 will be resolved before the nightly-to-beta-1.52 promotion.

I'll be posting a revert PR shortly.

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 18, 2021
…-fields, r=Mark-Simulacrum

Revert PR 81473 to resolve (on beta) issues 81626 and 81658.

Revert PR rust-lang#81473 to resolve (on beta) issues rust-lang#81626 and rust-lang#81658.

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

Successfully merging this pull request may close these issues.

7 participants