rustbot
commented
2 months ago 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
apiraino
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:
Context
The Reference has recently started adding rule identifiers that look like
destructors.scope.nesting.function-bodythat 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:
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:
@rustbot second.-C flag, then full team check-off is required.@rfcbot fcp mergeon either the MCP or the PR.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.