Rollup merge of #154450 - Zalathar:assert-incr-state, r=fmease

Use the normal arg-parsing machinery for `-Zassert-incr-state`

The flag parser for `-Zassert-incr-state` currently extracts an `Option<String>`, and then performs an ad-hoc parsing step slightly later. From looking at https://github.com/rust-lang/rust/pull/90386, I can't see any reason why it doesn't just use the normal flag-parsing machinery.

A second commit also extracts the underlying implementation to a separate helper function, so that it isn't cluttering up the main control flow.

I found this while working on larger cleanups to incremental-file loading.

compiler/rustc_incremental/src/persist/load.rs

@@ -38,19 +38,8 @@ pub enum LoadResult<T> {
 impl<T: Default> LoadResult<T> {
     /// Accesses the data returned in [`LoadResult::Ok`].
     pub fn open(self, sess: &Session) -> T {
-        // Check for errors when using `-Zassert-incremental-state`
-        match (sess.opts.assert_incr_state, &self) {
-            (Some(IncrementalStateAssertion::NotLoaded), LoadResult::Ok { .. }) => {
-                sess.dcx().emit_fatal(errors::AssertNotLoaded);
-            }
-            (
-                Some(IncrementalStateAssertion::Loaded),
-                LoadResult::LoadDepGraph(..) | LoadResult::DataOutOfDate,
-            ) => {
-                sess.dcx().emit_fatal(errors::AssertLoaded);
-            }
-            _ => {}
-        };
+        // Emit a fatal error if `-Zassert-incr-state` is present and unsatisfied.
+        maybe_assert_incr_state(sess, &self);
 
         match self {
             LoadResult::LoadDepGraph(path, err) => {
@@ -188,6 +177,32 @@ pub fn load_query_result_cache(sess: &Session) -> Option<OnDiskCache> {
     }
 }
 
+/// Emits a fatal error if the assertion in `-Zassert-incr-state` doesn't match
+/// the outcome of trying to load previous-session state.
+fn maybe_assert_incr_state(sess: &Session, load_result: &LoadResult<impl Sized>) {
+    // Return immediately if there's nothing to assert.
+    let Some(assertion) = sess.opts.unstable_opts.assert_incr_state else { return };
+
+    // Match exhaustively to make sure we don't miss any cases.
+    let loaded = match load_result {
+        LoadResult::Ok { .. } => true,
+        LoadResult::DataOutOfDate | LoadResult::LoadDepGraph(..) => false,
+    };
+
+    match assertion {
+        IncrementalStateAssertion::Loaded => {
+            if !loaded {
+                sess.dcx().emit_fatal(errors::AssertLoaded);
+            }
+        }
+        IncrementalStateAssertion::NotLoaded => {
+            if loaded {
+                sess.dcx().emit_fatal(errors::AssertNotLoaded)
+            }
+        }
+    }
+}
+
 /// Setups the dependency graph by loading an existing graph from disk and set up streaming of a
 /// new graph to an incremental session directory.
 pub fn setup_dep_graph(

compiler/rustc_interface/src/tests.rs

@@ -12,12 +12,12 @@
 use rustc_session::config::{
     AnnotateMoves, AutoDiff, BranchProtection, CFGuard, Cfg, CoverageLevel, CoverageOptions,
     DebugInfo, DumpMonoStatsFormat, ErrorOutputType, ExternEntry, ExternLocation, Externs,
-    FmtDebug, FunctionReturn, InliningThreshold, Input, InstrumentCoverage, InstrumentXRay,
-    LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli, MirIncludeSpans, NextSolverConfig,
-    Offload, Options, OutFileName, OutputType, OutputTypes, PAuthKey, PacRet, Passes,
-    PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip, SwitchWithOptPath,
-    SymbolManglingVersion, WasiExecModel, build_configuration, build_session_options,
-    rustc_optgroups,
+    FmtDebug, FunctionReturn, IncrementalStateAssertion, InliningThreshold, Input,
+    InstrumentCoverage, InstrumentXRay, LinkSelfContained, LinkerPluginLto, LocationDetail, LtoCli,
+    MirIncludeSpans, NextSolverConfig, Offload, Options, OutFileName, OutputType, OutputTypes,
+    PAuthKey, PacRet, Passes, PatchableFunctionEntry, Polonius, ProcMacroExecutionStrategy, Strip,
+    SwitchWithOptPath, SymbolManglingVersion, WasiExecModel, build_configuration,
+    build_session_options, rustc_optgroups,
 };
 use rustc_session::lint::Level;
 use rustc_session::search_paths::SearchPath;
@@ -686,7 +686,7 @@ macro_rules! untracked {
 
     // Make sure that changing an [UNTRACKED] option leaves the hash unchanged.
     // tidy-alphabetical-start
-    untracked!(assert_incr_state, Some(String::from("loaded")));
+    untracked!(assert_incr_state, Some(IncrementalStateAssertion::Loaded));
     untracked!(codegen_source_order, true);
     untracked!(deduplicate_diagnostics, false);
     untracked!(dump_dep_graph, true);

compiler/rustc_session/src/config.rs

@@ -1405,7 +1405,6 @@ fn default() -> Options {
         };
 
         Options {
-            assert_incr_state: None,
             crate_types: Vec::new(),
             optimize: OptLevel::No,
             debuginfo: DebugInfo::None,
@@ -2287,20 +2286,6 @@ fn select_debuginfo(matches: &getopts::Matches, cg: &CodegenOptions) -> DebugInf
     if max_g > max_c { DebugInfo::Full } else { cg.debuginfo }
 }
 
-fn parse_assert_incr_state(
-    early_dcx: &EarlyDiagCtxt,
-    opt_assertion: &Option<String>,
-) -> Option<IncrementalStateAssertion> {
-    match opt_assertion {
-        Some(s) if s.as_str() == "loaded" => Some(IncrementalStateAssertion::Loaded),
-        Some(s) if s.as_str() == "not-loaded" => Some(IncrementalStateAssertion::NotLoaded),
-        Some(s) => {
-            early_dcx.early_fatal(format!("unexpected incremental state assertion value: {s}"))
-        }
-        None => None,
-    }
-}
-
 pub fn parse_externs(
     early_dcx: &EarlyDiagCtxt,
     matches: &getopts::Matches,
@@ -2506,8 +2491,6 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
 
     let incremental = cg.incremental.as_ref().map(PathBuf::from);
 
-    let assert_incr_state = parse_assert_incr_state(early_dcx, &unstable_opts.assert_incr_state);
-
     if cg.profile_generate.enabled() && cg.profile_use.is_some() {
         early_dcx.early_fatal("options `-C profile-generate` and `-C profile-use` are exclusive");
     }
@@ -2759,7 +2742,6 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
     let verbose = matches.opt_present("verbose") || unstable_opts.verbose_internals;
 
     Options {
-        assert_incr_state,
         crate_types,
         optimize: opt_level,
         debuginfo,

compiler/rustc_session/src/options.rs

@@ -419,7 +419,6 @@ pub struct Options {
         /// If `Some`, enable incremental compilation, using the given
         /// directory to store intermediate results.
         incremental: Option<PathBuf> [UNTRACKED],
-        assert_incr_state: Option<IncrementalStateAssertion> [UNTRACKED],
         /// Set based on the result of the `Config::track_state` callback
         /// for custom drivers to invalidate the incremental cache.
         #[rustc_lint_opt_deny_field_access("should only be used via `Config::track_state`")]
@@ -889,6 +888,7 @@ mod desc {
     pub(crate) const parse_mir_include_spans: &str =
         "either a boolean (`yes`, `no`, `on`, `off`, etc), or `nll` (default: `nll`)";
     pub(crate) const parse_align: &str = "a number that is a power of 2 between 1 and 2^29";
+    pub(crate) const parse_assert_incr_state: &str = "one of: `loaded`, `not-loaded`";
 }
 
 pub mod parse {
@@ -2061,6 +2061,18 @@ pub(crate) fn parse_align(slot: &mut Option<Align>, v: Option<&str>) -> bool {
 
         true
     }
+
+    pub(crate) fn parse_assert_incr_state(
+        slot: &mut Option<IncrementalStateAssertion>,
+        v: Option<&str>,
+    ) -> bool {
+        *slot = match v {
+            Some("loaded") => Some(IncrementalStateAssertion::Loaded),
+            Some("not-loaded") => Some(IncrementalStateAssertion::NotLoaded),
+            _ => return false,
+        };
+        true
+    }
 }
 
 options! {
@@ -2230,7 +2242,7 @@ pub(crate) fn parse_align(slot: &mut Option<Align>, v: Option<&str>) -> bool {
     annotate_moves: AnnotateMoves = (AnnotateMoves::Disabled, parse_annotate_moves, [TRACKED],
         "emit debug info for compiler-generated move and copy operations \
         to make them visible in profilers. Can be a boolean or a size limit in bytes (default: disabled)"),
-    assert_incr_state: Option<String> = (None, parse_opt_string, [UNTRACKED],
+    assert_incr_state: Option<IncrementalStateAssertion> = (None, parse_assert_incr_state, [UNTRACKED],
         "assert that the incremental cache is in given state: \
          either `loaded` or `not-loaded`."),
     assume_incomplete_release: bool = (false, parse_bool, [TRACKED],