Change compiletest declarations parsing

[pietroalbini]

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.

[rustbot]

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

[oli-obk]

@rustbot second

[apiraino]

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