rust-lang
Emit `check-cfg` lints during attribute parsing rather than evaluation
The goal of this PR is to make the eval_config_entry not have any side effects, by moving the check-cfg lints to the attribute parsing. This also helps ensure we do emit the lint in situations where the attribute happens to be parsed, but never evaluated.
cc @jdonszelmann @Urgau for a vibe check if you feel like it
The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
|
14 | if let EvalConfigResult::True = attr::eval_config_entry(
| ^^^^^^^^^^^^^^^^^^^^^^^
...
17 | ecx.current_expansion.lint_node_id,
| ---------------------------------- unexpected argument #3 of type `NodeId`
18 | ShouldEmit::ErrorsAndLints,
| -------------------------- unexpected argument #4 of type `ShouldEmit`
|
note: function defined here
--> /checkout/compiler/rustc_attr_parsing/src/attributes/cfg.rs:207:8
|
207 | pub fn eval_config_entry(sess: &Session, cfg_entry: &CfgEntry) -> EvalConfigResult {
| ^^^^^^^^^^^^^^^^^
help: remove the extra arguments
|
16 - &cfg,
17 - ecx.current_expansion.lint_node_id,
Does it mean we would get warnings for non-active cfg attributes?
#[cfg(target_os = "lol")] // warn and not active
#[cfg(unknown)] // currently doesn't warn, since above is not active, but will it with this PR?
fn foo() {}
Regarding the non-accessible TyCtxt, would it be possible to continue to buffer the lint in ParseSess::buffered_lints, using sess.parse_sess.buffer_lint? Or is the problem that you don't emit the lint as a builtin lint and as such no longer have access to TyCtxt?
The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking rustc_builtin_macros v0.0.0 (/checkout/compiler/rustc_builtin_macros)
error: unused import: `ShouldEmit`
--> compiler/rustc_builtin_macros/src/cfg_select.rs:4:62
|
4 | CfgSelectBranches, CfgSelectPredicate, EvalConfigResult, ShouldEmit, parse_cfg_select,
| ^^^^^^^^^^
|
= note: `-D unused-imports` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(unused_imports)]`
The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)
Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] askama test:false 0.171
error[E0061]: this function takes 3 arguments but 4 arguments were supplied
--> src/librustdoc/passes/check_doc_cfg.rs:34:13
|
34 | rustc_lint::decorate_builtin_lint(sess, Some(self.0), builtin_diag, diag)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ------------ unexpected argument #2 of type `std::option::Option<TyCtxt<'a>>`
|
note: function defined here
--> /checkout/compiler/rustc_lint/src/early/diagnostics.rs:15:8
|
15 | pub fn decorate_builtin_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &mut Diag<'_, ()>) {
| ^^^^^^^^^^^^^^^^^^^^^
help: remove the extra argument
|
34 - rustc_lint::decorate_builtin_lint(sess, Some(self.0), builtin_diag, diag)
34 + rustc_lint::decorate_builtin_lint(sess, builtin_diag, diag)
|
For more information about this error, try `rustc --explain E0061`.
[RUSTC-TIMING] rustdoc test:false 3.572
error: could not compile `rustdoc` (lib) due to 1 previous error
Attributes that are configured out (like in your example) are currently not parsed, and therefore this would not cause this warning, preserving the old behavior. I didn't think of this edgecase so I'll make sure to add a test for this.
Indeed currently attribute lints are special and not emitted as a builtin lint. I'm gonna play around with this a bit and see if anything is stopping me from changing that
:umbrella: The latest upstream changes (presumably #149410) made this pull request unmergeable. Please resolve the merge conflicts.
:umbrella: The latest upstream changes (presumably #149646) made this pull request unmergeable. Please resolve the merge conflicts.
Rebased on https://github.com/rust-lang/rust/pull/149662
After that PR, this change becomes trivial, we don't need to move the lints and we have access to TyCtxt
Some changes occurred in compiler/rustc_attr_parsing
cc @jdonszelmann
Some changes occurred in compiler/rustc_hir/src/attrs
cc @jdonszelmann
