From 9fef3d9e0a386f8b1e277d4b598e6254c9c0516b Mon Sep 17 00:00:00 2001 From: xFrednet <xFrednet@gmail.com> Date: Fri, 6 Aug 2021 23:18:16 +0200 Subject: [PATCH] Added `Expect` lint level and attribute (RFC-2383) * Also added the `LintExpectationId` which will be used in future commits --- Cargo.lock | 1 + .../src/annotate_snippet_emitter_writer.rs | 1 + compiler/rustc_errors/src/diagnostic.rs | 6 +- compiler/rustc_errors/src/lib.rs | 14 ++++- compiler/rustc_feature/src/builtin_attrs.rs | 4 ++ compiler/rustc_lint/src/context.rs | 4 ++ compiler/rustc_lint_defs/Cargo.toml | 1 + compiler/rustc_lint_defs/src/lib.rs | 56 ++++++++++++++++++- compiler/rustc_middle/src/lint.rs | 22 ++++++++ compiler/rustc_session/src/session.rs | 3 + 10 files changed, 107 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f0c6e371c38..bc2c77d9271 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3882,6 +3882,7 @@ version = "0.0.0" dependencies = [ "rustc_ast", "rustc_data_structures", + "rustc_index", "rustc_macros", "rustc_serialize", "rustc_span", diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 7d7ab1ed4e5..c380455012d 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -75,6 +75,7 @@ fn annotation_type_for_level(level: Level) -> AnnotationType { // FIXME(#59346): Not sure how to map this level Level::FailureNote => AnnotationType::Error, Level::Allow => panic!("Should not call with Allow"), + Level::Expect(_) => panic!("Should not call with Expect"), } } diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 6d6ada86428..802f2560601 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -133,7 +133,11 @@ impl Diagnostic { | Level::Error { .. } | Level::FailureNote => true, - Level::Warning | Level::Note | Level::Help | Level::Allow => false, + Level::Warning + | Level::Note + | Level::Help + | Level::Allow + | Level::Expect(_) => false, } } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index fdfedffc529..41d2b285997 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -20,6 +20,7 @@ extern crate tracing; pub use emitter::ColorConfig; +use rustc_lint_defs::LintExpectationId; use Level::*; use emitter::{is_case_difference, Emitter, EmitterWriter}; @@ -677,6 +678,11 @@ impl Handler { DiagnosticBuilder::new(self, Level::Allow, msg) } + /// Construct a builder at the `Expect` level with the `msg`. + pub fn struct_expect(&self, msg: &str, id: LintExpectationId) -> DiagnosticBuilder<'_, ()> { + DiagnosticBuilder::new(self, Level::Expect(id), msg) + } + /// Construct a builder at the `Error` level at the given `span` and with the `msg`. pub fn struct_span_err( &self, @@ -953,7 +959,9 @@ impl HandlerInner { (*TRACK_DIAGNOSTICS)(diagnostic); - if diagnostic.level == Allow { + if let Level::Expect(_) = diagnostic.level { + return; + } else if diagnostic.level == Allow { return; } @@ -1250,6 +1258,7 @@ pub enum Level { Help, FailureNote, Allow, + Expect(LintExpectationId), } impl fmt::Display for Level { @@ -1275,7 +1284,7 @@ impl Level { spec.set_fg(Some(Color::Cyan)).set_intense(true); } FailureNote => {} - Allow => unreachable!(), + Allow | Expect(_) => unreachable!(), } spec } @@ -1289,6 +1298,7 @@ impl Level { Help => "help", FailureNote => "failure-note", Allow => panic!("Shouldn't call on allowed error"), + Expect(_) => panic!("Shouldn't call on expected error"), } } diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 53762eef785..4b9cf784495 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -282,6 +282,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ungated!( allow, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk ), + gated!( + expect, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk, + lint_reasons, experimental!(expect) + ), ungated!( forbid, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk ), diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index ad9a16fb39a..c34e9f1ba78 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -109,6 +109,7 @@ struct LintGroup { depr: Option<LintAlias>, } +#[derive(Debug)] pub enum CheckLintNameResult<'a> { Ok(&'a [LintId]), /// Lint doesn't exist. Potentially contains a suggestion for a correct lint name. @@ -377,6 +378,9 @@ impl LintStore { Level::ForceWarn => "--force-warn", Level::Deny => "-D", Level::Forbid => "-F", + Level::Expect(_) => { + unreachable!("lints with the level of `expect` should not run this code"); + } }, lint_name ); diff --git a/compiler/rustc_lint_defs/Cargo.toml b/compiler/rustc_lint_defs/Cargo.toml index 798d50819e2..c37e45b46ce 100644 --- a/compiler/rustc_lint_defs/Cargo.toml +++ b/compiler/rustc_lint_defs/Cargo.toml @@ -10,3 +10,4 @@ rustc_span = { path = "../rustc_span" } rustc_serialize = { path = "../rustc_serialize" } rustc_macros = { path = "../rustc_macros" } rustc_target = { path = "../rustc_target" } +rustc_index = { path = "../rustc_index" } diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index e9c62fc4006..e6643cfd544 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -1,3 +1,5 @@ +#![feature(min_specialization)] + #[macro_use] extern crate rustc_macros; @@ -46,13 +48,60 @@ pub enum Applicability { Unspecified, } +rustc_index::newtype_index! { + /// FIXME: The lint expectation ID is currently a simple copy of the `AttrId` + /// that the expectation originated from. In the future it should be generated + /// by other means. This is for one to keep the IDs independent of each other + /// and also to ensure that it is actually stable between compilation sessions. + /// (The `AttrId` for instance, is not stable). + /// + /// Additionally, it would be nice if this generation could be moved into + /// [`Level::from_symbol`] to have it all contained in one module and to + /// make it simpler to use. + pub struct LintExpectationId { + DEBUG_FORMAT = "LintExpectationId({})" + } +} + +rustc_data_structures::impl_stable_hash_via_hash!(LintExpectationId); + +impl<HCX> ToStableHashKey<HCX> for LintExpectationId { + type KeyType = u32; + + #[inline] + fn to_stable_hash_key(&self, _: &HCX) -> Self::KeyType { + self.as_u32() + } +} + /// Setting for how to handle a lint. +/// +/// See: https://doc.rust-lang.org/rustc/lints/levels.html #[derive(Clone, Copy, PartialEq, PartialOrd, Eq, Ord, Debug, Hash)] pub enum Level { + /// The `allow` level will not issue any message. Allow, + /// The `expect` level will suppress the lint message but intern produce a message + /// if the lint wasn't issued in the expected scope. `Expect` should not be used as + /// an initial level for a lint. + /// + /// Note that this still means that the lint is enabled in this position and should + /// be emitted, this will intern fulfill the expectation and suppress the lint. + /// + /// See RFC 2383. + /// + /// The `LintExpectationId` is used to later link a lint emission to the actual + /// expectation. It can be ignored in most cases. + Expect(LintExpectationId), + /// The `warn` level will produce a warning if the lint was violated, however the + /// compiler will continue with its execution. Warn, ForceWarn, + /// The `deny` level will produce an error and stop further execution after the lint + /// pass is complete. Deny, + /// `Forbid` is equivalent to the `deny` level but can't be overwritten like the previous + /// levels. Forbid, } @@ -63,6 +112,7 @@ impl Level { pub fn as_str(self) -> &'static str { match self { Level::Allow => "allow", + Level::Expect(_) => "expect", Level::Warn => "warn", Level::ForceWarn => "force-warn", Level::Deny => "deny", @@ -70,14 +120,15 @@ impl Level { } } - /// Converts a lower-case string to a level. + /// Converts a lower-case string to a level. This will never construct the expect + /// level as that would require a [`LintExpectationId`] pub fn from_str(x: &str) -> Option<Level> { match x { "allow" => Some(Level::Allow), "warn" => Some(Level::Warn), "deny" => Some(Level::Deny), "forbid" => Some(Level::Forbid), - _ => None, + "expect" | _ => None, } } @@ -85,6 +136,7 @@ impl Level { pub fn from_symbol(x: Symbol) -> Option<Level> { match x { sym::allow => Some(Level::Allow), + sym::expect => Some(Level::Expect(LintExpectationId::from(0u32))), sym::warn => Some(Level::Warn), sym::deny => Some(Level::Deny), sym::forbid => Some(Level::Forbid), diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index 35e1558600d..5e7fc33953a 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -225,6 +225,7 @@ pub fn explain_lint_level_source( Level::Forbid => "-F", Level::Allow => "-A", Level::ForceWarn => "--force-warn", + Level::Expect(_) => unreachable!("the expect level does not have a commandline flag"), }; let hyphen_case_lint_name = name.replace('_', "-"); if lint_flag_val.as_str() == name { @@ -314,6 +315,16 @@ pub fn struct_lint_level<'s, 'd>( return; } } + (Level::Expect(expect_id), _) => { + // This case is special as we actually allow the lint itself in this context, but + // we can't return early like in the case for `Level::Allow` because we still + // need the lint diagnostic to be emitted to `rustc_error::HanderInner`. + // + // We can also not mark the lint expectation as fulfilled here right away, as it + // can still be cancelled in the decorate function. All of this means that we simply + // create a `DiagnosticBuilder` and continue as we would for warnings. + sess.struct_expect("", expect_id) + } (Level::Warn | Level::ForceWarn, Some(span)) => sess.struct_span_warn(span, ""), (Level::Warn | Level::ForceWarn, None) => sess.struct_warn(""), (Level::Deny | Level::Forbid, Some(span)) => { @@ -346,6 +357,17 @@ pub fn struct_lint_level<'s, 'd>( } } + // Lint diagnostics that are covered by the expect level will not be emitted outside + // the compiler. It is therefore not necessary to add any information for the user. + // This will therefore directly call the decorate function which will intern emit + // the `Diagnostic`. + if let Level::Expect(_) = level { + let name = lint.name_lower(); + err.code(DiagnosticId::Lint { name, has_future_breakage, is_force_warn: false }); + decorate(LintDiagnosticBuilder::new(err)); + return; + } + explain_lint_level_source(sess, lint, level, src, &mut err); let name = lint.name_lower(); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index b72be735ce4..edc98abca29 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -331,6 +331,9 @@ impl Session { pub fn struct_allow(&self, msg: &str) -> DiagnosticBuilder<'_, ()> { self.diagnostic().struct_allow(msg) } + pub fn struct_expect(&self, msg: &str, id: lint::LintExpectationId) -> DiagnosticBuilder<'_, ()> { + self.diagnostic().struct_expect(msg, id) + } pub fn struct_span_err<S: Into<MultiSpan>>( &self, sp: S,