rust-lang / rust-lang/rust

Arguably-internal lint scope `rustc` isn't protected by a feature gate

Open
#138,787 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-lints C-bug T-compiler
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

While we manually reject[^1] attribute paths that contain a path segment that starts with the string rustc[^2] in order to protect internal attributes

/* assume the user defined `rustc`, `rustc_attr`, `rustcorrosion` or don't */

#[rustc::property] //~ ERROR attributes starting with `rustc` are reserved
#[rustc_attr] //~ ERROR attributes starting with `rustc` are reserved
#[rustcorrosion] //~ ERROR attributes starting with `rustc` are reserved
fn main() {}

we don't have a similar mechanism in place for "lint paths", i.e., paths given as arguments to lint attributes like allow and deny. The following code passes compilation without any warnings which I didn't expect:

#![allow(rustc::internals)]
#![deny(rustc::i_do_not_exist)]

Why could this be considered problematic? Rustc should be allowed to remove or rename the lint scope rustc without impacting stable users. However, if rustc did do so, it would break the code of hypothetical stable users who started to "rely" on these lint paths existing. Why would it break their code? Well, undefined lint scopes currently result in a hard error (unlike unknown lints for which we emit the warn-by-default lint unknown-lints):

#![allow(undefined::lint)] //~ ERROR unknown tool name `undefined` found in scoped lint: `undefined::lint` [E0710]

Of course, that scenario is quite unlikely to happen in practice. This is slightly above P-low territory I'd say.


I think we should reject lint scope rustc unless internal feature rustc_attrs is enabled.

Alternatively, we could think about replacing the hard error E0710 with a deny-by-default lint unknown-lint-scopes (rendering "breakages" caused by renaming/removing the lint scope acceptable as per Rust's stability guarantees) but that might do a disservice to tools using register_tool and wouldn't signal to users that crashes/ICEs caused by running these internal lint passes are not considered bugs.

[^1]: Unless internal feature rustc_attrs is enabled.
[^2]: Indeed, this includes "benign" segment idents like rustcorrosion which I find questionable personally speaking (I would at least relax it to IS(rustc) OR STARTSWITH(rustc_)) since users could theoretically encounter this in the wild if they define or use attribute proc macros. I remember there having been heated discussions about replacing that with a proper "tool" module (ToolMod) called rustc with the main(?) counter argument being "rustc is not a tool" which I do understand.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing how lint attributes such as allow and deny resolve scoped lint paths and how E0710 is produced for unknown lint scopes. Compare that path with the existing rustc_attrs handling for internal attributes; done means rustc lint scopes are rejected without the feature and accepted with it, with coverage for the examples in the issue.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.