From a63d4761053c2f807ac9f10e79610b288eb23c5e Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 26 Mar 2026 14:09:18 +1100 Subject: [PATCH 1/3] Move `build_dep_graph` into `rustc_incremental::persist::load` Moving this code here makes the subsequent cleanup easier. --- .../rustc_incremental/src/persist/load.rs | 40 +++++++++++++++++-- .../rustc_incremental/src/persist/save.rs | 40 +------------------ 2 files changed, 38 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index c0466a4b5211..6cd57f91d5a0 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -7,8 +7,8 @@ use rustc_hashes::Hash64; use rustc_middle::dep_graph::{DepGraph, SerializedDepGraph, WorkProductMap}; use rustc_middle::query::on_disk_cache::OnDiskCache; -use rustc_serialize::Decodable; -use rustc_serialize::opaque::MemDecoder; +use rustc_serialize::opaque::{FileEncoder, MemDecoder}; +use rustc_serialize::{Decodable, Encodable}; use rustc_session::config::IncrementalStateAssertion; use rustc_session::{Session, StableCrateId}; use rustc_span::Symbol; @@ -16,7 +16,6 @@ use super::data::*; use super::fs::*; -use super::save::build_dep_graph; use super::{file_format, work_product}; use crate::errors; use crate::persist::file_format::{OpenFile, OpenFileError}; @@ -233,3 +232,38 @@ pub fn setup_dep_graph( }) .unwrap_or_else(DepGraph::new_disabled) } + +/// Builds the dependency graph. +/// +/// This function creates the *staging dep-graph*. When the dep-graph is modified by a query +/// execution, the new dependency information is not kept in memory but directly +/// output to this file. `save_dep_graph` then finalizes the staging dep-graph +/// and moves it to the permanent dep-graph path +pub(crate) fn build_dep_graph( + sess: &Session, + prev_graph: Arc, + prev_work_products: WorkProductMap, +) -> Option { + if sess.opts.incremental.is_none() { + // No incremental compilation. + return None; + } + + // Stream the dep-graph to an alternate file, to avoid overwriting anything in case of errors. + let path_buf = staging_dep_graph_path(sess); + + let mut encoder = match FileEncoder::new(&path_buf) { + Ok(encoder) => encoder, + Err(err) => { + sess.dcx().emit_err(errors::CreateDepGraph { path: &path_buf, err }); + return None; + } + }; + + file_format::write_file_header(&mut encoder, sess); + + // First encode the commandline arguments hash + sess.opts.dep_tracking_hash(false).encode(&mut encoder); + + Some(DepGraph::new(sess, prev_graph, prev_work_products, encoder)) +} diff --git a/compiler/rustc_incremental/src/persist/save.rs b/compiler/rustc_incremental/src/persist/save.rs index dc14a8c5e2ec..ab695dddf06e 100644 --- a/compiler/rustc_incremental/src/persist/save.rs +++ b/compiler/rustc_incremental/src/persist/save.rs @@ -1,11 +1,8 @@ use std::fs; -use std::sync::Arc; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::sync::par_join; -use rustc_middle::dep_graph::{ - DepGraph, SerializedDepGraph, WorkProduct, WorkProductId, WorkProductMap, -}; +use rustc_middle::dep_graph::{DepGraph, WorkProduct, WorkProductId}; use rustc_middle::query::on_disk_cache; use rustc_middle::ty::TyCtxt; use rustc_serialize::Encodable as RustcEncodable; @@ -149,38 +146,3 @@ fn encode_work_product_index( serialized_products.encode(encoder) } - -/// Builds the dependency graph. -/// -/// This function creates the *staging dep-graph*. When the dep-graph is modified by a query -/// execution, the new dependency information is not kept in memory but directly -/// output to this file. `save_dep_graph` then finalizes the staging dep-graph -/// and moves it to the permanent dep-graph path -pub(crate) fn build_dep_graph( - sess: &Session, - prev_graph: Arc, - prev_work_products: WorkProductMap, -) -> Option { - if sess.opts.incremental.is_none() { - // No incremental compilation. - return None; - } - - // Stream the dep-graph to an alternate file, to avoid overwriting anything in case of errors. - let path_buf = staging_dep_graph_path(sess); - - let mut encoder = match FileEncoder::new(&path_buf) { - Ok(encoder) => encoder, - Err(err) => { - sess.dcx().emit_err(errors::CreateDepGraph { path: &path_buf, err }); - return None; - } - }; - - file_format::write_file_header(&mut encoder, sess); - - // First encode the commandline arguments hash - sess.opts.dep_tracking_hash(false).encode(&mut encoder); - - Some(DepGraph::new(sess, prev_graph, prev_work_products, encoder)) -} From 6e6701e2bd3b2116c9996c7cb1be7381f030ba0d Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 26 Mar 2026 14:12:56 +1100 Subject: [PATCH 2/3] Make `setup_dep_graph` incremental-only and more straightforward The existing code contains some very strange control flow that can put the dep graph into an inconsistent and untested state if streaming output setup fails. (Specifically, that failure would put `DepGraph` into an "empty" state intended for non-incremental compilation, but other parts of the compiler would still think that incremental mode is enabled due to `sess.opts.incremental`.) This commit therefore performs a big overhaul of `setup_dep_graph` by: - Returning immediately in non-incremental mode. - Exiting immediately if dep-graph streaming output couldn't be set up. - Inlining some "helper" functions that were more confusing than helpful. --- compiler/rustc_incremental/src/persist/fs.rs | 4 +- .../rustc_incremental/src/persist/load.rs | 116 +++++++----------- 2 files changed, 43 insertions(+), 77 deletions(-) diff --git a/compiler/rustc_incremental/src/persist/fs.rs b/compiler/rustc_incremental/src/persist/fs.rs index cf80a7ac2c46..9e22935bb646 100644 --- a/compiler/rustc_incremental/src/persist/fs.rs +++ b/compiler/rustc_incremental/src/persist/fs.rs @@ -215,9 +215,7 @@ pub(crate) fn prepare_session_directory( crate_name: Symbol, stable_crate_id: StableCrateId, ) { - if sess.opts.incremental.is_none() { - return; - } + assert!(sess.opts.incremental.is_some()); let _timer = sess.timer("incr_comp_prepare_session_directory"); diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index 6cd57f91d5a0..03dcf30de7c5 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -34,41 +34,13 @@ pub enum LoadResult { LoadDepGraph(PathBuf, std::io::Error), } -impl LoadResult { - /// Accesses the data returned in [`LoadResult::Ok`]. - pub fn open(self, sess: &Session) -> T { - // Emit a fatal error if `-Zassert-incr-state` is present and unsatisfied. - maybe_assert_incr_state(sess, &self); - - match self { - LoadResult::LoadDepGraph(path, err) => { - sess.dcx().emit_warn(errors::LoadDepGraph { path, err }); - Default::default() - } - LoadResult::DataOutOfDate => { - if let Err(err) = delete_all_session_dir_contents(sess) { - sess.dcx() - .emit_err(errors::DeleteIncompatible { path: dep_graph_path(sess), err }); - } - Default::default() - } - LoadResult::Ok { data } => data, - } - } -} - fn delete_dirty_work_product(sess: &Session, swp: SerializedWorkProduct) { debug!("delete_dirty_work_product({:?})", swp); work_product::delete_workproduct_files(sess, &swp.work_product); } fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkProductMap)> { - let prof = sess.prof.clone(); - - if sess.opts.incremental.is_none() { - // No incremental compilation. - return LoadResult::Ok { data: Default::default() }; - } + assert!(sess.opts.incremental.is_some()); let _timer = sess.prof.generic_activity("incr_comp_prepare_load_dep_graph"); @@ -116,7 +88,7 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkPr } } - let _prof_timer = prof.generic_activity("incr_comp_load_dep_graph"); + let _prof_timer = sess.prof.generic_activity("incr_comp_load_dep_graph"); match file_format::open_incremental_file(sess, &path) { Err(OpenFileError::NotFoundOrHeaderMismatch) => LoadResult::DataOutOfDate, @@ -202,68 +174,64 @@ fn maybe_assert_incr_state(sess: &Session, load_result: &LoadResult) } } -/// Setups the dependency graph by loading an existing graph from disk and set up streaming of a -/// new graph to an incremental session directory. +/// Loads the previous session's dependency graph from disk if possible, and +/// sets up streaming output for the current session's dep graph data into an +/// incremental session directory. +/// +/// In non-incremental mode, a dummy dep graph is returned immediately. pub fn setup_dep_graph( sess: &Session, crate_name: Symbol, stable_crate_id: StableCrateId, ) -> DepGraph { + if sess.opts.incremental.is_none() { + return DepGraph::new_disabled(); + } + // `load_dep_graph` can only be called after `prepare_session_directory`. prepare_session_directory(sess, crate_name, stable_crate_id); + // Try to load the previous session's dep graph and work products. + let load_result = load_dep_graph(sess); - let res = sess.opts.build_dep_graph().then(|| load_dep_graph(sess)); + sess.time("incr_comp_garbage_collect_session_directories", || { + if let Err(e) = garbage_collect_session_directories(sess) { + warn!( + "Error while trying to garbage collect incremental compilation \ + cache directory: {e}", + ); + } + }); - if sess.opts.incremental.is_some() { - sess.time("incr_comp_garbage_collect_session_directories", || { - if let Err(e) = garbage_collect_session_directories(sess) { - warn!( - "Error while trying to garbage collect incremental \ - compilation cache directory: {}", - e - ); + // Emit a fatal error if `-Zassert-incr-state` is present and unsatisfied. + maybe_assert_incr_state(sess, &load_result); + + let (prev_graph, prev_work_products) = match load_result { + LoadResult::LoadDepGraph(path, err) => { + sess.dcx().emit_warn(errors::LoadDepGraph { path, err }); + Default::default() + } + LoadResult::DataOutOfDate => { + if let Err(err) = delete_all_session_dir_contents(sess) { + sess.dcx().emit_err(errors::DeleteIncompatible { path: dep_graph_path(sess), err }); } - }); - } - - res.and_then(|result| { - let (prev_graph, prev_work_products) = result.open(sess); - build_dep_graph(sess, prev_graph, prev_work_products) - }) - .unwrap_or_else(DepGraph::new_disabled) -} - -/// Builds the dependency graph. -/// -/// This function creates the *staging dep-graph*. When the dep-graph is modified by a query -/// execution, the new dependency information is not kept in memory but directly -/// output to this file. `save_dep_graph` then finalizes the staging dep-graph -/// and moves it to the permanent dep-graph path -pub(crate) fn build_dep_graph( - sess: &Session, - prev_graph: Arc, - prev_work_products: WorkProductMap, -) -> Option { - if sess.opts.incremental.is_none() { - // No incremental compilation. - return None; - } + Default::default() + } + LoadResult::Ok { data } => data, + }; // Stream the dep-graph to an alternate file, to avoid overwriting anything in case of errors. let path_buf = staging_dep_graph_path(sess); - let mut encoder = match FileEncoder::new(&path_buf) { - Ok(encoder) => encoder, - Err(err) => { - sess.dcx().emit_err(errors::CreateDepGraph { path: &path_buf, err }); - return None; - } - }; + let mut encoder = FileEncoder::new(&path_buf).unwrap_or_else(|err| { + // We're in incremental mode but couldn't set up streaming output of the dep graph. + // Exit immediately instead of continuing in an inconsistent and untested state. + sess.dcx().emit_fatal(errors::CreateDepGraph { path: &path_buf, err }) + }); file_format::write_file_header(&mut encoder, sess); // First encode the commandline arguments hash sess.opts.dep_tracking_hash(false).encode(&mut encoder); - Some(DepGraph::new(sess, prev_graph, prev_work_products, encoder)) + DepGraph::new(sess, prev_graph, prev_work_products, encoder) } From 369c801eafe69b13c1464f00b0cbf829ff99e721 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 26 Mar 2026 14:41:34 +1100 Subject: [PATCH 3/3] Clean up enum `LoadResult` - Make the enum non-public - Replace the generic `` with concrete fields - Rename variant `LoadDepGraph` to `IoError` --- compiler/rustc_incremental/src/lib.rs | 5 ++-- .../rustc_incremental/src/persist/load.rs | 28 +++++++++---------- compiler/rustc_incremental/src/persist/mod.rs | 2 +- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_incremental/src/lib.rs b/compiler/rustc_incremental/src/lib.rs index 591ade379e6a..ebaed7bd1634 100644 --- a/compiler/rustc_incremental/src/lib.rs +++ b/compiler/rustc_incremental/src/lib.rs @@ -10,9 +10,8 @@ mod persist; pub use persist::{ - LoadResult, copy_cgu_workproduct_to_incr_comp_cache_dir, finalize_session_directory, - in_incr_comp_dir, in_incr_comp_dir_sess, load_query_result_cache, save_work_product_index, - setup_dep_graph, + copy_cgu_workproduct_to_incr_comp_cache_dir, finalize_session_directory, in_incr_comp_dir, + in_incr_comp_dir_sess, load_query_result_cache, save_work_product_index, setup_dep_graph, }; use rustc_middle::util::Providers; diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index 03dcf30de7c5..0e2cda68c6cc 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -1,5 +1,6 @@ //! Code to load the dep-graph from files. +use std::io; use std::path::PathBuf; use std::sync::Arc; @@ -22,16 +23,13 @@ #[derive(Debug)] /// Represents the result of an attempt to load incremental compilation data. -pub enum LoadResult { +enum LoadResult { /// Loading was successful. - Ok { - #[allow(missing_docs)] - data: T, - }, + Ok { prev_graph: Arc, prev_work_products: WorkProductMap }, /// The file either didn't exist or was produced by an incompatible compiler version. DataOutOfDate, - /// Loading the dep graph failed. - LoadDepGraph(PathBuf, std::io::Error), + /// Loading failed due to an unexpected I/O error. + IoError { path: PathBuf, err: io::Error }, } fn delete_dirty_work_product(sess: &Session, swp: SerializedWorkProduct) { @@ -39,7 +37,7 @@ fn delete_dirty_work_product(sess: &Session, swp: SerializedWorkProduct) { work_product::delete_workproduct_files(sess, &swp.work_product); } -fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkProductMap)> { +fn load_dep_graph(sess: &Session) -> LoadResult { assert!(sess.opts.incremental.is_some()); let _timer = sess.prof.generic_activity("incr_comp_prepare_load_dep_graph"); @@ -92,7 +90,7 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkPr match file_format::open_incremental_file(sess, &path) { Err(OpenFileError::NotFoundOrHeaderMismatch) => LoadResult::DataOutOfDate, - Err(OpenFileError::IoError { err }) => LoadResult::LoadDepGraph(path.to_owned(), err), + Err(OpenFileError::IoError { err }) => LoadResult::IoError { path: path.to_owned(), err }, Ok(OpenFile { mmap, start_pos }) => { let Ok(mut decoder) = MemDecoder::new(&mmap, start_pos) else { sess.dcx().emit_warn(errors::CorruptFile { path: &path }); @@ -114,9 +112,9 @@ fn load_dep_graph(sess: &Session) -> LoadResult<(Arc, WorkPr return LoadResult::DataOutOfDate; } - let dep_graph = SerializedDepGraph::decode(&mut decoder); + let prev_graph = SerializedDepGraph::decode(&mut decoder); - LoadResult::Ok { data: (dep_graph, prev_work_products) } + LoadResult::Ok { prev_graph, prev_work_products } } } } @@ -150,14 +148,14 @@ pub fn load_query_result_cache(sess: &Session) -> Option { /// 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) { +fn maybe_assert_incr_state(sess: &Session, load_result: &LoadResult) { // 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, + LoadResult::DataOutOfDate | LoadResult::IoError { .. } => false, }; match assertion { @@ -206,7 +204,7 @@ pub fn setup_dep_graph( maybe_assert_incr_state(sess, &load_result); let (prev_graph, prev_work_products) = match load_result { - LoadResult::LoadDepGraph(path, err) => { + LoadResult::IoError { path, err } => { sess.dcx().emit_warn(errors::LoadDepGraph { path, err }); Default::default() } @@ -216,7 +214,7 @@ pub fn setup_dep_graph( } Default::default() } - LoadResult::Ok { data } => data, + LoadResult::Ok { prev_graph, prev_work_products } => (prev_graph, prev_work_products), }; // Stream the dep-graph to an alternate file, to avoid overwriting anything in case of errors. diff --git a/compiler/rustc_incremental/src/persist/mod.rs b/compiler/rustc_incremental/src/persist/mod.rs index a3857967ab08..c25c5f1ea47f 100644 --- a/compiler/rustc_incremental/src/persist/mod.rs +++ b/compiler/rustc_incremental/src/persist/mod.rs @@ -11,7 +11,7 @@ mod work_product; pub use fs::{finalize_session_directory, in_incr_comp_dir, in_incr_comp_dir_sess}; -pub use load::{LoadResult, load_query_result_cache, setup_dep_graph}; +pub use load::{load_query_result_cache, setup_dep_graph}; pub(crate) use save::save_dep_graph; pub use save::save_work_product_index; pub use work_product::copy_cgu_workproduct_to_incr_comp_cache_dir;