Make the test cfg a userspace check-cfg #785
Description
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 rustc1.
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. rust-lang/rust#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 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 this would only affect users of --check-cfg (I'm only aware of Cargo for now).
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 or which cfgs 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 generally it's usefulness.
Given this, I don't think we should be doing this more general mechanism until we have actual use cases.
It as also be not noted as 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:
- 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.
Footnotes
-
Per
explicit_builtin_cfgs_in_flagslint. ↩