Test rule annotations

[ehuss]

This is a proposal to add a compiletest header that marks the language rules covered by that test.

Summary of what the spec team is asking from the compiler team:

• Approve changes to compiletest to add a new "reference" header, and support to collect those headers into a JSON file.
• Permit the spec team to independently review and approve adding reference headers to existing tests. (Reviews from compiler contributors would also be appreciated, but not required.)
• Review additions to Rust's test suite for parts of the language that are not covered (incrementally, ongoing).

Context

The Reference has recently started adding rule identifiers that look like destructors.scope.nesting.function-body that we use to refer to specific rules in the language. We would like to start tying those identifiers to tests within Rust's testsuite. We plan to create a coverage summary similar to the FLS traceability matrix report, as well as links to those tests from the Reference.

This is modeled after how Ferrocene's fork of compiletest adds annotations for the FLS.

Proposal

Compiletest will be modified to support headers that might look something like:

//@ reference: destructors.scope.nesting

Compiletest will be modified to be able to collect these identifiers into a JSON file that the Reference tooling can use to generate coverage information and to provide links to the tests.

The spec team will be responsible for maintaining these annotations. However, any assistance from compiler contributors is welcome. Cooperation here can potentially help ease the stabilization process of future features (since being able to see the tests associated with specific rules is very helpful in reviewing).

An optional change we may need to make to compiletest is the ability to have non-rust files which can be used to mark rule identifiers for an entire directory. I don't expect to make this change until we determine that we actually need it. Ferrocene has employed this technique for their own annotations (example). This makes it easier to mark a large number of tests that all cover a similar concept.

Granularity

Although standards assessors do not require individual rules to be associated with individual tests, I think it will be useful from the standpoint of a Reference reviewer, and from the standpoint of Rust programmers trying to better understand some concept.

When reviewing changes to the Reference, it can be very difficult to actually know if the content is correct without fully reviewing the rustc implementation or looking at the corresponding tests. These annotations can go a long way towards seeing that there is an explicit test covering some concept and to validate our understanding. Similarly, a reader of the Reference may be confused by some statement, but seeing an example in Rust code can go a long way to improve their understanding.

We are not expecting 100% coverage at any time. Although it would be nice, it is unrealistic due to the sequence things are developed, and resources available.

New tests

The spec team plans to add tests to Rust's testsuite in cases where coverage is lacking. This will likely be done as-needed and incrementally in small steps over a long period of time. This will be an ask from the compiler team to be willing to review the addition of new tests.

Mentors or Reviewers

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.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

@rustbot concern reason-for-concern 
<description of the concern> 

Concerns can be lifted with:

@rustbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

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

[jieyouxu]

Yes, this is something that seems reasonable, and I would be interested in helping with (in terms of implementation or reviews).
@rustbot second

[apiraino]

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