Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change compiletest declarations parsing #512

Closed
1 of 3 tasks
pietroalbini opened this issue Apr 29, 2022 · 3 comments
Closed
1 of 3 tasks

Change compiletest declarations parsing #512

pietroalbini opened this issue Apr 29, 2022 · 3 comments
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team

Comments

@pietroalbini
Copy link
Member

pietroalbini commented Apr 29, 2022 •
edited
Loading

Proposal

Right now compiletest declarations (like // only-x86_64) are only parsed up until the first function or module, ignoring any subsequent declaration, and use the same syntax as normal comments. This causes two problems:

  1. It's not possible to add declarations to the bottom of the file. This is desirable whenever a downstream fork needs to add its own declarations, as adding declarations to the top of the file causes all line numbers in error messages to shift.
  2. There is no error when an invalid declaration is written (or there is simply a typo), as compiletest has no way to know whether a comment is meant to be a declaration or just a comment. This is done often enough that there is hardcoded special handling for compile-flags (erroring out when compiler-test is written instead).

This MCP proposes to:

  • Add a new declaration syntax (//@) in addition to the current one (//). Declarations with the new syntax will be validated, and invalid declarations will result in an error.
  • Allow new-style declarations in the whole file rather than just the top part of the file.
  • Phase out old-style declarations while minimizing churn:
    1. Open a big PR to migrate declarations to the new style for the files currently in master. This will encourage new contributors looking at other test files to use the new style of declarations.
    2. Periodically (monthly?) open PRs to convert newly added old-style declarations to the equivalent new-style ones.
    3. After a while, make it an error to use old-style declarations.

Thanks to @bjorn3 for the idea of the //@ syntax.

Alternative: avoid a deprecation period for old-style declarations

We could avoid a long deprecation period and just create a PR converting all declarations, erroring out when an old declaration is present. This would reduce the amount of work needed to make the change, but it would create a lot of churn for all open PRs.

Alternative: only use the new syntax for end-of-style declarations

We could continue accepting // declarations at the top of the file, and require //@ at the bottom of the file. This would prevent all of the churn, but would also create inconsistencies and confusions at how declarations are parsed. It would also not fix problem 2.

Alternative: remove all line numbers from test outputs

We could remove all line and column numbers from the test output, removing the need for adding declarations at the bottom of the file. This would solve problem 1, but wouldn't fix problem 2.

Mentors or Reviewers

@Mark-Simulacrum

Process

The main points of the Major Change Process are as follows:

  • File an issue describing the proposal.
  • A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
    • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
    • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
  • Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.
@pietroalbini pietroalbini added T-compiler Add this label so rfcbot knows to poll the compiler team major-change A proposal to make a major change to rustc labels Apr 29, 2022
@rustbot
Copy link
Collaborator

rustbot commented Apr 29, 2022

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

@rustbot rustbot added the to-announce Announce this issue on triage meeting label Apr 29, 2022
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label May 6, 2022
@pietroalbini pietroalbini mentioned this issue May 30, 2022
Merged
@oli-obk
Copy link
Contributor

oli-obk commented May 30, 2022

@rustbot second

@rustbot rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label May 30, 2022
@apiraino
Copy link
Contributor

apiraino commented Jun 9, 2022

@rustbot label -final-comment-period +major-change-accepted

@apiraino apiraino closed this as completed Jun 9, 2022
@rustbot rustbot added major-change-accepted A major change proposal that was accepted to-announce Announce this issue on triage meeting and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Jun 9, 2022
@RalfJung RalfJung mentioned this issue Jun 9, 2022
Closed
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jun 9, 2022
@oli-obk oli-obk mentioned this issue Jul 28, 2022
Closed
3 tasks
This was referenced Apr 19, 2023
Merged
Merged
@jieyouxu jieyouxu mentioned this issue Feb 10, 2024
Merged
3 tasks
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2024
@bors
Auto merge of rust-lang#120881 - jieyouxu:migrate-ui-test-directives,…
be8d2f6 
… r=oli-obk

Migrate ui tests from legacy compiletest-style directives `//` to `ui_test`-style directives `//@`

## Preface

There's an on-going effort to rewrite parts of or the entirety of compiletest
(<rust-lang/compiler-team#536>). A step towards this involve migrating
ui tests to use the [`ui_test`](https://github.com/oli-obk/ui_test) framework, which involves
changing compiletest directives in `// <directive-name>` style to `ui_test` `//@ <directive-name>`
style (rust-lang/compiler-team#512).

This PR aims to implement the directive-style change from `//` to `//`@`` for ui tests only and
make compiletest only accept `//`@`` directives in ui test mode (only).

## Key Changes

1. All ui test `//` directives are replaced by `//`@`` directives.
2. Only accept `//`@`` directives for ui tests (specifically ui tests).
3. Errors if a comment could be interpreted as a legacy-style `//` directive.

## Diff Generation

The diff is generated by:

- Collecting directives from ui tests via hacking on compiletest.
- Using a migration tool to replace `//` directives in ui tests with `//`@`.`

### Reproduction Steps

0. Delete the temporary directory `$RUSTC_REPO_PATH/build/x86_64-apple-darwin/test/ui/__directive_lines` and the temporary file `$RUSTC_REPO_PATH/build/x86_64-apple-darwin/test/ui/__directive_lines.txt` (if you ran the collection script before).
1. Use the <https://github.com/jieyouxu/rust/tree/collect-test-directives> collect-test-directives
   script, which outputs temporary files recording headers occuring in each ui test.
   - You need to checkout this branch: `git checkout collect-test-directives`.
   - You might need to rebase on lastest master and ensure there are no conflicts.
   - You likely need to run `./x test tests/ui --stage 1 --force-rerun` to generate the temporary
     files consistently.
2. Checkout the `migrate-ui-test-directives` branch.
4. Run the migration tool <https://github.com/jieyouxu/compiletest-ui_test-header-migration>.
    - You will need to first generate a `migration_config.toml` via `cargo run -- generate-config` under `$CWD`.
    - Then, update `manual_directives = ["// should-fail"]` in `migration_config.toml`. This is required because the collection script doesn't deal with some special meta ui tests and there are no other `// should-fail` occurrences.
5. Check that the migration at least does not cause UI test failures if you change compiletest to
   accept `//`@`` directives for ui tests only.
   - `RUSTC_TEST_FAIL_FAST=1 ./x test tests/ui --stage 1 --bless`
6. Confirm that there is no difference after running the migration tool when you are on the
   `migrate-ui-test-directives` branch.

## Next Steps

- [x] ~~Need to implement some kind of warning or tidy script to help contributors catch old-style
  `// <directive-name>` directives, while only accepting `ui_test`-style `//@ <directive-name>`
  directives.~~ An error is emitted if a comment that could be interpreted as legacy-style test directive is encountered.
- [ ] Need to properly document this change in e.g. rustc-dev-guide (rust-lang/rustc-dev-guide#1885).
    - [x] Add a `README.md` to `tests/ui` describing the directive style change.
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 16, 2024
@bors
Auto merge of rust-lang#120881 - jieyouxu:migrate-ui-test-directives,…
bccb9bb 
… r=oli-obk

Migrate ui tests from legacy compiletest-style directives `//` to `ui_test`-style directives `//@`

## Preface

There's an on-going effort to rewrite parts of or the entirety of compiletest
(<rust-lang/compiler-team#536>). A step towards this involve migrating
ui tests to use the [`ui_test`](https://github.com/oli-obk/ui_test) framework, which involves
changing compiletest directives in `// <directive-name>` style to `ui_test` `//@ <directive-name>`
style (rust-lang/compiler-team#512).

This PR aims to implement the directive-style change from `//` to `//`@`` for ui tests only and
make compiletest only accept `//`@`` directives in the "ui" test suite (only).

## Key Changes

1. All ui test `//` directives are replaced by `//`@`` directives.
2. Only accept `//`@`` directives for "ui" test suite.
3. Errors if a comment could be interpreted as a legacy-style `//` directive.

## Diff Generation

The diff is generated by:

- Collecting directives from ui tests via hacking on compiletest.
- Using a migration tool to replace `//` directives in ui tests with `//`@`.`

### Reproduction Steps

0. Delete the temporary directory `$RUSTC_REPO_PATH/build/x86_64-apple-darwin/test/ui/__directive_lines` and the temporary file `$RUSTC_REPO_PATH/build/x86_64-apple-darwin/test/ui/__directive_lines.txt` (if you ran the collection script before).
1. Use the <https://github.com/jieyouxu/rust/tree/collect-test-directives> collect-test-directives
   script, which outputs temporary files recording headers occuring in each ui test.
   - You need to checkout this branch: `git checkout collect-test-directives`.
   - You might need to rebase on lastest master and ensure there are no conflicts.
   - You likely need to run `./x test tests/ui --stage 1 --force-rerun` to generate the temporary
     files consistently.
2. Checkout the `migrate-ui-test-directives` branch.
4. Run the migration tool <https://github.com/jieyouxu/compiletest-ui_test-header-migration>.
    - You will need to first generate a `migration_config.toml` via `cargo run -- generate-config` under `$CWD`.
    - Then, update `manual_directives = ["// should-fail"]` in `migration_config.toml`. This is required because the collection script doesn't deal with some special meta ui tests and there are no other `// should-fail` occurrences.
5. Check that the migration at least does not cause UI test failures if you change compiletest to
   accept `//`@`` directives for ui tests only.
   - `RUSTC_TEST_FAIL_FAST=1 ./x test tests/ui --stage 1 --bless`
6. Confirm that there is no difference after running the migration tool when you are on the
   `migrate-ui-test-directives` branch.

## Next Steps

- [x] ~~Need to implement some kind of warning or tidy script to help contributors catch old-style
  `// <directive-name>` directives, while only accepting `ui_test`-style `//@ <directive-name>`
  directives.~~ An error is emitted if a comment that could be interpreted as legacy-style test directive is encountered.
- [ ] Need to properly document this change in e.g. rustc-dev-guide (rust-lang/rustc-dev-guide#1885).
    - [x] Add a `README.md` to `tests/ui` describing the directive style change.
@jieyouxu jieyouxu mentioned this issue Feb 20, 2024
Merged
1 task
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 21, 2024
@bors
Auto merge of rust-lang#121370 - jieyouxu:migrate-compiletest-directi…
806b54b 
…ves, r=oli-obk

Migrate compiletest to use `ui_test`-style `//@` directives

## Preface

There's an on-going effort to rewrite parts of or the entirety of compiletest
(<rust-lang/compiler-team#536>). A step towards this involve migrating
compiletest tests to use the [`ui_test`](https://github.com/oli-obk/ui_test) framework, which
involves changing compiletest directives in `// <directive-name>` style to `ui_test`
`//@ <directive-name>` style (rust-lang/compiler-team#512).

This PR aims to implement the directive-style change from `//` to `//`@`` for the remaining
non-"ui" test suite tests.

## Key Changes

1. All `tests/` tests now use `//`@`` directives.
2. Compiletest only accepts `//`@`` and issues an error if an old-style directive is detected.
3. `// ignore-tidy` and `// ignore-tidy-*` are considered tidy directives and are ignored by
   compiletest header parsing.

## Diff Generation

The diff is generated by:

- Collecting directives from `tests/` via hijacking compiletest to emit successfully parsed
  directive lines.
- Using a migration tool
  (<https://github.com/jieyouxu/compiletest-ui_test-header-migration/tree/master>) to replace
  `//` directives in compiletest tests with `//`@`.`

### Reproduction Steps

0. Delete the temporary file `$RUSTC_REPO_PATH/build/<target_triple>/test/__directive_lines.txt`,
   if the collection script was previously ran.
1. Use the <https://github.com/jieyouxu/rust/tree/collect-test-directives> collect-test-directives
   script, which outputs a temporary file recording headers occuring in each compiletest test.
   - You need to checkout this branch: `git checkout collect-test-directives`.
   - This needs to be rebased on latest master to ensure up-to-date test directives can be collected.
   - You need to run `./x test` on each of the `test/*` subfolders once:

        ```bash
        ./x test tests/assembly/ --stage 1 --force-rerun
        ./x test tests/codegen/ --stage 1 --force-rerun
        ./x test tests/codegen-units/ --stage 1 --force-rerun
        ./x test tests/coverage/ --stage 1 --force-rerun
        ./x test tests/coverage-run-rustdoc/ --stage 1 --force-rerun
        ./x test tests/debuginfo/ --stage 1 --force-rerun
        ./x test tests/incremental/ --stage 1 --force-rerun
        ./x test tests/mir-opt/ --stage 1 --force-rerun
        ./x test tests/pretty/ --stage 1 --force-rerun
        ./x test tests/run-make/ --stage 1 --force-rerun
        ./x test tests/run-make-fulldeps/ --stage 1 --force-rerun
        ./x test tests/run-pass-valgrind/ --stage 1 --force-rerun
        ./x test tests/rustdoc/ --stage 1 --force-rerun
        TARGET=<target-triple> ./x test tests/rustdoc-gui/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js-std/ --stage 1 --force-rerun
        ./x test tests/rustdoc-json/ --stage 1 --force-rerun
        ./x test tests/rustdoc-ui/ --stage 1 --force-rerun
        ./x test tests/ui/ --stage 1 --force-rerun
        ./x test tests/ui-fulldeps/ --stage 1 --force-rerun
        ```

2. Checkout the `migrate-compiletest-directives` branch.
3. Run the migration tool <https://github.com/jieyouxu/compiletest-ui_test-header-migration>.
4. Check that the migration at least does not cause test failures if you change compiletest to
   accept `//`@`` directives only. This is also required if the test outputs somehow need to be
   blessed.
   - `RUSTC_TEST_FAIL_FAST=1 ./x test tests/<secondary-directory>  --stage 1 --bless`
5. Confirm that there is no difference after running the migration tool when you are on the
   `migrate-compiletest-directives` branch.

## Follow Up Work

- [ ] Adjust rustc-dev-guide docs for compiletest directives (this time for all the other suites and modes).
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 21, 2024
@bors
Auto merge of rust-lang#121370 - jieyouxu:migrate-compiletest-directi…
ec712cb 
…ves, r=oli-obk

Migrate compiletest to use `ui_test`-style `//@` directives

## Preface

There's an on-going effort to rewrite parts of or the entirety of compiletest
(<rust-lang/compiler-team#536>). A step towards this involve migrating
compiletest tests to use the [`ui_test`](https://github.com/oli-obk/ui_test) framework, which
involves changing compiletest directives in `// <directive-name>` style to `ui_test`
`//@ <directive-name>` style (rust-lang/compiler-team#512).

This PR aims to implement the directive-style change from `//` to `//`@`` for the remaining
non-"ui" test suite tests.

## Key Changes

1. All `tests/` tests now use `//`@`` directives.
2. Compiletest only accepts `//`@`` and issues an error if an old-style directive is detected.
3. `// ignore-tidy` and `// ignore-tidy-*` are considered tidy directives and are ignored by
   compiletest header parsing.

## Diff Generation

The diff is generated by:

- Collecting directives from `tests/` via hijacking compiletest to emit successfully parsed
  directive lines.
- Using a migration tool
  (<https://github.com/jieyouxu/compiletest-ui_test-header-migration/tree/master>) to replace
  `//` directives in compiletest tests with `//`@`.`

### Reproduction Steps

0. Delete the temporary file `$RUSTC_REPO_PATH/build/<target_triple>/test/__directive_lines.txt`,
   if the collection script was previously ran.
1. Use the <https://github.com/jieyouxu/rust/tree/collect-test-directives> collect-test-directives
   script, which outputs a temporary file recording headers occuring in each compiletest test.
   - You need to checkout this branch: `git checkout collect-test-directives`.
   - This needs to be rebased on latest master to ensure up-to-date test directives can be collected.
   - You need to run `./x test` on each of the `test/*` subfolders once:

        ```bash
        ./x test tests/assembly/ --stage 1 --force-rerun
        ./x test tests/codegen/ --stage 1 --force-rerun
        ./x test tests/codegen-units/ --stage 1 --force-rerun
        ./x test tests/coverage/ --stage 1 --force-rerun
        ./x test tests/coverage-run-rustdoc/ --stage 1 --force-rerun
        ./x test tests/debuginfo/ --stage 1 --force-rerun
        ./x test tests/incremental/ --stage 1 --force-rerun
        ./x test tests/mir-opt/ --stage 1 --force-rerun
        ./x test tests/pretty/ --stage 1 --force-rerun
        ./x test tests/run-make/ --stage 1 --force-rerun
        ./x test tests/run-make-fulldeps/ --stage 1 --force-rerun
        ./x test tests/run-pass-valgrind/ --stage 1 --force-rerun
        ./x test tests/rustdoc/ --stage 1 --force-rerun
        TARGET=<target-triple> ./x test tests/rustdoc-gui/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js-std/ --stage 1 --force-rerun
        ./x test tests/rustdoc-json/ --stage 1 --force-rerun
        ./x test tests/rustdoc-ui/ --stage 1 --force-rerun
        ./x test tests/ui/ --stage 1 --force-rerun
        ./x test tests/ui-fulldeps/ --stage 1 --force-rerun
        ```

2. Checkout the `migrate-compiletest-directives` branch.
3. Run the migration tool <https://github.com/jieyouxu/compiletest-ui_test-header-migration>.
4. Check that the migration at least does not cause test failures if you change compiletest to
   accept `//`@`` directives only. This is also required if the test outputs somehow need to be
   blessed.
   - `RUSTC_TEST_FAIL_FAST=1 ./x test tests/<secondary-directory>  --stage 1 --bless`
5. Confirm that there is no difference after running the migration tool when you are on the
   `migrate-compiletest-directives` branch.

## Follow Up Work

- [ ] Adjust rustc-dev-guide docs for compiletest directives (this time for all the other suites and modes).
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 22, 2024
@bors
Auto merge of rust-lang#121370 - jieyouxu:migrate-compiletest-directi…
057b6de 
…ves, r=oli-obk

Migrate compiletest to use `ui_test`-style `//@` directives

## Preface

There's an on-going effort to rewrite parts of or the entirety of compiletest
(<rust-lang/compiler-team#536>). A step towards this involve migrating
compiletest tests to use the [`ui_test`](https://github.com/oli-obk/ui_test) framework, which
involves changing compiletest directives in `// <directive-name>` style to `ui_test`
`//@ <directive-name>` style (rust-lang/compiler-team#512).

This PR aims to implement the directive-style change from `//` to `//`@`` for the remaining
non-"ui" test suite tests.

## Key Changes

1. All `tests/` tests now use `//`@`` directives.
2. Compiletest only accepts `//`@`` and issues an error if an old-style directive is detected.
3. `// ignore-tidy` and `// ignore-tidy-*` are considered tidy directives and are ignored by
   compiletest header parsing.

## Diff Generation

The diff is generated by:

- Collecting directives from `tests/` via hijacking compiletest to emit successfully parsed
  directive lines.
- Using a migration tool
  (<https://github.com/jieyouxu/compiletest-ui_test-header-migration/tree/master>) to replace
  `//` directives in compiletest tests with `//`@`.`

### Reproduction Steps

0. Delete the temporary file `$RUSTC_REPO_PATH/build/<target_triple>/test/__directive_lines.txt`,
   if the collection script was previously ran.
1. Use the <https://github.com/jieyouxu/rust/tree/collect-test-directives> collect-test-directives
   script, which outputs a temporary file recording headers occuring in each compiletest test.
   - You need to checkout this branch: `git checkout collect-test-directives`.
   - This needs to be rebased on latest master to ensure up-to-date test directives can be collected.
   - You need to run `./x test` on each of the `test/*` subfolders once:

        ```bash
        ./x test tests/assembly/ --stage 1 --force-rerun
        ./x test tests/codegen/ --stage 1 --force-rerun
        ./x test tests/codegen-units/ --stage 1 --force-rerun
        ./x test tests/coverage/ --stage 1 --force-rerun
        ./x test tests/coverage-run-rustdoc/ --stage 1 --force-rerun
        ./x test tests/debuginfo/ --stage 1 --force-rerun
        ./x test tests/incremental/ --stage 1 --force-rerun
        ./x test tests/mir-opt/ --stage 1 --force-rerun
        ./x test tests/pretty/ --stage 1 --force-rerun
        ./x test tests/run-make/ --stage 1 --force-rerun
        ./x test tests/run-make-fulldeps/ --stage 1 --force-rerun
        ./x test tests/run-pass-valgrind/ --stage 1 --force-rerun
        ./x test tests/rustdoc/ --stage 1 --force-rerun
        TARGET=<target-triple> ./x test tests/rustdoc-gui/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js-std/ --stage 1 --force-rerun
        ./x test tests/rustdoc-json/ --stage 1 --force-rerun
        ./x test tests/rustdoc-ui/ --stage 1 --force-rerun
        ./x test tests/ui/ --stage 1 --force-rerun
        ./x test tests/ui-fulldeps/ --stage 1 --force-rerun
        ```

2. Checkout the `migrate-compiletest-directives` branch.
3. Run the migration tool <https://github.com/jieyouxu/compiletest-ui_test-header-migration>.
4. Check that the migration at least does not cause test failures if you change compiletest to
   accept `//`@`` directives only. This is also required if the test outputs somehow need to be
   blessed.
   - `RUSTC_TEST_FAIL_FAST=1 ./x test tests/<secondary-directory>  --stage 1 --bless`
5. Confirm that there is no difference after running the migration tool when you are on the
   `migrate-compiletest-directives` branch.

## Follow Up Work

- [ ] Adjust rustc-dev-guide docs for compiletest directives (this time for all the other suites and modes).
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 22, 2024
@bors
Auto merge of rust-lang#121370 - jieyouxu:migrate-compiletest-directi…
f62f490 
…ves, r=oli-obk

Migrate compiletest to use `ui_test`-style `//@` directives

## Preface

There's an on-going effort to rewrite parts of or the entirety of compiletest
(<rust-lang/compiler-team#536>). A step towards this involve migrating
compiletest tests to use the [`ui_test`](https://github.com/oli-obk/ui_test) framework, which
involves changing compiletest directives in `// <directive-name>` style to `ui_test`
`//@ <directive-name>` style (rust-lang/compiler-team#512).

This PR aims to implement the directive-style change from `//` to `//`@`` for the remaining
non-"ui" test suite tests.

## Key Changes

1. All `tests/` tests now use `//`@`` directives.
2. Compiletest only accepts `//`@`` and issues an error if an old-style directive is detected.
3. `// ignore-tidy` and `// ignore-tidy-*` are considered tidy directives and are ignored by
   compiletest header parsing.

## Diff Generation

The diff is generated by:

- Collecting directives from `tests/` via hijacking compiletest to emit successfully parsed
  directive lines.
- Using a migration tool
  (<https://github.com/jieyouxu/compiletest-ui_test-header-migration/tree/master>) to replace
  `//` directives in compiletest tests with `//`@`.`

### Reproduction Steps

0. Delete the temporary file `$RUSTC_REPO_PATH/build/<target_triple>/test/__directive_lines.txt`,
   if the collection script was previously ran.
1. Use the <https://github.com/jieyouxu/rust/tree/collect-test-directives> collect-test-directives
   script, which outputs a temporary file recording headers occuring in each compiletest test.
   - You need to checkout this branch: `git checkout collect-test-directives`.
   - This needs to be rebased on latest master to ensure up-to-date test directives can be collected.
   - You need to run `./x test` on each of the `test/*` subfolders once:

        ```bash
        ./x test tests/assembly/ --stage 1 --force-rerun
        ./x test tests/codegen/ --stage 1 --force-rerun
        ./x test tests/codegen-units/ --stage 1 --force-rerun
        ./x test tests/coverage/ --stage 1 --force-rerun
        ./x test tests/coverage-run-rustdoc/ --stage 1 --force-rerun
        ./x test tests/debuginfo/ --stage 1 --force-rerun
        ./x test tests/incremental/ --stage 1 --force-rerun
        ./x test tests/mir-opt/ --stage 1 --force-rerun
        ./x test tests/pretty/ --stage 1 --force-rerun
        ./x test tests/run-make/ --stage 1 --force-rerun
        ./x test tests/run-make-fulldeps/ --stage 1 --force-rerun
        ./x test tests/run-pass-valgrind/ --stage 1 --force-rerun
        ./x test tests/rustdoc/ --stage 1 --force-rerun
        TARGET=<target-triple> ./x test tests/rustdoc-gui/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js/ --stage 1 --force-rerun
        ./x test tests/rustdoc-js-std/ --stage 1 --force-rerun
        ./x test tests/rustdoc-json/ --stage 1 --force-rerun
        ./x test tests/rustdoc-ui/ --stage 1 --force-rerun
        ./x test tests/ui/ --stage 1 --force-rerun
        ./x test tests/ui-fulldeps/ --stage 1 --force-rerun
        ```

2. Checkout the `migrate-compiletest-directives` branch.
3. Run the migration tool <https://github.com/jieyouxu/compiletest-ui_test-header-migration>.
4. Check that the migration at least does not cause test failures if you change compiletest to
   accept `//`@`` directives only. This is also required if the test outputs somehow need to be
   blessed.
   - `RUSTC_TEST_FAIL_FAST=1 ./x test tests/<secondary-directory>  --stage 1 --bless`
5. Confirm that there is no difference after running the migration tool when you are on the
   `migrate-compiletest-directives` branch.

## Follow Up Work

- [ ] Adjust rustc-dev-guide docs for compiletest directives (this time for all the other suites and modes). <rust-lang/rustc-dev-guide#1895>.
@jieyouxu jieyouxu mentioned this issue Jul 22, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted T-compiler Add this label so rfcbot knows to poll the compiler team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@oli-obk @pietroalbini @apiraino @rustbot