Policy for ICEs on incorrect usage of internal-only features · Issue #620 · rust-lang/compiler-team (original) (raw)

Proposal

When using internal features like lang_items or intrinsics, rustc is very panicky. For examples, see rust-lang/rust#110538, rust-lang/rust#102465, rust-lang/rust#101964.

rustc makes many assumptions about the shape and presence of lang items and intrinsics. These assumptions are deep within the compiler and make the code simpler or faster. Trying to give proper errors for these broken assumptions would increase the code complexity of the compiler.

These errors are only being hit either by people working on the lang items (which will be people with some knowledge of the compiler, so they should be fine with the ICEs) or by people just playing around, which we don't need to support when it costs us something. Additionally, after #596 users using these panicky features will be made aware of what they're doing.

Therefore, I propose the following policy when dealing with ICEs/confusing errors from problems with the declared lang items or intrinsics in the source code:

ICEs or confusing diagnostics when using internal-only features like intrinsics or lang items incorrectly are not considered rustc bugs. Therefore, diagnostics improvements or ICE fixes for issues related to usage of internal only features are generally not accepted unless they are particularly useful in general. For example, the compiler currently validates that the item kind and amount of generic args of lang items matches, the code for which is fairly simple and useful for all lang items so we will keep it. In particular, one-off fixes for particular assumptions about lang items or intrinsics that introduce additional complexity into the compiler are not accepted. Error or ICE messages on incorrect usages of internal-only features are intended for compiler developers and not general users.

Exceptions could always be made on a case-by-case basis as deemed appropriate by T-compiler or compiler contributors, but this policy is there to have something to point at when closing PRs or issues as wontfix.

As a concrete example of an application of this policy, this PR of mine would have been denied: rust-lang/rust#104216. It added some complexity and a test for a broken lang item definition.

We should PR the policy do the dev guide if it is accepted. We may also want to skim the test suite for tests that test the absence of ICEs on broken lang item definitions like added in the PR shown above and delete those tests.

Mentors or Reviewers

@compiler-errors asked me to make this and helped with the wording

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.

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.