Make the test cfg a userspace check-cfg
Proposal
The test cfg is special, in that it's the only built-in cfg that is set by rustc and is allowed to be set by users of rustc[^1].
[^1]: Per explicit_builtin_cfgs_in_flags lint.
Cargo for examples sets it when running cargo test with harness = false (i.e. without rustc --test).
This creates a tension with --check-cfg as to who is the "owner" of the cfg, since it is currently always marked as expected.
However, there are cases where the user might not want to have unit test inside a crate and, as such not have the test cfg being marked as expected, which is not currently possible (we don't have a way to opt-out of the well-known list).
This is particularly relevant for the lib.test = false config in Cargo.toml which disables unit-testing for the package.
In those cases, having test as a well-known config is counter-productive, as it won't produce the warning that is being expected (e.g. https://github.com/rust-lang/rust/issues/117778 and https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/check-cfg.20forbid.20rather.20than.20accept).
As an aside, this is relevant for us regarding core/coretests, alloc/alloctests crates.
Given all of the above, I propose that we make users of rustc to be owner of the test cfg.
Concretely, that would mean:
- Removing the
testfromrustcwell known cfgs list - Adjusting the documentation (regarding the owner of
testcfg) - (for Cargo) Adding the
testto it's well known cfgs list (and making it conditional onlib.test)
This should make it clear that it's the responsibility of the user/build system to explicitly "mark" a crate as unit-testable, instead of "forcing" every crate.
The unexpected_cfgs lint is warn-by-default but only activated with --check-cfg (opt-in) so I don't think this could break any workflow.
This proposal is the result of multiple discussions over the issues and Zulip topic, including with @epage (Cargo maintainer and "liaison" for check-cfg).
Alternative/future work: general mechanism to "un-expect" cfgs
An alternative to this proposal would be to have a more general mechanism to "un-expect" any (or maybe just well known) cfgs. This was explored in https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/check-cfg.20forbid.20rather.20than.20accept.
However, it's unclear who would benefit from this general mechanism apart for the test cfg. It would also introduce many complexities, in particular regarding precedence or scope, and more generaly it's usefulness.
Given this, I don't think we should be doing this more general mechanism until we have actual use cases. This alternative is forward-compatible with the proposal above.
Mentors or Reviewers
my-self (I think)
Process
The main points of the Major Change Process are as follows:
- [x] 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 mergeon either the MCP or the PR.
- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a
- [ ] 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.
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
@rfcbot fcp merge
Team member @petrochenkov has proposed to merge this. The next step is review by the rest of the tagged team members:
- [ ] @Aaron1011
- [x] @cjgillot
- [x] @compiler-errors
- [x] @davidtwco
- [x] @eddyb
- [x] @estebank
- [x] @jackh726
- [x] @lcnr
- [x] @matthewjasper
- [x] @nagisa
- [x] @oli-obk
- [x] @petrochenkov
- [x] @pnkfelix
- [ ] @wesleywiser
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
See this document for info about what commands tagged team members can give me.
Implementation - https://github.com/rust-lang/rust/pull/131729.
This FCP is waiting on your feedback @Aaron1011 @cjgillot @estebank @pnkfelix @wesleywiser.
:bell: This is now entering its final comment period, as per the review above. :bell:
psst @petrochenkov, I wasn't able to add the final-comment-period label, please do so.
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
@rustbot label -final-comment-period +major-change-accepted