From 272015190d058b7c802331e870b23857eeba22cd Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 15 Apr 2021 19:25:01 -0400 Subject: [PATCH] Add [TRACKED_NO_CRATE_HASH] and [SUBSTRUCT] directives This is necessary for options that should invalidate the incremental hash but *not* affect the crate hash (e.g. --remap-path-prefix). This doesn't add `for_crate_hash` to the trait directly because it's not relevant for *types*, only for *options*, which are fields on a larger struct. Instead, it adds a new `SUBSTRUCT` directive for options, which does take a `for_crate_hash` parameter. - Use TRACKED_NO_CRATE_HASH for --remap-path-prefix - Add test that `remap_path_prefix` is tracked - Reduce duplication in the test suite to avoid future churn --- .../rustc_incremental/src/persist/load.rs | 2 +- .../rustc_incremental/src/persist/save.rs | 2 +- compiler/rustc_interface/src/tests.rs | 131 +++++++++--------- compiler/rustc_middle/src/hir/map/mod.rs | 2 +- compiler/rustc_session/src/config.rs | 1 + compiler/rustc_session/src/options.rs | 85 ++++++++---- 6 files changed, 133 insertions(+), 90 deletions(-) diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index 259e540c612..2661afd7ffc 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -104,7 +104,7 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture { // Fortunately, we just checked that this isn't the case. let path = dep_graph_path_from(&sess.incr_comp_session_dir()); let report_incremental_info = sess.opts.debugging_opts.incremental_info; - let expected_hash = sess.opts.dep_tracking_hash(); + let expected_hash = sess.opts.dep_tracking_hash(false); let mut prev_work_products = FxHashMap::default(); let nightly_build = sess.is_nightly_build(); diff --git a/compiler/rustc_incremental/src/persist/save.rs b/compiler/rustc_incremental/src/persist/save.rs index d558af3c1d5..1484088837a 100644 --- a/compiler/rustc_incremental/src/persist/save.rs +++ b/compiler/rustc_incremental/src/persist/save.rs @@ -219,7 +219,7 @@ pub fn build_dep_graph( } // First encode the commandline arguments hash - if let Err(err) = sess.opts.dep_tracking_hash().encode(&mut encoder) { + if let Err(err) = sess.opts.dep_tracking_hash(false).encode(&mut encoder) { sess.err(&format!( "failed to write dependency graph hash `{}`: {}", path_buf.display(), diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 5800fa173dd..62c2d3c722f 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -19,6 +19,7 @@ use rustc_span::symbol::sym; use rustc_span::SourceFileHashAlgorithm; use rustc_target::spec::{CodeModel, LinkerFlavor, MergeFunctions, PanicStrategy}; use rustc_target::spec::{RelocModel, RelroLevel, SanitizerSet, SplitDebuginfo, TlsModel}; + use std::collections::{BTreeMap, BTreeSet}; use std::iter::FromIterator; use std::num::NonZeroUsize; @@ -74,6 +75,27 @@ fn mk_map(entries: Vec<(K, V)>) -> BTreeMap { BTreeMap::from_iter(entries.into_iter()) } +fn assert_same_clone(x: &Options) { + assert_eq!(x.dep_tracking_hash(true), x.clone().dep_tracking_hash(true)); + assert_eq!(x.dep_tracking_hash(false), x.clone().dep_tracking_hash(false)); +} + +fn assert_same_hash(x: &Options, y: &Options) { + assert_eq!(x.dep_tracking_hash(true), y.dep_tracking_hash(true)); + assert_eq!(x.dep_tracking_hash(false), y.dep_tracking_hash(false)); + // Check clone + assert_same_clone(x); + assert_same_clone(y); +} + +fn assert_different_hash(x: &Options, y: &Options) { + assert_ne!(x.dep_tracking_hash(true), y.dep_tracking_hash(true)); + assert_ne!(x.dep_tracking_hash(false), y.dep_tracking_hash(false)); + // Check clone + assert_same_clone(x); + assert_same_clone(y); +} + // When the user supplies --test we should implicitly supply --cfg test #[test] fn test_switch_implies_cfg_test() { @@ -130,14 +152,9 @@ fn test_output_types_tracking_hash_different_paths() { v2.output_types = OutputTypes::new(&[(OutputType::Exe, Some(PathBuf::from("/some/thing")))]); v3.output_types = OutputTypes::new(&[(OutputType::Exe, None)]); - assert!(v1.dep_tracking_hash() != v2.dep_tracking_hash()); - assert!(v1.dep_tracking_hash() != v3.dep_tracking_hash()); - assert!(v2.dep_tracking_hash() != v3.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v2.clone().dep_tracking_hash()); - assert_eq!(v3.dep_tracking_hash(), v3.clone().dep_tracking_hash()); + assert_different_hash(&v1, &v2); + assert_different_hash(&v1, &v3); + assert_different_hash(&v2, &v3); } #[test] @@ -155,10 +172,7 @@ fn test_output_types_tracking_hash_different_construction_order() { (OutputType::Exe, Some(PathBuf::from("./some/thing"))), ]); - assert_eq!(v1.dep_tracking_hash(), v2.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); + assert_same_hash(&v1, &v2); } #[test] @@ -182,14 +196,9 @@ fn test_externs_tracking_hash_different_construction_order() { (String::from("d"), new_public_extern_entry(vec!["f", "e"])), ])); - assert_eq!(v1.dep_tracking_hash(), v2.dep_tracking_hash()); - assert_eq!(v1.dep_tracking_hash(), v3.dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v3.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v2.clone().dep_tracking_hash()); - assert_eq!(v3.dep_tracking_hash(), v3.clone().dep_tracking_hash()); + assert_same_hash(&v1, &v2); + assert_same_hash(&v1, &v3); + assert_same_hash(&v2, &v3); } #[test] @@ -219,14 +228,9 @@ fn test_lints_tracking_hash_different_values() { (String::from("d"), Level::Deny), ]; - assert!(v1.dep_tracking_hash() != v2.dep_tracking_hash()); - assert!(v1.dep_tracking_hash() != v3.dep_tracking_hash()); - assert!(v2.dep_tracking_hash() != v3.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v2.clone().dep_tracking_hash()); - assert_eq!(v3.dep_tracking_hash(), v3.clone().dep_tracking_hash()); + assert_different_hash(&v1, &v2); + assert_different_hash(&v1, &v3); + assert_different_hash(&v2, &v3); } #[test] @@ -248,11 +252,7 @@ fn test_lints_tracking_hash_different_construction_order() { (String::from("d"), Level::Forbid), ]; - assert_eq!(v1.dep_tracking_hash(), v2.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v2.clone().dep_tracking_hash()); + assert_same_hash(&v1, &v2); } #[test] @@ -292,15 +292,9 @@ fn test_search_paths_tracking_hash_different_order() { v4.search_paths.push(SearchPath::from_cli_opt("dependency=ghi", JSON)); v4.search_paths.push(SearchPath::from_cli_opt("framework=jkl", JSON)); - assert!(v1.dep_tracking_hash() == v2.dep_tracking_hash()); - assert!(v1.dep_tracking_hash() == v3.dep_tracking_hash()); - assert!(v1.dep_tracking_hash() == v4.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v2.clone().dep_tracking_hash()); - assert_eq!(v3.dep_tracking_hash(), v3.clone().dep_tracking_hash()); - assert_eq!(v4.dep_tracking_hash(), v4.clone().dep_tracking_hash()); + assert_same_hash(&v1, &v2); + assert_same_hash(&v1, &v3); + assert_same_hash(&v1, &v4); } #[test] @@ -338,15 +332,9 @@ fn test_native_libs_tracking_hash_different_values() { (String::from("c"), None, NativeLibKind::Unspecified), ]; - assert!(v1.dep_tracking_hash() != v2.dep_tracking_hash()); - assert!(v1.dep_tracking_hash() != v3.dep_tracking_hash()); - assert!(v1.dep_tracking_hash() != v4.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v2.clone().dep_tracking_hash()); - assert_eq!(v3.dep_tracking_hash(), v3.clone().dep_tracking_hash()); - assert_eq!(v4.dep_tracking_hash(), v4.clone().dep_tracking_hash()); + assert_different_hash(&v1, &v2); + assert_different_hash(&v1, &v3); + assert_different_hash(&v1, &v4); } #[test] @@ -374,14 +362,9 @@ fn test_native_libs_tracking_hash_different_order() { (String::from("b"), None, NativeLibKind::Framework), ]; - assert!(v1.dep_tracking_hash() == v2.dep_tracking_hash()); - assert!(v1.dep_tracking_hash() == v3.dep_tracking_hash()); - assert!(v2.dep_tracking_hash() == v3.dep_tracking_hash()); - - // Check clone - assert_eq!(v1.dep_tracking_hash(), v1.clone().dep_tracking_hash()); - assert_eq!(v2.dep_tracking_hash(), v2.clone().dep_tracking_hash()); - assert_eq!(v3.dep_tracking_hash(), v3.clone().dep_tracking_hash()); + assert_same_hash(&v1, &v2); + assert_same_hash(&v1, &v3); + assert_same_hash(&v2, &v3); } #[test] @@ -393,7 +376,7 @@ fn test_codegen_options_tracking_hash() { ($name: ident, $non_default_value: expr) => { assert_ne!(opts.cg.$name, $non_default_value); opts.cg.$name = $non_default_value; - assert_eq!(reference.dep_tracking_hash(), opts.dep_tracking_hash()); + assert_same_hash(&reference, &opts); }; } @@ -419,7 +402,7 @@ fn test_codegen_options_tracking_hash() { opts = reference.clone(); assert_ne!(opts.cg.$name, $non_default_value); opts.cg.$name = $non_default_value; - assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash()); + assert_different_hash(&reference, &opts); }; } @@ -456,6 +439,28 @@ fn test_codegen_options_tracking_hash() { tracked!(target_feature, String::from("all the features, all of them")); } +#[test] +fn test_top_level_options_tracked_no_crate() { + let reference = Options::default(); + let mut opts; + + macro_rules! tracked { + ($name: ident, $non_default_value: expr) => { + opts = reference.clone(); + assert_ne!(opts.$name, $non_default_value); + opts.$name = $non_default_value; + // The crate hash should be the same + assert_eq!(reference.dep_tracking_hash(true), opts.dep_tracking_hash(true)); + // The incremental hash should be different + assert_ne!(reference.dep_tracking_hash(false), opts.dep_tracking_hash(false)); + }; + } + + // Make sure that changing a [TRACKED_NO_CRATE_HASH] option leaves the crate hash unchanged but changes the incremental hash. + // This list is in alphabetical order. + tracked!(remap_path_prefix, vec![("/home/bors/rust".into(), "src".into())]); +} + #[test] fn test_debugging_options_tracking_hash() { let reference = Options::default(); @@ -465,7 +470,7 @@ fn test_debugging_options_tracking_hash() { ($name: ident, $non_default_value: expr) => { assert_ne!(opts.debugging_opts.$name, $non_default_value); opts.debugging_opts.$name = $non_default_value; - assert_eq!(reference.dep_tracking_hash(), opts.dep_tracking_hash()); + assert_same_hash(&reference, &opts); }; } @@ -537,7 +542,7 @@ fn test_debugging_options_tracking_hash() { opts = reference.clone(); assert_ne!(opts.debugging_opts.$name, $non_default_value); opts.debugging_opts.$name = $non_default_value; - assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash()); + assert_different_hash(&reference, &opts); }; } diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index d155276051e..4cd126988f9 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -943,7 +943,7 @@ pub(super) fn index_hir<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> &'tcx Indexe intravisit::walk_crate(&mut collector, tcx.untracked_crate); let crate_disambiguator = tcx.sess.local_crate_disambiguator(); - let cmdline_args = tcx.sess.opts.dep_tracking_hash(); + let cmdline_args = tcx.sess.opts.dep_tracking_hash(true); collector.finalize_and_compute_crate_hash(crate_disambiguator, &*tcx.cstore, cmdline_args) }; diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index a7b9d03844c..5d588ad1be2 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -2374,6 +2374,7 @@ crate mod dep_tracking { impl_dep_tracking_hash_for_sortable_vec_of!(String); impl_dep_tracking_hash_for_sortable_vec_of!(PathBuf); + impl_dep_tracking_hash_for_sortable_vec_of!((PathBuf, PathBuf)); impl_dep_tracking_hash_for_sortable_vec_of!(CrateType); impl_dep_tracking_hash_for_sortable_vec_of!((String, lint::Level)); impl_dep_tracking_hash_for_sortable_vec_of!((String, Option, NativeLibKind)); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 759110f6859..226c7bda052 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -20,21 +20,41 @@ use std::num::NonZeroUsize; use std::path::PathBuf; use std::str; -macro_rules! hash_option { - ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, [UNTRACKED]) => {{}}; - ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, [TRACKED]) => {{ +macro_rules! insert { + ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr) => { if $sub_hashes .insert(stringify!($opt_name), $opt_expr as &dyn dep_tracking::DepTrackingHash) .is_some() { panic!("duplicate key in CLI DepTrackingHash: {}", stringify!($opt_name)) } + }; +} + +macro_rules! hash_opt { + ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, $_for_crate_hash: ident, [UNTRACKED]) => {{}}; + ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, $_for_crate_hash: ident, [TRACKED]) => {{ insert!($opt_name, $opt_expr, $sub_hashes) }}; + ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, $for_crate_hash: ident, [TRACKED_NO_CRATE_HASH]) => {{ + if !$for_crate_hash { + insert!($opt_name, $opt_expr, $sub_hashes) + } }}; + ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, $_for_crate_hash: ident, [SUBSTRUCT]) => {{}}; +} + +macro_rules! hash_substruct { + ($opt_name:ident, $opt_expr:expr, $error_format:expr, $for_crate_hash:expr, $hasher:expr, [UNTRACKED]) => {{}}; + ($opt_name:ident, $opt_expr:expr, $error_format:expr, $for_crate_hash:expr, $hasher:expr, [TRACKED]) => {{}}; + ($opt_name:ident, $opt_expr:expr, $error_format:expr, $for_crate_hash:expr, $hasher:expr, [TRACKED_NO_CRATE_HASH]) => {{}}; + ($opt_name:ident, $opt_expr:expr, $error_format:expr, $for_crate_hash:expr, $hasher:expr, [SUBSTRUCT]) => { + use crate::config::dep_tracking::DepTrackingHash; + $opt_expr.dep_tracking_hash($for_crate_hash, $error_format).hash($hasher, $error_format); + }; } macro_rules! top_level_options { (pub struct Options { $( - $opt:ident : $t:ty [$dep_tracking_marker:ident $($warn_val:expr, $warn_text:expr)*], + $opt:ident : $t:ty [$dep_tracking_marker:ident], )* } ) => ( #[derive(Clone)] pub struct Options { @@ -42,20 +62,27 @@ macro_rules! top_level_options { } impl Options { - pub fn dep_tracking_hash(&self) -> u64 { + pub fn dep_tracking_hash(&self, for_crate_hash: bool) -> u64 { let mut sub_hashes = BTreeMap::new(); $({ - hash_option!($opt, - &self.$opt, - &mut sub_hashes, - [$dep_tracking_marker $($warn_val, - $warn_text, - self.error_format)*]); + hash_opt!($opt, + &self.$opt, + &mut sub_hashes, + for_crate_hash, + [$dep_tracking_marker]); })* let mut hasher = DefaultHasher::new(); dep_tracking::stable_hash(sub_hashes, &mut hasher, self.error_format); + $({ + hash_substruct!($opt, + &self.$opt, + self.error_format, + for_crate_hash, + &mut hasher, + [$dep_tracking_marker]); + })* hasher.finish() } } @@ -72,9 +99,16 @@ macro_rules! top_level_options { // A change in the given field will cause the compiler to completely clear the // incremental compilation cache before proceeding. // +// [TRACKED_NO_CRATE_HASH] +// Same as [TRACKED], but will not affect the crate hash. This is useful for options that only +// affect the incremental cache. +// // [UNTRACKED] // Incremental compilation is not influenced by this option. // +// [SUBSTRUCT] +// Second-level sub-structs containing more options. +// // If you add a new option to this struct or one of the sub-structs like // `CodegenOptions`, think about how it influences incremental compilation. If in // doubt, specify [TRACKED], which is always "correct" but might lead to @@ -106,12 +140,12 @@ top_level_options!( // directory to store intermediate results. incremental: Option [UNTRACKED], - debugging_opts: DebuggingOptions [TRACKED], + debugging_opts: DebuggingOptions [SUBSTRUCT], prints: Vec [UNTRACKED], // Determines which borrow checker(s) to run. This is the parsed, sanitized // version of `debugging_opts.borrowck`, which is just a plain string. borrowck_mode: BorrowckMode [UNTRACKED], - cg: CodegenOptions [TRACKED], + cg: CodegenOptions [SUBSTRUCT], externs: Externs [UNTRACKED], extern_dep_specs: ExternDepSpecs [UNTRACKED], crate_name: Option [TRACKED], @@ -139,7 +173,7 @@ top_level_options!( cli_forced_thinlto_off: bool [UNTRACKED], // Remap source path prefixes in all output (messages, object files, debug, etc.). - remap_path_prefix: Vec<(PathBuf, PathBuf)> [UNTRACKED], + remap_path_prefix: Vec<(PathBuf, PathBuf)> [TRACKED_NO_CRATE_HASH], edition: Edition [TRACKED], @@ -169,7 +203,7 @@ macro_rules! options { $($opt:ident : $t:ty = ( $init:expr, $parse:ident, - [$dep_tracking_marker:ident $(($dep_warn_val:expr, $dep_warn_text:expr))*], + [$dep_tracking_marker:ident], $desc:expr) ),* ,) => ( @@ -219,18 +253,21 @@ macro_rules! options { return op; } - impl dep_tracking::DepTrackingHash for $struct_name { - fn hash(&self, hasher: &mut DefaultHasher, error_format: ErrorOutputType) { + impl $struct_name { + fn dep_tracking_hash(&self, for_crate_hash: bool, error_format: ErrorOutputType) -> u64 { let mut sub_hashes = BTreeMap::new(); $({ - hash_option!($opt, - &self.$opt, - &mut sub_hashes, - [$dep_tracking_marker $($dep_warn_val, - $dep_warn_text, - error_format)*]); + hash_opt!($opt, + &self.$opt, + &mut sub_hashes, + for_crate_hash, + [$dep_tracking_marker]); })* - dep_tracking::stable_hash(sub_hashes, hasher, error_format); + let mut hasher = DefaultHasher::new(); + dep_tracking::stable_hash(sub_hashes, + &mut hasher, + error_format); + hasher.finish() } }