From 3090b0151c896dd47dedcbd26f1223a534f55f17 Mon Sep 17 00:00:00 2001 From: Tor Hovland Date: Thu, 22 Apr 2021 22:33:54 +0200 Subject: [PATCH 01/16] Use flex more consistently. --- src/librustdoc/html/static/rustdoc.css | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index 213ca9ec9e3e..cf934efbb15d 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -117,9 +117,12 @@ h4:not(.method):not(.type):not(.tymethod):not(.associatedconstant) { } h1.fqn { display: flex; - width: 100%; border-bottom: 1px dashed; margin-top: 0; + + /* workaround to keep flex from breaking below 700 px width due to the float: right on the nav + above the h1 */ + padding-left: 1px; } h1.fqn > .in-band > a:hover { text-decoration: underline; @@ -453,20 +456,14 @@ nav.sub { } .content .out-of-band { - float: right; + flex-grow: 0; + text-align: right; font-size: 23px; margin: 0px; - padding: 0px; + padding: 0 0 0 12px; font-weight: normal; } -h1.fqn > .out-of-band { - float: unset; - flex: 1; - text-align: right; - margin-left: 8px; -} - h3.impl > .out-of-band { font-size: 21px; } @@ -486,6 +483,7 @@ h4 > code, h3 > code, .invisible > code { } .content .in-band { + flex-grow: 1; margin: 0px; padding: 0px; } @@ -1483,10 +1481,6 @@ h4 > .notable-traits { display: none !important; } - h1.fqn { - overflow: initial; - } - .theme-picker { left: 10px; top: 54px; From fb7018b41e66aa2edfaf2d4fbb8fe260cb411ac2 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 15 Apr 2021 23:06:32 -0400 Subject: [PATCH 02/16] Test that non_default_option is not the default option Otherwise the test is useless and does nothing. This caught 2 bugs in the test suite. --- compiler/rustc_interface/src/tests.rs | 8 ++++++-- compiler/rustc_session/src/config.rs | 6 +++--- compiler/rustc_session/src/options.rs | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 9685d21762b7..5800fa173dd8 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -391,6 +391,7 @@ fn test_codegen_options_tracking_hash() { macro_rules! untracked { ($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()); }; @@ -416,6 +417,7 @@ fn test_codegen_options_tracking_hash() { macro_rules! tracked { ($name: ident, $non_default_value: expr) => { 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()); }; @@ -461,6 +463,7 @@ fn test_debugging_options_tracking_hash() { macro_rules! untracked { ($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()); }; @@ -471,7 +474,7 @@ fn test_debugging_options_tracking_hash() { untracked!(ast_json, true); untracked!(ast_json_noexpand, true); untracked!(borrowck, String::from("other")); - untracked!(deduplicate_diagnostics, true); + untracked!(deduplicate_diagnostics, false); untracked!(dep_tasks, true); untracked!(dont_buffer_diagnostics, true); untracked!(dump_dep_graph, true); @@ -515,7 +518,7 @@ fn test_debugging_options_tracking_hash() { untracked!(self_profile_events, Some(vec![String::new()])); untracked!(span_debug, true); untracked!(span_free_formats, true); - untracked!(strip, Strip::None); + untracked!(strip, Strip::Debuginfo); untracked!(terminal_width, Some(80)); untracked!(threads, 99); untracked!(time, true); @@ -532,6 +535,7 @@ fn test_debugging_options_tracking_hash() { macro_rules! tracked { ($name: ident, $non_default_value: expr) => { 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()); }; diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 52a6e4ff924f..a7b9d03844c2 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -156,7 +156,7 @@ pub enum InstrumentCoverage { Off, } -#[derive(Clone, PartialEq, Hash)] +#[derive(Clone, PartialEq, Hash, Debug)] pub enum LinkerPluginLto { LinkerPlugin(PathBuf), LinkerPluginAuto, @@ -172,7 +172,7 @@ impl LinkerPluginLto { } } -#[derive(Clone, PartialEq, Hash)] +#[derive(Clone, PartialEq, Hash, Debug)] pub enum SwitchWithOptPath { Enabled(Option), Disabled, @@ -778,7 +778,7 @@ pub enum CrateType { impl_stable_hash_via_hash!(CrateType); -#[derive(Clone, Hash)] +#[derive(Clone, Hash, Debug, PartialEq, Eq)] pub enum Passes { Some(Vec), All, diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index fd26f50da5a8..759110f68598 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1220,7 +1220,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, // - compiler/rustc_interface/src/tests.rs } -#[derive(Clone, Hash)] +#[derive(Clone, Hash, PartialEq, Eq, Debug)] pub enum WasiExecModel { Command, Reactor, From 272015190d058b7c802331e870b23857eeba22cd Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 15 Apr 2021 19:25:01 -0400 Subject: [PATCH 03/16] 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 259e540c6125..2661afd7ffc3 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 d558af3c1d55..1484088837a4 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 5800fa173dd8..62c2d3c722f5 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 d155276051e4..4cd126988f91 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 a7b9d03844c2..5d588ad1be2d 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 759110f68598..226c7bda0522 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() } } From 39648ea467a39afa3676d900656874947c747690 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 27 Apr 2021 16:25:12 +0000 Subject: [PATCH 04/16] Make `real_rust_path_dir` a TRACKED_NO_CRATE_HASH option This also adds support for doc-comments to Options. --- compiler/rustc_interface/src/tests.rs | 4 +++ compiler/rustc_metadata/src/rmeta/decoder.rs | 6 ++-- compiler/rustc_session/src/config.rs | 30 ++++++++++++++++++++ compiler/rustc_session/src/options.rs | 18 ++++++++++-- compiler/rustc_session/src/session.rs | 30 -------------------- 5 files changed, 52 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 62c2d3c722f5..d8c1a7a26822 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -459,6 +459,10 @@ fn test_top_level_options_tracked_no_crate() { // 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())]); + tracked!( + real_rust_source_base_dir, + Some("/home/bors/rust/.rustup/toolchains/nightly/lib/rustlib/src/rust".into()) + ); } #[test] diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 19ae5ce69c13..2ade1bb4f95d 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1617,7 +1617,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { .map(Path::new) .filter(|_| { // Only spend time on further checks if we have what to translate *to*. - sess.real_rust_source_base_dir.is_some() + sess.opts.real_rust_source_base_dir.is_some() }) .filter(|virtual_dir| { // Don't translate away `/rustc/$hash` if we're still remapping to it, @@ -1629,11 +1629,11 @@ impl<'a, 'tcx> CrateMetadataRef<'a> { debug!( "try_to_translate_virtual_to_real(name={:?}): \ virtual_rust_source_base_dir={:?}, real_rust_source_base_dir={:?}", - name, virtual_rust_source_base_dir, sess.real_rust_source_base_dir, + name, virtual_rust_source_base_dir, sess.opts.real_rust_source_base_dir, ); if let Some(virtual_dir) = virtual_rust_source_base_dir { - if let Some(real_dir) = &sess.real_rust_source_base_dir { + if let Some(real_dir) = &sess.opts.real_rust_source_base_dir { if let rustc_span::FileName::Real(old_name) = name { if let rustc_span::RealFileName::Named(one_path) = old_name { if let Ok(rest) = one_path.strip_prefix(virtual_dir) { diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 5d588ad1be2d..1f5cb5b8abc8 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -702,6 +702,7 @@ impl Default for Options { cli_forced_codegen_units: None, cli_forced_thinlto_off: false, remap_path_prefix: Vec::new(), + real_rust_source_base_dir: None, edition: DEFAULT_EDITION, json_artifact_notifications: false, json_unused_externs: false, @@ -1980,6 +1981,34 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { } } + // Try to find a directory containing the Rust `src`, for more details see + // the doc comment on the `real_rust_source_base_dir` field. + let tmp_buf; + let sysroot = match &sysroot_opt { + Some(s) => s, + None => { + tmp_buf = crate::filesearch::get_or_default_sysroot(); + &tmp_buf + } + }; + let real_rust_source_base_dir = { + // This is the location used by the `rust-src` `rustup` component. + let mut candidate = sysroot.join("lib/rustlib/src/rust"); + if let Ok(metadata) = candidate.symlink_metadata() { + // Replace the symlink rustbuild creates, with its destination. + // We could try to use `fs::canonicalize` instead, but that might + // produce unnecessarily verbose path. + if metadata.file_type().is_symlink() { + if let Ok(symlink_dest) = std::fs::read_link(&candidate) { + candidate = symlink_dest; + } + } + } + + // Only use this directory if it has a file we can expect to always find. + if candidate.join("library/std/src/lib.rs").is_file() { Some(candidate) } else { None } + }; + Options { crate_types, optimize: opt_level, @@ -2010,6 +2039,7 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { cli_forced_codegen_units: codegen_units, cli_forced_thinlto_off: disable_thinlto, remap_path_prefix, + real_rust_source_base_dir, edition, json_artifact_notifications, json_unused_externs, diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 226c7bda0522..d15dec622e58 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -54,11 +54,15 @@ macro_rules! hash_substruct { macro_rules! top_level_options { (pub struct Options { $( + $( #[$attr:meta] )* $opt:ident : $t:ty [$dep_tracking_marker:ident], )* } ) => ( #[derive(Clone)] pub struct Options { - $(pub $opt: $t),* + $( + $( #[$attr] )* + pub $opt: $t + ),* } impl Options { @@ -174,6 +178,14 @@ top_level_options!( // Remap source path prefixes in all output (messages, object files, debug, etc.). remap_path_prefix: Vec<(PathBuf, PathBuf)> [TRACKED_NO_CRATE_HASH], + /// Base directory containing the `src/` for the Rust standard library, and + /// potentially `rustc` as well, if we can can find it. Right now it's always + /// `$sysroot/lib/rustlib/src/rust` (i.e. the `rustup` `rust-src` component). + /// + /// This directory is what the virtual `/rustc/$hash` is translated back to, + /// if Rust was built with path remapping to `/rustc/$hash` enabled + /// (the `rust.remap-debuginfo` option in `config.toml`). + real_rust_source_base_dir: Option [TRACKED_NO_CRATE_HASH], edition: Edition [TRACKED], @@ -254,13 +266,13 @@ macro_rules! options { } impl $struct_name { - fn dep_tracking_hash(&self, for_crate_hash: bool, error_format: ErrorOutputType) -> u64 { + fn dep_tracking_hash(&self, _for_crate_hash: bool, error_format: ErrorOutputType) -> u64 { let mut sub_hashes = BTreeMap::new(); $({ hash_opt!($opt, &self.$opt, &mut sub_hashes, - for_crate_hash, + _for_crate_hash, [$dep_tracking_marker]); })* let mut hasher = DefaultHasher::new(); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 7bff634fb2dd..e7dfc4b8c412 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -214,15 +214,6 @@ pub struct Session { /// drown everything else in noise. miri_unleashed_features: Lock)>>, - /// Base directory containing the `src/` for the Rust standard library, and - /// potentially `rustc` as well, if we can can find it. Right now it's always - /// `$sysroot/lib/rustlib/src/rust` (i.e. the `rustup` `rust-src` component). - /// - /// This directory is what the virtual `/rustc/$hash` is translated back to, - /// if Rust was built with path remapping to `/rustc/$hash` enabled - /// (the `rust.remap-debuginfo` option in `config.toml`). - pub real_rust_source_base_dir: Option, - /// Architecture to use for interpreting asm!. pub asm_arch: Option, @@ -1390,26 +1381,6 @@ pub fn build_session( _ => CtfeBacktrace::Disabled, }); - // Try to find a directory containing the Rust `src`, for more details see - // the doc comment on the `real_rust_source_base_dir` field. - let real_rust_source_base_dir = { - // This is the location used by the `rust-src` `rustup` component. - let mut candidate = sysroot.join("lib/rustlib/src/rust"); - if let Ok(metadata) = candidate.symlink_metadata() { - // Replace the symlink rustbuild creates, with its destination. - // We could try to use `fs::canonicalize` instead, but that might - // produce unnecessarily verbose path. - if metadata.file_type().is_symlink() { - if let Ok(symlink_dest) = std::fs::read_link(&candidate) { - candidate = symlink_dest; - } - } - } - - // Only use this directory if it has a file we can expect to always find. - if candidate.join("library/std/src/lib.rs").is_file() { Some(candidate) } else { None } - }; - let asm_arch = if target_cfg.allow_asm { InlineAsmArch::from_str(&target_cfg.arch).ok() } else { None }; @@ -1453,7 +1424,6 @@ pub fn build_session( system_library_path: OneThread::new(RefCell::new(Default::default())), ctfe_backtrace, miri_unleashed_features: Lock::new(Default::default()), - real_rust_source_base_dir, asm_arch, target_features: FxHashSet::default(), known_attrs: Lock::new(MarkedAttrs::new()), From 9d170be94454340e212e186413d1cf738cfef7c7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 27 Apr 2021 16:44:14 +0000 Subject: [PATCH 05/16] Use doc-comment instad of comments consistently This makes the comments show up in the generated docs. --- compiler/rustc_session/src/options.rs | 117 +++++++++++++------------- 1 file changed, 59 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index d15dec622e58..993b9d6ec78d 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -53,11 +53,12 @@ macro_rules! hash_substruct { } macro_rules! top_level_options { - (pub struct Options { $( + ( $( #[$top_level_attr:meta] )* pub struct Options { $( $( #[$attr:meta] )* $opt:ident : $t:ty [$dep_tracking_marker:ident], )* } ) => ( #[derive(Clone)] + $( #[$top_level_attr] )* pub struct Options { $( $( #[$attr] )* @@ -93,38 +94,38 @@ macro_rules! top_level_options { ); } -// The top-level command-line options struct. -// -// For each option, one has to specify how it behaves with regard to the -// dependency tracking system of incremental compilation. This is done via the -// square-bracketed directive after the field type. The options are: -// -// [TRACKED] -// 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 -// unnecessary re-compilation. top_level_options!( + /// The top-level command-line options struct. + /// + /// For each option, one has to specify how it behaves with regard to the + /// dependency tracking system of incremental compilation. This is done via the + /// square-bracketed directive after the field type. The options are: + /// + /// [TRACKED] + /// 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 + /// unnecessary re-compilation. pub struct Options { - // The crate config requested for the session, which may be combined - // with additional crate configurations during the compile process. + /// The crate config requested for the session, which may be combined + /// with additional crate configurations during the compile process. crate_types: Vec [TRACKED], optimize: OptLevel [TRACKED], - // Include the `debug_assertions` flag in dependency tracking, since it - // can influence whether overflow checks are done or not. + /// Include the `debug_assertions` flag in dependency tracking, since it + /// can influence whether overflow checks are done or not. debug_assertions: bool [TRACKED], debuginfo: DebugInfo [TRACKED], lint_opts: Vec<(String, lint::Level)> [TRACKED], @@ -140,43 +141,43 @@ top_level_options!( test: bool [TRACKED], error_format: ErrorOutputType [UNTRACKED], - // If `Some`, enable incremental compilation, using the given - // directory to store intermediate results. + /// If `Some`, enable incremental compilation, using the given + /// directory to store intermediate results. incremental: Option [UNTRACKED], 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. + /// 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 [SUBSTRUCT], externs: Externs [UNTRACKED], extern_dep_specs: ExternDepSpecs [UNTRACKED], crate_name: Option [TRACKED], - // An optional name to use as the crate for std during std injection, - // written `extern crate name as std`. Defaults to `std`. Used by - // out-of-tree drivers. + /// An optional name to use as the crate for std during std injection, + /// written `extern crate name as std`. Defaults to `std`. Used by + /// out-of-tree drivers. alt_std_name: Option [TRACKED], - // Indicates how the compiler should treat unstable features. + /// Indicates how the compiler should treat unstable features. unstable_features: UnstableFeatures [TRACKED], - // Indicates whether this run of the compiler is actually rustdoc. This - // is currently just a hack and will be removed eventually, so please - // try to not rely on this too much. + /// Indicates whether this run of the compiler is actually rustdoc. This + /// is currently just a hack and will be removed eventually, so please + /// try to not rely on this too much. actually_rustdoc: bool [TRACKED], - // Control path trimming. + /// Control path trimming. trimmed_def_paths: TrimmedDefPaths [TRACKED], - // Specifications of codegen units / ThinLTO which are forced as a - // result of parsing command line options. These are not necessarily - // what rustc was invoked with, but massaged a bit to agree with - // commands like `--emit llvm-ir` which they're often incompatible with - // if we otherwise use the defaults of rustc. + /// Specifications of codegen units / ThinLTO which are forced as a + /// result of parsing command line options. These are not necessarily + /// what rustc was invoked with, but massaged a bit to agree with + /// commands like `--emit llvm-ir` which they're often incompatible with + /// if we otherwise use the defaults of rustc. cli_forced_codegen_units: Option [UNTRACKED], cli_forced_thinlto_off: bool [UNTRACKED], - // Remap source path prefixes in all output (messages, object files, debug, etc.). + /// Remap source path prefixes in all output (messages, object files, debug, etc.). remap_path_prefix: Vec<(PathBuf, PathBuf)> [TRACKED_NO_CRATE_HASH], /// Base directory containing the `src/` for the Rust standard library, and /// potentially `rustc` as well, if we can can find it. Right now it's always @@ -189,11 +190,11 @@ top_level_options!( edition: Edition [TRACKED], - // `true` if we're emitting JSON blobs about each artifact produced - // by the compiler. + /// `true` if we're emitting JSON blobs about each artifact produced + /// by the compiler. json_artifact_notifications: bool [TRACKED], - // `true` if we're emitting a JSON blob containing the unused externs + /// `true` if we're emitting a JSON blob containing the unused externs json_unused_externs: bool [UNTRACKED], pretty: Option [UNTRACKED], @@ -212,7 +213,7 @@ macro_rules! options { ($struct_name:ident, $setter_name:ident, $defaultfn:ident, $buildfn:ident, $prefix:expr, $outputname:expr, $stat:ident, $mod_desc:ident, $mod_set:ident, - $($opt:ident : $t:ty = ( + $($( #[$attr:meta] )* $opt:ident : $t:ty = ( $init:expr, $parse:ident, [$dep_tracking_marker:ident], @@ -223,7 +224,7 @@ macro_rules! options { pub struct $struct_name { $(pub $opt: $t),* } pub fn $defaultfn() -> $struct_name { - $struct_name { $($opt: $init),* } + $struct_name { $( $( #[$attr] )* $opt: $init),* } } pub fn $buildfn(matches: &getopts::Matches, error_format: ErrorOutputType) -> $struct_name @@ -1177,7 +1178,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, self_profile: SwitchWithOptPath = (SwitchWithOptPath::Disabled, parse_switch_with_opt_path, [UNTRACKED], "run the self profiler and output the raw event data"), - // keep this in sync with the event filter names in librustc_data_structures/profiling.rs + /// keep this in sync with the event filter names in librustc_data_structures/profiling.rs self_profile_events: Option> = (None, parse_opt_comma_list, [UNTRACKED], "specify the events recorded by the self profiler; for example: `-Z self-profile-events=default,query-keys` @@ -1189,7 +1190,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "show spans for compiler debugging (expr|pat|ty)"), span_debug: bool = (false, parse_bool, [UNTRACKED], "forward proc_macro::Span's `Debug` impl to `Span`"), - // o/w tests have closure@path + /// o/w tests have closure@path span_free_formats: bool = (false, parse_bool, [UNTRACKED], "exclude spans when debug-printing compiler state (default: no)"), src_hash_algorithm: Option = (None, parse_src_file_hash, [TRACKED], @@ -1210,10 +1211,10 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "select processor to schedule for (`rustc --print target-cpus` for details)"), thinlto: Option = (None, parse_opt_bool, [TRACKED], "enable ThinLTO when possible"), - // We default to 1 here since we want to behave like - // a sequential compiler for now. This'll likely be adjusted - // in the future. Note that -Zthreads=0 is the way to get - // the num_cpus behavior. + /// We default to 1 here since we want to behave like + /// a sequential compiler for now. This'll likely be adjusted + /// in the future. Note that -Zthreads=0 is the way to get + /// the num_cpus behavior. threads: usize = (1, parse_threads, [UNTRACKED], "use a thread pool with N threads"), time: bool = (false, parse_bool, [UNTRACKED], From 8c25e27f1676ac40918731975cbcd47c6acff0a7 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 14 Apr 2021 09:20:49 -0400 Subject: [PATCH 06/16] Implement `x.py test src/tools/clippy --bless` - Add clippy_dev to the rust workspace Before, it would give an error that it wasn't either included or excluded from the workspace: ``` error: current package believes it's in a workspace when it's not: current: /home/joshua/rustc/src/tools/clippy/clippy_dev/Cargo.toml workspace: /home/joshua/rustc/Cargo.toml this may be fixable by adding `src/tools/clippy/clippy_dev` to the `workspace.members` array of the manifest located at: /home/joshua/rustc/Cargo.toml Alternatively, to keep it out of the workspace, add the package to the `workspace.exclude` array, or add an empty `[workspace]` table to the package's manifest. ``` - Change clippy's copy of compiletest not to special-case rust-lang/rust. Using OUT_DIR confused `clippy_dev` and it couldn't find the test outputs. This is one of the reasons why `cargo dev bless` used to silently do nothing (the others were that `CARGO_TARGET_DIR` and `PROFILE` weren't set appropriately). - Run clippy_dev on test failure I tested this by removing a couple lines from a stderr file, and they were correctly replaced. - Fix clippy_dev warnings --- Cargo.lock | 13 +++++++++++++ Cargo.toml | 1 + src/bootstrap/builder.rs | 5 +++++ src/bootstrap/test.rs | 20 ++++++++++++++++++++ src/tools/clippy/clippy_dev/src/new_lint.rs | 4 ++-- src/tools/clippy/tests/compile-test.rs | 9 +-------- 6 files changed, 42 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8fec4bf128f7..3fa2771a1dad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -599,6 +599,19 @@ dependencies = [ name = "clippy-mini-macro-test" version = "0.2.0" +[[package]] +name = "clippy_dev" +version = "0.0.1" +dependencies = [ + "bytecount", + "clap", + "itertools 0.9.0", + "opener", + "regex", + "shell-escape", + "walkdir", +] + [[package]] name = "clippy_lints" version = "0.1.53" diff --git a/Cargo.toml b/Cargo.toml index 02011357eac9..327afe35c2fb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ members = [ "src/rustdoc-json-types", "src/tools/cargotest", "src/tools/clippy", + "src/tools/clippy/clippy_dev", "src/tools/compiletest", "src/tools/error_index_generator", "src/tools/linkchecker", diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 38901a35296e..62a3a87eeb85 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -1627,6 +1627,11 @@ impl Cargo { pub fn add_rustc_lib_path(&mut self, builder: &Builder<'_>, compiler: Compiler) { builder.add_rustc_lib_path(compiler, &mut self.command); } + + pub fn current_dir(&mut self, dir: &Path) -> &mut Cargo { + self.command.current_dir(dir); + self + } } impl From for Command { diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index b9d7ecf8c0eb..965d11621450 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -632,6 +632,26 @@ impl Step for Clippy { cargo.add_rustc_lib_path(builder, compiler); + if builder.try_run(&mut cargo.into()) { + // The tests succeeded; nothing to do. + return; + } + + if !builder.config.cmd.bless() { + std::process::exit(1); + } + + let mut cargo = builder.cargo(compiler, Mode::ToolRustc, SourceType::InTree, host, "run"); + cargo.arg("-p").arg("clippy_dev"); + // clippy_dev gets confused if it can't find `clippy/Cargo.toml` + cargo.current_dir(&builder.src.join("src").join("tools").join("clippy")); + if builder.config.rust_optimize { + cargo.env("PROFILE", "release"); + } else { + cargo.env("PROFILE", "debug"); + } + cargo.arg("--"); + cargo.arg("bless"); builder.run(&mut cargo.into()); } } diff --git a/src/tools/clippy/clippy_dev/src/new_lint.rs b/src/tools/clippy/clippy_dev/src/new_lint.rs index d951ca0e6308..4676c2affad7 100644 --- a/src/tools/clippy/clippy_dev/src/new_lint.rs +++ b/src/tools/clippy/clippy_dev/src/new_lint.rs @@ -44,7 +44,7 @@ pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str create_test(&lint).context("Unable to create a test for the new lint") } -fn create_lint(lint: &LintData) -> io::Result<()> { +fn create_lint(lint: &LintData<'_>) -> io::Result<()> { let (pass_type, pass_lifetimes, pass_import, context_import) = match lint.pass { "early" => ("EarlyLintPass", "", "use rustc_ast::ast::*;", "EarlyContext"), "late" => ("LateLintPass", "<'_>", "use rustc_hir::*;", "LateContext"), @@ -68,7 +68,7 @@ fn create_lint(lint: &LintData) -> io::Result<()> { write_file(lint.project_root.join(&lint_path), lint_contents.as_bytes()) } -fn create_test(lint: &LintData) -> io::Result<()> { +fn create_test(lint: &LintData<'_>) -> io::Result<()> { fn create_project_layout>(lint_name: &str, location: P, case: &str, hint: &str) -> io::Result<()> { let mut path = location.into().join(case); fs::create_dir(&path)?; diff --git a/src/tools/clippy/tests/compile-test.rs b/src/tools/clippy/tests/compile-test.rs index f4d354f0bf94..e1110721f6ec 100644 --- a/src/tools/clippy/tests/compile-test.rs +++ b/src/tools/clippy/tests/compile-test.rs @@ -83,14 +83,7 @@ fn default_config() -> compiletest::Config { third_party_crates(), )); - config.build_base = if cargo::is_rustc_test_suite() { - // This make the stderr files go to clippy OUT_DIR on rustc repo build dir - let mut path = PathBuf::from(env!("OUT_DIR")); - path.push("test_build_base"); - path - } else { - host_lib().join("test_build_base") - }; + config.build_base = host_lib().join("test_build_base"); config.rustc_path = clippy_driver_path(); config } From aeb67ad870af2abde47b7be81651aa5a3fec6faa Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 26 Apr 2021 02:45:46 -0700 Subject: [PATCH 07/16] Drop branching blocks with same span as expanded macro Fixes: #84561 --- .../rustc_mir/src/transform/coverage/spans.rs | 84 ++++++++++++--- .../rustc_mir/src/transform/coverage/tests.rs | 13 ++- .../expected_show_coverage.issue-84561.txt | 102 ++++++++++++++++++ .../run-make-fulldeps/coverage/issue-84561.rs | 94 ++++++++++++++++ 4 files changed, 276 insertions(+), 17 deletions(-) create mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt create mode 100644 src/test/run-make-fulldeps/coverage/issue-84561.rs diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index 249f5e835cd7..8dd6f7402fee 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -11,7 +11,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::TyCtxt; use rustc_span::source_map::original_sp; -use rustc_span::{BytePos, Span}; +use rustc_span::{BytePos, ExpnKind, MacroKind, Span}; use std::cmp::Ordering; @@ -67,6 +67,7 @@ impl CoverageStatement { #[derive(Debug, Clone)] pub(super) struct CoverageSpan { pub span: Span, + pub is_macro_expansion: bool, pub bcb: BasicCoverageBlock, pub coverage_statements: Vec, pub is_closure: bool, @@ -74,12 +75,22 @@ pub(super) struct CoverageSpan { impl CoverageSpan { pub fn for_fn_sig(fn_sig_span: Span) -> Self { - Self { span: fn_sig_span, bcb: START_BCB, coverage_statements: vec![], is_closure: false } + // Whether the function signature is from a macro or not, it should not be treated like + // macro-expanded statements and terminators. + let is_macro_expansion = false; + Self { + span: fn_sig_span, + is_macro_expansion, + bcb: START_BCB, + coverage_statements: vec![], + is_closure: false, + } } pub fn for_statement( statement: &Statement<'tcx>, span: Span, + is_macro_expansion: bool, bcb: BasicCoverageBlock, bb: BasicBlock, stmt_index: usize, @@ -94,15 +105,22 @@ impl CoverageSpan { Self { span, + is_macro_expansion, bcb, coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], is_closure, } } - pub fn for_terminator(span: Span, bcb: BasicCoverageBlock, bb: BasicBlock) -> Self { + pub fn for_terminator( + span: Span, + is_macro_expansion: bool, + bcb: BasicCoverageBlock, + bb: BasicBlock, + ) -> Self { Self { span, + is_macro_expansion, bcb, coverage_statements: vec![CoverageStatement::Terminator(bb, span)], is_closure: false, @@ -344,7 +362,27 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } else if self.prev_original_span == self.curr().span { // Note that this compares the new span to `prev_original_span`, which may not // be the full `prev.span` (if merged during the previous iteration). - self.hold_pending_dups_unless_dominated(); + if self.prev().is_macro_expansion && self.curr().is_macro_expansion { + // Macros that expand to include branching (such as + // `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or + // `trace!()) typically generate callee spans with identical + // ranges (typically the full span of the macro) for all + // `BasicBlocks`. This makes it impossible to distinguish + // the condition (`if val1 != val2`) from the optional + // branched statements (such as the call to `panic!()` on + // assert failure). In this case it is better (or less + // worse) to drop the optional branch bcbs and keep the + // non-conditional statements, to count when reached. + debug!( + " curr and prev are part of a macro expansion, and curr has the same span \ + as prev, but is in a different bcb. Drop curr and keep prev for next iter. \ + prev={:?}", + self.prev() + ); + self.take_curr(); + } else { + self.hold_pending_dups_unless_dominated(); + } } else { self.cutoff_prev_at_overlapping_curr(); } @@ -401,14 +439,24 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { .iter() .enumerate() .filter_map(move |(index, statement)| { - filtered_statement_span(statement, self.body_span).map(|span| { - CoverageSpan::for_statement(statement, span, bcb, bb, index) - }) + filtered_statement_span(statement, self.body_span).map( + |(span, is_macro_expansion)| { + CoverageSpan::for_statement( + statement, + span, + is_macro_expansion, + bcb, + bb, + index, + ) + }, + ) }) - .chain( - filtered_terminator_span(data.terminator(), self.body_span) - .map(|span| CoverageSpan::for_terminator(span, bcb, bb)), - ) + .chain(filtered_terminator_span(data.terminator(), self.body_span).map( + |(span, is_macro_expansion)| { + CoverageSpan::for_terminator(span, is_macro_expansion, bcb, bb) + }, + )) }) .collect() } @@ -656,7 +704,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { pub(super) fn filtered_statement_span( statement: &'a Statement<'tcx>, body_span: Span, -) -> Option { +) -> Option<(Span, bool)> { match statement.kind { // These statements have spans that are often outside the scope of the executed source code // for their parent `BasicBlock`. @@ -701,7 +749,7 @@ pub(super) fn filtered_statement_span( pub(super) fn filtered_terminator_span( terminator: &'a Terminator<'tcx>, body_span: Span, -) -> Option { +) -> Option<(Span, bool)> { match terminator.kind { // These terminators have spans that don't positively contribute to computing a reasonable // span of actually executed source code. (For example, SwitchInt terminators extracted from @@ -732,7 +780,13 @@ pub(super) fn filtered_terminator_span( } #[inline] -fn function_source_span(span: Span, body_span: Span) -> Span { +fn function_source_span(span: Span, body_span: Span) -> (Span, bool) { + let is_macro_expansion = span.ctxt() != body_span.ctxt() + && if let ExpnKind::Macro(MacroKind::Bang, _) = span.ctxt().outer_expn_data().kind { + true + } else { + false + }; let span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); - if body_span.contains(span) { span } else { body_span } + (if body_span.contains(span) { span } else { body_span }, is_macro_expansion) } diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs index dee112443d33..dbbd677fd638 100644 --- a/compiler/rustc_mir/src/transform/coverage/tests.rs +++ b/compiler/rustc_mir/src/transform/coverage/tests.rs @@ -1,6 +1,10 @@ //! This crate hosts a selection of "unit tests" for components of the `InstrumentCoverage` MIR //! pass. //! +//! ```shell +//! ./x.py test --keep-stage 1 compiler/rustc_mir --test-args '--show-output coverage' +//! ``` +//! //! The tests construct a few "mock" objects, as needed, to support the `InstrumentCoverage` //! functions and algorithms. Mocked objects include instances of `mir::Body`; including //! `Terminator`s of various `kind`s, and `Span` objects. Some functions used by or used on @@ -679,10 +683,15 @@ fn test_make_bcb_counters() { let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); let mut coverage_spans = Vec::new(); for (bcb, data) in basic_coverage_blocks.iter_enumerated() { - if let Some(span) = + if let Some((span, is_macro_expansion)) = spans::filtered_terminator_span(data.terminator(&mir_body), body_span) { - coverage_spans.push(spans::CoverageSpan::for_terminator(span, bcb, data.last_bb())); + coverage_spans.push(spans::CoverageSpan::for_terminator( + span, + is_macro_expansion, + bcb, + data.last_bb(), + )); } } let mut coverage_counters = counters::CoverageCounters::new(0); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt new file mode 100644 index 000000000000..34d584f9eae6 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt @@ -0,0 +1,102 @@ + 1| |// FIXME(#84561): function-like macros produce unintuitive coverage results. + 2| |// This test demonstrates some of the problems. + 3| | + 4| 18|#[derive(Debug, PartialEq, Eq)] + ^5 ^0 + ------------------ + | ::eq: + | 4| 18|#[derive(Debug, PartialEq, Eq)] + ------------------ + | Unexecuted instantiation: ::ne + ------------------ + 5| |struct Foo(u32); + 6| | + 7| 1|fn main() { + 8| 1| let bar = Foo(1); + 9| 1| assert_eq!(bar, Foo(1)); + 10| 1| let baz = Foo(0); + 11| 1| assert_ne!(baz, Foo(1)); + 12| 1| println!("{:?}", Foo(1)); + 13| 1| println!("{:?}", bar); + 14| 1| println!("{:?}", baz); + 15| 1| + 16| 1| assert_eq!(Foo(1), Foo(1)); + 17| 1| assert_ne!(Foo(0), Foo(1)); + 18| 1| assert_eq!(Foo(2), Foo(2)); + 19| 1| let bar = Foo(1); + 20| 1| assert_ne!(Foo(0), Foo(3)); + 21| 1| assert_ne!(Foo(0), Foo(4)); + 22| 1| assert_eq!(Foo(3), Foo(3)); + 23| 1| assert_ne!(Foo(0), Foo(5)); + 24| 1| println!("{:?}", bar); + 25| 1| println!("{:?}", Foo(1)); + 26| 1| + 27| 1| let is_true = std::env::args().len() == 1; + 28| 1| + 29| 1| assert_eq!( + 30| 1| Foo(1), + 31| 1| Foo(1) + 32| 1| ); + 33| 1| assert_ne!( + 34| 1| Foo(0), + 35| 1| Foo(1) + 36| 1| ); + 37| 1| assert_eq!( + 38| 1| Foo(2), + 39| 1| Foo(2) + 40| 1| ); + 41| 1| let bar = Foo(1 + 42| 1| ); + 43| 1| assert_ne!( + 44| 1| Foo(0), + 45| 1| Foo(3) + 46| 1| ); + 47| 1| if is_true { + 48| 1| assert_ne!( + 49| 1| Foo(0), + 50| 1| Foo(4) + 51| 1| ); + 52| | } else { + 53| 0| assert_eq!( + 54| 0| Foo(3), + 55| 0| Foo(3) + 56| 0| ); + 57| | } + 58| | assert_ne!( + 59| 1| if is_true { + 60| 1| Foo(0) + 61| | } else { + 62| 0| Foo(1) + 63| | }, + 64| | Foo(5) + 65| | ); + 66| 1| assert_ne!( + 67| 1| Foo(5), + 68| 1| if is_true { + 69| 1| Foo(0) + 70| | } else { + 71| 0| Foo(1) + 72| | } + 73| | ); + 74| | assert_ne!( + 75| 1| if is_true { + 76| 1| assert_eq!( + 77| 1| Foo(3), + 78| 1| Foo(3) + 79| 1| ); + 80| 1| Foo(0) + 81| | } else { + 82| | assert_ne!( + 83| 0| if is_true { + 84| 0| Foo(0) + 85| | } else { + 86| 0| Foo(1) + 87| | }, + 88| | Foo(5) + 89| | ); + 90| 0| Foo(1) + 91| | }, + 92| | Foo(5) + 93| | ); + 94| 1|} + diff --git a/src/test/run-make-fulldeps/coverage/issue-84561.rs b/src/test/run-make-fulldeps/coverage/issue-84561.rs new file mode 100644 index 000000000000..a5a0e1dc7581 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/issue-84561.rs @@ -0,0 +1,94 @@ +// FIXME(#84561): function-like macros produce unintuitive coverage results. +// This test demonstrates some of the problems. + +#[derive(Debug, PartialEq, Eq)] +struct Foo(u32); + +fn main() { + let bar = Foo(1); + assert_eq!(bar, Foo(1)); + let baz = Foo(0); + assert_ne!(baz, Foo(1)); + println!("{:?}", Foo(1)); + println!("{:?}", bar); + println!("{:?}", baz); + + assert_eq!(Foo(1), Foo(1)); + assert_ne!(Foo(0), Foo(1)); + assert_eq!(Foo(2), Foo(2)); + let bar = Foo(1); + assert_ne!(Foo(0), Foo(3)); + assert_ne!(Foo(0), Foo(4)); + assert_eq!(Foo(3), Foo(3)); + assert_ne!(Foo(0), Foo(5)); + println!("{:?}", bar); + println!("{:?}", Foo(1)); + + let is_true = std::env::args().len() == 1; + + assert_eq!( + Foo(1), + Foo(1) + ); + assert_ne!( + Foo(0), + Foo(1) + ); + assert_eq!( + Foo(2), + Foo(2) + ); + let bar = Foo(1 + ); + assert_ne!( + Foo(0), + Foo(3) + ); + if is_true { + assert_ne!( + Foo(0), + Foo(4) + ); + } else { + assert_eq!( + Foo(3), + Foo(3) + ); + } + assert_ne!( + if is_true { + Foo(0) + } else { + Foo(1) + }, + Foo(5) + ); + assert_ne!( + Foo(5), + if is_true { + Foo(0) + } else { + Foo(1) + } + ); + assert_ne!( + if is_true { + assert_eq!( + Foo(3), + Foo(3) + ); + Foo(0) + } else { + assert_ne!( + if is_true { + Foo(0) + } else { + Foo(1) + }, + Foo(5) + ); + Foo(1) + }, + Foo(5) + ); +} From 2c4fc3e8f4e3886d1c3b9cb437e428cc4e3d2113 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 26 Apr 2021 16:27:54 -0700 Subject: [PATCH 08/16] More improvements to macro coverage --- .../rustc_mir/src/transform/coverage/spans.rs | 159 ++++++++---- .../rustc_mir/src/transform/coverage/tests.rs | 4 +- .../expected_show_coverage.inner_items.txt | 6 +- .../expected_show_coverage.issue-84561.txt | 236 ++++++++++++------ .../expected_show_coverage.uses_crate.txt | 6 +- ...pected_show_coverage.uses_inline_crate.txt | 12 +- .../run-make-fulldeps/coverage/issue-84561.rs | 98 +++++++- 7 files changed, 366 insertions(+), 155 deletions(-) diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index 8dd6f7402fee..97eda1e37e1f 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -11,7 +11,7 @@ use rustc_middle::mir::{ use rustc_middle::ty::TyCtxt; use rustc_span::source_map::original_sp; -use rustc_span::{BytePos, ExpnKind, MacroKind, Span}; +use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol}; use std::cmp::Ordering; @@ -67,7 +67,7 @@ impl CoverageStatement { #[derive(Debug, Clone)] pub(super) struct CoverageSpan { pub span: Span, - pub is_macro_expansion: bool, + pub expn_span: Span, pub bcb: BasicCoverageBlock, pub coverage_statements: Vec, pub is_closure: bool, @@ -75,12 +75,9 @@ pub(super) struct CoverageSpan { impl CoverageSpan { pub fn for_fn_sig(fn_sig_span: Span) -> Self { - // Whether the function signature is from a macro or not, it should not be treated like - // macro-expanded statements and terminators. - let is_macro_expansion = false; Self { span: fn_sig_span, - is_macro_expansion, + expn_span: fn_sig_span, bcb: START_BCB, coverage_statements: vec![], is_closure: false, @@ -90,7 +87,7 @@ impl CoverageSpan { pub fn for_statement( statement: &Statement<'tcx>, span: Span, - is_macro_expansion: bool, + expn_span: Span, bcb: BasicCoverageBlock, bb: BasicBlock, stmt_index: usize, @@ -105,7 +102,7 @@ impl CoverageSpan { Self { span, - is_macro_expansion, + expn_span, bcb, coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], is_closure, @@ -114,13 +111,13 @@ impl CoverageSpan { pub fn for_terminator( span: Span, - is_macro_expansion: bool, + expn_span: Span, bcb: BasicCoverageBlock, bb: BasicBlock, ) -> Self { Self { span, - is_macro_expansion, + expn_span, bcb, coverage_statements: vec![CoverageStatement::Terminator(bb, span)], is_closure: false, @@ -176,6 +173,34 @@ impl CoverageSpan { .collect::>() .join("\n") } + + /// If the span is part of a macro, and the macro is visible (expands directly to the given + /// body_span), returns the macro name symbol. + pub fn current_macro(&self) -> Option { + if let ExpnKind::Macro(MacroKind::Bang, current_macro) = + self.expn_span.ctxt().outer_expn_data().kind + { + return Some(current_macro); + } + None + } + + /// If the span is part of a macro, and the macro is visible (expands directly to the given + /// body_span), returns the macro name symbol. + pub fn visible_macro(&self, body_span: Span) -> Option { + if let Some(current_macro) = self.current_macro() { + if self.expn_span.parent().unwrap_or_else(|| bug!("macro must have a parent")).ctxt() + == body_span.ctxt() + { + return Some(current_macro); + } + } + None + } + + pub fn is_macro_expansion(&self) -> bool { + self.current_macro().is_some() + } } /// Converts the initial set of `CoverageSpan`s (one per MIR `Statement` or `Terminator`) into a @@ -219,6 +244,9 @@ pub struct CoverageSpans<'a, 'tcx> { /// Assigned from `curr_original_span` from the previous iteration. prev_original_span: Span, + /// A copy of the expn_span from the prior iteration. + prev_expn_span: Option, + /// One or more `CoverageSpan`s with the same `Span` but different `BasicCoverageBlock`s, and /// no `BasicCoverageBlock` in this list dominates another `BasicCoverageBlock` in the list. /// If a new `curr` span also fits this criteria (compared to an existing list of @@ -273,15 +301,13 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { curr_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), some_prev: None, prev_original_span: Span::with_root_ctxt(BytePos(0), BytePos(0)), + prev_expn_span: None, pending_dups: Vec::new(), }; let sorted_spans = coverage_spans.mir_to_initial_sorted_coverage_spans(); coverage_spans.sorted_spans_iter = Some(sorted_spans.into_iter()); - coverage_spans.some_prev = coverage_spans.sorted_spans_iter.as_mut().unwrap().next(); - coverage_spans.prev_original_span = - coverage_spans.some_prev.as_ref().expect("at least one span").span; coverage_spans.to_refined_spans() } @@ -335,10 +361,14 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { /// de-duplicated `CoverageSpan`s. fn to_refined_spans(mut self) -> Vec { while self.next_coverage_span() { - if self.curr().is_mergeable(self.prev()) { + if self.some_prev.is_none() { + debug!(" initial span"); + self.check_invoked_macro_name_span(); + } else if self.curr().is_mergeable(self.prev()) { debug!(" same bcb (and neither is a closure), merge with prev={:?}", self.prev()); let prev = self.take_prev(); self.curr_mut().merge_from(prev); + self.check_invoked_macro_name_span(); // Note that curr.span may now differ from curr_original_span } else if self.prev_ends_before_curr() { debug!( @@ -347,7 +377,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { self.prev() ); let prev = self.take_prev(); - self.refined_spans.push(prev); + self.push_refined_span(prev); + self.check_invoked_macro_name_span(); } else if self.prev().is_closure { // drop any equal or overlapping span (`curr`) and keep `prev` to test again in the // next iter @@ -362,7 +393,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } else if self.prev_original_span == self.curr().span { // Note that this compares the new span to `prev_original_span`, which may not // be the full `prev.span` (if merged during the previous iteration). - if self.prev().is_macro_expansion && self.curr().is_macro_expansion { + if self.prev().is_macro_expansion() && self.curr().is_macro_expansion() { // Macros that expand to include branching (such as // `assert_eq!()`, `assert_ne!()`, `info!()`, `debug!()`, or // `trace!()) typically generate callee spans with identical @@ -385,15 +416,16 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } } else { self.cutoff_prev_at_overlapping_curr(); + self.check_invoked_macro_name_span(); } } debug!(" AT END, adding last prev={:?}", self.prev()); let prev = self.take_prev(); - let CoverageSpans { pending_dups, mut refined_spans, .. } = self; + let pending_dups = self.pending_dups.split_off(0); for dup in pending_dups { debug!(" ...adding at least one pending dup={:?}", dup); - refined_spans.push(dup); + self.push_refined_span(dup); } // Async functions wrap a closure that implements the body to be executed. The enclosing @@ -403,21 +435,60 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { // excluded. The closure's `Return` is the only one that will be counted. This provides // adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace // of the function body.) - let body_ends_with_closure = if let Some(last_covspan) = refined_spans.last() { + let body_ends_with_closure = if let Some(last_covspan) = self.refined_spans.last() { last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi() } else { false }; if !body_ends_with_closure { - refined_spans.push(prev); + self.push_refined_span(prev); } // Remove `CoverageSpan`s derived from closures, originally added to ensure the coverage // regions for the current function leave room for the closure's own coverage regions // (injected separately, from the closure's own MIR). - refined_spans.retain(|covspan| !covspan.is_closure); - refined_spans + self.refined_spans.retain(|covspan| !covspan.is_closure); + self.refined_spans + } + + fn push_refined_span(&mut self, covspan: CoverageSpan) { + let len = self.refined_spans.len(); + if len > 0 { + let last = &mut self.refined_spans[len - 1]; + if last.is_mergeable(&covspan) { + debug!( + "merging new refined span with last refined span, last={:?}, covspan={:?}", + last, covspan + ); + last.merge_from(covspan); + return; + } + } + self.refined_spans.push(covspan) + } + + fn check_invoked_macro_name_span(&mut self) { + if let Some(visible_macro) = self.curr().visible_macro(self.body_span) { + if self.prev_expn_span.map_or(true, |prev_expn_span| { + self.curr().expn_span.ctxt() != prev_expn_span.ctxt() + }) { + let merged_prefix_len = self.curr_original_span.lo() - self.curr().span.lo(); + let after_macro_bang = merged_prefix_len + + BytePos(visible_macro.to_string().bytes().count() as u32 + 1); + let mut macro_name_cov = self.curr().clone(); + self.curr_mut().span = + self.curr().span.with_lo(self.curr().span.lo() + after_macro_bang); + macro_name_cov.span = + macro_name_cov.span.with_hi(macro_name_cov.span.lo() + after_macro_bang); + debug!( + " and curr starts a new macro expansion, so add a new span just for \ + the macro `{}!`, new span={:?}", + visible_macro, macro_name_cov + ); + self.push_refined_span(macro_name_cov); + } + } } // Generate a set of `CoverageSpan`s from the filtered set of `Statement`s and `Terminator`s of @@ -440,22 +511,15 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { .enumerate() .filter_map(move |(index, statement)| { filtered_statement_span(statement, self.body_span).map( - |(span, is_macro_expansion)| { + |(span, expn_span)| { CoverageSpan::for_statement( - statement, - span, - is_macro_expansion, - bcb, - bb, - index, + statement, span, expn_span, bcb, bb, index, ) }, ) }) .chain(filtered_terminator_span(data.terminator(), self.body_span).map( - |(span, is_macro_expansion)| { - CoverageSpan::for_terminator(span, is_macro_expansion, bcb, bb) - }, + |(span, expn_span)| CoverageSpan::for_terminator(span, expn_span, bcb, bb), )) }) .collect() @@ -509,7 +573,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { let pending_dups = self.pending_dups.split_off(0); for dup in pending_dups.into_iter() { debug!(" ...adding at least one pending={:?}", dup); - self.refined_spans.push(dup); + self.push_refined_span(dup); } } else { self.pending_dups.clear(); @@ -521,12 +585,13 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { /// Advance `prev` to `curr` (if any), and `curr` to the next `CoverageSpan` in sorted order. fn next_coverage_span(&mut self) -> bool { if let Some(curr) = self.some_curr.take() { + self.prev_expn_span = Some(curr.expn_span); self.some_prev = Some(curr); self.prev_original_span = self.curr_original_span; } while let Some(curr) = self.sorted_spans_iter.as_mut().unwrap().next() { debug!("FOR curr={:?}", curr); - if self.prev_starts_after_next(&curr) { + if self.some_prev.is_some() && self.prev_starts_after_next(&curr) { debug!( " prev.span starts after curr.span, so curr will be dropped (skipping past \ closure?); prev={:?}", @@ -583,10 +648,10 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { for mut dup in pending_dups.iter().cloned() { dup.span = dup.span.with_hi(left_cutoff); debug!(" ...and at least one pre_closure dup={:?}", dup); - self.refined_spans.push(dup); + self.push_refined_span(dup); } } - self.refined_spans.push(pre_closure); + self.push_refined_span(pre_closure); } if has_post_closure_span { // Update prev.span to start after the closure (and discard curr) @@ -597,7 +662,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } self.pending_dups.append(&mut pending_dups); let closure_covspan = self.take_curr(); - self.refined_spans.push(closure_covspan); // since self.prev() was already updated + self.push_refined_span(closure_covspan); // since self.prev() was already updated } else { pending_dups.clear(); } @@ -688,7 +753,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } else { debug!(" ... adding modified prev={:?}", self.prev()); let prev = self.take_prev(); - self.refined_spans.push(prev); + self.push_refined_span(prev); } } else { // with `pending_dups`, `prev` cannot have any statements that don't overlap @@ -704,7 +769,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { pub(super) fn filtered_statement_span( statement: &'a Statement<'tcx>, body_span: Span, -) -> Option<(Span, bool)> { +) -> Option<(Span, Span)> { match statement.kind { // These statements have spans that are often outside the scope of the executed source code // for their parent `BasicBlock`. @@ -749,7 +814,7 @@ pub(super) fn filtered_statement_span( pub(super) fn filtered_terminator_span( terminator: &'a Terminator<'tcx>, body_span: Span, -) -> Option<(Span, bool)> { +) -> Option<(Span, Span)> { match terminator.kind { // These terminators have spans that don't positively contribute to computing a reasonable // span of actually executed source code. (For example, SwitchInt terminators extracted from @@ -779,14 +844,10 @@ pub(super) fn filtered_terminator_span( } } +/// Returns the span within the function source body, and the given span, which will be different +/// if the given span is an expansion (macro, syntactic sugar, etc.). #[inline] -fn function_source_span(span: Span, body_span: Span) -> (Span, bool) { - let is_macro_expansion = span.ctxt() != body_span.ctxt() - && if let ExpnKind::Macro(MacroKind::Bang, _) = span.ctxt().outer_expn_data().kind { - true - } else { - false - }; - let span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); - (if body_span.contains(span) { span } else { body_span }, is_macro_expansion) +fn function_source_span(span: Span, body_span: Span) -> (Span, Span) { + let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); + (if body_span.contains(original_span) { original_span } else { body_span }, span) } diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs index dbbd677fd638..9b84173c8a29 100644 --- a/compiler/rustc_mir/src/transform/coverage/tests.rs +++ b/compiler/rustc_mir/src/transform/coverage/tests.rs @@ -683,12 +683,12 @@ fn test_make_bcb_counters() { let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); let mut coverage_spans = Vec::new(); for (bcb, data) in basic_coverage_blocks.iter_enumerated() { - if let Some((span, is_macro_expansion)) = + if let Some((span, expn_span)) = spans::filtered_terminator_span(data.terminator(&mir_body), body_span) { coverage_spans.push(spans::CoverageSpan::for_terminator( span, - is_macro_expansion, + expn_span, bcb, data.last_bb(), )); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt index f5b5184044f6..883254a09ba7 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inner_items.txt @@ -1,9 +1,9 @@ 1| |#![allow(unused_assignments, unused_variables, dead_code)] 2| | 3| 1|fn main() { - 4| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 5| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 6| | // dependent conditions. + 4| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 5| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 6| 1| // dependent conditions. 7| 1| let is_true = std::env::args().len() == 1; 8| 1| 9| 1| let mut countdown = 0; diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt index 34d584f9eae6..5d266b5db15f 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt @@ -1,17 +1,17 @@ - 1| |// FIXME(#84561): function-like macros produce unintuitive coverage results. - 2| |// This test demonstrates some of the problems. - 3| | - 4| 18|#[derive(Debug, PartialEq, Eq)] - ^5 ^0 + 1| |// This demonstrated Issue #84561: function-like macros produce unintuitive coverage results. + 2| | + 3| |// expect-exit-status-101 + 4| 21|#[derive(PartialEq, Eq)] + ^0 ------------------ | ::eq: - | 4| 18|#[derive(Debug, PartialEq, Eq)] + | 4| 21|#[derive(PartialEq, Eq)] ------------------ | Unexecuted instantiation: ::ne ------------------ 5| |struct Foo(u32); - 6| | - 7| 1|fn main() { + 6| 1|fn test2() { + 7| 1| let is_true = std::env::args().len() == 1; 8| 1| let bar = Foo(1); 9| 1| assert_eq!(bar, Foo(1)); 10| 1| let baz = Foo(0); @@ -23,80 +23,158 @@ 16| 1| assert_eq!(Foo(1), Foo(1)); 17| 1| assert_ne!(Foo(0), Foo(1)); 18| 1| assert_eq!(Foo(2), Foo(2)); - 19| 1| let bar = Foo(1); - 20| 1| assert_ne!(Foo(0), Foo(3)); + 19| 1| let bar = Foo(0); + 20| 1| assert_ne!(bar, Foo(3)); 21| 1| assert_ne!(Foo(0), Foo(4)); - 22| 1| assert_eq!(Foo(3), Foo(3)); - 23| 1| assert_ne!(Foo(0), Foo(5)); - 24| 1| println!("{:?}", bar); - 25| 1| println!("{:?}", Foo(1)); - 26| 1| - 27| 1| let is_true = std::env::args().len() == 1; - 28| 1| - 29| 1| assert_eq!( - 30| 1| Foo(1), - 31| 1| Foo(1) - 32| 1| ); - 33| 1| assert_ne!( - 34| 1| Foo(0), - 35| 1| Foo(1) - 36| 1| ); - 37| 1| assert_eq!( - 38| 1| Foo(2), - 39| 1| Foo(2) - 40| 1| ); - 41| 1| let bar = Foo(1 - 42| 1| ); - 43| 1| assert_ne!( - 44| 1| Foo(0), - 45| 1| Foo(3) - 46| 1| ); - 47| 1| if is_true { - 48| 1| assert_ne!( - 49| 1| Foo(0), - 50| 1| Foo(4) - 51| 1| ); - 52| | } else { - 53| 0| assert_eq!( - 54| 0| Foo(3), - 55| 0| Foo(3) - 56| 0| ); - 57| | } - 58| | assert_ne!( - 59| 1| if is_true { - 60| 1| Foo(0) - 61| | } else { - 62| 0| Foo(1) - 63| | }, - 64| | Foo(5) - 65| | ); - 66| 1| assert_ne!( - 67| 1| Foo(5), - 68| 1| if is_true { - 69| 1| Foo(0) - 70| | } else { - 71| 0| Foo(1) - 72| | } - 73| | ); - 74| | assert_ne!( - 75| 1| if is_true { - 76| 1| assert_eq!( - 77| 1| Foo(3), - 78| 1| Foo(3) - 79| 1| ); - 80| 1| Foo(0) - 81| | } else { - 82| | assert_ne!( - 83| 0| if is_true { - 84| 0| Foo(0) - 85| | } else { - 86| 0| Foo(1) - 87| | }, - 88| | Foo(5) - 89| | ); + 22| 1| assert_eq!(Foo(3), Foo(3), "with a message"); + ^0 + 23| 1| println!("{:?}", bar); + 24| 1| println!("{:?}", Foo(1)); + 25| 1| + 26| 1| assert_ne!(Foo(0), Foo(5), "{}", if is_true { "true message" } else { "false message" }); + ^0 ^0 ^0 + 27| 1| assert_ne!( + 28| | Foo(0) + 29| | , + 30| | Foo(5) + 31| | , + 32| 0| "{}" + 33| 0| , + 34| 0| if + 35| 0| is_true + 36| | { + 37| 0| "true message" + 38| | } else { + 39| 0| "false message" + 40| | } + 41| | ); + 42| | + 43| 1| let is_true = std::env::args().len() == 1; + 44| 1| + 45| 1| assert_eq!( + 46| 1| Foo(1), + 47| 1| Foo(1) + 48| 1| ); + 49| 1| assert_ne!( + 50| 1| Foo(0), + 51| 1| Foo(1) + 52| 1| ); + 53| 1| assert_eq!( + 54| 1| Foo(2), + 55| 1| Foo(2) + 56| 1| ); + 57| 1| let bar = Foo(1); + 58| 1| assert_ne!( + 59| 1| bar, + 60| 1| Foo(3) + 61| 1| ); + 62| 1| if is_true { + 63| 1| assert_ne!( + 64| 1| Foo(0), + 65| 1| Foo(4) + 66| 1| ); + 67| | } else { + 68| 0| assert_eq!( + 69| 0| Foo(3), + 70| 0| Foo(3) + 71| 0| ); + 72| | } + 73| 1| if is_true { + 74| 1| assert_ne!( + 75| | Foo(0), + 76| | Foo(4), + 77| 0| "with a message" + 78| | ); + 79| | } else { + 80| 0| assert_eq!( + 81| | Foo(3), + 82| | Foo(3), + 83| 0| "with a message" + 84| | ); + 85| | } + 86| 1| assert_ne!( + 87| 1| if is_true { + 88| 1| Foo(0) + 89| | } else { 90| 0| Foo(1) 91| | }, 92| | Foo(5) 93| | ); - 94| 1|} + 94| 1| assert_ne!( + 95| 1| Foo(5), + 96| 1| if is_true { + 97| 1| Foo(0) + 98| | } else { + 99| 0| Foo(1) + 100| | } + 101| | ); + 102| 1| assert_ne!( + 103| 1| if is_true { + 104| 1| assert_eq!( + 105| 1| Foo(3), + 106| 1| Foo(3) + 107| 1| ); + 108| 1| Foo(0) + 109| | } else { + 110| 0| assert_ne!( + 111| 0| if is_true { + 112| 0| Foo(0) + 113| | } else { + 114| 0| Foo(1) + 115| | }, + 116| | Foo(5) + 117| | ); + 118| 0| Foo(1) + 119| | }, + 120| | Foo(5), + 121| 0| "with a message" + 122| | ); + 123| 1| assert_eq!( + 124| | Foo(1), + 125| | Foo(3), + 126| 1| "this assert should fail" + 127| | ); + 128| 0| assert_eq!( + 129| | Foo(3), + 130| | Foo(3), + 131| 0| "this assert should not be reached" + 132| | ); + 133| 0|} + 134| | + 135| |impl std::fmt::Debug for Foo { + 136| | fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + 137| 7| write!(f, "try and succeed")?; + ^0 + 138| 7| Ok(()) + 139| 7| } + 140| |} + 141| | + 142| |static mut DEBUG_LEVEL_ENABLED: bool = false; + 143| | + 144| |macro_rules! debug { + 145| | ($($arg:tt)+) => ( + 146| | if unsafe { DEBUG_LEVEL_ENABLED } { + 147| | println!($($arg)+); + 148| | } + 149| | ); + 150| |} + 151| | + 152| 1|fn test1() { + 153| 1| debug!("debug is enabled"); + ^0 + 154| 1| debug!("debug is enabled"); + ^0 + 155| 1| let _ = 0; + 156| 1| debug!("debug is enabled"); + ^0 + 157| 1| unsafe { + 158| 1| DEBUG_LEVEL_ENABLED = true; + 159| 1| } + 160| 1| debug!("debug is enabled"); + 161| 1|} + 162| | + 163| 1|fn main() { + 164| 1| test1(); + 165| 1| test2(); + 166| 1|} diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt index f5beb9ef24a0..c2d5143a6181 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt @@ -3,9 +3,9 @@ 3| |use std::fmt::Debug; 4| | 5| 1|pub fn used_function() { - 6| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 7| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 8| | // dependent conditions. + 6| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 7| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 8| 1| // dependent conditions. 9| 1| let is_true = std::env::args().len() == 1; 10| 1| let mut countdown = 0; 11| 1| if is_true { diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt index cc98956e3073..dab31cbf4ac9 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_inline_crate.txt @@ -5,9 +5,9 @@ 5| |use std::fmt::Debug; 6| | 7| 1|pub fn used_function() { - 8| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 9| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 10| | // dependent conditions. + 8| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 9| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 10| 1| // dependent conditions. 11| 1| let is_true = std::env::args().len() == 1; 12| 1| let mut countdown = 0; 13| 1| if is_true { @@ -19,9 +19,9 @@ 18| | 19| |#[inline(always)] 20| 1|pub fn used_inline_function() { - 21| | // Initialize test constants in a way that cannot be determined at compile time, to ensure - 22| | // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - 23| | // dependent conditions. + 21| 1| // Initialize test constants in a way that cannot be determined at compile time, to ensure + 22| 1| // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from + 23| 1| // dependent conditions. 24| 1| let is_true = std::env::args().len() == 1; 25| 1| let mut countdown = 0; 26| 1| if is_true { diff --git a/src/test/run-make-fulldeps/coverage/issue-84561.rs b/src/test/run-make-fulldeps/coverage/issue-84561.rs index a5a0e1dc7581..5c8fd0b7caea 100644 --- a/src/test/run-make-fulldeps/coverage/issue-84561.rs +++ b/src/test/run-make-fulldeps/coverage/issue-84561.rs @@ -1,10 +1,10 @@ -// FIXME(#84561): function-like macros produce unintuitive coverage results. -// This test demonstrates some of the problems. +// This demonstrated Issue #84561: function-like macros produce unintuitive coverage results. -#[derive(Debug, PartialEq, Eq)] +// expect-exit-status-101 +#[derive(PartialEq, Eq)] struct Foo(u32); - -fn main() { +fn test2() { + let is_true = std::env::args().len() == 1; let bar = Foo(1); assert_eq!(bar, Foo(1)); let baz = Foo(0); @@ -16,14 +16,30 @@ fn main() { assert_eq!(Foo(1), Foo(1)); assert_ne!(Foo(0), Foo(1)); assert_eq!(Foo(2), Foo(2)); - let bar = Foo(1); - assert_ne!(Foo(0), Foo(3)); + let bar = Foo(0); + assert_ne!(bar, Foo(3)); assert_ne!(Foo(0), Foo(4)); - assert_eq!(Foo(3), Foo(3)); - assert_ne!(Foo(0), Foo(5)); + assert_eq!(Foo(3), Foo(3), "with a message"); println!("{:?}", bar); println!("{:?}", Foo(1)); + assert_ne!(Foo(0), Foo(5), "{}", if is_true { "true message" } else { "false message" }); + assert_ne!( + Foo(0) + , + Foo(5) + , + "{}" + , + if + is_true + { + "true message" + } else { + "false message" + } + ); + let is_true = std::env::args().len() == 1; assert_eq!( @@ -38,10 +54,9 @@ fn main() { Foo(2), Foo(2) ); - let bar = Foo(1 - ); + let bar = Foo(1); assert_ne!( - Foo(0), + bar, Foo(3) ); if is_true { @@ -55,6 +70,19 @@ fn main() { Foo(3) ); } + if is_true { + assert_ne!( + Foo(0), + Foo(4), + "with a message" + ); + } else { + assert_eq!( + Foo(3), + Foo(3), + "with a message" + ); + } assert_ne!( if is_true { Foo(0) @@ -89,6 +117,50 @@ fn main() { ); Foo(1) }, - Foo(5) + Foo(5), + "with a message" + ); + assert_eq!( + Foo(1), + Foo(3), + "this assert should fail" + ); + assert_eq!( + Foo(3), + Foo(3), + "this assert should not be reached" + ); +} + +impl std::fmt::Debug for Foo { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "try and succeed")?; + Ok(()) + } +} + +static mut DEBUG_LEVEL_ENABLED: bool = false; + +macro_rules! debug { + ($($arg:tt)+) => ( + if unsafe { DEBUG_LEVEL_ENABLED } { + println!($($arg)+); + } ); } + +fn test1() { + debug!("debug is enabled"); + debug!("debug is enabled"); + let _ = 0; + debug!("debug is enabled"); + unsafe { + DEBUG_LEVEL_ENABLED = true; + } + debug!("debug is enabled"); +} + +fn main() { + test1(); + test2(); +} From bbf6bcee80dcfb566b4d975ff6c3476c0864352e Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Sat, 24 Apr 2021 17:22:29 -0700 Subject: [PATCH 09/16] spanview debug output caused ICE when a function had no body --- compiler/rustc_mir/src/util/spanview.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir/src/util/spanview.rs b/compiler/rustc_mir/src/util/spanview.rs index a9a30e407b4b..9abfa4a8dc68 100644 --- a/compiler/rustc_mir/src/util/spanview.rs +++ b/compiler/rustc_mir/src/util/spanview.rs @@ -99,7 +99,11 @@ where W: Write, { let def_id = body.source.def_id(); - let body_span = hir_body(tcx, def_id).value.span; + let hir_body = hir_body(tcx, def_id); + if hir_body.is_none() { + return Ok(()); + } + let body_span = hir_body.unwrap().value.span; let mut span_viewables = Vec::new(); for (bb, data) in body.basic_blocks().iter_enumerated() { match spanview { @@ -664,19 +668,16 @@ fn fn_span<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Span { let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.as_local().expect("expected DefId is local")); let fn_decl_span = tcx.hir().span(hir_id); - let body_span = hir_body(tcx, def_id).value.span; - if fn_decl_span.ctxt() == body_span.ctxt() { - fn_decl_span.to(body_span) + if let Some(body_span) = hir_body(tcx, def_id).map(|hir_body| hir_body.value.span) { + if fn_decl_span.ctxt() == body_span.ctxt() { fn_decl_span.to(body_span) } else { body_span } } else { - // This probably occurs for functions defined via macros - body_span + fn_decl_span } } -fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx rustc_hir::Body<'tcx> { +fn hir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option<&'tcx rustc_hir::Body<'tcx>> { let hir_node = tcx.hir().get_if_local(def_id).expect("expected DefId is local"); - let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); - tcx.hir().body(fn_body_id) + hir::map::associated_body(hir_node).map(|fn_body_id| tcx.hir().body(fn_body_id)) } fn escape_html(s: &str) -> String { From fd85fd308b536bd42073f58636358ca147822947 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Tue, 27 Apr 2021 21:45:30 -0700 Subject: [PATCH 10/16] addressed review feedback --- .../rustc_mir/src/transform/coverage/spans.rs | 52 ++++++++++++++----- .../expected_show_coverage.issue-84561.txt | 26 ++++++++-- .../run-make-fulldeps/coverage/issue-84561.rs | 18 ++++++- 3 files changed, 78 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index 97eda1e37e1f..ed373f03b5a5 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -13,6 +13,7 @@ use rustc_middle::ty::TyCtxt; use rustc_span::source_map::original_sp; use rustc_span::{BytePos, ExpnKind, MacroKind, Span, Symbol}; +use std::cell::RefCell; use std::cmp::Ordering; #[derive(Debug, Copy, Clone)] @@ -68,6 +69,7 @@ impl CoverageStatement { pub(super) struct CoverageSpan { pub span: Span, pub expn_span: Span, + pub current_macro_or_none: RefCell>>, pub bcb: BasicCoverageBlock, pub coverage_statements: Vec, pub is_closure: bool, @@ -78,6 +80,7 @@ impl CoverageSpan { Self { span: fn_sig_span, expn_span: fn_sig_span, + current_macro_or_none: Default::default(), bcb: START_BCB, coverage_statements: vec![], is_closure: false, @@ -103,6 +106,7 @@ impl CoverageSpan { Self { span, expn_span, + current_macro_or_none: Default::default(), bcb, coverage_statements: vec![CoverageStatement::Statement(bb, span, stmt_index)], is_closure, @@ -118,6 +122,7 @@ impl CoverageSpan { Self { span, expn_span, + current_macro_or_none: Default::default(), bcb, coverage_statements: vec![CoverageStatement::Terminator(bb, span)], is_closure: false, @@ -174,15 +179,19 @@ impl CoverageSpan { .join("\n") } - /// If the span is part of a macro, and the macro is visible (expands directly to the given - /// body_span), returns the macro name symbol. + /// If the span is part of a macro, returns the macro name symbol. pub fn current_macro(&self) -> Option { - if let ExpnKind::Macro(MacroKind::Bang, current_macro) = - self.expn_span.ctxt().outer_expn_data().kind - { - return Some(current_macro); - } - None + self.current_macro_or_none + .borrow_mut() + .get_or_insert_with(|| { + if let ExpnKind::Macro(MacroKind::Bang, current_macro) = + self.expn_span.ctxt().outer_expn_data().kind + { + return Some(current_macro); + } + None + }) + .map(|symbol| symbol) } /// If the span is part of a macro, and the macro is visible (expands directly to the given @@ -474,8 +483,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { self.curr().expn_span.ctxt() != prev_expn_span.ctxt() }) { let merged_prefix_len = self.curr_original_span.lo() - self.curr().span.lo(); - let after_macro_bang = merged_prefix_len - + BytePos(visible_macro.to_string().bytes().count() as u32 + 1); + let after_macro_bang = + merged_prefix_len + BytePos(visible_macro.as_str().bytes().count() as u32 + 1); let mut macro_name_cov = self.curr().clone(); self.curr_mut().span = self.curr().span.with_lo(self.curr().span.lo() + after_macro_bang); @@ -766,6 +775,9 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } } +/// See `function_source_span()` for a description of the two returned spans. +/// If the MIR `Statement` is not contributive to computing coverage spans, +/// returns `None`. pub(super) fn filtered_statement_span( statement: &'a Statement<'tcx>, body_span: Span, @@ -811,6 +823,9 @@ pub(super) fn filtered_statement_span( } } +/// See `function_source_span()` for a description of the two returned spans. +/// If the MIR `Terminator` is not contributive to computing coverage spans, +/// returns `None`. pub(super) fn filtered_terminator_span( terminator: &'a Terminator<'tcx>, body_span: Span, @@ -844,8 +859,21 @@ pub(super) fn filtered_terminator_span( } } -/// Returns the span within the function source body, and the given span, which will be different -/// if the given span is an expansion (macro, syntactic sugar, etc.). +/// Returns two spans from the given span (the span associated with a +/// `Statement` or `Terminator`): +/// +/// 1. An extrapolated span (pre-expansion[^1]) corresponding to a range within +/// the function's body source. This span is guaranteed to be contained +/// within, or equal to, the `body_span`. If the extrapolated span is not +/// contained within the `body_span`, the `body_span` is returned. +/// 2. The actual `span` value from the `Statement`, before expansion. +/// +/// Only the first span is used when computing coverage code regions. The second +/// span is useful if additional expansion data is needed (such as to look up +/// the macro name for a composed span within that macro).) +/// +/// [^1]Expansions result from Rust syntax including macros, syntactic +/// sugar, etc.). #[inline] fn function_source_span(span: Span, body_span: Span) -> (Span, Span) { let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt()); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt index 5d266b5db15f..8256daa1419b 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.issue-84561.txt @@ -10,7 +10,7 @@ | Unexecuted instantiation: ::ne ------------------ 5| |struct Foo(u32); - 6| 1|fn test2() { + 6| 1|fn test3() { 7| 1| let is_true = std::env::args().len() == 1; 8| 1| let bar = Foo(1); 9| 1| assert_eq!(bar, Foo(1)); @@ -173,8 +173,24 @@ 160| 1| debug!("debug is enabled"); 161| 1|} 162| | - 163| 1|fn main() { - 164| 1| test1(); - 165| 1| test2(); - 166| 1|} + 163| |macro_rules! call_debug { + 164| | ($($arg:tt)+) => ( + 165| 1| fn call_print(s: &str) { + 166| 1| print!("{}", s); + 167| 1| } + 168| | + 169| | call_print("called from call_debug: "); + 170| | debug!($($arg)+); + 171| | ); + 172| |} + 173| | + 174| 1|fn test2() { + 175| 1| call_debug!("debug is enabled"); + 176| 1|} + 177| | + 178| 1|fn main() { + 179| 1| test1(); + 180| 1| test2(); + 181| 1| test3(); + 182| 1|} diff --git a/src/test/run-make-fulldeps/coverage/issue-84561.rs b/src/test/run-make-fulldeps/coverage/issue-84561.rs index 5c8fd0b7caea..b39a289c45e2 100644 --- a/src/test/run-make-fulldeps/coverage/issue-84561.rs +++ b/src/test/run-make-fulldeps/coverage/issue-84561.rs @@ -3,7 +3,7 @@ // expect-exit-status-101 #[derive(PartialEq, Eq)] struct Foo(u32); -fn test2() { +fn test3() { let is_true = std::env::args().len() == 1; let bar = Foo(1); assert_eq!(bar, Foo(1)); @@ -160,7 +160,23 @@ fn test1() { debug!("debug is enabled"); } +macro_rules! call_debug { + ($($arg:tt)+) => ( + fn call_print(s: &str) { + print!("{}", s); + } + + call_print("called from call_debug: "); + debug!($($arg)+); + ); +} + +fn test2() { + call_debug!("debug is enabled"); +} + fn main() { test1(); test2(); + test3(); } From 31ae3b2bdb9376b749fc1d64b531e86806e03c73 Mon Sep 17 00:00:00 2001 From: Jack Huey Date: Wed, 28 Apr 2021 10:18:52 -0400 Subject: [PATCH 11/16] Add HAS_RE_LATE_BOUND if there are bound vars --- compiler/rustc_middle/src/ty/flags.rs | 4 ++++ .../ui/lifetimes/issue-83737-erasing-bound-vars.rs | 14 ++++++++++++++ src/test/ui/lifetimes/issue-84604.rs | 9 +++++++++ 3 files changed, 27 insertions(+) create mode 100644 src/test/ui/lifetimes/issue-83737-erasing-bound-vars.rs create mode 100644 src/test/ui/lifetimes/issue-84604.rs diff --git a/compiler/rustc_middle/src/ty/flags.rs b/compiler/rustc_middle/src/ty/flags.rs index 01bc5cc761ca..92288c898274 100644 --- a/compiler/rustc_middle/src/ty/flags.rs +++ b/compiler/rustc_middle/src/ty/flags.rs @@ -59,6 +59,10 @@ impl FlagComputation { { let mut computation = FlagComputation::new(); + if !value.bound_vars().is_empty() { + computation.flags = computation.flags | TypeFlags::HAS_RE_LATE_BOUND; + } + f(&mut computation, value.skip_binder()); self.add_flags(computation.flags); diff --git a/src/test/ui/lifetimes/issue-83737-erasing-bound-vars.rs b/src/test/ui/lifetimes/issue-83737-erasing-bound-vars.rs new file mode 100644 index 000000000000..c496a3556c84 --- /dev/null +++ b/src/test/ui/lifetimes/issue-83737-erasing-bound-vars.rs @@ -0,0 +1,14 @@ +// build-pass +// compile-flags: --edition 2018 +// compile-flags: --crate-type rlib + +use std::future::Future; + +async fn handle(slf: &F) +where + F: Fn(&()) -> Box Future + Unpin>, +{ + (slf)(&()).await; +} + +fn main() {} diff --git a/src/test/ui/lifetimes/issue-84604.rs b/src/test/ui/lifetimes/issue-84604.rs new file mode 100644 index 000000000000..df8368da0a09 --- /dev/null +++ b/src/test/ui/lifetimes/issue-84604.rs @@ -0,0 +1,9 @@ +// run-pass +// compile-flags: -Zsymbol-mangling-version=v0 + +pub fn f() {} +pub trait Frob {} +fn main() { + f::>(); + f:: Frob>(); +} From 27fc7cbba9026add8a469587ea7aba9d9291941d Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 28 Apr 2021 07:19:49 -0700 Subject: [PATCH 12/16] Update LLVM for more wasm simd updates This fixes the temporary regression introduced in #84339 where the wasm target uses `fpto{s,u}i` intrinsics but the codegen for those intrinsics with the `+nontrapping-fptoint` LLVM feature wasn't very good (aka it didn't use the wasm instruction). The fixes brought in here fix that and also implement the second-to-last simd instruction in LLVM. --- src/llvm-project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llvm-project b/src/llvm-project index 0ed6038a318e..06c2b9d1e23b 160000 --- a/src/llvm-project +++ b/src/llvm-project @@ -1 +1 @@ -Subproject commit 0ed6038a318e34e3d76a9e55bdebc4cfd17f902a +Subproject commit 06c2b9d1e23b73ffb5059769542097b86d5a4623 From e6a731eb90fe3d47d89416e199832af4248399f6 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 28 Apr 2021 16:28:59 +0100 Subject: [PATCH 13/16] Be stricter about rejecting LLVM reserved registers in asm! --- compiler/rustc_target/src/asm/aarch64.rs | 10 +++--- compiler/rustc_target/src/asm/arm.rs | 3 +- compiler/rustc_target/src/asm/hexagon.rs | 3 +- compiler/rustc_target/src/asm/riscv.rs | 3 +- compiler/rustc_target/src/asm/x86.rs | 32 +++++++++++++++++-- .../unstable-book/src/library-features/asm.md | 25 ++++++++------- src/test/codegen/asm-multiple-options.rs | 2 +- src/test/codegen/asm-options.rs | 2 +- src/test/pretty/asm.pp | 2 +- src/test/pretty/asm.rs | 2 +- 10 files changed, 59 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_target/src/asm/aarch64.rs b/compiler/rustc_target/src/asm/aarch64.rs index e7c9edea7653..dd51574efca0 100644 --- a/compiler/rustc_target/src/asm/aarch64.rs +++ b/compiler/rustc_target/src/asm/aarch64.rs @@ -83,10 +83,8 @@ def_regs! { x13: reg = ["x13", "w13"], x14: reg = ["x14", "w14"], x15: reg = ["x15", "w15"], - x16: reg = ["x16", "w16"], x17: reg = ["x17", "w17"], x18: reg = ["x18", "w18"], - x19: reg = ["x19", "w19"], x20: reg = ["x20", "w20"], x21: reg = ["x21", "w21"], x22: reg = ["x22", "w22"], @@ -96,7 +94,7 @@ def_regs! { x26: reg = ["x26", "w26"], x27: reg = ["x27", "w27"], x28: reg = ["x28", "w28"], - x30: reg = ["x30", "w30", "lr"], + x30: reg = ["x30", "w30", "lr", "wlr"], v0: vreg, vreg_low16 = ["v0", "b0", "h0", "s0", "d0", "q0"], v1: vreg, vreg_low16 = ["v1", "b1", "h1", "s1", "d1", "q1"], v2: vreg, vreg_low16 = ["v2", "b2", "h2", "s2", "d2", "q2"], @@ -129,7 +127,11 @@ def_regs! { v29: vreg = ["v29", "b29", "h29", "s29", "d29", "q29"], v30: vreg = ["v30", "b30", "h30", "s30", "d30", "q30"], v31: vreg = ["v31", "b31", "h31", "s31", "d31", "q31"], - #error = ["x29", "fp"] => + #error = ["x16", "w16"] => + "x16 is used internally by LLVM and cannot be used as an operand for inline asm", + #error = ["x19", "w19"] => + "x19 is used internally by LLVM and cannot be used as an operand for inline asm", + #error = ["x29", "w29", "fp", "wfp"] => "the frame pointer cannot be used as an operand for inline asm", #error = ["sp", "wsp"] => "the stack pointer cannot be used as an operand for inline asm", diff --git a/compiler/rustc_target/src/asm/arm.rs b/compiler/rustc_target/src/asm/arm.rs index a7a708fe7dec..4c323fc35d64 100644 --- a/compiler/rustc_target/src/asm/arm.rs +++ b/compiler/rustc_target/src/asm/arm.rs @@ -98,7 +98,6 @@ def_regs! { r5: reg, reg_thumb = ["r5", "v2"], r7: reg, reg_thumb = ["r7", "v4"] % frame_pointer_r7, r8: reg = ["r8", "v5"], - r9: reg = ["r9", "v6", "rfp"], r10: reg = ["r10", "sl"], r11: reg = ["r11", "fp"] % frame_pointer_r11, r12: reg = ["r12", "ip"], @@ -185,6 +184,8 @@ def_regs! { q15: qreg = ["q15"], #error = ["r6", "v3"] => "r6 is used internally by LLVM and cannot be used as an operand for inline asm", + #error = ["r9", "v6", "rfp"] => + "r9 is used internally by LLVM and cannot be used as an operand for inline asm", #error = ["r13", "sp"] => "the stack pointer cannot be used as an operand for inline asm", #error = ["r15", "pc"] => diff --git a/compiler/rustc_target/src/asm/hexagon.rs b/compiler/rustc_target/src/asm/hexagon.rs index d41941d0b4cd..74afddb69dc7 100644 --- a/compiler/rustc_target/src/asm/hexagon.rs +++ b/compiler/rustc_target/src/asm/hexagon.rs @@ -60,7 +60,6 @@ def_regs! { r16: reg = ["r16"], r17: reg = ["r17"], r18: reg = ["r18"], - r19: reg = ["r19"], r20: reg = ["r20"], r21: reg = ["r21"], r22: reg = ["r22"], @@ -70,6 +69,8 @@ def_regs! { r26: reg = ["r26"], r27: reg = ["r27"], r28: reg = ["r28"], + #error = ["r19"] => + "r19 is used internally by LLVM and cannot be used as an operand for inline asm", #error = ["r29", "sp"] => "the stack pointer cannot be used as an operand for inline asm", #error = ["r30", "fr"] => diff --git a/compiler/rustc_target/src/asm/riscv.rs b/compiler/rustc_target/src/asm/riscv.rs index 185d6ac8246c..e276a9175f9a 100644 --- a/compiler/rustc_target/src/asm/riscv.rs +++ b/compiler/rustc_target/src/asm/riscv.rs @@ -66,7 +66,6 @@ def_regs! { x5: reg = ["x5", "t0"], x6: reg = ["x6", "t1"], x7: reg = ["x7", "t2"], - x9: reg = ["x9", "s1"], x10: reg = ["x10", "a0"], x11: reg = ["x11", "a1"], x12: reg = ["x12", "a2"], @@ -121,6 +120,8 @@ def_regs! { f29: freg = ["f29", "ft9"], f30: freg = ["f30", "ft10"], f31: freg = ["f31", "ft11"], + #error = ["x9", "s1"] => + "s1 is used internally by LLVM and cannot be used as an operand for inline asm", #error = ["x8", "s0", "fp"] => "the frame pointer cannot be used as an operand for inline asm", #error = ["x2", "sp"] => diff --git a/compiler/rustc_target/src/asm/x86.rs b/compiler/rustc_target/src/asm/x86.rs index 90660dad4c2a..48f83ca7cd49 100644 --- a/compiler/rustc_target/src/asm/x86.rs +++ b/compiler/rustc_target/src/asm/x86.rs @@ -152,13 +152,41 @@ fn high_byte( } } +fn rbx_reserved( + arch: InlineAsmArch, + _has_feature: impl FnMut(&str) -> bool, + _target: &Target, +) -> Result<(), &'static str> { + match arch { + InlineAsmArch::X86 => Ok(()), + InlineAsmArch::X86_64 => { + Err("rbx is used internally by LLVM and cannot be used as an operand for inline asm") + } + _ => unreachable!(), + } +} + +fn esi_reserved( + arch: InlineAsmArch, + _has_feature: impl FnMut(&str) -> bool, + _target: &Target, +) -> Result<(), &'static str> { + match arch { + InlineAsmArch::X86 => { + Err("esi is used internally by LLVM and cannot be used as an operand for inline asm") + } + InlineAsmArch::X86_64 => Ok(()), + _ => unreachable!(), + } +} + def_regs! { X86 X86InlineAsmReg X86InlineAsmRegClass { ax: reg, reg_abcd = ["ax", "eax", "rax"], - bx: reg, reg_abcd = ["bx", "ebx", "rbx"], + bx: reg, reg_abcd = ["bx", "ebx", "rbx"] % rbx_reserved, cx: reg, reg_abcd = ["cx", "ecx", "rcx"], dx: reg, reg_abcd = ["dx", "edx", "rdx"], - si: reg = ["si", "esi", "rsi"], + si: reg = ["si", "esi", "rsi"] % esi_reserved, di: reg = ["di", "edi", "rdi"], r8: reg = ["r8", "r8w", "r8d"] % x86_64_only, r9: reg = ["r9", "r9w", "r9d"] % x86_64_only, diff --git a/src/doc/unstable-book/src/library-features/asm.md b/src/doc/unstable-book/src/library-features/asm.md index 4f9033cedc3f..7c2bf6218553 100644 --- a/src/doc/unstable-book/src/library-features/asm.md +++ b/src/doc/unstable-book/src/library-features/asm.md @@ -535,20 +535,20 @@ Here is the list of currently supported register classes: | Architecture | Register class | Registers | LLVM constraint code | | ------------ | -------------- | --------- | -------------------- | -| x86 | `reg` | `ax`, `bx`, `cx`, `dx`, `si`, `di`, `r[8-15]` (x86-64 only) | `r` | +| x86 | `reg` | `ax`, `bx`, `cx`, `dx`, `si`, `di`, `bp`, `r[8-15]` (x86-64 only) | `r` | | x86 | `reg_abcd` | `ax`, `bx`, `cx`, `dx` | `Q` | | x86-32 | `reg_byte` | `al`, `bl`, `cl`, `dl`, `ah`, `bh`, `ch`, `dh` | `q` | -| x86-64 | `reg_byte`\* | `al`, `bl`, `cl`, `dl`, `sil`, `dil`, `r[8-15]b` | `q` | +| x86-64 | `reg_byte`\* | `al`, `bl`, `cl`, `dl`, `sil`, `dil`, `bpl`, `r[8-15]b` | `q` | | x86 | `xmm_reg` | `xmm[0-7]` (x86) `xmm[0-15]` (x86-64) | `x` | | x86 | `ymm_reg` | `ymm[0-7]` (x86) `ymm[0-15]` (x86-64) | `x` | | x86 | `zmm_reg` | `zmm[0-7]` (x86) `zmm[0-31]` (x86-64) | `v` | | x86 | `kreg` | `k[1-7]` | `Yk` | -| AArch64 | `reg` | `x[0-28]`, `x30` | `r` | +| AArch64 | `reg` | `x[0-30]` | `r` | | AArch64 | `vreg` | `v[0-31]` | `w` | | AArch64 | `vreg_low16` | `v[0-15]` | `x` | -| ARM | `reg` | `r[0-5]` `r7`\*, `r[8-10]`, `r11`\*, `r12`, `r14` | `r` | +| ARM | `reg` | `r[0-12]`, `r14` | `r` | | ARM (Thumb) | `reg_thumb` | `r[0-r7]` | `l` | -| ARM (ARM) | `reg_thumb` | `r[0-r10]`, `r12`, `r14` | `l` | +| ARM (ARM) | `reg_thumb` | `r[0-r12]`, `r14` | `l` | | ARM | `sreg` | `s[0-31]` | `t` | | ARM | `sreg_low16` | `s[0-15]` | `x` | | ARM | `dreg` | `d[0-31]` | `w` | @@ -573,9 +573,7 @@ Here is the list of currently supported register classes: > > Note #3: NVPTX doesn't have a fixed register set, so named registers are not supported. > -> Note #4: On ARM the frame pointer is either `r7` or `r11` depending on the platform. -> -> Note #5: WebAssembly doesn't have registers, so named registers are not supported. +> Note #4: WebAssembly doesn't have registers, so named registers are not supported. Additional register classes may be added in the future based on demand (e.g. MMX, x87, etc). @@ -677,13 +675,15 @@ Some registers cannot be used for input or output operands: | All | `sp` | The stack pointer must be restored to its original value at the end of an asm code block. | | All | `bp` (x86), `x29` (AArch64), `x8` (RISC-V), `fr` (Hexagon), `$fp` (MIPS) | The frame pointer cannot be used as an input or output. | | ARM | `r7` or `r11` | On ARM the frame pointer can be either `r7` or `r11` depending on the target. The frame pointer cannot be used as an input or output. | -| ARM | `r6` | `r6` is used internally by LLVM as a base pointer and therefore cannot be used as an input or output. | +| All | `si` (x86-32), `bx` (x86-64), `r6` (ARM), `x19` (AArch64), `r19` (Hexagon), `x9` (RISC-V) | This is used internally by LLVM as a "base pointer" for functions with complex stack frames. | | x86 | `k0` | This is a constant zero register which can't be modified. | | x86 | `ip` | This is the program counter, not a real register. | | x86 | `mm[0-7]` | MMX registers are not currently supported (but may be in the future). | | x86 | `st([0-7])` | x87 registers are not currently supported (but may be in the future). | | AArch64 | `xzr` | This is a constant zero register which can't be modified. | +| AArch64 | `x16` | This is used internally by LLVM for speculative load hardening. | | ARM | `pc` | This is the program counter, not a real register. | +| ARM | `r9` | This is a reserved register on some ARM targets. | | MIPS | `$0` or `$zero` | This is a constant zero register which can't be modified. | | MIPS | `$1` or `$at` | Reserved for assembler. | | MIPS | `$26`/`$k0`, `$27`/`$k1` | OS-reserved registers. | @@ -693,9 +693,10 @@ Some registers cannot be used for input or output operands: | RISC-V | `gp`, `tp` | These registers are reserved and cannot be used as inputs or outputs. | | Hexagon | `lr` | This is the link register which cannot be used as an input or output. | -In some cases LLVM will allocate a "reserved register" for `reg` operands even though this register cannot be explicitly specified. Assembly code making use of reserved registers should be careful since `reg` operands may alias with those registers. Reserved registers are: -- The frame pointer on all architectures. -- `r6` on ARM. +In some cases LLVM will allocate a "reserved register" for `reg` operands even though this register cannot be explicitly specified. Assembly code making use of reserved registers should be careful since `reg` operands may alias with those registers. Reserved registers are the frame pointer and base pointer +- The frame pointer and LLVM base pointer on all architectures. +- `x16` on AArch64. +- `r6` and `r9` on ARM. ## Template modifiers diff --git a/src/test/codegen/asm-multiple-options.rs b/src/test/codegen/asm-multiple-options.rs index c702742bf1a6..baf9f3e9bd14 100644 --- a/src/test/codegen/asm-multiple-options.rs +++ b/src/test/codegen/asm-multiple-options.rs @@ -10,7 +10,7 @@ #[no_mangle] pub unsafe fn pure(x: i32) { let y: i32; - asm!("", out("ax") y, in("bx") x, options(pure), options(nomem)); + asm!("", out("ax") y, in("cx") x, options(pure), options(nomem)); } pub static mut VAR: i32 = 0; diff --git a/src/test/codegen/asm-options.rs b/src/test/codegen/asm-options.rs index 21e7eb437963..70391661b0cf 100644 --- a/src/test/codegen/asm-options.rs +++ b/src/test/codegen/asm-options.rs @@ -10,7 +10,7 @@ #[no_mangle] pub unsafe fn pure(x: i32) { let y: i32; - asm!("", out("ax") y, in("bx") x, options(pure, nomem)); + asm!("", out("ax") y, in("cx") x, options(pure, nomem)); } // CHECK-LABEL: @noreturn diff --git a/src/test/pretty/asm.pp b/src/test/pretty/asm.pp index c86d8a119718..a2065039692b 100644 --- a/src/test/pretty/asm.pp +++ b/src/test/pretty/asm.pp @@ -21,7 +21,7 @@ asm!("{0}", out(reg) a); asm!("{0}", inout(reg) b); asm!("{0} {1}", out(reg) _, inlateout(reg) b => _); - asm!("", out("al") _, lateout("rbx") _); + asm!("", out("al") _, lateout("rcx") _); asm!("inst1\ninst2"); asm!("inst1 {0}, 42\ninst2 {1}, 24", in(reg) a, out(reg) b); asm!("inst2 {1}, 24\ninst1 {0}, 42", in(reg) a, out(reg) b); diff --git a/src/test/pretty/asm.rs b/src/test/pretty/asm.rs index 33f25e5216b4..1156ab769a04 100644 --- a/src/test/pretty/asm.rs +++ b/src/test/pretty/asm.rs @@ -15,7 +15,7 @@ pub fn main() { asm!("{0}", out(reg) a); asm!("{name}", name = inout(reg) b); asm!("{} {}", out(reg) _, inlateout(reg) b => _); - asm!("", out("al") _, lateout("rbx") _); + asm!("", out("al") _, lateout("rcx") _); asm!("inst1", "inst2"); asm!("inst1 {}, 42", "inst2 {}, 24", in(reg) a, out(reg) b); asm!("inst2 {1}, 24", "inst1 {0}, 42", in(reg) a, out(reg) b); From c9fbaa6a9f68c74608324cc01e5985d9635cb021 Mon Sep 17 00:00:00 2001 From: Jack Huey <31162821+jackh726@users.noreply.github.com> Date: Wed, 28 Apr 2021 15:51:54 -0400 Subject: [PATCH 14/16] Add a comment --- compiler/rustc_middle/src/ty/flags.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_middle/src/ty/flags.rs b/compiler/rustc_middle/src/ty/flags.rs index 92288c898274..d2e233f67d7b 100644 --- a/compiler/rustc_middle/src/ty/flags.rs +++ b/compiler/rustc_middle/src/ty/flags.rs @@ -59,6 +59,9 @@ impl FlagComputation { { let mut computation = FlagComputation::new(); + // In some cases, there are binders with variables that are unused (e.g., `for<'a> fn(u32)`). + // Set the flag to represent the `'a` in this example. Note that if there are late bound types + // or consts, this flag will also get set. if !value.bound_vars().is_empty() { computation.flags = computation.flags | TypeFlags::HAS_RE_LATE_BOUND; } From 51b0cb25cbc8932ef06cccbc3866ed912248b1fb Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 28 Apr 2021 21:47:26 +0000 Subject: [PATCH 15/16] Add integration test for `--remap-pathh-prefix` --- src/test/incremental/commandline-args.rs | 7 +++++-- .../run-make-fulldeps/incr-add-rust-src-component/Makefile | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/incremental/commandline-args.rs b/src/test/incremental/commandline-args.rs index 08a0232f661f..35b7183db7fa 100644 --- a/src/test/incremental/commandline-args.rs +++ b/src/test/incremental/commandline-args.rs @@ -2,20 +2,23 @@ // the cache while changing an untracked one doesn't. // ignore-asmjs wasm2js does not support source maps yet -// revisions:rpass1 rpass2 rpass3 +// revisions:rpass1 rpass2 rpass3 rpass4 // compile-flags: -Z query-dep-graph #![feature(rustc_attrs)] #![rustc_partition_codegened(module="commandline_args", cfg="rpass2")] #![rustc_partition_reused(module="commandline_args", cfg="rpass3")] +#![rustc_partition_codegened(module="commandline_args", cfg="rpass4")] // Between revisions 1 and 2, we are changing the debuginfo-level, which should // invalidate the cache. Between revisions 2 and 3, we are adding `--verbose` -// which should have no effect on the cache: +// which should have no effect on the cache. Between revisions, we are adding +// `--remap-path-prefix` which should invalidate the cache: //[rpass1] compile-flags: -C debuginfo=0 //[rpass2] compile-flags: -C debuginfo=2 //[rpass3] compile-flags: -C debuginfo=2 --verbose +//[rpass4] compile-flags: -C debuginfo=2 --verbose --remap-path-prefix=/home/bors/rust=src pub fn main() { // empty diff --git a/src/test/run-make-fulldeps/incr-add-rust-src-component/Makefile b/src/test/run-make-fulldeps/incr-add-rust-src-component/Makefile index 50ff3dd56ce9..371f94715a8c 100644 --- a/src/test/run-make-fulldeps/incr-add-rust-src-component/Makefile +++ b/src/test/run-make-fulldeps/incr-add-rust-src-component/Makefile @@ -1,7 +1,7 @@ -include ../tools.mk # rust-lang/rust#70924: Test that if we add rust-src component in between two -# incremetnal compiles, the compiler does not ICE on the second. +# incremental compiles, the compiler does not ICE on the second. # This test uses `ln -s` rather than copying to save testing time, but its # usage doesn't work on windows. So ignore windows. From 8c0469552e879f6319f8f96db660bab9eae1de5c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 29 Apr 2021 10:40:10 +0200 Subject: [PATCH 16/16] Remove unnecessary CSS rules for search results --- src/librustdoc/html/static/rustdoc.css | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index a024fa49b0e8..3eda2bea7fe6 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -385,13 +385,6 @@ nav.sub { position: relative; } -#results { - position: absolute; - right: 0; - left: 0; - overflow: auto; -} - #results > table { width: 100%; table-layout: fixed;