Skip to content

Conversation

jieyouxu
Copy link
Member

I did a survey pass over tests/, and this test seems like the only candidate suitable for conversion into a known-bug test. (Other tests had varying degrees of other issues that known-bug would not be suitable.)

r? compiler

@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 Apr 13, 2025
@jieyouxu jieyouxu added the A-testsuite Area: The testsuite used to check the correctness of rustc label Apr 13, 2025
//! - `dead_code` lint expansion for self-assignments was implemented in #87129.
//! - Unfortunately implementation components of #87129 had to be disabled as part of reverts
//! #86212, #83171 (to revert #81473) to address regressions #81626 and #81658.
//! - Consequently, none of the following warnings are emitted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice explanation.

@nnethercote
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 13, 2025

📌 Commit 16334cd has been approved by nnethercote

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 Apr 13, 2025
@jieyouxu
Copy link
Member Author

@bors rollup

ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 13, 2025
Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test

I did a survey pass over `tests/`, and this test seems like the only candidate suitable for conversion into a known-bug test. (Other tests had varying degrees of other issues that known-bug would not be suitable.)

r? compiler
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Apr 13, 2025
Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test

I did a survey pass over `tests/`, and this test seems like the only candidate suitable for conversion into a known-bug test. (Other tests had varying degrees of other issues that known-bug would not be suitable.)

r? compiler
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
…enton

Rollup of 12 pull requests

Successful merges:

 - rust-lang#138744 (Add methods to TCP and UDP sockets to modify hop limit (refresh of rust-lang#94678))
 - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast)
 - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate)
 - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration)
 - rust-lang#139582 (Various coercion cleanups)
 - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`)
 - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`)
 - rust-lang#139666 (cleanup `mir_borrowck`)
 - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints)
 - rust-lang#139695 (compiletest: consistently use `camino::{Utf8Path,Utf8PathBuf}` throughout)
 - rust-lang#139722 (Move some things to rustc_type_ir)
 - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Apr 13, 2025
Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test

I did a survey pass over `tests/`, and this test seems like the only candidate suitable for conversion into a known-bug test. (Other tests had varying degrees of other issues that known-bug would not be suitable.)

r? compiler
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 13, 2025
Rollup of 11 pull requests

Successful merges:

 - rust-lang#137043 (Initial `UnsafePinned` implementation [Part 1: Libs])
 - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast)
 - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate)
 - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration)
 - rust-lang#139582 (Various coercion cleanups)
 - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`)
 - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`)
 - rust-lang#139671 (Proc macro span API redesign: Replace proc_macro::SourceFile by Span::{file, local_file})
 - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints)
 - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test)
 - rust-lang#139741 (fix smir's run! doc and import)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#137043 (Initial `UnsafePinned` implementation [Part 1: Libs])
 - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast)
 - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate)
 - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration)
 - rust-lang#139582 (Various coercion cleanups)
 - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`)
 - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`)
 - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints)
 - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test)
 - rust-lang#139741 (fix smir's run! doc and import)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7ba09db into rust-lang:master Apr 14, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 14, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 14, 2025
Rollup merge of rust-lang#139740 - jieyouxu:known-bug, r=nnethercote

Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test

I did a survey pass over `tests/`, and this test seems like the only candidate suitable for conversion into a known-bug test. (Other tests had varying degrees of other issues that known-bug would not be suitable.)

r? compiler
@jieyouxu jieyouxu deleted the known-bug branch April 14, 2025 04:18
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 14, 2025
Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? `@Zalathar` (or compiler/bootstrap)
Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 15, 2025
Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? ``@Zalathar`` (or compiler/bootstrap)
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Apr 15, 2025
Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? ```@Zalathar``` (or compiler/bootstrap)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 15, 2025
Rollup merge of rust-lang#139783 - jieyouxu:ignore-dir, r=Zalathar

Use `compiletest-ignore-dir` for bootstrap self-tests

Follow-up to rust-lang#139705 and rust-lang#139740.

I did another survey pass over `//@ ignore-test` under `tests/`, and this is the only 2 non-tests that should use `compiletest-ignore-dir`.

r? `@Zalathar` (or compiler/bootstrap)
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 17, 2025
Introduce and use specialized `//@ ignore-auxiliary` for test support files instead of using `//@ ignore-test`

### Summary

Add a semantically meaningful directive for ignoring test *auxiliary* files. This is for auxiliary files that *participate* in actual tests but should not be built by `compiletest` (i.e. these files are involved through `mod xxx;` or `include!()` or `#[path = "xxx"]`, etc.).

### Motivation

A specialized directive like `//@ ignore-auxiliary` makes it way easier to audit disabled tests via `//@ ignore-test`.
  - These support files cannot use the canonical `auxiliary/` dir because they participate in module resolution or are included, or their relative paths can be important for test intention otherwise.

Follow-up to:
- rust-lang#139705
- rust-lang#139783
- rust-lang#139740

See also discussions in:

- [#t-compiler > Directive name for non-test aux files?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Directive.20name.20for.20non-test.20aux.20files.3F/with/512773817)
- [#t-compiler > Handling disabled &rust-lang#96;//@ ignore-test&rust-lang#96; tests](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Handling.20disabled.20.60.2F.2F.40.20ignore-test.60.20tests/with/512005974)
- [#t-compiler/meetings > &rust-lang#91;steering&rust-lang#93; 2025-04-11 Dealing with disabled tests](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bsteering.5D.202025-04-11.20Dealing.20with.20disabled.20tests/with/511717981)

### Remarks on remaining unconditionally disabled tests under `tests/`

After this PR, against commit 79a272c, only **14** remaining test files are disabled through `//@ ignore-test`:

<details>
<summary>Remaining `//@ ignore-test` files under `tests/`</summary>

```
tests/debuginfo/drop-locations.rs
4://@ ignore-test (broken, see rust-lang#128971)

tests/rustdoc/macro-document-private-duplicate.rs
1://@ ignore-test (fails spuriously, see issue rust-lang#89228)

tests/rustdoc/inline_cross/assoc-const-equality.rs
3://@ ignore-test (FIXME: rust-lang#125092)

tests/ui/match/issue-27021.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/match/issue-26996.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-49298.rs
9://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-59756.rs
2://@ ignore-test (rustfix needs multiple suggestions)

tests/ui/precondition-checks/write.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/read.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/write_bytes.rs
5://@ ignore-test (unimplemented)

tests/ui/explicit-tail-calls/drop-order.rs
2://@ ignore-test: tail calls are not implemented in rustc_codegen_ssa yet, so this causes 🧊

tests/ui/panics/panic-short-backtrace-windows-x86_64.rs
3://@ ignore-test (rust-lang#92000)

tests/ui/json/json-bom-plus-crlf-multifile-aux.rs
3://@ ignore-test Not a test. Used by other tests

tests/ui/traits/next-solver/object-soundness-requires-generalization.rs
2://@ ignore-test (see rust-lang#114196)
```
</details>

Of these, most are either **unimplemented**, or **spurious**, or **known-broken**. The outstanding one is `tests/ui/json/json-bom-plus-crlf-multifile-aux.rs` which I did not want to touch in *this* PR -- that aux file has load-bearing BOM and carriage returns and byte offset matters. I think those test files that require special encoding / BOM probably are better off as `run-make` tests. See rust-lang#139968 for that aux file.

### Review advice

- Best reviewed commit-by-commit.
- The directive name diverged from the most voted `//@ auxiliary` because I think that's easy to confuse with `//@ aux-{crate,dir}`.

r? compiler
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2025
Rollup merge of rust-lang#139967 - jieyouxu:auxiliary, r=wesleywiser

Introduce and use specialized `//@ ignore-auxiliary` for test support files instead of using `//@ ignore-test`

### Summary

Add a semantically meaningful directive for ignoring test *auxiliary* files. This is for auxiliary files that *participate* in actual tests but should not be built by `compiletest` (i.e. these files are involved through `mod xxx;` or `include!()` or `#[path = "xxx"]`, etc.).

### Motivation

A specialized directive like `//@ ignore-auxiliary` makes it way easier to audit disabled tests via `//@ ignore-test`.
  - These support files cannot use the canonical `auxiliary/` dir because they participate in module resolution or are included, or their relative paths can be important for test intention otherwise.

Follow-up to:
- rust-lang#139705
- rust-lang#139783
- rust-lang#139740

See also discussions in:

- [#t-compiler > Directive name for non-test aux files?](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Directive.20name.20for.20non-test.20aux.20files.3F/with/512773817)
- [#t-compiler > Handling disabled &rust-lang#96;//@ ignore-test&rust-lang#96; tests](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Handling.20disabled.20.60.2F.2F.40.20ignore-test.60.20tests/with/512005974)
- [#t-compiler/meetings > &rust-lang#91;steering&rust-lang#93; 2025-04-11 Dealing with disabled tests](https://rust-lang.zulipchat.com/#narrow/channel/238009-t-compiler.2Fmeetings/topic/.5Bsteering.5D.202025-04-11.20Dealing.20with.20disabled.20tests/with/511717981)

### Remarks on remaining unconditionally disabled tests under `tests/`

After this PR, against commit 79a272c, only **14** remaining test files are disabled through `//@ ignore-test`:

<details>
<summary>Remaining `//@ ignore-test` files under `tests/`</summary>

```
tests/debuginfo/drop-locations.rs
4://@ ignore-test (broken, see rust-lang#128971)

tests/rustdoc/macro-document-private-duplicate.rs
1://@ ignore-test (fails spuriously, see issue rust-lang#89228)

tests/rustdoc/inline_cross/assoc-const-equality.rs
3://@ ignore-test (FIXME: rust-lang#125092)

tests/ui/match/issue-27021.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/match/issue-26996.rs
7://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-49298.rs
9://@ ignore-test (rust-lang#54987)

tests/ui/issues/issue-59756.rs
2://@ ignore-test (rustfix needs multiple suggestions)

tests/ui/precondition-checks/write.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/read.rs
5://@ ignore-test (unimplemented)

tests/ui/precondition-checks/write_bytes.rs
5://@ ignore-test (unimplemented)

tests/ui/explicit-tail-calls/drop-order.rs
2://@ ignore-test: tail calls are not implemented in rustc_codegen_ssa yet, so this causes 🧊

tests/ui/panics/panic-short-backtrace-windows-x86_64.rs
3://@ ignore-test (rust-lang#92000)

tests/ui/json/json-bom-plus-crlf-multifile-aux.rs
3://@ ignore-test Not a test. Used by other tests

tests/ui/traits/next-solver/object-soundness-requires-generalization.rs
2://@ ignore-test (see rust-lang#114196)
```
</details>

Of these, most are either **unimplemented**, or **spurious**, or **known-broken**. The outstanding one is `tests/ui/json/json-bom-plus-crlf-multifile-aux.rs` which I did not want to touch in *this* PR -- that aux file has load-bearing BOM and carriage returns and byte offset matters. I think those test files that require special encoding / BOM probably are better off as `run-make` tests. See rust-lang#139968 for that aux file.

### Review advice

- Best reviewed commit-by-commit.
- The directive name diverged from the most voted `//@ auxiliary` because I think that's easy to confuse with `//@ aux-{crate,dir}`.

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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.

4 participants