Prevent invalid LevelSpec values

`LevelSpec` has two related fields, `level` and `lint_id`. This commit
makes the fields private (and introduces getters) to ensure they are set
together using only valid combinations.

The commit also introduces `is_allow` and `is_expect` methods because
those are useful.

Author: nnethercote

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -739,7 +739,7 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
                         rustc_session::lint::builtin::LONG_RUNNING_CONST_EVAL,
                         hir_id,
                     )
-                    .level
+                    .level()
                     .is_error();
                 let span = ecx.cur_span();
                 ecx.tcx.emit_node_span_lint(

compiler/rustc_driver_impl/src/lib.rs

@@ -729,7 +729,7 @@ fn print_crate_info(
                         // lint is unstable and feature gate isn't active, don't print
                         continue;
                     }
-                    let level = builder.lint_level_spec(lint).level;
+                    let level = builder.lint_level_spec(lint).level();
                     println_info!("{}={}", lint.name_lower(), level.as_str());
                 }
             }

compiler/rustc_hir_typeck/src/upvar.rs

@@ -2415,11 +2415,8 @@ fn should_do_rust_2021_incompatible_closure_captures_analysis(
         return false;
     }
 
-    let level = tcx
-        .lint_level_spec_at_node(lint::builtin::RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES, closure_id)
-        .level;
-
-    !matches!(level, lint::Level::Allow)
+    !tcx.lint_level_spec_at_node(lint::builtin::RUST_2021_INCOMPATIBLE_CLOSURE_CAPTURES, closure_id)
+        .is_allow()
 }
 
 /// Return a two string tuple (s1, s2)

compiler/rustc_lint/src/builtin.rs

@@ -60,7 +60,8 @@ use crate::lints::{
     BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment,
     BuiltinUnusedDocCommentSub, BuiltinWhileTrue, EqInternalMethodImplemented, InvalidAsmLabel,
 };
-use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext};
+use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
+
 declare_lint! {
     /// The `while_true` lint detects `while true { }`.
     ///
@@ -694,9 +695,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDebugImplementations {
         }
 
         // Avoid listing trait impls if the trait is allowed.
-        let level =
-            cx.tcx.lint_level_spec_at_node(MISSING_DEBUG_IMPLEMENTATIONS, item.hir_id()).level;
-        if level == Level::Allow {
+        if cx.tcx.lint_level_spec_at_node(MISSING_DEBUG_IMPLEMENTATIONS, item.hir_id()).is_allow() {
             return;
         }
 

compiler/rustc_lint/src/levels.rs

@@ -129,7 +129,7 @@ fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> UnordSet<LintId> {
             let level_spec =
                 root_map.lint_level_spec_at_node(tcx, LintId::of(lint), hir::CRATE_HIR_ID);
             // Only include lints that are allowed at crate root or by default.
-            matches!(level_spec.level, Level::Allow)
+            level_spec.is_allow()
                 || (matches!(level_spec.src, LintLevelSource::Default)
                     && lint.default_level(tcx.sess.edition()) == Level::Allow)
         })
@@ -142,7 +142,7 @@ fn lints_that_dont_need_to_run(tcx: TyCtxt<'_>, (): ()) -> UnordSet<LintId> {
         // All lints that appear with a non-allow level must be run.
         for (_, specs) in map.specs.iter() {
             for (lint, level_spec) in specs.iter() {
-                if !matches!(level_spec.level, Level::Allow) {
+                if !level_spec.is_allow() {
                     dont_need_to_run.remove(lint);
                 }
             }
@@ -520,15 +520,15 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
             };
             for &id in ids {
                 // ForceWarn and Forbid cannot be overridden
-                if let Some(LevelSpec { level: Level::ForceWarn | Level::Forbid, .. }) =
-                    self.current_specs().get(&id)
+                if let Some(level_spec) = self.current_specs().get(&id)
+                    && matches!(level_spec.level(), Level::ForceWarn | Level::Forbid)
                 {
                     continue;
                 }
 
                 if self.check_gated_lint(id, DUMMY_SP, true) {
                     let src = LintLevelSource::CommandLine(lint_flag_val, level);
-                    self.insert(id, LevelSpec { level, lint_id: None, src });
+                    self.insert(id, LevelSpec::new(level, None, src));
                 }
             }
         }
@@ -537,9 +537,14 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
     /// Attempts to insert the `id` to `LevelSpec` map entry. If unsuccessful
     /// (e.g. if a forbid was already inserted on the same scope), then emits a
     /// diagnostic with no change to `specs`.
-    fn insert_spec(&mut self, id: LintId, LevelSpec { level, lint_id, src }: LevelSpec) {
-        let LevelSpec { level: old_level, src: old_src, .. } =
-            self.provider.get_lint_level_spec(id.lint, self.sess);
+    fn insert_spec(&mut self, id: LintId, level_spec: LevelSpec) {
+        let level = level_spec.level();
+        let lint_id = level_spec.lint_id();
+        let src = level_spec.src;
+
+        let old_level_spec = self.provider.get_lint_level_spec(id.lint, self.sess);
+        let old_level = old_level_spec.level();
+        let old_src = old_level_spec.src;
 
         // Setting to a non-forbid level is an error if the lint previously had
         // a forbid level. Note that this is not necessarily true even with a
@@ -622,14 +627,14 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
         match (old_level, level) {
             // If the new level is an expectation store it in `ForceWarn`
             (Level::ForceWarn, Level::Expect) => {
-                self.insert(id, LevelSpec { level: Level::ForceWarn, lint_id, src: old_src })
+                self.insert(id, LevelSpec::new(Level::ForceWarn, lint_id, old_src))
             }
             // Keep `ForceWarn` level but drop the expectation
             (Level::ForceWarn, _) => {
-                self.insert(id, LevelSpec { level: Level::ForceWarn, lint_id: None, src: old_src })
+                self.insert(id, LevelSpec::new(Level::ForceWarn, None, old_src))
             }
             // Set the lint level as normal
-            _ => self.insert(id, LevelSpec { level, lint_id, src }),
+            _ => self.insert(id, LevelSpec::new(level, lint_id, src)),
         };
     }
 
@@ -644,7 +649,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
             if attr.is_automatically_derived_attr() {
                 self.insert(
                     LintId::of(SINGLE_USE_LIFETIMES),
-                    LevelSpec { level: Level::Allow, lint_id: None, src: LintLevelSource::Default },
+                    LevelSpec::new(Level::Allow, None, LintLevelSource::Default),
                 );
                 continue;
             }
@@ -653,7 +658,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
             if attr.is_doc_hidden() {
                 self.insert(
                     LintId::of(MISSING_DOCS),
-                    LevelSpec { level: Level::Allow, lint_id: None, src: LintLevelSource::Default },
+                    LevelSpec::new(Level::Allow, None, LintLevelSource::Default),
                 );
                 continue;
             }
@@ -866,7 +871,7 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
                 let src = LintLevelSource::Node { name, span: sp, reason };
                 for &id in ids {
                     if self.check_gated_lint(id, sp, false) {
-                        self.insert_spec(id, LevelSpec { level, lint_id, src });
+                        self.insert_spec(id, LevelSpec::new(level, lint_id, src));
                     }
                 }
 
@@ -897,20 +902,24 @@ impl<'s, P: LintLevelsProvider> LintLevelsBuilder<'s, P> {
         }
 
         if self.lint_added_lints && !is_crate_node {
-            for (id, &LevelSpec { level, ref src, .. }) in self.current_specs().iter() {
+            for (id, level_spec) in self.current_specs().iter() {
                 if !id.lint.crate_level_only {
                     continue;
                 }
 
-                let LintLevelSource::Node { name: lint_attr_name, span: lint_attr_span, .. } = *src
+                let LintLevelSource::Node { name: lint_attr_name, span: lint_attr_span, .. } =
+                    level_spec.src
                 else {
                     continue;
                 };
 
                 self.emit_span_lint(
                     UNUSED_ATTRIBUTES,
                     lint_attr_span.into(),
-                    IgnoredUnlessCrateSpecified { level: level.as_str(), name: lint_attr_name },
+                    IgnoredUnlessCrateSpecified {
+                        level: level_spec.level().as_str(),
+                        name: lint_attr_name,
+                    },
                 );
                 // don't set a separate error for every lint in the group
                 break;

compiler/rustc_lint/src/non_ascii_idents.rs

@@ -155,18 +155,14 @@ impl EarlyLintPass for NonAsciiIdents {
     fn check_crate(&mut self, cx: &EarlyContext<'_>, _: &ast::Crate) {
         use std::collections::BTreeMap;
 
-        use rustc_session::lint::Level;
         use rustc_span::Span;
         use unicode_security::GeneralSecurityProfile;
 
-        let check_non_ascii_idents =
-            cx.builder.lint_level_spec(NON_ASCII_IDENTS).level != Level::Allow;
-        let check_uncommon_codepoints =
-            cx.builder.lint_level_spec(UNCOMMON_CODEPOINTS).level != Level::Allow;
-        let check_confusable_idents =
-            cx.builder.lint_level_spec(CONFUSABLE_IDENTS).level != Level::Allow;
+        let check_non_ascii_idents = !cx.builder.lint_level_spec(NON_ASCII_IDENTS).is_allow();
+        let check_uncommon_codepoints = !cx.builder.lint_level_spec(UNCOMMON_CODEPOINTS).is_allow();
+        let check_confusable_idents = !cx.builder.lint_level_spec(CONFUSABLE_IDENTS).is_allow();
         let check_mixed_script_confusables =
-            cx.builder.lint_level_spec(MIXED_SCRIPT_CONFUSABLES).level != Level::Allow;
+            !cx.builder.lint_level_spec(MIXED_SCRIPT_CONFUSABLES).is_allow();
 
         if !check_non_ascii_idents
             && !check_uncommon_codepoints

compiler/rustc_metadata/src/creader.rs

@@ -332,7 +332,7 @@ impl CStore {
                 lint::builtin::UNUSED_CRATE_DEPENDENCIES,
                 rustc_hir::CRATE_HIR_ID,
             )
-            .level;
+            .level();
         if level != lint::Level::Allow {
             let unused_externs =
                 self.unused_externs.iter().map(|ident| ident.to_ident_string()).collect::<Vec<_>>();

compiler/rustc_middle/src/lint.rs

@@ -56,11 +56,56 @@ impl LintLevelSource {
 /// Convenience helper for things that are frequently used together.
 #[derive(Copy, Clone, Debug, StableHash, Encodable, Decodable)]
 pub struct LevelSpec {
-    pub level: Level,
-    pub lint_id: Option<LintExpectationId>,
+    // This field *must* be private. It must be set in tandem with `lint_id`, only in
+    // `LevelSpec::new`, because only certain `level`/`lint_id` combinations are valid. See
+    // `LevelSpec::new` for those combinations.
+    //
+    // If you are thinking right now that `level` and `lint_id` should be combined into a single
+    // type that excludes the invalid combinations, that's a reasonable thought, but in practice
+    // it's painful because `level` needs to be used by itself, without `lint_id`, in many places.
+    // Making the fields private prevents invalid combinations while retaining the flexibility of
+    // two separate fields.
+    level: Level,
+
+    // This field *must* be private. See the comment on `level`.
+    lint_id: Option<LintExpectationId>,
+
     pub src: LintLevelSource,
 }
 
+impl LevelSpec {
+    // Panics if an invalid `level`/`lint_id` combination is given.
+    pub fn new(
+        level: Level,
+        lint_id: Option<LintExpectationId>,
+        src: LintLevelSource,
+    ) -> LevelSpec {
+        match (level, lint_id) {
+            (Level::Allow | Level::Warn | Level::Deny | Level::Forbid, None) => {}
+            (Level::Expect, Some(_)) => {}
+            (Level::ForceWarn, _) => {}
+            _ => panic!("invalid level/lint_id combination"),
+        }
+        LevelSpec { level, lint_id, src }
+    }
+
+    pub fn level(self) -> Level {
+        self.level
+    }
+
+    pub fn is_allow(self) -> bool {
+        self.level == Level::Allow
+    }
+
+    pub fn is_expect(self) -> bool {
+        self.level == Level::Expect
+    }
+
+    pub fn lint_id(self) -> Option<LintExpectationId> {
+        self.lint_id
+    }
+}
+
 /// Return type for the `shallow_lint_levels_on` query.
 ///
 /// This map represents the set of allowed lints and allowance levels given
@@ -82,10 +127,8 @@ pub fn reveal_actual_level_spec(
     let level_spec = probe_for_lint_level_spec(lint);
 
     // If `level` is none then we actually assume the default level for this lint.
-    let mut level_spec = level_spec.unwrap_or_else(|| LevelSpec {
-        level: lint.lint.default_level(sess.edition()),
-        lint_id: None,
-        src: LintLevelSource::Default,
+    let mut level_spec = level_spec.unwrap_or_else(|| {
+        LevelSpec::new(lint.lint.default_level(sess.edition()), None, LintLevelSource::Default)
     });
 
     // If we're about to issue a warning, check at the last minute for any

compiler/rustc_middle/src/middle/stability.rs

@@ -13,7 +13,7 @@ use rustc_macros::{Decodable, Encodable, StableHash, Subdiagnostic};
 use rustc_session::Session;
 use rustc_session::errors::feature_err_issue;
 use rustc_session::lint::builtin::{DEPRECATED, DEPRECATED_IN_FUTURE};
-use rustc_session::lint::{DeprecatedSinceKind, Level, Lint};
+use rustc_session::lint::{DeprecatedSinceKind, Lint};
 use rustc_span::{Span, Symbol, sym};
 use tracing::debug;
 
@@ -234,7 +234,7 @@ fn late_report_deprecation(
     // Calculating message for lint involves calling `self.def_path_str`,
     // which will by default invoke the expensive `visible_parent_map` query.
     // Skip all that work if the lint is allowed anyway.
-    if tcx.lint_level_spec_at_node(lint, hir_id).level == Level::Allow {
+    if tcx.lint_level_spec_at_node(lint, hir_id).is_allow() {
         return;
     }
 

compiler/rustc_mir_build/src/check_unsafety.rs

@@ -12,7 +12,6 @@ use rustc_middle::thir::visit::Visitor;
 use rustc_middle::thir::*;
 use rustc_middle::ty::print::with_no_trimmed_paths;
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_session::lint::Level;
 use rustc_session::lint::builtin::{DEPRECATED_SAFE_2024, UNSAFE_OP_IN_UNSAFE_FN, UNUSED_UNSAFE};
 use rustc_span::def_id::{DefId, LocalDefId};
 use rustc_span::{Span, Symbol};
@@ -175,8 +174,7 @@ impl<'tcx> UnsafetyVisitor<'_, 'tcx> {
 
     /// Whether the `unsafe_op_in_unsafe_fn` lint is `allow`ed at the current HIR node.
     fn unsafe_op_in_unsafe_fn_allowed(&self) -> bool {
-        self.tcx.lint_level_spec_at_node(UNSAFE_OP_IN_UNSAFE_FN, self.hir_context).level
-            == Level::Allow
+        self.tcx.lint_level_spec_at_node(UNSAFE_OP_IN_UNSAFE_FN, self.hir_context).is_allow()
     }
 
     /// Handle closures/coroutines/inline-consts, which is unsafecked with their parent body.
@@ -234,10 +232,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
                 });
             }
             BlockSafety::ExplicitUnsafe(hir_id) => {
-                let used = matches!(
-                    self.tcx.lint_level_spec_at_node(UNUSED_UNSAFE, hir_id).level,
-                    Level::Allow
-                );
+                let used = self.tcx.lint_level_spec_at_node(UNUSED_UNSAFE, hir_id).is_allow();
                 self.in_safety_context(
                     SafetyContext::UnsafeBlock {
                         span: block.span,