From 3dd0a7d6eba173e5169b66fd4417025916aa0f3e Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Sun, 25 Oct 2020 20:41:28 +0900 Subject: [PATCH 01/26] Do not call `unwrap` with `signatures` option enabled --- compiler/rustc_save_analysis/src/sig.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_save_analysis/src/sig.rs b/compiler/rustc_save_analysis/src/sig.rs index 747e198cd9324..446a0723d8c35 100644 --- a/compiler/rustc_save_analysis/src/sig.rs +++ b/compiler/rustc_save_analysis/src/sig.rs @@ -21,7 +21,7 @@ // references. // // Signatures do not include visibility info. I'm not sure if this is a feature -// or an ommission (FIXME). +// or an omission (FIXME). // // FIXME where clauses need implementing, defs/refs in generics are mostly missing. @@ -677,7 +677,7 @@ impl<'hir> Sig for hir::Variant<'hir> { let mut text = self.ident.to_string(); match self.data { hir::VariantData::Struct(fields, r) => { - let id = parent_id.unwrap(); + let id = parent_id.ok_or("Missing id for Variant's parent")?; let name_def = SigElement { id: id_from_hir_id(id, scx), start: offset, From 443b45fa9fd87cf0939e80a64bead413530e375c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 11 Nov 2020 20:10:15 +0300 Subject: [PATCH 02/26] rustc_target: Change os from "unknown" to "none" for bare metal targets x86_64-fortanix-unknown-sgx and wasm32-unknown-unknown still have os == "unknown" because both have libstd --- compiler/rustc_target/src/spec/avr_gnu_base.rs | 1 - compiler/rustc_target/src/spec/mod.rs | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/avr_gnu_base.rs b/compiler/rustc_target/src/spec/avr_gnu_base.rs index 9cc10032c71da..67a7684da2c70 100644 --- a/compiler/rustc_target/src/spec/avr_gnu_base.rs +++ b/compiler/rustc_target/src/spec/avr_gnu_base.rs @@ -11,7 +11,6 @@ pub fn target(target_cpu: String) -> Target { pointer_width: 16, options: TargetOptions { c_int_width: "16".to_string(), - os: "unknown".to_string(), cpu: target_cpu.clone(), exe_suffix: ".elf".to_string(), diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 2a4ae786fb752..dc8c41ecedc2c 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -713,6 +713,9 @@ pub struct TargetOptions { /// Width of c_int type. Defaults to "32". pub c_int_width: String, /// OS name to use for conditional compilation. Defaults to "none". + /// "none" implies a bare metal target without `std` library. + /// A couple of targets having `std` also use "unknown" as an `os` value, + /// but they are exceptions. pub os: String, /// Environment name to use for conditional compilation. Defaults to "". pub env: String, From 1def24c5f45df9f8d101cdaec4b6485aa9ad1aae Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 11 Nov 2020 20:40:51 +0300 Subject: [PATCH 03/26] rustc_target: Normalize vendor from "" to "unknown" for all targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Majority of targets use "unknown" vendor and changing it from "unknown" to omitted doesn't make sense. From the LLVM docs (https://clang.llvm.org/docs/CrossCompilation.html#target-triple): >Most of the time it can be omitted (and Unknown) will be assumed, which sets the defaults for the specified architecture. >When a parameter is not important, it can be omitted, or you can choose unknown and the defaults will be used. If you choose a parameter that Clang doesn’t know, like blerg, it’ll ignore and assume unknown --- compiler/rustc_target/src/spec/aarch64_unknown_none.rs | 1 - .../rustc_target/src/spec/aarch64_unknown_none_softfloat.rs | 1 - compiler/rustc_target/src/spec/armebv7r_none_eabi.rs | 1 - compiler/rustc_target/src/spec/armebv7r_none_eabihf.rs | 1 - compiler/rustc_target/src/spec/armv7a_none_eabi.rs | 4 ---- compiler/rustc_target/src/spec/armv7a_none_eabihf.rs | 1 - compiler/rustc_target/src/spec/armv7r_none_eabi.rs | 1 - compiler/rustc_target/src/spec/armv7r_none_eabihf.rs | 1 - compiler/rustc_target/src/spec/fuchsia_base.rs | 1 - compiler/rustc_target/src/spec/mipsel_unknown_none.rs | 1 - compiler/rustc_target/src/spec/mod.rs | 6 +++--- compiler/rustc_target/src/spec/msp430_none_elf.rs | 1 - compiler/rustc_target/src/spec/thumb_base.rs | 1 - compiler/rustc_target/src/spec/wasm32_wasi.rs | 1 - 14 files changed, 3 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_target/src/spec/aarch64_unknown_none.rs b/compiler/rustc_target/src/spec/aarch64_unknown_none.rs index d0ad45153d677..c9f622820dea7 100644 --- a/compiler/rustc_target/src/spec/aarch64_unknown_none.rs +++ b/compiler/rustc_target/src/spec/aarch64_unknown_none.rs @@ -10,7 +10,6 @@ use super::{LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, Target, TargetOp pub fn target() -> Target { let opts = TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), linker: Some("rust-lld".to_owned()), features: "+strict-align,+neon,+fp-armv8".to_string(), diff --git a/compiler/rustc_target/src/spec/aarch64_unknown_none_softfloat.rs b/compiler/rustc_target/src/spec/aarch64_unknown_none_softfloat.rs index 41bd2182905c9..0811871c993aa 100644 --- a/compiler/rustc_target/src/spec/aarch64_unknown_none_softfloat.rs +++ b/compiler/rustc_target/src/spec/aarch64_unknown_none_softfloat.rs @@ -10,7 +10,6 @@ use super::{LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, Target, TargetOp pub fn target() -> Target { let opts = TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), linker: Some("rust-lld".to_owned()), features: "+strict-align,-neon,-fp-armv8".to_string(), diff --git a/compiler/rustc_target/src/spec/armebv7r_none_eabi.rs b/compiler/rustc_target/src/spec/armebv7r_none_eabi.rs index 3685630572371..c6586b79b87f8 100644 --- a/compiler/rustc_target/src/spec/armebv7r_none_eabi.rs +++ b/compiler/rustc_target/src/spec/armebv7r_none_eabi.rs @@ -12,7 +12,6 @@ pub fn target() -> Target { options: TargetOptions { endian: "big".to_string(), - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), executables: true, linker: Some("rust-lld".to_owned()), diff --git a/compiler/rustc_target/src/spec/armebv7r_none_eabihf.rs b/compiler/rustc_target/src/spec/armebv7r_none_eabihf.rs index 2ff3c8950c483..e3d4397f6123f 100644 --- a/compiler/rustc_target/src/spec/armebv7r_none_eabihf.rs +++ b/compiler/rustc_target/src/spec/armebv7r_none_eabihf.rs @@ -12,7 +12,6 @@ pub fn target() -> Target { options: TargetOptions { endian: "big".to_string(), - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), executables: true, linker: Some("rust-lld".to_owned()), diff --git a/compiler/rustc_target/src/spec/armv7a_none_eabi.rs b/compiler/rustc_target/src/spec/armv7a_none_eabi.rs index 742b403cff90c..74deab0191652 100644 --- a/compiler/rustc_target/src/spec/armv7a_none_eabi.rs +++ b/compiler/rustc_target/src/spec/armv7a_none_eabi.rs @@ -10,9 +10,6 @@ // bare-metal binaries (the `gcc` linker has the advantage that it knows where C // libraries and crt*.o are but it's not much of an advantage here); LLD is also // faster -// - `os` set to `none`. rationale: matches `thumb` targets -// - `env` and `vendor` are set to an empty string. rationale: matches `thumb` -// targets // - `panic_strategy` set to `abort`. rationale: matches `thumb` targets // - `relocation-model` set to `static`; also no PIE, no relro and no dynamic // linking. rationale: matches `thumb` targets @@ -21,7 +18,6 @@ use super::{LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, Target, TargetOp pub fn target() -> Target { let opts = TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), linker: Some("rust-lld".to_owned()), features: "+v7,+thumb2,+soft-float,-neon,+strict-align".to_string(), diff --git a/compiler/rustc_target/src/spec/armv7a_none_eabihf.rs b/compiler/rustc_target/src/spec/armv7a_none_eabihf.rs index b9cda18d6b46f..c5c720f5fbde4 100644 --- a/compiler/rustc_target/src/spec/armv7a_none_eabihf.rs +++ b/compiler/rustc_target/src/spec/armv7a_none_eabihf.rs @@ -9,7 +9,6 @@ use super::{LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, Target, TargetOp pub fn target() -> Target { let opts = TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), linker: Some("rust-lld".to_owned()), features: "+v7,+vfp3,-d32,+thumb2,-neon,+strict-align".to_string(), diff --git a/compiler/rustc_target/src/spec/armv7r_none_eabi.rs b/compiler/rustc_target/src/spec/armv7r_none_eabi.rs index 440c2434907be..3f49bd8786937 100644 --- a/compiler/rustc_target/src/spec/armv7r_none_eabi.rs +++ b/compiler/rustc_target/src/spec/armv7r_none_eabi.rs @@ -11,7 +11,6 @@ pub fn target() -> Target { arch: "arm".to_string(), options: TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), executables: true, linker: Some("rust-lld".to_owned()), diff --git a/compiler/rustc_target/src/spec/armv7r_none_eabihf.rs b/compiler/rustc_target/src/spec/armv7r_none_eabihf.rs index c1bf332a72ddf..9b2e8a8058fe7 100644 --- a/compiler/rustc_target/src/spec/armv7r_none_eabihf.rs +++ b/compiler/rustc_target/src/spec/armv7r_none_eabihf.rs @@ -11,7 +11,6 @@ pub fn target() -> Target { arch: "arm".to_string(), options: TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), executables: true, linker: Some("rust-lld".to_owned()), diff --git a/compiler/rustc_target/src/spec/fuchsia_base.rs b/compiler/rustc_target/src/spec/fuchsia_base.rs index e467c7c8f21e9..5c39773cbe381 100644 --- a/compiler/rustc_target/src/spec/fuchsia_base.rs +++ b/compiler/rustc_target/src/spec/fuchsia_base.rs @@ -21,7 +21,6 @@ pub fn opts() -> TargetOptions { TargetOptions { os: "fuchsia".to_string(), - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), linker: Some("rust-lld".to_owned()), lld_flavor: LldFlavor::Ld, diff --git a/compiler/rustc_target/src/spec/mipsel_unknown_none.rs b/compiler/rustc_target/src/spec/mipsel_unknown_none.rs index a8005927a7beb..0f9d3c3de1543 100644 --- a/compiler/rustc_target/src/spec/mipsel_unknown_none.rs +++ b/compiler/rustc_target/src/spec/mipsel_unknown_none.rs @@ -14,7 +14,6 @@ pub fn target() -> Target { arch: "mips".to_string(), options: TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), cpu: "mips32r2".to_string(), features: "+mips32r2,+soft-float,+noabicalls".to_string(), diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index dc8c41ecedc2c..129cab6fa4493 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -712,14 +712,14 @@ pub struct TargetOptions { pub endian: String, /// Width of c_int type. Defaults to "32". pub c_int_width: String, - /// OS name to use for conditional compilation. Defaults to "none". + /// OS name to use for conditional compilation (`target_os`). Defaults to "none". /// "none" implies a bare metal target without `std` library. /// A couple of targets having `std` also use "unknown" as an `os` value, /// but they are exceptions. pub os: String, - /// Environment name to use for conditional compilation. Defaults to "". + /// Environment name to use for conditional compilation (`target_env`). Defaults to "". pub env: String, - /// Vendor name to use for conditional compilation. Defaults to "unknown". + /// Vendor name to use for conditional compilation (`target_vendor`). Defaults to "unknown". pub vendor: String, /// Default linker flavor used if `-C linker-flavor` or `-C linker` are not passed /// on the command line. Defaults to `LinkerFlavor::Gcc`. diff --git a/compiler/rustc_target/src/spec/msp430_none_elf.rs b/compiler/rustc_target/src/spec/msp430_none_elf.rs index ef966cb702ec4..cc2578aa578e8 100644 --- a/compiler/rustc_target/src/spec/msp430_none_elf.rs +++ b/compiler/rustc_target/src/spec/msp430_none_elf.rs @@ -9,7 +9,6 @@ pub fn target() -> Target { options: TargetOptions { c_int_width: "16".to_string(), - vendor: String::new(), executables: true, // The LLVM backend currently can't generate object files. To diff --git a/compiler/rustc_target/src/spec/thumb_base.rs b/compiler/rustc_target/src/spec/thumb_base.rs index e550467502754..ec24807fec4ea 100644 --- a/compiler/rustc_target/src/spec/thumb_base.rs +++ b/compiler/rustc_target/src/spec/thumb_base.rs @@ -32,7 +32,6 @@ use crate::spec::{LinkerFlavor, LldFlavor, PanicStrategy, RelocModel, TargetOpti pub fn opts() -> TargetOptions { // See rust-lang/rfcs#1645 for a discussion about these defaults TargetOptions { - vendor: String::new(), linker_flavor: LinkerFlavor::Lld(LldFlavor::Ld), executables: true, // In most cases, LLD is good enough diff --git a/compiler/rustc_target/src/spec/wasm32_wasi.rs b/compiler/rustc_target/src/spec/wasm32_wasi.rs index 9c697674f397a..3f44acdc36b2d 100644 --- a/compiler/rustc_target/src/spec/wasm32_wasi.rs +++ b/compiler/rustc_target/src/spec/wasm32_wasi.rs @@ -79,7 +79,6 @@ pub fn target() -> Target { let mut options = wasm32_base::options(); options.os = "wasi".to_string(); - options.vendor = String::new(); options.linker_flavor = LinkerFlavor::Lld(LldFlavor::Wasm); options .pre_link_args From e0a8f22053868982be59a82865fa11f4fbdf6af0 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 11 Nov 2020 20:59:37 +0300 Subject: [PATCH 04/26] rustc_target: Make sure that in-tree targets follow conventions for os and vendor values --- compiler/rustc_target/src/spec/tests/tests_impl.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/compiler/rustc_target/src/spec/tests/tests_impl.rs b/compiler/rustc_target/src/spec/tests/tests_impl.rs index f348df7d5a716..bde7cb09bdf82 100644 --- a/compiler/rustc_target/src/spec/tests/tests_impl.rs +++ b/compiler/rustc_target/src/spec/tests/tests_impl.rs @@ -36,5 +36,18 @@ impl Target { && self.post_link_objects_fallback.is_empty()) || self.crt_objects_fallback.is_some() ); + // Keep the default "unknown" vendor instead. + assert_ne!(self.vendor, ""); + if !self.can_use_os_unknown() { + // Keep the default "none" for bare metal targets instead. + assert_ne!(self.os, "unknown"); + } + } + + // Add your target to the whitelist if it has `std` library + // and you certainly want "unknown" for the OS name. + fn can_use_os_unknown(&self) -> bool { + self.llvm_target == "wasm32-unknown-unknown" + || (self.env == "sgx" && self.vendor == "fortanix") } } From f5e67b5ee1d442ddfc580224c69f260a07b8840b Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 12 Nov 2020 01:05:27 +0100 Subject: [PATCH 05/26] Add a test for r# identifiers --- .../rustdoc/raw-ident-eliminate-r-hashtag.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs diff --git a/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs new file mode 100644 index 0000000000000..2221b5d47acf9 --- /dev/null +++ b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs @@ -0,0 +1,17 @@ +pub mod internal { + pub struct r#mod; + + /// See [name], [other name] + /// + /// [name]: mod + /// [other name]: ../internal/struct.mod.html + // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'name' + // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'other name' + pub struct B; +} + +/// See [name]. +/// +/// [name]: internal::mod +// @has 'raw_ident_eliminate_r_hashtag/struct.A.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'name' +struct A; From 9c7069645cf919cb0a848175a208d6eb4d92802e Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 12 Nov 2020 01:39:06 +0100 Subject: [PATCH 06/26] Ignore tidy linelength --- src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs index 2221b5d47acf9..d758d9936132f 100644 --- a/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs +++ b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs @@ -1,3 +1,5 @@ +// ignore-tidy-linelength + pub mod internal { pub struct r#mod; From bd0eb07af2deaff2c9f1e7553ea97341787779cd Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 2 Nov 2020 21:32:48 -0800 Subject: [PATCH 07/26] Added some unit tests as requested As discussed in PR #78267, for example: * https://github.com/rust-lang/rust/pull/78267#discussion_r515404722 * https://github.com/rust-lang/rust/pull/78267#discussion_r515405958 --- Cargo.lock | 8 + compiler/rustc_mir/Cargo.toml | 3 + .../src/transform/coverage/counters.rs | 4 +- .../rustc_mir/src/transform/coverage/debug.rs | 16 +- .../rustc_mir/src/transform/coverage/graph.rs | 16 +- .../rustc_mir/src/transform/coverage/mod.rs | 5 +- .../rustc_mir/src/transform/coverage/spans.rs | 6 +- .../transform/coverage/test_macros/Cargo.toml | 12 + .../transform/coverage/test_macros/src/lib.rs | 8 + .../rustc_mir/src/transform/coverage/tests.rs | 631 ++++++++++++++++++ 10 files changed, 687 insertions(+), 22 deletions(-) create mode 100644 compiler/rustc_mir/src/transform/coverage/test_macros/Cargo.toml create mode 100644 compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs create mode 100644 compiler/rustc_mir/src/transform/coverage/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 45a19fd79634e..4ab46a9cc871b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -721,6 +721,13 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9a21fa21941700a3cd8fcb4091f361a6a712fac632f85d9f487cc892045d55c6" +[[package]] +name = "coverage_test_macros" +version = "0.0.0" +dependencies = [ + "proc-macro2", +] + [[package]] name = "cpuid-bool" version = "0.1.2" @@ -3922,6 +3929,7 @@ dependencies = [ name = "rustc_mir" version = "0.0.0" dependencies = [ + "coverage_test_macros", "either", "itertools 0.9.0", "polonius-engine", diff --git a/compiler/rustc_mir/Cargo.toml b/compiler/rustc_mir/Cargo.toml index 487668cfa1109..9bfd1da039120 100644 --- a/compiler/rustc_mir/Cargo.toml +++ b/compiler/rustc_mir/Cargo.toml @@ -31,3 +31,6 @@ rustc_ast = { path = "../rustc_ast" } rustc_span = { path = "../rustc_span" } rustc_apfloat = { path = "../rustc_apfloat" } smallvec = { version = "1.0", features = ["union", "may_dangle"] } + +[dev-dependencies] +coverage_test_macros = { path = "src/transform/coverage/test_macros" } diff --git a/compiler/rustc_mir/src/transform/coverage/counters.rs b/compiler/rustc_mir/src/transform/coverage/counters.rs index d6c2f7f7aaf1d..7c4b30ca9e790 100644 --- a/compiler/rustc_mir/src/transform/coverage/counters.rs +++ b/compiler/rustc_mir/src/transform/coverage/counters.rs @@ -14,7 +14,7 @@ use rustc_middle::mir::coverage::*; /// Manages the counter and expression indexes/IDs to generate `CoverageKind` components for MIR /// `Coverage` statements. -pub(crate) struct CoverageCounters { +pub(super) struct CoverageCounters { function_source_hash: u64, next_counter_id: u32, num_expressions: u32, @@ -37,7 +37,7 @@ impl CoverageCounters { self.debug_counters.enable(); } - /// Makes `CoverageKind` `Counter`s and `Expressions` for the `BasicCoverageBlocks` directly or + /// Makes `CoverageKind` `Counter`s and `Expressions` for the `BasicCoverageBlock`s directly or /// indirectly associated with `CoverageSpans`, and returns additional `Expression`s /// representing intermediate values. pub fn make_bcb_counters( diff --git a/compiler/rustc_mir/src/transform/coverage/debug.rs b/compiler/rustc_mir/src/transform/coverage/debug.rs index ffa795134e257..7f1dc3844b21d 100644 --- a/compiler/rustc_mir/src/transform/coverage/debug.rs +++ b/compiler/rustc_mir/src/transform/coverage/debug.rs @@ -127,7 +127,7 @@ pub const NESTED_INDENT: &str = " "; const RUSTC_COVERAGE_DEBUG_OPTIONS: &str = "RUSTC_COVERAGE_DEBUG_OPTIONS"; -pub(crate) fn debug_options<'a>() -> &'a DebugOptions { +pub(super) fn debug_options<'a>() -> &'a DebugOptions { static DEBUG_OPTIONS: SyncOnceCell = SyncOnceCell::new(); &DEBUG_OPTIONS.get_or_init(|| DebugOptions::from_env()) @@ -136,7 +136,7 @@ pub(crate) fn debug_options<'a>() -> &'a DebugOptions { /// Parses and maintains coverage-specific debug options captured from the environment variable /// "RUSTC_COVERAGE_DEBUG_OPTIONS", if set. #[derive(Debug, Clone)] -pub(crate) struct DebugOptions { +pub(super) struct DebugOptions { pub allow_unused_expressions: bool, counter_format: ExpressionFormat, } @@ -250,7 +250,7 @@ impl Default for ExpressionFormat { /// /// `DebugCounters` supports a recursive rendering of `Expression` counters, so they can be /// presented as nested expressions such as `(bcb3 - (bcb0 + bcb1))`. -pub(crate) struct DebugCounters { +pub(super) struct DebugCounters { some_counters: Option>, } @@ -386,7 +386,7 @@ impl DebugCounter { /// If enabled, this data structure captures additional debugging information used when generating /// a Graphviz (.dot file) representation of the `CoverageGraph`, for debugging purposes. -pub(crate) struct GraphvizData { +pub(super) struct GraphvizData { some_bcb_to_coverage_spans_with_counters: Option>>, some_bcb_to_dependency_counters: Option>>, @@ -496,7 +496,7 @@ impl GraphvizData { /// directly or indirectly, to compute the coverage counts for all `CoverageSpan`s, and any that are /// _not_ used are retained in the `unused_expressions` Vec, to be included in debug output (logs /// and/or a `CoverageGraph` graphviz output). -pub(crate) struct UsedExpressions { +pub(super) struct UsedExpressions { some_used_expression_operands: Option>>, some_unused_expressions: @@ -626,7 +626,7 @@ impl UsedExpressions { } /// Generates the MIR pass `CoverageSpan`-specific spanview dump file. -pub(crate) fn dump_coverage_spanview( +pub(super) fn dump_coverage_spanview( tcx: TyCtxt<'tcx>, mir_body: &mir::Body<'tcx>, basic_coverage_blocks: &CoverageGraph, @@ -666,7 +666,7 @@ fn span_viewables( } /// Generates the MIR pass coverage-specific graphviz dump file. -pub(crate) fn dump_coverage_graphviz( +pub(super) fn dump_coverage_graphviz( tcx: TyCtxt<'tcx>, mir_body: &mir::Body<'tcx>, pass_name: &str, @@ -815,7 +815,7 @@ fn bcb_to_string_sections( /// Returns a simple string representation of a `TerminatorKind` variant, indenpendent of any /// values it might hold. -pub(crate) fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { +pub(super) fn term_type(kind: &TerminatorKind<'tcx>) -> &'static str { match kind { TerminatorKind::Goto { .. } => "Goto", TerminatorKind::SwitchInt { .. } => "SwitchInt", diff --git a/compiler/rustc_mir/src/transform/coverage/graph.rs b/compiler/rustc_mir/src/transform/coverage/graph.rs index c2ed2cbb10002..9d375633dcf51 100644 --- a/compiler/rustc_mir/src/transform/coverage/graph.rs +++ b/compiler/rustc_mir/src/transform/coverage/graph.rs @@ -17,7 +17,8 @@ const ID_SEPARATOR: &str = ","; /// `CoverageKind` counter (to be added by `CoverageCounters::make_bcb_counters`), and an optional /// set of additional counters--if needed--to count incoming edges, if there are more than one. /// (These "edge counters" are eventually converted into new MIR `BasicBlock`s.) -pub(crate) struct CoverageGraph { +#[derive(Debug)] +pub(super) struct CoverageGraph { bcbs: IndexVec, bb_to_bcb: IndexVec>, pub successors: IndexVec>, @@ -275,7 +276,7 @@ impl graph::WithPredecessors for CoverageGraph { rustc_index::newtype_index! { /// A node in the [control-flow graph][CFG] of CoverageGraph. - pub(crate) struct BasicCoverageBlock { + pub(super) struct BasicCoverageBlock { DEBUG_FORMAT = "bcb{}", } } @@ -305,7 +306,7 @@ rustc_index::newtype_index! { /// queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch (control flow) /// significance. #[derive(Debug, Clone)] -pub(crate) struct BasicCoverageBlockData { +pub(super) struct BasicCoverageBlockData { pub basic_blocks: Vec, pub counter_kind: Option, edge_from_bcbs: Option>, @@ -431,7 +432,7 @@ impl BasicCoverageBlockData { /// the specific branching BCB, representing the edge between the two. The latter case /// distinguishes this incoming edge from other incoming edges to the same `target_bcb`. #[derive(Clone, Copy, PartialEq, Eq)] -pub(crate) struct BcbBranch { +pub(super) struct BcbBranch { pub edge_from_bcb: Option, pub target_bcb: BasicCoverageBlock, } @@ -498,9 +499,8 @@ fn bcb_filtered_successors<'a, 'tcx>( /// Maintains separate worklists for each loop in the BasicCoverageBlock CFG, plus one for the /// CoverageGraph outside all loops. This supports traversing the BCB CFG in a way that /// ensures a loop is completely traversed before processing Blocks after the end of the loop. -// FIXME(richkadel): Add unit tests for TraversalContext. #[derive(Debug)] -pub(crate) struct TraversalContext { +pub(super) struct TraversalContext { /// From one or more backedges returning to a loop header. pub loop_backedges: Option<(Vec, BasicCoverageBlock)>, @@ -510,7 +510,7 @@ pub(crate) struct TraversalContext { pub worklist: Vec, } -pub(crate) struct TraverseCoverageGraphWithLoops { +pub(super) struct TraverseCoverageGraphWithLoops { pub backedges: IndexVec>, pub context_stack: Vec, visited: BitSet, @@ -642,7 +642,7 @@ impl TraverseCoverageGraphWithLoops { } } -fn find_loop_backedges( +pub(super) fn find_loop_backedges( basic_coverage_blocks: &CoverageGraph, ) -> IndexVec> { let num_bcbs = basic_coverage_blocks.num_nodes(); diff --git a/compiler/rustc_mir/src/transform/coverage/mod.rs b/compiler/rustc_mir/src/transform/coverage/mod.rs index c55349239b034..de37a67a1742f 100644 --- a/compiler/rustc_mir/src/transform/coverage/mod.rs +++ b/compiler/rustc_mir/src/transform/coverage/mod.rs @@ -5,6 +5,9 @@ mod debug; mod graph; mod spans; +#[cfg(test)] +mod tests; + use counters::CoverageCounters; use graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph}; use spans::{CoverageSpan, CoverageSpans}; @@ -31,7 +34,7 @@ use rustc_span::{CharPos, Pos, SourceFile, Span, Symbol}; /// A simple error message wrapper for `coverage::Error`s. #[derive(Debug)] -pub(crate) struct Error { +pub(self) struct Error { message: String, } diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index cda4fc125442f..f880d69bd64f6 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -17,7 +17,7 @@ use rustc_span::{BytePos, Span, SyntaxContext}; use std::cmp::Ordering; #[derive(Debug, Copy, Clone)] -pub(crate) enum CoverageStatement { +pub(super) enum CoverageStatement { Statement(BasicBlock, Span, usize), Terminator(BasicBlock, Span), } @@ -66,7 +66,7 @@ impl CoverageStatement { /// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock` /// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`. #[derive(Debug, Clone)] -pub(crate) struct CoverageSpan { +pub(super) struct CoverageSpan { pub span: Span, pub bcb: BasicCoverageBlock, pub coverage_statements: Vec, @@ -214,7 +214,7 @@ pub struct CoverageSpans<'a, 'tcx> { } impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { - pub(crate) fn generate_coverage_spans( + pub(super) fn generate_coverage_spans( mir_body: &'a mir::Body<'tcx>, body_span: Span, basic_coverage_blocks: &'a CoverageGraph, diff --git a/compiler/rustc_mir/src/transform/coverage/test_macros/Cargo.toml b/compiler/rustc_mir/src/transform/coverage/test_macros/Cargo.toml new file mode 100644 index 0000000000000..a9d6f0c803d2e --- /dev/null +++ b/compiler/rustc_mir/src/transform/coverage/test_macros/Cargo.toml @@ -0,0 +1,12 @@ +[package] +authors = ["The Rust Project Developers"] +name = "coverage_test_macros" +version = "0.0.0" +edition = "2018" + +[lib] +proc-macro = true +doctest = false + +[dependencies] +proc-macro2 = "1" diff --git a/compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs b/compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs new file mode 100644 index 0000000000000..ea551c7745556 --- /dev/null +++ b/compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs @@ -0,0 +1,8 @@ +use proc_macro::TokenStream; + +#[proc_macro] +pub fn let_bcb(item: TokenStream) -> TokenStream { + format!("let bcb{} = graph::BasicCoverageBlock::from_usize({}); let _ = {};", item, item, item) + .parse() + .unwrap() +} diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs new file mode 100644 index 0000000000000..2231fe6427fa2 --- /dev/null +++ b/compiler/rustc_mir/src/transform/coverage/tests.rs @@ -0,0 +1,631 @@ +use super::debug; +use super::graph; + +use coverage_test_macros::let_bcb; + +use rustc_data_structures::graph::WithNumNodes; +use rustc_data_structures::graph::WithSuccessors; +use rustc_index::vec::{Idx, IndexVec}; +use rustc_middle::mir::*; +use rustc_middle::ty::{self, TyS}; +use rustc_span::DUMMY_SP; + +use std::lazy::SyncOnceCell; + +fn dummy_ty<'tcx>() -> &'static TyS<'tcx> { + static DUMMY_TYS: SyncOnceCell> = SyncOnceCell::new(); + + &DUMMY_TYS.get_or_init(|| { + let fake_type_bytes = vec![0 as u8; std::mem::size_of::>()]; + unsafe { std::ptr::read_unaligned::>(fake_type_bytes.as_ptr() as *const TyS<'_>) } + }) +} + +struct MockBlocks<'tcx> { + blocks: IndexVec>, + source_info: SourceInfo, + dummy_place: Place<'tcx>, + next_local: usize, +} + +impl<'tcx> MockBlocks<'tcx> { + fn new() -> Self { + Self { + blocks: IndexVec::new(), + source_info: SourceInfo::outermost(DUMMY_SP), + dummy_place: Place { local: RETURN_PLACE, projection: ty::List::empty() }, + next_local: 0, + } + } + + fn new_temp(&mut self) -> Local { + let index = self.next_local; + self.next_local += 1; + Local::new(index) + } + + fn push(&mut self, num_nops: usize, kind: TerminatorKind<'tcx>) -> BasicBlock { + let nop = Statement { source_info: self.source_info, kind: StatementKind::Nop }; + + self.blocks.push(BasicBlockData { + statements: std::iter::repeat(&nop).cloned().take(num_nops).collect(), + terminator: Some(Terminator { source_info: self.source_info, kind }), + is_cleanup: false, + }) + } + + fn link(&mut self, from_block: BasicBlock, to_block: BasicBlock) { + match self.blocks[from_block].terminator_mut().kind { + TerminatorKind::Assert { ref mut target, .. } + | TerminatorKind::Call { destination: Some((_, ref mut target)), .. } + | TerminatorKind::Drop { ref mut target, .. } + | TerminatorKind::DropAndReplace { ref mut target, .. } + | TerminatorKind::FalseEdge { real_target: ref mut target, .. } + | TerminatorKind::FalseUnwind { real_target: ref mut target, .. } + | TerminatorKind::Goto { ref mut target } + | TerminatorKind::InlineAsm { destination: Some(ref mut target), .. } + | TerminatorKind::Yield { resume: ref mut target, .. } => *target = to_block, + ref invalid => bug!("Invalid from_block: {:?}", invalid), + } + } + + fn add_block_from( + &mut self, + some_from_block: Option, + to_kind: TerminatorKind<'tcx>, + ) -> BasicBlock { + let new_block = self.push(1, to_kind); + if let Some(from_block) = some_from_block { + self.link(from_block, new_block); + } + new_block + } + + fn set_branch(&mut self, switchint: BasicBlock, branch_index: usize, to_block: BasicBlock) { + match self.blocks[switchint].terminator_mut().kind { + TerminatorKind::SwitchInt { ref mut targets, .. } => { + let mut branches = targets.iter().collect::>(); + let otherwise = if branch_index == branches.len() { + to_block + } else { + let old_otherwise = targets.otherwise(); + if branch_index > branches.len() { + branches.push((branches.len() as u128, old_otherwise)); + while branches.len() < branch_index { + branches.push((branches.len() as u128, START_BLOCK)); + } + to_block + } else { + branches[branch_index] = (branch_index as u128, to_block); + old_otherwise + } + }; + *targets = SwitchTargets::new(branches.into_iter(), otherwise); + } + ref invalid => bug!("Invalid BasicBlock kind or no to_block: {:?}", invalid), + } + } + + fn call(&mut self, some_from_block: Option) -> BasicBlock { + self.add_block_from( + some_from_block, + TerminatorKind::Call { + func: Operand::Copy(self.dummy_place.clone()), + args: vec![], + destination: Some((self.dummy_place.clone(), START_BLOCK)), + cleanup: None, + from_hir_call: false, + fn_span: DUMMY_SP, + }, + ) + } + + fn goto(&mut self, some_from_block: Option) -> BasicBlock { + self.add_block_from(some_from_block, TerminatorKind::Goto { target: START_BLOCK }) + } + + fn switchint(&mut self, some_from_block: Option) -> BasicBlock { + let move_ = |place: Place<'tcx>| Operand::Move(place); + let discriminant = Place::from(self.new_temp()); + let switchint_kind = TerminatorKind::SwitchInt { + discr: move_(discriminant), + switch_ty: dummy_ty(), + targets: SwitchTargets::static_if(0, START_BLOCK, START_BLOCK), + }; + self.add_block_from(some_from_block, switchint_kind) + } + + fn return_(&mut self, some_from_block: Option) -> BasicBlock { + self.add_block_from(some_from_block, TerminatorKind::Return) + } + + fn to_body(self) -> Body<'tcx> { + Body::new_cfg_only(self.blocks) + } +} + +fn debug_basic_blocks(mir_body: &Body<'tcx>) -> String { + format!( + "{:?}", + mir_body + .basic_blocks() + .iter_enumerated() + .map(|(bb, data)| { + let kind = &data.terminator().kind; + match kind { + TerminatorKind::Assert { target, .. } + | TerminatorKind::Call { destination: Some((_, target)), .. } + | TerminatorKind::Drop { target, .. } + | TerminatorKind::DropAndReplace { target, .. } + | TerminatorKind::FalseEdge { real_target: target, .. } + | TerminatorKind::FalseUnwind { real_target: target, .. } + | TerminatorKind::Goto { target } + | TerminatorKind::InlineAsm { destination: Some(target), .. } + | TerminatorKind::Yield { resume: target, .. } => { + format!("{:?}:{} -> {:?}", bb, debug::term_type(kind), target) + } + TerminatorKind::SwitchInt { targets, .. } => { + format!("{:?}:{} -> {:?}", bb, debug::term_type(kind), targets) + } + _ => format!("{:?}:{}", bb, debug::term_type(kind)), + } + }) + .collect::>() + ) +} + +static PRINT_GRAPHS: bool = false; + +fn print_mir_graphviz(name: &str, mir_body: &Body<'_>) { + if PRINT_GRAPHS { + println!( + "digraph {} {{\n{}\n}}", + name, + mir_body + .basic_blocks() + .iter_enumerated() + .map(|(bb, data)| { + format!( + " {:?} [label=\"{:?}: {}\"];\n{}", + bb, + bb, + debug::term_type(&data.terminator().kind), + mir_body + .successors(bb) + .map(|successor| { format!(" {:?} -> {:?};", bb, successor) }) + .collect::>() + .join("\n") + ) + }) + .collect::>() + .join("\n") + ); + } +} + +fn print_coverage_graphviz( + name: &str, + mir_body: &Body<'_>, + basic_coverage_blocks: &graph::CoverageGraph, +) { + if PRINT_GRAPHS { + println!( + "digraph {} {{\n{}\n}}", + name, + basic_coverage_blocks + .iter_enumerated() + .map(|(bcb, bcb_data)| { + format!( + " {:?} [label=\"{:?}: {}\"];\n{}", + bcb, + bcb, + debug::term_type(&bcb_data.terminator(mir_body).kind), + basic_coverage_blocks + .successors(bcb) + .map(|successor| { format!(" {:?} -> {:?};", bcb, successor) }) + .collect::>() + .join("\n") + ) + }) + .collect::>() + .join("\n") + ); + } +} + +/// Create a mock `Body` with a simple flow. +fn mir_goto_switchint() -> Body<'a> { + let mut blocks = MockBlocks::new(); + let start = blocks.call(None); + let goto = blocks.goto(Some(start)); + let switchint = blocks.switchint(Some(goto)); + let then_call = blocks.call(None); + let else_call = blocks.call(None); + blocks.set_branch(switchint, 0, then_call); + blocks.set_branch(switchint, 1, else_call); + blocks.return_(Some(then_call)); + blocks.return_(Some(else_call)); + + let mir_body = blocks.to_body(); + print_mir_graphviz("mir_goto_switchint", &mir_body); + /* Graphviz character plots created using: `graph-easy --as=boxart`: + ┌────────────────┐ + │ bb0: Call │ + └────────────────┘ + │ + │ + ▼ + ┌────────────────┐ + │ bb1: Goto │ + └────────────────┘ + │ + │ + ▼ + ┌─────────────┐ ┌────────────────┐ + │ bb4: Call │ ◀── │ bb2: SwitchInt │ + └─────────────┘ └────────────────┘ + │ │ + │ │ + ▼ ▼ + ┌─────────────┐ ┌────────────────┐ + │ bb6: Return │ │ bb3: Call │ + └─────────────┘ └────────────────┘ + │ + │ + ▼ + ┌────────────────┐ + │ bb5: Return │ + └────────────────┘ + */ + mir_body +} + +fn covgraph_goto_switchint() -> graph::CoverageGraph { + let mir_body = mir_goto_switchint(); + if false { + println!("basic_blocks = {}", debug_basic_blocks(&mir_body)); + } + let covgraph = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz("covgraph_goto_switchint ", &mir_body, &covgraph); + /* + ┌──────────────┐ ┌─────────────────┐ + │ bcb2: Return │ ◀── │ bcb0: SwitchInt │ + └──────────────┘ └─────────────────┘ + │ + │ + ▼ + ┌─────────────────┐ + │ bcb1: Return │ + └─────────────────┘ + */ + covgraph +} + +/// Create a mock `Body` with a loop. +fn mir_switchint_then_loop_else_return() -> Body<'a> { + let mut blocks = MockBlocks::new(); + let start = blocks.call(None); + let switchint = blocks.switchint(Some(start)); + let then_call = blocks.call(None); + blocks.set_branch(switchint, 0, then_call); + let backedge_goto = blocks.goto(Some(then_call)); + blocks.link(backedge_goto, switchint); + let else_return = blocks.return_(None); + blocks.set_branch(switchint, 1, else_return); + + let mir_body = blocks.to_body(); + print_mir_graphviz("mir_switchint_then_loop_else_return", &mir_body); + /* + ┌────────────────┐ + │ bb0: Call │ + └────────────────┘ + │ + │ + ▼ + ┌─────────────┐ ┌────────────────┐ + │ bb4: Return │ ◀── │ bb1: SwitchInt │ ◀┐ + └─────────────┘ └────────────────┘ │ + │ │ + │ │ + ▼ │ + ┌────────────────┐ │ + │ bb2: Call │ │ + └────────────────┘ │ + │ │ + │ │ + ▼ │ + ┌────────────────┐ │ + │ bb3: Goto │ ─┘ + └────────────────┘ + */ + mir_body +} + +fn covgraph_switchint_then_loop_else_return() -> graph::CoverageGraph { + let mir_body = mir_switchint_then_loop_else_return(); + let covgraph = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz("covgraph_switchint_then_loop_else_return", &mir_body, &covgraph); + /* + ┌─────────────────┐ + │ bcb0: Call │ + └─────────────────┘ + │ + │ + ▼ + ┌────────────┐ ┌─────────────────┐ + │ bcb3: Goto │ ◀── │ bcb1: SwitchInt │ ◀┐ + └────────────┘ └─────────────────┘ │ + │ │ │ + │ │ │ + │ ▼ │ + │ ┌─────────────────┐ │ + │ │ bcb2: Return │ │ + │ └─────────────────┘ │ + │ │ + └─────────────────────────────────────┘ + */ + covgraph +} + +/// Create a mock `Body` with nested loops. +fn mir_switchint_loop_then_inner_loop_else_break() -> Body<'a> { + let mut blocks = MockBlocks::new(); + let start = blocks.call(None); + let switchint = blocks.switchint(Some(start)); + let then_call = blocks.call(None); + blocks.set_branch(switchint, 0, then_call); + let else_return = blocks.return_(None); + blocks.set_branch(switchint, 1, else_return); + + let inner_start = blocks.call(Some(then_call)); + let inner_switchint = blocks.switchint(Some(inner_start)); + let inner_then_call = blocks.call(None); + blocks.set_branch(inner_switchint, 0, inner_then_call); + let inner_backedge_goto = blocks.goto(Some(inner_then_call)); + blocks.link(inner_backedge_goto, inner_switchint); + let inner_else_break_goto = blocks.goto(None); + blocks.set_branch(inner_switchint, 1, inner_else_break_goto); + + let backedge_goto = blocks.goto(Some(inner_else_break_goto)); + blocks.link(backedge_goto, switchint); + + let mir_body = blocks.to_body(); + print_mir_graphviz("mir_switchint_loop_then_inner_loop_else_break", &mir_body); + /* + ┌────────────────┐ + │ bb0: Call │ + └────────────────┘ + │ + │ + ▼ + ┌─────────────┐ ┌────────────────┐ + │ bb3: Return │ ◀── │ bb1: SwitchInt │ ◀─────┐ + └─────────────┘ └────────────────┘ │ + │ │ + │ │ + ▼ │ + ┌────────────────┐ │ + │ bb2: Call │ │ + └────────────────┘ │ + │ │ + │ │ + ▼ │ + ┌────────────────┐ │ + │ bb4: Call │ │ + └────────────────┘ │ + │ │ + │ │ + ▼ │ + ┌─────────────┐ ┌────────────────┐ │ + │ bb8: Goto │ ◀── │ bb5: SwitchInt │ ◀┐ │ + └─────────────┘ └────────────────┘ │ │ + │ │ │ │ + │ │ │ │ + ▼ ▼ │ │ + ┌─────────────┐ ┌────────────────┐ │ │ + │ bb9: Goto │ ─┐ │ bb6: Call │ │ │ + └─────────────┘ │ └────────────────┘ │ │ + │ │ │ │ + │ │ │ │ + │ ▼ │ │ + │ ┌────────────────┐ │ │ + │ │ bb7: Goto │ ─┘ │ + │ └────────────────┘ │ + │ │ + └───────────────────────────┘ + */ + mir_body +} + +fn covgraph_switchint_loop_then_inner_loop_else_break() -> graph::CoverageGraph { + let mir_body = mir_switchint_loop_then_inner_loop_else_break(); + let covgraph = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz( + "covgraph_switchint_loop_then_inner_loop_else_break", + &mir_body, + &covgraph, + ); + /* + ┌─────────────────┐ + │ bcb0: Call │ + └─────────────────┘ + │ + │ + ▼ + ┌──────────────┐ ┌─────────────────┐ + │ bcb2: Return │ ◀── │ bcb1: SwitchInt │ ◀┐ + └──────────────┘ └─────────────────┘ │ + │ │ + │ │ + ▼ │ + ┌─────────────────┐ │ + │ bcb3: Call │ │ + └─────────────────┘ │ + │ │ + │ │ + ▼ │ + ┌──────────────┐ ┌─────────────────┐ │ + │ bcb6: Goto │ ◀── │ bcb4: SwitchInt │ ◀┼────┐ + └──────────────┘ └─────────────────┘ │ │ + │ │ │ │ + │ │ │ │ + │ ▼ │ │ + │ ┌─────────────────┐ │ │ + │ │ bcb5: Goto │ ─┘ │ + │ └─────────────────┘ │ + │ │ + └────────────────────────────────────────────┘ + */ + covgraph +} + +macro_rules! assert_successors { + ($basic_coverage_blocks:ident, $i:ident, [$($successor:ident),*]) => { + let mut successors = $basic_coverage_blocks.successors[$i].clone(); + successors.sort_unstable(); + assert_eq!(successors, vec![$($successor),*]); + } +} + +#[test] +fn test_covgraph_goto_switchint() { + let basic_coverage_blocks = covgraph_goto_switchint(); + assert_eq!( + basic_coverage_blocks.num_nodes(), + 3, + "basic_coverage_blocks: {:?}", + basic_coverage_blocks.iter_enumerated().collect::>() + ); + + let_bcb!(0); + let_bcb!(1); + let_bcb!(2); + + assert_successors!(basic_coverage_blocks, bcb0, [bcb1, bcb2]); + assert_successors!(basic_coverage_blocks, bcb1, []); + assert_successors!(basic_coverage_blocks, bcb2, []); +} + +#[test] +fn test_find_loop_backedges_none() { + let basic_coverage_blocks = covgraph_goto_switchint(); + if false { + println!( + "basic_coverage_blocks = {:?}", + basic_coverage_blocks.iter_enumerated().collect::>() + ); + println!("successors = {:?}", basic_coverage_blocks.successors); + } + let backedges = graph::find_loop_backedges(&basic_coverage_blocks); + assert_eq!( + backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), + 0, + "backedges: {:?}", + backedges + ); +} + +#[test] +fn test_covgraph_switchint_then_loop_else_return() { + let basic_coverage_blocks = covgraph_switchint_then_loop_else_return(); + assert_eq!( + basic_coverage_blocks.num_nodes(), + 4, + "basic_coverage_blocks: {:?}", + basic_coverage_blocks.iter_enumerated().collect::>() + ); + + let_bcb!(0); + let_bcb!(1); + let_bcb!(2); + let_bcb!(3); + + assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); + assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); + assert_successors!(basic_coverage_blocks, bcb2, []); + assert_successors!(basic_coverage_blocks, bcb3, [bcb1]); +} + +#[test] +fn test_find_loop_backedges_one() { + let basic_coverage_blocks = covgraph_switchint_then_loop_else_return(); + let backedges = graph::find_loop_backedges(&basic_coverage_blocks); + assert_eq!( + backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), + 1, + "backedges: {:?}", + backedges + ); + + let_bcb!(1); + let_bcb!(3); + + assert_eq!(backedges[bcb1], vec![bcb3]); +} + +#[test] +fn test_covgraph_switchint_loop_then_inner_loop_else_break() { + let basic_coverage_blocks = covgraph_switchint_loop_then_inner_loop_else_break(); + assert_eq!( + basic_coverage_blocks.num_nodes(), + 7, + "basic_coverage_blocks: {:?}", + basic_coverage_blocks.iter_enumerated().collect::>() + ); + + let_bcb!(0); + let_bcb!(1); + let_bcb!(2); + let_bcb!(3); + let_bcb!(4); + let_bcb!(5); + let_bcb!(6); + + assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); + assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); + assert_successors!(basic_coverage_blocks, bcb2, []); + assert_successors!(basic_coverage_blocks, bcb3, [bcb4]); + assert_successors!(basic_coverage_blocks, bcb4, [bcb5, bcb6]); + assert_successors!(basic_coverage_blocks, bcb5, [bcb1]); + assert_successors!(basic_coverage_blocks, bcb6, [bcb4]); +} + +#[test] +fn test_find_loop_backedges_two() { + let basic_coverage_blocks = covgraph_switchint_loop_then_inner_loop_else_break(); + let backedges = graph::find_loop_backedges(&basic_coverage_blocks); + assert_eq!( + backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), + 2, + "backedges: {:?}", + backedges + ); + + let_bcb!(1); + let_bcb!(4); + let_bcb!(5); + let_bcb!(6); + + assert_eq!(backedges[bcb1], vec![bcb5]); + assert_eq!(backedges[bcb4], vec![bcb6]); +} + +#[test] +fn test_traverse_coverage_with_loops() { + let basic_coverage_blocks = covgraph_switchint_loop_then_inner_loop_else_break(); + let mut traversed_in_order = Vec::new(); + let mut traversal = graph::TraverseCoverageGraphWithLoops::new(&basic_coverage_blocks); + while let Some(bcb) = traversal.next(&basic_coverage_blocks) { + traversed_in_order.push(bcb); + } + + let_bcb!(6); + + // bcb0 is visited first. Then bcb1 starts the first loop, and all remaining nodes, *except* + // bcb6 are inside the first loop. + assert_eq!( + *traversed_in_order.last().expect("should have elements"), + bcb6, + "bcb6 should not be visited until all nodes inside the first loop have been visited" + ); +} From ecfeac58aae332b5f88388624d9656ec6750e011 Mon Sep 17 00:00:00 2001 From: Poliorcetics Date: Thu, 12 Nov 2020 01:55:28 +0100 Subject: [PATCH 08/26] Use intradoc-links for the whole test, add a @has check Co-authored-by: Joshua Nelson --- src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs index d758d9936132f..0c047e6249a53 100644 --- a/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs +++ b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs @@ -1,12 +1,13 @@ // ignore-tidy-linelength pub mod internal { + // @has 'raw_ident_eliminate_r_hashtag/internal/struct.mod.html' pub struct r#mod; /// See [name], [other name] /// /// [name]: mod - /// [other name]: ../internal/struct.mod.html + /// [other name]: crate::internal::mod // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'name' // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'other name' pub struct B; From 562d50eb7b67109ca420eab8705673b138863e6a Mon Sep 17 00:00:00 2001 From: Zachary Catlin Date: Wed, 11 Nov 2020 21:02:21 -0500 Subject: [PATCH 09/26] Include llvm-as in llvm-tools-preview component Including llvm-as adds the ability to include assembly language fragments that can be inlined using LTO. --- src/bootstrap/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 3d111839dc725..ad8b9c7253c68 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -177,6 +177,7 @@ const LLVM_TOOLS: &[&str] = &[ "llvm-size", // used to prints the size of the linker sections of a program "llvm-strip", // used to discard symbols from binary files to reduce their size "llvm-ar", // used for creating and modifying archive files + "llvm-as", // used to convert LLVM assembly to LLVM bitcode "llvm-dis", // used to disassemble LLVM bitcode "llc", // used to compile LLVM bytecode "opt", // used to optimize LLVM bytecode From eb9f2bb3b0a3f7e712efa28743acf6134d49de5c Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 11 Nov 2020 20:01:10 -0800 Subject: [PATCH 10/26] Overcome Sync issues with non-parallel compiler Per Mark's recommendation at: https://github.com/rust-lang/rust/pull/78963#issuecomment-725790071 --- compiler/rustc_middle/src/ty/mod.rs | 12 ++++++++++++ .../rustc_mir/src/transform/coverage/tests.rs | 19 ++++++++++--------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 0042b4a3a4279..d4f299b3f37b9 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -611,6 +611,18 @@ pub struct TyS<'tcx> { outer_exclusive_binder: ty::DebruijnIndex, } +impl<'tcx> TyS<'tcx> { + /// A constructor used only for internal testing. + #[allow(rustc::usage_of_ty_tykind)] + pub fn make_for_test( + kind: TyKind<'tcx>, + flags: TypeFlags, + outer_exclusive_binder: ty::DebruijnIndex, + ) -> TyS<'tcx> { + TyS { kind, flags, outer_exclusive_binder } + } +} + // `TyS` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] static_assert_size!(TyS<'_>, 32); diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs index 2231fe6427fa2..3e305a58c6bb2 100644 --- a/compiler/rustc_mir/src/transform/coverage/tests.rs +++ b/compiler/rustc_mir/src/transform/coverage/tests.rs @@ -7,18 +7,19 @@ use rustc_data_structures::graph::WithNumNodes; use rustc_data_structures::graph::WithSuccessors; use rustc_index::vec::{Idx, IndexVec}; use rustc_middle::mir::*; -use rustc_middle::ty::{self, TyS}; +use rustc_middle::ty::{self, DebruijnIndex, TyS, TypeFlags}; use rustc_span::DUMMY_SP; -use std::lazy::SyncOnceCell; - -fn dummy_ty<'tcx>() -> &'static TyS<'tcx> { - static DUMMY_TYS: SyncOnceCell> = SyncOnceCell::new(); +fn dummy_ty() -> &'static TyS<'static> { + thread_local! { + static DUMMY_TYS: &'static TyS<'static> = Box::leak(box TyS::make_for_test( + ty::Bool, + TypeFlags::empty(), + DebruijnIndex::from_usize(0), + )); + } - &DUMMY_TYS.get_or_init(|| { - let fake_type_bytes = vec![0 as u8; std::mem::size_of::>()]; - unsafe { std::ptr::read_unaligned::>(fake_type_bytes.as_ptr() as *const TyS<'_>) } - }) + &DUMMY_TYS.with(|tys| *tys) } struct MockBlocks<'tcx> { From fe56d267cae200d0722c3304e893c0d3f91d68e8 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Sun, 8 Nov 2020 11:06:05 -0800 Subject: [PATCH 11/26] Fix and re-enable two coverage tests on MacOS Note, in the coverage-reports test, the comment about MacOS was wrong. The setting is based on config.toml llvm `optimize` setting. There doesn't appear to be any environment variable I can check, and I don't think we should add one. Testing the binary itself is a more reliable way to check anyway. For the coverage-spanview test, I removed the dependency on sed altogether, which is much less ugly than trying to work around the MacOS sed differences. I tested these changes on Linux, Windows, and Mac. --- .../coverage-reports-base/Makefile | 14 +++--- .../coverage-spanview-base/Makefile | 44 +++++++------------ 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/src/test/run-make-fulldeps/coverage-reports-base/Makefile b/src/test/run-make-fulldeps/coverage-reports-base/Makefile index b175768e19962..1e2aa056e4008 100644 --- a/src/test/run-make-fulldeps/coverage-reports-base/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports-base/Makefile @@ -13,15 +13,13 @@ BASEDIR=../coverage-reports-base SOURCEDIR=../coverage -ifeq ($(UNAME),Darwin) -# FIXME(richkadel): It appears that --debug is not available on MacOS even when not running -# under CI. -NO_LLVM_ASSERTIONS=1 -endif - # The `llvm-cov show` flag `--debug`, used to generate the `counters` output files, is only enabled -# if LLVM assertions are enabled. Some CI builds disable debug assertions. -ifndef NO_LLVM_ASSERTIONS +# if LLVM assertions are enabled. Requires Rust config `llvm/optimize` and not +# `llvm/release_debuginfo`. Note that some CI builds disable debug assertions (by setting +# `NO_LLVM_ASSERTIONS=1`), so it is not OK to fail the test, but `bless`ed test results cannot be +# generated without debug assertions. +LLVM_COV_DEBUG := $(shell "$(LLVM_BIN_DIR)"/llvm-cov show --debug 2>&1 | grep -q "Unknown command line argument '--debug'"; echo $$?) +ifeq ($(LLVM_COV_DEBUG), 1) DEBUG_FLAG=--debug endif diff --git a/src/test/run-make-fulldeps/coverage-spanview-base/Makefile b/src/test/run-make-fulldeps/coverage-spanview-base/Makefile index fb9f5215fe8ee..03ef04776a03b 100644 --- a/src/test/run-make-fulldeps/coverage-spanview-base/Makefile +++ b/src/test/run-make-fulldeps/coverage-spanview-base/Makefile @@ -9,9 +9,20 @@ BASEDIR=../coverage-spanview-base SOURCEDIR=../coverage -ifeq ($(UNAME),Darwin) -SED_HAS_ISSUES=1 -endif +define SPANVIEW_HEADER + + +endef +export SPANVIEW_HEADER all: $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs)) @@ -33,31 +44,12 @@ endif -Zdump-mir-spanview \ -Zdump-mir-dir="$(TMPDIR)"/mir_dump.$@ -ifdef SED_HAS_ISSUES - # FIXME(richkadel): MacOS's default sed has some significant limitations. Until I've come up - # with a better workaround, I'm disabling this test for MacOS. - # - # For future reference, see if `gsed` is available as an alternative. - which gsed || echo "no gsed" -else - for path in "$(TMPDIR)"/mir_dump.$@/*; do \ - echo $$path; \ file="$$(basename "$$path")"; \ - echo $$file; \ urlescaped="$$("$(PYTHON)" $(BASEDIR)/escape_url.py $$file)" || exit $$?; \ - echo $$urlescaped; \ - sed -i -e '1a\ -' "$$path"; \ + printf "$$SPANVIEW_HEADER\n" "$@" "$$urlescaped" > "$$path".modified; \ + tail -n +2 "$$path" >> "$$path".modified; \ + mv "$$path".modified "$$path"; \ done && true # for/done ends in non-zero status ifdef RUSTC_BLESS_TEST @@ -70,5 +62,3 @@ else cp "$(TMPDIR)"/mir_dump.$@/*InstrumentCoverage.0.html "$(TMPDIR)"/actual_mir_dump.$@/ $(DIFF) -r expected_mir_dump.$@/ "$(TMPDIR)"/actual_mir_dump.$@/ endif - -endif From 775f1e5acdeae38eae7de41b22bdf6ab71e34efd Mon Sep 17 00:00:00 2001 From: thiolliere Date: Thu, 12 Nov 2020 12:41:19 +0100 Subject: [PATCH 12/26] fix pretty print for qpath --- compiler/rustc_ast_pretty/src/pprust/state.rs | 11 ++++++----- src/test/pretty/qpath-associated-type-bound.rs | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 src/test/pretty/qpath-associated-type-bound.rs diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index a64014f5acbb8..62a9452c9f0aa 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -2328,11 +2328,12 @@ impl<'a> State<'a> { self.print_path(path, false, depth); } self.s.word(">"); - self.s.word("::"); - let item_segment = path.segments.last().unwrap(); - self.print_ident(item_segment.ident); - if let Some(ref args) = item_segment.args { - self.print_generic_args(args, colons_before_params) + for item_segment in &path.segments[qself.position..] { + self.s.word("::"); + self.print_ident(item_segment.ident); + if let Some(ref args) = item_segment.args { + self.print_generic_args(args, colons_before_params) + } } } diff --git a/src/test/pretty/qpath-associated-type-bound.rs b/src/test/pretty/qpath-associated-type-bound.rs new file mode 100644 index 0000000000000..e06885e03882b --- /dev/null +++ b/src/test/pretty/qpath-associated-type-bound.rs @@ -0,0 +1,16 @@ +// pp-exact + + +mod m { + pub trait Tr { + type Ts: super::Tu; + } +} + +trait Tu { + fn dummy() { } +} + +fn foo() { ::Ts::dummy(); } + +fn main() { } From 04d41e1f4083307b1434d305899da739b21e5155 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 12 Nov 2020 00:54:23 +0300 Subject: [PATCH 13/26] rustc_target: Mark UEFI targets as `is_like_windows`/`is_like_msvc` Document what `is_like_windows` and `is_like_msvc` mean in more detail. --- compiler/rustc_codegen_llvm/src/back/write.rs | 4 +--- compiler/rustc_codegen_ssa/src/back/write.rs | 2 ++ compiler/rustc_target/src/spec/mod.rs | 19 ++++++++++++++++--- .../rustc_target/src/spec/tests/tests_impl.rs | 2 ++ .../rustc_target/src/spec/uefi_msvc_base.rs | 9 --------- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index e6acb6860be9e..6237a4c0020eb 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -925,9 +925,7 @@ unsafe fn embed_bitcode( || cgcx.opts.target_triple.triple().starts_with("asmjs") { // nothing to do here - } else if cgcx.opts.target_triple.triple().contains("windows") - || cgcx.opts.target_triple.triple().contains("uefi") - { + } else if cgcx.is_pe_coff { let asm = " .section .llvmbc,\"n\" .section .llvmcmd,\"n\" diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index b34bee3358b40..7f2bb7b5bcdaf 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -307,6 +307,7 @@ pub struct CodegenContext { pub allocator_module_config: Arc, pub tm_factory: TargetMachineFactory, pub msvc_imps_needed: bool, + pub is_pe_coff: bool, pub target_pointer_width: u32, pub target_arch: String, pub debuginfo: config::DebugInfo, @@ -1022,6 +1023,7 @@ fn start_executing_work( tm_factory: TargetMachineFactory(backend.target_machine_factory(tcx.sess, ol)), total_cgus, msvc_imps_needed: msvc_imps_needed(tcx), + is_pe_coff: tcx.sess.target.is_like_windows, target_pointer_width: tcx.sess.target.pointer_width, target_arch: tcx.sess.target.arch.clone(), debuginfo: tcx.sess.opts.debuginfo, diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index f949bf95a502a..cf45d8b7f6abd 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -819,10 +819,23 @@ pub struct TargetOptions { /// Only useful for compiling against Illumos/Solaris, /// as they have a different set of linker flags. Defaults to false. pub is_like_solaris: bool, - /// Whether the target toolchain is like Windows'. Only useful for compiling against Windows, - /// only really used for figuring out how to find libraries, since Windows uses its own - /// library naming convention. Defaults to false. + /// Whether the target is like Windows. + /// This is a combination of several more specific properties represented as a single flag: + /// - The target uses a Windows ABI, + /// - uses PE/COFF as a format for object code, + /// - uses Windows-style dllexport/dllimport for shared libraries, + /// - uses import libraries and .def files for symbol exports, + /// - executables support setting a subsystem. pub is_like_windows: bool, + /// Whether the target is like MSVC. + /// This is a combination of several more specific properties represented as a single flag: + /// - The target has all the properties from `is_like_windows` + /// (for in-tree targets "is_like_msvc ⇒ is_like_windows" is ensured by a unit test), + /// - has some MSVC-specific Windows ABI properties, + /// - uses a link.exe-like linker, + /// - uses CodeView/PDB for debuginfo and natvis for its visualization, + /// - uses SEH-based unwinding, + /// - supports control flow guard mechanism. pub is_like_msvc: bool, /// Whether the target toolchain is like Emscripten's. Only useful for compiling with /// Emscripten toolchain. diff --git a/compiler/rustc_target/src/spec/tests/tests_impl.rs b/compiler/rustc_target/src/spec/tests/tests_impl.rs index f348df7d5a716..10e62f6b672e3 100644 --- a/compiler/rustc_target/src/spec/tests/tests_impl.rs +++ b/compiler/rustc_target/src/spec/tests/tests_impl.rs @@ -8,6 +8,7 @@ pub(super) fn test_target(target: Target) { impl Target { fn check_consistency(&self) { + assert!(self.is_like_windows || !self.is_like_msvc); // Check that LLD with the given flavor is treated identically to the linker it emulates. // If your target really needs to deviate from the rules below, except it and document the // reasons. @@ -16,6 +17,7 @@ impl Target { || self.linker_flavor == LinkerFlavor::Lld(LldFlavor::Link), self.lld_flavor == LldFlavor::Link, ); + assert_eq!(self.is_like_msvc, self.lld_flavor == LldFlavor::Link); for args in &[ &self.pre_link_args, &self.late_link_args, diff --git a/compiler/rustc_target/src/spec/uefi_msvc_base.rs b/compiler/rustc_target/src/spec/uefi_msvc_base.rs index 79fe77495e731..322b6f530e9fd 100644 --- a/compiler/rustc_target/src/spec/uefi_msvc_base.rs +++ b/compiler/rustc_target/src/spec/uefi_msvc_base.rs @@ -46,15 +46,6 @@ pub fn opts() -> TargetOptions { stack_probes: true, singlethread: true, linker: Some("rust-lld".to_string()), - // FIXME: This should likely be `true` inherited from `msvc_base` - // because UEFI follows Windows ABI and uses PE/COFF. - // The `false` is probably causing ABI bugs right now. - is_like_windows: false, - // FIXME: This should likely be `true` inherited from `msvc_base` - // because UEFI follows Windows ABI and uses PE/COFF. - // The `false` is probably causing ABI bugs right now. - is_like_msvc: false, - ..base } } From 0b4af1614d91152564b852572afa63fab71162a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 11 Nov 2020 00:00:00 +0000 Subject: [PATCH 14/26] Never inline when `no_sanitize` attributes differ The inliner looks if a sanitizer is enabled before considering `no_sanitize` attribute as possible source of incompatibility. The MIR inlining could happen in a crate with sanitizer disabled, but code generation in a crate with sanitizer enabled, thus the attribute would be incorrectly ignored. To avoid the issue never inline functions with different `no_sanitize` attributes. --- compiler/rustc_mir/src/transform/inline.rs | 6 +----- src/test/mir-opt/inline/inline-compatibility.rs | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 7737672dbde66..7332d4194944e 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -218,11 +218,7 @@ impl Inliner<'tcx> { return false; } - let self_no_sanitize = - self.codegen_fn_attrs.no_sanitize & self.tcx.sess.opts.debugging_opts.sanitizer; - let callee_no_sanitize = - codegen_fn_attrs.no_sanitize & self.tcx.sess.opts.debugging_opts.sanitizer; - if self_no_sanitize != callee_no_sanitize { + if self.codegen_fn_attrs.no_sanitize != codegen_fn_attrs.no_sanitize { debug!("`callee has incompatible no_sanitize attribute - not inlining"); return false; } diff --git a/src/test/mir-opt/inline/inline-compatibility.rs b/src/test/mir-opt/inline/inline-compatibility.rs index ff9049edb4f2c..2e9edf5260f53 100644 --- a/src/test/mir-opt/inline/inline-compatibility.rs +++ b/src/test/mir-opt/inline/inline-compatibility.rs @@ -1,8 +1,6 @@ // Checks that only functions with compatible attributes are inlined. // // only-x86_64 -// needs-sanitizer-address -// compile-flags: -Zsanitizer=address #![crate_type = "lib"] #![feature(no_sanitize)] @@ -35,5 +33,5 @@ pub unsafe fn not_inlined_no_sanitize() { pub unsafe fn target_feature() {} #[inline] -#[no_sanitize(address, memory)] +#[no_sanitize(address)] pub unsafe fn no_sanitize() {} From ae4332643de6a672ab3aefd62a3063c9af21166c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 11 Nov 2020 00:00:00 +0000 Subject: [PATCH 15/26] Never inline cold functions The information about cold attribute is lost during inlining, Avoid the issue by never inlining cold functions. --- compiler/rustc_mir/src/transform/inline.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 7332d4194944e..5023e49df3abb 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -252,9 +252,9 @@ impl Inliner<'tcx> { self.tcx.sess.opts.debugging_opts.inline_mir_threshold }; - // Significantly lower the threshold for inlining cold functions if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::COLD) { - threshold /= 5; + debug!("#[cold] present - not inlining"); + return false; } // Give a bonus functions with a small number of blocks, From 9bb3d6b7d472e2116312ea45db07a5338af205fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 11 Nov 2020 00:00:00 +0000 Subject: [PATCH 16/26] Remove check for impossible condition The callee body is already transformed; the condition is always false. --- compiler/rustc_mir/src/transform/inline.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 5023e49df3abb..0d6d9e397ac9f 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -203,12 +203,6 @@ impl Inliner<'tcx> { debug!("should_inline({:?})", callsite); let tcx = self.tcx; - // Cannot inline generators which haven't been transformed yet - if callee_body.yield_ty.is_some() { - debug!(" yield ty present - not inlining"); - return false; - } - let codegen_fn_attrs = tcx.codegen_fn_attrs(callsite.callee.def_id()); let self_features = &self.codegen_fn_attrs.target_features; From 2879ab793e10a1bf5c158e3301474be96192aa7a Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 4 Nov 2020 16:27:11 +0300 Subject: [PATCH 17/26] rustc_parse: Remove optimization for 0-length streams in `collect_tokens` The optimization conflates empty token streams with unknown token stream, which is at least suspicious, and doesn't affect performance because 0-length token streams are very rare. --- compiler/rustc_parse/src/parser/mod.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index da1c54e88b5e2..40aa2db58c720 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1180,8 +1180,7 @@ impl<'a> Parser<'a> { /// Records all tokens consumed by the provided callback, /// including the current token. These tokens are collected /// into a `LazyTokenStream`, and returned along with the result - /// of the callback. The returned `LazyTokenStream` will be `None` - /// if not tokens were captured. + /// of the callback. /// /// Note: If your callback consumes an opening delimiter /// (including the case where you call `collect_tokens` @@ -1203,17 +1202,14 @@ impl<'a> Parser<'a> { let ret = f(self)?; - // We didn't capture any tokens - let num_calls = self.token_cursor.num_next_calls - cursor_snapshot.num_next_calls; - if num_calls == 0 { - return Ok((ret, None)); - } - // Produces a `TokenStream` on-demand. Using `cursor_snapshot` // and `num_calls`, we can reconstruct the `TokenStream` seen // by the callback. This allows us to avoid producing a `TokenStream` // if it is never needed - for example, a captured `macro_rules!` // argument that is never passed to a proc macro. + // In practice token stream creation happens rarely compared to + // calls to `collect_tokens` (see some statistics in #78736), + // so we are doing as little up-front work as possible. // // This also makes `Parser` very cheap to clone, since // there is no intermediate collection buffer to clone. @@ -1247,8 +1243,8 @@ impl<'a> Parser<'a> { let lazy_impl = LazyTokenStreamImpl { start_token, + num_calls: self.token_cursor.num_next_calls - cursor_snapshot.num_next_calls, cursor_snapshot, - num_calls, desugar_doc_comments: self.desugar_doc_comments, }; Ok((ret, Some(LazyTokenStream::new(lazy_impl)))) From 66cadec1763ac645337c1ac58f06ea48b9b72a26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 11 Nov 2020 00:00:00 +0000 Subject: [PATCH 18/26] Fix generator inlining by checking for rust-call abi and spread arg --- compiler/rustc_mir/src/transform/inline.rs | 26 +++++++++++---------- src/test/mir-opt/inline/inline-generator.rs | 16 +++++++++++++ 2 files changed, 30 insertions(+), 12 deletions(-) create mode 100644 src/test/mir-opt/inline/inline-generator.rs diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 0d6d9e397ac9f..2ccb9b3709f2f 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -7,6 +7,7 @@ use rustc_index::vec::Idx; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; +use rustc_middle::ty::subst::Subst; use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt}; use rustc_span::{hygiene::ExpnKind, ExpnData, Span}; use rustc_target::spec::abi::Abi; @@ -28,6 +29,7 @@ pub struct Inline; #[derive(Copy, Clone, Debug)] struct CallSite<'tcx> { callee: Instance<'tcx>, + fn_sig: ty::PolyFnSig<'tcx>, block: BasicBlock, target: Option, source_info: SourceInfo, @@ -173,22 +175,23 @@ impl Inliner<'tcx> { // Only consider direct calls to functions let terminator = bb_data.terminator(); - if let TerminatorKind::Call { func: ref op, ref destination, .. } = terminator.kind { - if let ty::FnDef(callee_def_id, substs) = *op.ty(caller_body, self.tcx).kind() { - // To resolve an instance its substs have to be fully normalized, so - // we do this here. - let normalized_substs = self.tcx.normalize_erasing_regions(self.param_env, substs); + if let TerminatorKind::Call { ref func, ref destination, .. } = terminator.kind { + let func_ty = func.ty(caller_body, self.tcx); + if let ty::FnDef(def_id, substs) = *func_ty.kind() { + // To resolve an instance its substs have to be fully normalized. + let substs = self.tcx.normalize_erasing_regions(self.param_env, substs); let callee = - Instance::resolve(self.tcx, self.param_env, callee_def_id, normalized_substs) - .ok() - .flatten()?; + Instance::resolve(self.tcx, self.param_env, def_id, substs).ok().flatten()?; if let InstanceDef::Virtual(..) | InstanceDef::Intrinsic(_) = callee.def { return None; } + let fn_sig = self.tcx.fn_sig(def_id).subst(self.tcx, substs); + return Some(CallSite { callee, + fn_sig, block: bb, target: destination.map(|(_, target)| target), source_info: terminator.source_info, @@ -437,7 +440,7 @@ impl Inliner<'tcx> { }; // Copy the arguments if needed. - let args: Vec<_> = self.make_call_args(args, &callsite, caller_body); + let args: Vec<_> = self.make_call_args(args, &callsite, caller_body, &callee_body); let mut integrator = Integrator { args: &args, @@ -518,6 +521,7 @@ impl Inliner<'tcx> { args: Vec>, callsite: &CallSite<'tcx>, caller_body: &mut Body<'tcx>, + callee_body: &Body<'tcx>, ) -> Vec { let tcx = self.tcx; @@ -544,9 +548,7 @@ impl Inliner<'tcx> { // tmp2 = tuple_tmp.2 // // and the vector is `[closure_ref, tmp0, tmp1, tmp2]`. - // FIXME(eddyb) make this check for `"rust-call"` ABI combined with - // `callee_body.spread_arg == None`, instead of special-casing closures. - if tcx.is_closure(callsite.callee.def_id()) { + if callsite.fn_sig.abi() == Abi::RustCall && callee_body.spread_arg.is_none() { let mut args = args.into_iter(); let self_ = self.create_temp_if_necessary(args.next().unwrap(), callsite, caller_body); let tuple = self.create_temp_if_necessary(args.next().unwrap(), callsite, caller_body); diff --git a/src/test/mir-opt/inline/inline-generator.rs b/src/test/mir-opt/inline/inline-generator.rs new file mode 100644 index 0000000000000..d11b3e548f721 --- /dev/null +++ b/src/test/mir-opt/inline/inline-generator.rs @@ -0,0 +1,16 @@ +// ignore-wasm32-bare compiled with panic=abort by default +#![feature(generators, generator_trait)] + +use std::ops::Generator; +use std::pin::Pin; + +// EMIT_MIR inline_generator.main.Inline.diff +fn main() { + let _r = Pin::new(&mut g()).resume(false); +} + +#[inline(always)] +pub fn g() -> impl Generator { + #[inline(always)] + |a| { yield if a { 7 } else { 13 } } +} From 79d853eccebf6d9704adeea45f20f11bae0d7396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 11 Nov 2020 00:00:00 +0000 Subject: [PATCH 19/26] Never inline C variadic functions --- compiler/rustc_mir/src/transform/inline.rs | 5 +++++ .../mir-opt/inline/inline-compatibility.rs | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 2ccb9b3709f2f..aae98f5b6d8d6 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -206,6 +206,11 @@ impl Inliner<'tcx> { debug!("should_inline({:?})", callsite); let tcx = self.tcx; + if callsite.fn_sig.c_variadic() { + debug!("callee is variadic - not inlining"); + return false; + } + let codegen_fn_attrs = tcx.codegen_fn_attrs(callsite.callee.def_id()); let self_features = &self.codegen_fn_attrs.target_features; diff --git a/src/test/mir-opt/inline/inline-compatibility.rs b/src/test/mir-opt/inline/inline-compatibility.rs index 2e9edf5260f53..30aff0a64efb9 100644 --- a/src/test/mir-opt/inline/inline-compatibility.rs +++ b/src/test/mir-opt/inline/inline-compatibility.rs @@ -5,6 +5,7 @@ #![crate_type = "lib"] #![feature(no_sanitize)] #![feature(target_feature_11)] +#![feature(c_variadic)] // EMIT_MIR inline_compatibility.inlined_target_feature.Inline.diff #[target_feature(enable = "sse2")] @@ -35,3 +36,20 @@ pub unsafe fn target_feature() {} #[inline] #[no_sanitize(address)] pub unsafe fn no_sanitize() {} + +// EMIT_MIR inline_compatibility.not_inlined_c_variadic.Inline.diff +pub unsafe fn not_inlined_c_variadic() { + let s = sum(4u32, 4u32, 30u32, 200u32, 1000u32); +} + +#[no_mangle] +#[inline(always)] +unsafe extern "C" fn sum(n: u32, mut vs: ...) -> u32 { + let mut s = 0; + let mut i = 0; + while i != n { + s += vs.arg::(); + i += 1; + } + s +} From 2a010dd3404256ea774e5338ad1952d09ab2cf03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Wed, 11 Nov 2020 00:00:00 +0000 Subject: [PATCH 20/26] ./x.py test --bless --- ...patibility.inlined_no_sanitize.Inline.diff | 20 +-- ...ibility.inlined_target_feature.Inline.diff | 20 +-- ...ibility.not_inlined_c_variadic.Inline.diff | 25 ++++ ...bility.not_inlined_no_sanitize.Inline.diff | 16 +-- ...ity.not_inlined_target_feature.Inline.diff | 16 +-- .../inline/inline_generator.main.Inline.diff | 123 ++++++++++++++++++ 6 files changed, 184 insertions(+), 36 deletions(-) create mode 100644 src/test/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.diff create mode 100644 src/test/mir-opt/inline/inline_generator.main.Inline.diff diff --git a/src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff index 451ec39422fc4..c95cf47695785 100644 --- a/src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff +++ b/src/test/mir-opt/inline/inline_compatibility.inlined_no_sanitize.Inline.diff @@ -2,24 +2,24 @@ + // MIR for `inlined_no_sanitize` after Inline fn inlined_no_sanitize() -> () { - let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:24:37: 24:37 - let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18 -+ scope 1 (inlined no_sanitize) { // at $DIR/inline-compatibility.rs:25:5: 25:18 + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:23:37: 23:37 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:24:5: 24:18 ++ scope 1 (inlined no_sanitize) { // at $DIR/inline-compatibility.rs:24:5: 24:18 + } bb0: { - StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18 -- _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18 + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:24:5: 24:18 +- _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:24:5: 24:18 - // mir::Constant -- // + span: $DIR/inline-compatibility.rs:25:5: 25:16 +- // + span: $DIR/inline-compatibility.rs:24:5: 24:16 - // + literal: Const { ty: unsafe fn() {no_sanitize}, val: Value(Scalar()) } - } - - bb1: { -+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:25:5: 25:18 - StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:25:18: 25:19 - _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:24:37: 26:2 - return; // scope 0 at $DIR/inline-compatibility.rs:26:2: 26:2 ++ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:24:5: 24:18 + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:24:18: 24:19 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:23:37: 25:2 + return; // scope 0 at $DIR/inline-compatibility.rs:25:2: 25:2 } } diff --git a/src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff index a59ddd344cb26..2bb928343229f 100644 --- a/src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff +++ b/src/test/mir-opt/inline/inline_compatibility.inlined_target_feature.Inline.diff @@ -2,24 +2,24 @@ + // MIR for `inlined_target_feature` after Inline fn inlined_target_feature() -> () { - let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:13:40: 13:40 - let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21 -+ scope 1 (inlined target_feature) { // at $DIR/inline-compatibility.rs:14:5: 14:21 + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:12:40: 12:40 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:13:5: 13:21 ++ scope 1 (inlined target_feature) { // at $DIR/inline-compatibility.rs:13:5: 13:21 + } bb0: { - StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21 -- _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21 + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:13:5: 13:21 +- _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:13:5: 13:21 - // mir::Constant -- // + span: $DIR/inline-compatibility.rs:14:5: 14:19 +- // + span: $DIR/inline-compatibility.rs:13:5: 13:19 - // + literal: Const { ty: unsafe fn() {target_feature}, val: Value(Scalar()) } - } - - bb1: { -+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:14:5: 14:21 - StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:14:21: 14:22 - _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:13:40: 15:2 - return; // scope 0 at $DIR/inline-compatibility.rs:15:2: 15:2 ++ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:13:5: 13:21 + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:13:21: 13:22 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:12:40: 14:2 + return; // scope 0 at $DIR/inline-compatibility.rs:14:2: 14:2 } } diff --git a/src/test/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.diff new file mode 100644 index 0000000000000..09bca903c80e8 --- /dev/null +++ b/src/test/mir-opt/inline/inline_compatibility.not_inlined_c_variadic.Inline.diff @@ -0,0 +1,25 @@ +- // MIR for `not_inlined_c_variadic` before Inline ++ // MIR for `not_inlined_c_variadic` after Inline + + fn not_inlined_c_variadic() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:41:40: 41:40 + let _1: u32; // in scope 0 at $DIR/inline-compatibility.rs:42:9: 42:10 + scope 1 { + debug s => _1; // in scope 1 at $DIR/inline-compatibility.rs:42:9: 42:10 + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:42:9: 42:10 + _1 = sum(const 4_u32, const 4_u32, const 30_u32, const 200_u32, const 1000_u32) -> bb1; // scope 0 at $DIR/inline-compatibility.rs:42:13: 42:52 + // mir::Constant + // + span: $DIR/inline-compatibility.rs:42:13: 42:16 + // + literal: Const { ty: unsafe extern "C" fn(u32, ...) -> u32 {sum}, val: Value(Scalar()) } + } + + bb1: { + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:41:40: 43:2 + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:43:1: 43:2 + return; // scope 0 at $DIR/inline-compatibility.rs:43:2: 43:2 + } + } + diff --git a/src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff index 651eadc1e849c..5af3946f2e501 100644 --- a/src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff +++ b/src/test/mir-opt/inline/inline_compatibility.not_inlined_no_sanitize.Inline.diff @@ -2,21 +2,21 @@ + // MIR for `not_inlined_no_sanitize` after Inline fn not_inlined_no_sanitize() -> () { - let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:29:41: 29:41 - let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18 + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:28:41: 28:41 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:29:5: 29:18 bb0: { - StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18 - _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18 + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:29:5: 29:18 + _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:29:5: 29:18 // mir::Constant - // + span: $DIR/inline-compatibility.rs:30:5: 30:16 + // + span: $DIR/inline-compatibility.rs:29:5: 29:16 // + literal: Const { ty: unsafe fn() {no_sanitize}, val: Value(Scalar()) } } bb1: { - StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:30:18: 30:19 - _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:29:41: 31:2 - return; // scope 0 at $DIR/inline-compatibility.rs:31:2: 31:2 + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:29:18: 29:19 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:28:41: 30:2 + return; // scope 0 at $DIR/inline-compatibility.rs:30:2: 30:2 } } diff --git a/src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff b/src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff index 55b9edf3adc1f..8c9fa573ce218 100644 --- a/src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff +++ b/src/test/mir-opt/inline/inline_compatibility.not_inlined_target_feature.Inline.diff @@ -2,21 +2,21 @@ + // MIR for `not_inlined_target_feature` after Inline fn not_inlined_target_feature() -> () { - let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:18:44: 18:44 - let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21 + let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:17:44: 17:44 + let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:18:5: 18:21 bb0: { - StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21 - _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21 + StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:18:5: 18:21 + _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:18:5: 18:21 // mir::Constant - // + span: $DIR/inline-compatibility.rs:19:5: 19:19 + // + span: $DIR/inline-compatibility.rs:18:5: 18:19 // + literal: Const { ty: unsafe fn() {target_feature}, val: Value(Scalar()) } } bb1: { - StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:19:21: 19:22 - _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:18:44: 20:2 - return; // scope 0 at $DIR/inline-compatibility.rs:20:2: 20:2 + StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:18:21: 18:22 + _0 = const (); // scope 0 at $DIR/inline-compatibility.rs:17:44: 19:2 + return; // scope 0 at $DIR/inline-compatibility.rs:19:2: 19:2 } } diff --git a/src/test/mir-opt/inline/inline_generator.main.Inline.diff b/src/test/mir-opt/inline/inline_generator.main.Inline.diff new file mode 100644 index 0000000000000..aa32daa82dd51 --- /dev/null +++ b/src/test/mir-opt/inline/inline_generator.main.Inline.diff @@ -0,0 +1,123 @@ +- // MIR for `main` before Inline ++ // MIR for `main` after Inline + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/inline-generator.rs:8:11: 8:11 + let _1: std::ops::GeneratorState< as std::ops::Generator>::Yield, as std::ops::Generator>::Return>; // in scope 0 at $DIR/inline-generator.rs:9:9: 9:11 + let mut _2: std::pin::Pin<&mut impl std::ops::Generator>; // in scope 0 at $DIR/inline-generator.rs:9:14: 9:32 + let mut _3: &mut impl std::ops::Generator; // in scope 0 at $DIR/inline-generator.rs:9:23: 9:31 + let mut _4: impl std::ops::Generator; // in scope 0 at $DIR/inline-generator.rs:9:28: 9:31 ++ let mut _7: bool; // in scope 0 at $DIR/inline-generator.rs:9:14: 9:46 + scope 1 { + debug _r => _1; // in scope 1 at $DIR/inline-generator.rs:9:9: 9:11 + } ++ scope 2 (inlined g) { // at $DIR/inline-generator.rs:9:28: 9:31 ++ } ++ scope 3 (inlined Pin::<&mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]>::new) { // at $DIR/inline-generator.rs:9:14: 9:32 ++ debug pointer => _3; // in scope 3 at $DIR/inline-generator.rs:9:14: 9:32 ++ let mut _5: &mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]; // in scope 3 at $DIR/inline-generator.rs:9:14: 9:32 ++ scope 4 { ++ scope 5 (inlined Pin::<&mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]>::new_unchecked) { // at $DIR/inline-generator.rs:9:14: 9:32 ++ debug pointer => _5; // in scope 5 at $DIR/inline-generator.rs:9:14: 9:32 ++ let mut _6: &mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]; // in scope 5 at $DIR/inline-generator.rs:9:14: 9:32 ++ } ++ } ++ } ++ scope 6 (inlined g::{closure#0}) { // at $DIR/inline-generator.rs:9:14: 9:46 ++ debug a => _8; // in scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ let mut _8: bool; // in scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ let mut _9: u32; // in scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/inline-generator.rs:9:9: 9:11 + StorageLive(_2); // scope 0 at $DIR/inline-generator.rs:9:14: 9:32 + StorageLive(_3); // scope 0 at $DIR/inline-generator.rs:9:23: 9:31 + StorageLive(_4); // scope 0 at $DIR/inline-generator.rs:9:28: 9:31 +- _4 = g() -> bb1; // scope 0 at $DIR/inline-generator.rs:9:28: 9:31 +- // mir::Constant +- // + span: $DIR/inline-generator.rs:9:28: 9:29 +- // + literal: Const { ty: fn() -> impl std::ops::Generator {g}, val: Value(Scalar()) } +- } +- +- bb1: { ++ discriminant(_4) = 0; // scope 2 at $DIR/inline-generator.rs:9:28: 9:31 + _3 = &mut _4; // scope 0 at $DIR/inline-generator.rs:9:23: 9:31 +- _2 = Pin::<&mut impl Generator>::new(move _3) -> [return: bb2, unwind: bb4]; // scope 0 at $DIR/inline-generator.rs:9:14: 9:32 +- // mir::Constant +- // + span: $DIR/inline-generator.rs:9:14: 9:22 +- // + user_ty: UserType(0) +- // + literal: Const { ty: fn(&mut impl std::ops::Generator) -> std::pin::Pin<&mut impl std::ops::Generator> {std::pin::Pin::<&mut impl std::ops::Generator>::new}, val: Value(Scalar()) } +- } +- +- bb2: { ++ StorageLive(_5); // scope 4 at $DIR/inline-generator.rs:9:14: 9:32 ++ _5 = move _3; // scope 4 at $DIR/inline-generator.rs:9:14: 9:32 ++ StorageLive(_6); // scope 5 at $DIR/inline-generator.rs:9:14: 9:32 ++ _6 = move _5; // scope 5 at $DIR/inline-generator.rs:9:14: 9:32 ++ (_2.0: &mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]) = move _6; // scope 5 at $DIR/inline-generator.rs:9:14: 9:32 ++ StorageDead(_6); // scope 5 at $DIR/inline-generator.rs:9:14: 9:32 ++ StorageDead(_5); // scope 4 at $DIR/inline-generator.rs:9:14: 9:32 + StorageDead(_3); // scope 0 at $DIR/inline-generator.rs:9:31: 9:32 +- _1 = as Generator>::resume(move _2, const false) -> [return: bb3, unwind: bb4]; // scope 0 at $DIR/inline-generator.rs:9:14: 9:46 +- // mir::Constant +- // + span: $DIR/inline-generator.rs:9:33: 9:39 +- // + literal: Const { ty: for<'r> fn(std::pin::Pin<&'r mut impl std::ops::Generator>, bool) -> std::ops::GeneratorState< as std::ops::Generator>::Yield, as std::ops::Generator>::Return> { as std::ops::Generator>::resume}, val: Value(Scalar()) } ++ StorageLive(_7); // scope 0 at $DIR/inline-generator.rs:9:14: 9:46 ++ _7 = const false; // scope 0 at $DIR/inline-generator.rs:9:14: 9:46 ++ _9 = discriminant((*(_2.0: &mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]))); // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ switchInt(move _9) -> [0_u32: bb3, 1_u32: bb8, 3_u32: bb7, otherwise: bb9]; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 + } + +- bb3: { ++ bb1: { ++ StorageDead(_7); // scope 0 at $DIR/inline-generator.rs:9:14: 9:46 + StorageDead(_2); // scope 0 at $DIR/inline-generator.rs:9:45: 9:46 + StorageDead(_4); // scope 0 at $DIR/inline-generator.rs:9:46: 9:47 + _0 = const (); // scope 0 at $DIR/inline-generator.rs:8:11: 10:2 + StorageDead(_1); // scope 0 at $DIR/inline-generator.rs:10:1: 10:2 + return; // scope 0 at $DIR/inline-generator.rs:10:2: 10:2 + } + +- bb4 (cleanup): { ++ bb2 (cleanup): { + resume; // scope 0 at $DIR/inline-generator.rs:8:1: 10:2 ++ } ++ ++ bb3: { ++ _8 = move _7; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ switchInt(_8) -> [false: bb4, otherwise: bb5]; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ } ++ ++ bb4: { ++ ((_1 as Yielded).0: i32) = const 13_i32; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ goto -> bb6; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ } ++ ++ bb5: { ++ ((_1 as Yielded).0: i32) = const 7_i32; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ goto -> bb6; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ } ++ ++ bb6: { ++ discriminant(_1) = 0; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ discriminant((*(_2.0: &mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]))) = 3; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ goto -> bb1; // scope 0 at $DIR/inline-generator.rs:15:11: 15:39 ++ } ++ ++ bb7: { ++ ((_1 as Complete).0: bool) = move _7; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ discriminant(_1) = 1; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ discriminant((*(_2.0: &mut [generator@$DIR/inline-generator.rs:15:5: 15:41 {bool, i32}]))) = 1; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ goto -> bb1; // scope 0 at $DIR/inline-generator.rs:15:41: 15:41 ++ } ++ ++ bb8: { ++ assert(const false, "generator resumed after completion") -> [success: bb8, unwind: bb2]; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 ++ } ++ ++ bb9: { ++ unreachable; // scope 6 at $DIR/inline-generator.rs:9:14: 9:46 + } + } + From d486bfcbff107e8a6769e00c59d02b13c664b6ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 12 Nov 2020 00:00:00 +0000 Subject: [PATCH 21/26] Normalize function type during validation During inlining, the callee body is normalized and has types revealed, but some of locals corresponding to the arguments might come from the caller body which is not. As a result the caller body does not pass validation without additional normalization. --- compiler/rustc_mir/src/transform/validate.rs | 2 ++ .../auxiliary/lib.rs | 4 ---- .../ui/mir/mir-inlining/ice-issue-77306-1.rs | 18 ++++++++++++++---- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_mir/src/transform/validate.rs b/compiler/rustc_mir/src/transform/validate.rs index e1e6e71acb5a8..2fbfe13082f89 100644 --- a/compiler/rustc_mir/src/transform/validate.rs +++ b/compiler/rustc_mir/src/transform/validate.rs @@ -357,7 +357,9 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } TerminatorKind::Call { func, args, destination, cleanup, .. } => { + let param_env = self.param_env.with_reveal_all_normalized(self.tcx); let func_ty = func.ty(&self.body.local_decls, self.tcx); + let func_ty = self.tcx.normalize_erasing_regions(param_env, func_ty); match func_ty.kind() { ty::FnPtr(..) | ty::FnDef(..) => {} _ => self.fail( diff --git a/src/test/ui/issues/issue-50865-private-impl-trait/auxiliary/lib.rs b/src/test/ui/issues/issue-50865-private-impl-trait/auxiliary/lib.rs index fb4bf2b8b44e7..f3a51b415faca 100644 --- a/src/test/ui/issues/issue-50865-private-impl-trait/auxiliary/lib.rs +++ b/src/test/ui/issues/issue-50865-private-impl-trait/auxiliary/lib.rs @@ -1,7 +1,3 @@ -// revisions: default miropt -//[miropt]compile-flags: -Z mir-opt-level=2 -// ~^ This flag is for #77668, it used to be ICE. - #![crate_type = "lib"] pub fn bar

( // Error won't happen if "bar" is not generic diff --git a/src/test/ui/mir/mir-inlining/ice-issue-77306-1.rs b/src/test/ui/mir/mir-inlining/ice-issue-77306-1.rs index 4d083bf232155..ccb279f7fa212 100644 --- a/src/test/ui/mir/mir-inlining/ice-issue-77306-1.rs +++ b/src/test/ui/mir/mir-inlining/ice-issue-77306-1.rs @@ -1,17 +1,27 @@ -// run-pass +// Regression test for various issues related to normalization & inlining. +// * #68347, #77306, #77668 - missed normalization during inlining. +// * #78442 - missed normalization in validator after inlining. +// +// build-pass // compile-flags:-Zmir-opt-level=2 -// Previously ICEd because we did not normalize during inlining, -// see https://github.com/rust-lang/rust/pull/77306 for more discussion. - pub fn write() { create()() } +pub fn write_generic(_t: T) { + hide()(); +} + pub fn create() -> impl FnOnce() { || () } +pub fn hide() -> impl Fn() { + write +} + fn main() { write(); + write_generic(()); } From 99be78d135e73197e04221c139a219ea6436e72a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Thu, 12 Nov 2020 00:00:00 +0000 Subject: [PATCH 22/26] Always use param_env_reveal_all_normalized in validator --- compiler/rustc_mir/src/transform/validate.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_mir/src/transform/validate.rs b/compiler/rustc_mir/src/transform/validate.rs index 2fbfe13082f89..876ecee80c6a1 100644 --- a/compiler/rustc_mir/src/transform/validate.rs +++ b/compiler/rustc_mir/src/transform/validate.rs @@ -38,7 +38,9 @@ pub struct Validator { impl<'tcx> MirPass<'tcx> for Validator { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let def_id = body.source.def_id(); - let param_env = tcx.param_env(def_id); + // We need to param_env_reveal_all_normalized, as some optimizations + // change types in ways that require unfolding opaque types. + let param_env = tcx.param_env_reveal_all_normalized(def_id); let mir_phase = self.mir_phase; let always_live_locals = AlwaysLiveLocals::new(body); @@ -79,7 +81,6 @@ pub fn equal_up_to_regions( } // Normalize lifetimes away on both sides, then compare. - let param_env = param_env.with_reveal_all_normalized(tcx); let normalize = |ty: Ty<'tcx>| { tcx.normalize_erasing_regions( param_env, @@ -167,17 +168,14 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { return true; } // Normalize projections and things like that. - // FIXME: We need to reveal_all, as some optimizations change types in ways - // that require unfolding opaque types. - let param_env = self.param_env.with_reveal_all_normalized(self.tcx); - let src = self.tcx.normalize_erasing_regions(param_env, src); - let dest = self.tcx.normalize_erasing_regions(param_env, dest); + let src = self.tcx.normalize_erasing_regions(self.param_env, src); + let dest = self.tcx.normalize_erasing_regions(self.param_env, dest); // Type-changing assignments can happen when subtyping is used. While // all normal lifetimes are erased, higher-ranked types with their // late-bound lifetimes are still around and can lead to type // differences. So we compare ignoring lifetimes. - equal_up_to_regions(self.tcx, param_env, src, dest) + equal_up_to_regions(self.tcx, self.param_env, src, dest) } } @@ -357,9 +355,8 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { } } TerminatorKind::Call { func, args, destination, cleanup, .. } => { - let param_env = self.param_env.with_reveal_all_normalized(self.tcx); let func_ty = func.ty(&self.body.local_decls, self.tcx); - let func_ty = self.tcx.normalize_erasing_regions(param_env, func_ty); + let func_ty = self.tcx.normalize_erasing_regions(self.param_env, func_ty); match func_ty.kind() { ty::FnPtr(..) | ty::FnDef(..) => {} _ => self.fail( From c131063988ff8ac86bbf53a3bf998145c6b99d6f Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Thu, 12 Nov 2020 16:08:42 -0800 Subject: [PATCH 23/26] Added a unit test for BcbCounters Restructured the code a little, to allow getting both the mir::Body and coverage graph. --- .../src/transform/coverage/counters.rs | 1 - .../rustc_mir/src/transform/coverage/spans.rs | 10 +- .../rustc_mir/src/transform/coverage/tests.rs | 279 +++++++++++------- 3 files changed, 188 insertions(+), 102 deletions(-) diff --git a/compiler/rustc_mir/src/transform/coverage/counters.rs b/compiler/rustc_mir/src/transform/coverage/counters.rs index 7c4b30ca9e790..20f6a16e0f757 100644 --- a/compiler/rustc_mir/src/transform/coverage/counters.rs +++ b/compiler/rustc_mir/src/transform/coverage/counters.rs @@ -120,7 +120,6 @@ struct BcbCounters<'a> { basic_coverage_blocks: &'a mut CoverageGraph, } -// FIXME(richkadel): Add unit tests for `BcbCounters` functions/algorithms. impl<'a> BcbCounters<'a> { fn new( coverage_counters: &'a mut CoverageCounters, diff --git a/compiler/rustc_mir/src/transform/coverage/spans.rs b/compiler/rustc_mir/src/transform/coverage/spans.rs index f880d69bd64f6..95c49922262f6 100644 --- a/compiler/rustc_mir/src/transform/coverage/spans.rs +++ b/compiler/rustc_mir/src/transform/coverage/spans.rs @@ -645,7 +645,10 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> { } } -fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> Option { +pub(super) fn filtered_statement_span( + statement: &'a Statement<'tcx>, + body_span: Span, +) -> Option { match statement.kind { // These statements have spans that are often outside the scope of the executed source code // for their parent `BasicBlock`. @@ -686,7 +689,10 @@ fn filtered_statement_span(statement: &'a Statement<'tcx>, body_span: Span) -> O } } -fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -> Option { +pub(super) fn filtered_terminator_span( + terminator: &'a Terminator<'tcx>, + body_span: Span, +) -> Option { 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 diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs index 3e305a58c6bb2..7de965be53bde 100644 --- a/compiler/rustc_mir/src/transform/coverage/tests.rs +++ b/compiler/rustc_mir/src/transform/coverage/tests.rs @@ -1,14 +1,39 @@ +//! This crate hosts a selection of "unit tests" for components of the `InstrumentCoverage` MIR +//! pass. +//! +//! 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 +//! real, runtime versions of these mocked-up objects have constraints (such as cross-thread +//! limitations) and deep dependencies on other elements of the full Rust compiler (which is +//! *not* constructed or mocked for these tests). +//! +//! Of particular note, attempting to simply print elements of the `mir::Body` with default +//! `Debug` formatting can fail because some `Debug` format implementations require the +//! `TyCtxt`, obtained via a static global variable that is *not* set for these tests. +//! Initializing the global type context is prohibitively complex for the scope and scale of these +//! tests (essentially requiring initializing the entire compiler). +//! +//! Also note, some basic features of `Span` also rely on the `Span`s own "session globals", which +//! are unrelated to the `TyCtxt` global. Without initializing the `Span` session globals, some +//! basic, coverage-specific features would be impossible to test, but thankfully initializing these +//! globals is comparitively simpler. The easiest way is to wrap the test in a closure argument +//! to: `rustc_span::with_default_session_globals(|| { test_here(); })`. + +use super::counters; use super::debug; use super::graph; +use super::spans; use coverage_test_macros::let_bcb; use rustc_data_structures::graph::WithNumNodes; use rustc_data_structures::graph::WithSuccessors; use rustc_index::vec::{Idx, IndexVec}; +use rustc_middle::mir::coverage::CoverageKind; use rustc_middle::mir::*; use rustc_middle::ty::{self, DebruijnIndex, TyS, TypeFlags}; -use rustc_span::DUMMY_SP; +use rustc_span::{self, BytePos, Pos, Span, DUMMY_SP}; fn dummy_ty() -> &'static TyS<'static> { thread_local! { @@ -24,7 +49,6 @@ fn dummy_ty() -> &'static TyS<'static> { struct MockBlocks<'tcx> { blocks: IndexVec>, - source_info: SourceInfo, dummy_place: Place<'tcx>, next_local: usize, } @@ -33,7 +57,6 @@ impl<'tcx> MockBlocks<'tcx> { fn new() -> Self { Self { blocks: IndexVec::new(), - source_info: SourceInfo::outermost(DUMMY_SP), dummy_place: Place { local: RETURN_PLACE, projection: ty::List::empty() }, next_local: 0, } @@ -45,12 +68,19 @@ impl<'tcx> MockBlocks<'tcx> { Local::new(index) } - fn push(&mut self, num_nops: usize, kind: TerminatorKind<'tcx>) -> BasicBlock { - let nop = Statement { source_info: self.source_info, kind: StatementKind::Nop }; - + fn push(&mut self, kind: TerminatorKind<'tcx>) -> BasicBlock { + let next_lo = if let Some(last) = self.blocks.last() { + self.blocks[last].terminator().source_info.span.hi() + } else { + BytePos(1) + }; + let next_hi = next_lo + BytePos(1); self.blocks.push(BasicBlockData { - statements: std::iter::repeat(&nop).cloned().take(num_nops).collect(), - terminator: Some(Terminator { source_info: self.source_info, kind }), + statements: vec![], + terminator: Some(Terminator { + source_info: SourceInfo::outermost(Span::with_root_ctxt(next_lo, next_hi)), + kind, + }), is_cleanup: false, }) } @@ -75,7 +105,7 @@ impl<'tcx> MockBlocks<'tcx> { some_from_block: Option, to_kind: TerminatorKind<'tcx>, ) -> BasicBlock { - let new_block = self.push(1, to_kind); + let new_block = self.push(to_kind); if let Some(from_block) = some_from_block { self.link(from_block, new_block); } @@ -152,7 +182,10 @@ fn debug_basic_blocks(mir_body: &Body<'tcx>) -> String { .basic_blocks() .iter_enumerated() .map(|(bb, data)| { - let kind = &data.terminator().kind; + let term = &data.terminator(); + let kind = &term.kind; + let span = term.source_info.span; + let sp = format!("(span:{},{})", span.lo().to_u32(), span.hi().to_u32()); match kind { TerminatorKind::Assert { target, .. } | TerminatorKind::Call { destination: Some((_, target)), .. } @@ -163,12 +196,12 @@ fn debug_basic_blocks(mir_body: &Body<'tcx>) -> String { | TerminatorKind::Goto { target } | TerminatorKind::InlineAsm { destination: Some(target), .. } | TerminatorKind::Yield { resume: target, .. } => { - format!("{:?}:{} -> {:?}", bb, debug::term_type(kind), target) + format!("{}{:?}:{} -> {:?}", sp, bb, debug::term_type(kind), target) } TerminatorKind::SwitchInt { targets, .. } => { - format!("{:?}:{} -> {:?}", bb, debug::term_type(kind), targets) + format!("{}{:?}:{} -> {:?}", sp, bb, debug::term_type(kind), targets) } - _ => format!("{:?}:{}", bb, debug::term_type(kind)), + _ => format!("{}{:?}:{}", sp, bb, debug::term_type(kind)), } }) .collect::>() @@ -235,7 +268,7 @@ fn print_coverage_graphviz( } /// Create a mock `Body` with a simple flow. -fn mir_goto_switchint() -> Body<'a> { +fn goto_switchint() -> Body<'a> { let mut blocks = MockBlocks::new(); let start = blocks.call(None); let goto = blocks.goto(Some(start)); @@ -281,13 +314,22 @@ fn mir_goto_switchint() -> Body<'a> { mir_body } -fn covgraph_goto_switchint() -> graph::CoverageGraph { - let mir_body = mir_goto_switchint(); +macro_rules! assert_successors { + ($basic_coverage_blocks:ident, $i:ident, [$($successor:ident),*]) => { + let mut successors = $basic_coverage_blocks.successors[$i].clone(); + successors.sort_unstable(); + assert_eq!(successors, vec![$($successor),*]); + } +} + +#[test] +fn test_covgraph_goto_switchint() { + let mir_body = goto_switchint(); if false { println!("basic_blocks = {}", debug_basic_blocks(&mir_body)); } - let covgraph = graph::CoverageGraph::from_mir(&mir_body); - print_coverage_graphviz("covgraph_goto_switchint ", &mir_body, &covgraph); + let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz("covgraph_goto_switchint ", &mir_body, &basic_coverage_blocks); /* ┌──────────────┐ ┌─────────────────┐ │ bcb2: Return │ ◀── │ bcb0: SwitchInt │ @@ -299,11 +341,24 @@ fn covgraph_goto_switchint() -> graph::CoverageGraph { │ bcb1: Return │ └─────────────────┘ */ - covgraph + assert_eq!( + basic_coverage_blocks.num_nodes(), + 3, + "basic_coverage_blocks: {:?}", + basic_coverage_blocks.iter_enumerated().collect::>() + ); + + let_bcb!(0); + let_bcb!(1); + let_bcb!(2); + + assert_successors!(basic_coverage_blocks, bcb0, [bcb1, bcb2]); + assert_successors!(basic_coverage_blocks, bcb1, []); + assert_successors!(basic_coverage_blocks, bcb2, []); } /// Create a mock `Body` with a loop. -fn mir_switchint_then_loop_else_return() -> Body<'a> { +fn switchint_then_loop_else_return() -> Body<'a> { let mut blocks = MockBlocks::new(); let start = blocks.call(None); let switchint = blocks.switchint(Some(start)); @@ -342,10 +397,15 @@ fn mir_switchint_then_loop_else_return() -> Body<'a> { mir_body } -fn covgraph_switchint_then_loop_else_return() -> graph::CoverageGraph { - let mir_body = mir_switchint_then_loop_else_return(); - let covgraph = graph::CoverageGraph::from_mir(&mir_body); - print_coverage_graphviz("covgraph_switchint_then_loop_else_return", &mir_body, &covgraph); +#[test] +fn test_covgraph_switchint_then_loop_else_return() { + let mir_body = switchint_then_loop_else_return(); + let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); + print_coverage_graphviz( + "covgraph_switchint_then_loop_else_return", + &mir_body, + &basic_coverage_blocks, + ); /* ┌─────────────────┐ │ bcb0: Call │ @@ -365,11 +425,26 @@ fn covgraph_switchint_then_loop_else_return() -> graph::CoverageGraph { │ │ └─────────────────────────────────────┘ */ - covgraph + assert_eq!( + basic_coverage_blocks.num_nodes(), + 4, + "basic_coverage_blocks: {:?}", + basic_coverage_blocks.iter_enumerated().collect::>() + ); + + let_bcb!(0); + let_bcb!(1); + let_bcb!(2); + let_bcb!(3); + + assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); + assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); + assert_successors!(basic_coverage_blocks, bcb2, []); + assert_successors!(basic_coverage_blocks, bcb3, [bcb1]); } /// Create a mock `Body` with nested loops. -fn mir_switchint_loop_then_inner_loop_else_break() -> Body<'a> { +fn switchint_loop_then_inner_loop_else_break() -> Body<'a> { let mut blocks = MockBlocks::new(); let start = blocks.call(None); let switchint = blocks.switchint(Some(start)); @@ -438,13 +513,14 @@ fn mir_switchint_loop_then_inner_loop_else_break() -> Body<'a> { mir_body } -fn covgraph_switchint_loop_then_inner_loop_else_break() -> graph::CoverageGraph { - let mir_body = mir_switchint_loop_then_inner_loop_else_break(); - let covgraph = graph::CoverageGraph::from_mir(&mir_body); +#[test] +fn test_covgraph_switchint_loop_then_inner_loop_else_break() { + let mir_body = switchint_loop_then_inner_loop_else_break(); + let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); print_coverage_graphviz( "covgraph_switchint_loop_then_inner_loop_else_break", &mir_body, - &covgraph, + &basic_coverage_blocks, ); /* ┌─────────────────┐ @@ -477,23 +553,9 @@ fn covgraph_switchint_loop_then_inner_loop_else_break() -> graph::CoverageGraph │ │ └────────────────────────────────────────────┘ */ - covgraph -} - -macro_rules! assert_successors { - ($basic_coverage_blocks:ident, $i:ident, [$($successor:ident),*]) => { - let mut successors = $basic_coverage_blocks.successors[$i].clone(); - successors.sort_unstable(); - assert_eq!(successors, vec![$($successor),*]); - } -} - -#[test] -fn test_covgraph_goto_switchint() { - let basic_coverage_blocks = covgraph_goto_switchint(); assert_eq!( basic_coverage_blocks.num_nodes(), - 3, + 7, "basic_coverage_blocks: {:?}", basic_coverage_blocks.iter_enumerated().collect::>() ); @@ -501,15 +563,24 @@ fn test_covgraph_goto_switchint() { let_bcb!(0); let_bcb!(1); let_bcb!(2); + let_bcb!(3); + let_bcb!(4); + let_bcb!(5); + let_bcb!(6); - assert_successors!(basic_coverage_blocks, bcb0, [bcb1, bcb2]); - assert_successors!(basic_coverage_blocks, bcb1, []); + assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); + assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); assert_successors!(basic_coverage_blocks, bcb2, []); + assert_successors!(basic_coverage_blocks, bcb3, [bcb4]); + assert_successors!(basic_coverage_blocks, bcb4, [bcb5, bcb6]); + assert_successors!(basic_coverage_blocks, bcb5, [bcb1]); + assert_successors!(basic_coverage_blocks, bcb6, [bcb4]); } #[test] fn test_find_loop_backedges_none() { - let basic_coverage_blocks = covgraph_goto_switchint(); + let mir_body = goto_switchint(); + let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); if false { println!( "basic_coverage_blocks = {:?}", @@ -526,30 +597,10 @@ fn test_find_loop_backedges_none() { ); } -#[test] -fn test_covgraph_switchint_then_loop_else_return() { - let basic_coverage_blocks = covgraph_switchint_then_loop_else_return(); - assert_eq!( - basic_coverage_blocks.num_nodes(), - 4, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); - - let_bcb!(0); - let_bcb!(1); - let_bcb!(2); - let_bcb!(3); - - assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); - assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); - assert_successors!(basic_coverage_blocks, bcb2, []); - assert_successors!(basic_coverage_blocks, bcb3, [bcb1]); -} - #[test] fn test_find_loop_backedges_one() { - let basic_coverage_blocks = covgraph_switchint_then_loop_else_return(); + let mir_body = switchint_then_loop_else_return(); + let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); let backedges = graph::find_loop_backedges(&basic_coverage_blocks); assert_eq!( backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), @@ -564,36 +615,10 @@ fn test_find_loop_backedges_one() { assert_eq!(backedges[bcb1], vec![bcb3]); } -#[test] -fn test_covgraph_switchint_loop_then_inner_loop_else_break() { - let basic_coverage_blocks = covgraph_switchint_loop_then_inner_loop_else_break(); - assert_eq!( - basic_coverage_blocks.num_nodes(), - 7, - "basic_coverage_blocks: {:?}", - basic_coverage_blocks.iter_enumerated().collect::>() - ); - - let_bcb!(0); - let_bcb!(1); - let_bcb!(2); - let_bcb!(3); - let_bcb!(4); - let_bcb!(5); - let_bcb!(6); - - assert_successors!(basic_coverage_blocks, bcb0, [bcb1]); - assert_successors!(basic_coverage_blocks, bcb1, [bcb2, bcb3]); - assert_successors!(basic_coverage_blocks, bcb2, []); - assert_successors!(basic_coverage_blocks, bcb3, [bcb4]); - assert_successors!(basic_coverage_blocks, bcb4, [bcb5, bcb6]); - assert_successors!(basic_coverage_blocks, bcb5, [bcb1]); - assert_successors!(basic_coverage_blocks, bcb6, [bcb4]); -} - #[test] fn test_find_loop_backedges_two() { - let basic_coverage_blocks = covgraph_switchint_loop_then_inner_loop_else_break(); + let mir_body = switchint_loop_then_inner_loop_else_break(); + let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); let backedges = graph::find_loop_backedges(&basic_coverage_blocks); assert_eq!( backedges.iter_enumerated().map(|(_bcb, backedges)| backedges.len()).sum::(), @@ -613,7 +638,8 @@ fn test_find_loop_backedges_two() { #[test] fn test_traverse_coverage_with_loops() { - let basic_coverage_blocks = covgraph_switchint_loop_then_inner_loop_else_break(); + let mir_body = switchint_loop_then_inner_loop_else_break(); + let basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body); let mut traversed_in_order = Vec::new(); let mut traversal = graph::TraverseCoverageGraphWithLoops::new(&basic_coverage_blocks); while let Some(bcb) = traversal.next(&basic_coverage_blocks) { @@ -630,3 +656,58 @@ fn test_traverse_coverage_with_loops() { "bcb6 should not be visited until all nodes inside the first loop have been visited" ); } + +fn synthesize_body_span_from_terminators(mir_body: &Body<'_>) -> Span { + let mut some_span: Option = None; + for (_, data) in mir_body.basic_blocks().iter_enumerated() { + let term_span = data.terminator().source_info.span; + if let Some(span) = some_span.as_mut() { + *span = span.to(term_span); + } else { + some_span = Some(term_span) + } + } + some_span.expect("body must have at least one BasicBlock") +} + +#[test] +fn test_make_bcb_counters() { + rustc_span::with_default_session_globals(|| { + let mir_body = goto_switchint(); + let body_span = synthesize_body_span_from_terminators(&mir_body); + 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) = + spans::filtered_terminator_span(data.terminator(&mir_body), body_span) + { + coverage_spans.push(spans::CoverageSpan::for_terminator(span, bcb, data.last_bb())); + } + } + let mut coverage_counters = counters::CoverageCounters::new(0); + let intermediate_expressions = coverage_counters + .make_bcb_counters(&mut basic_coverage_blocks, &coverage_spans) + .expect("should be Ok"); + assert_eq!(intermediate_expressions.len(), 0); + + let_bcb!(1); + assert_eq!( + 1, // coincidentally, bcb1 has a `Counter` with id = 1 + match basic_coverage_blocks[bcb1].counter().expect("should have a counter") { + CoverageKind::Counter { id, .. } => id, + _ => panic!("expected a Counter"), + } + .as_u32() + ); + + let_bcb!(2); + assert_eq!( + 2, // coincidentally, bcb2 has a `Counter` with id = 2 + match basic_coverage_blocks[bcb2].counter().expect("should have a counter") { + CoverageKind::Counter { id, .. } => id, + _ => panic!("expected a Counter"), + } + .as_u32() + ); + }); +} From 309d863e372227f9a3eed89e46cba0ff3b1ea8c8 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Fri, 13 Nov 2020 14:58:21 +0100 Subject: [PATCH 24/26] Fix wrong XPath --- src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs index 0c047e6249a53..f895a4c2104ba 100644 --- a/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs +++ b/src/test/rustdoc/raw-ident-eliminate-r-hashtag.rs @@ -1,5 +1,7 @@ // ignore-tidy-linelength +#![crate_type="lib"] + pub mod internal { // @has 'raw_ident_eliminate_r_hashtag/internal/struct.mod.html' pub struct r#mod; @@ -8,13 +10,13 @@ pub mod internal { /// /// [name]: mod /// [other name]: crate::internal::mod - // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'name' - // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'other name' + // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//*a[@href="../../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'name' + // @has 'raw_ident_eliminate_r_hashtag/internal/struct.B.html' '//*a[@href="../../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'other name' pub struct B; } /// See [name]. /// /// [name]: internal::mod -// @has 'raw_ident_eliminate_r_hashtag/struct.A.html' '//a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'name' -struct A; +// @has 'raw_ident_eliminate_r_hashtag/struct.A.html' '//*a[@href="../raw_ident_eliminate_r_hashtag/internal/struct.mod.html"]' 'name' +pub struct A; From b4b0ef3e4b6ebf1571b2c38b952c940c8b94e91e Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Fri, 13 Nov 2020 09:02:25 -0800 Subject: [PATCH 25/26] Addressed feedback --- compiler/rustc_mir/src/transform/coverage/mod.rs | 2 +- .../src/transform/coverage/test_macros/src/lib.rs | 4 +--- .../rustc_mir/src/transform/coverage/tests.rs | 15 ++++++++------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_mir/src/transform/coverage/mod.rs b/compiler/rustc_mir/src/transform/coverage/mod.rs index de37a67a1742f..192bb6680e420 100644 --- a/compiler/rustc_mir/src/transform/coverage/mod.rs +++ b/compiler/rustc_mir/src/transform/coverage/mod.rs @@ -34,7 +34,7 @@ use rustc_span::{CharPos, Pos, SourceFile, Span, Symbol}; /// A simple error message wrapper for `coverage::Error`s. #[derive(Debug)] -pub(self) struct Error { +struct Error { message: String, } diff --git a/compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs b/compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs index ea551c7745556..3d6095d2738cb 100644 --- a/compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs +++ b/compiler/rustc_mir/src/transform/coverage/test_macros/src/lib.rs @@ -2,7 +2,5 @@ use proc_macro::TokenStream; #[proc_macro] pub fn let_bcb(item: TokenStream) -> TokenStream { - format!("let bcb{} = graph::BasicCoverageBlock::from_usize({}); let _ = {};", item, item, item) - .parse() - .unwrap() + format!("let bcb{} = graph::BasicCoverageBlock::from_usize({});", item, item).parse().unwrap() } diff --git a/compiler/rustc_mir/src/transform/coverage/tests.rs b/compiler/rustc_mir/src/transform/coverage/tests.rs index 7de965be53bde..d36f1b8e5f670 100644 --- a/compiler/rustc_mir/src/transform/coverage/tests.rs +++ b/compiler/rustc_mir/src/transform/coverage/tests.rs @@ -35,6 +35,9 @@ use rustc_middle::mir::*; use rustc_middle::ty::{self, DebruijnIndex, TyS, TypeFlags}; use rustc_span::{self, BytePos, Pos, Span, DUMMY_SP}; +// All `TEMP_BLOCK` targets should be replaced before calling `to_body() -> mir::Body`. +const TEMP_BLOCK: BasicBlock = BasicBlock::MAX; + fn dummy_ty() -> &'static TyS<'static> { thread_local! { static DUMMY_TYS: &'static TyS<'static> = Box::leak(box TyS::make_for_test( @@ -123,7 +126,7 @@ impl<'tcx> MockBlocks<'tcx> { if branch_index > branches.len() { branches.push((branches.len() as u128, old_otherwise)); while branches.len() < branch_index { - branches.push((branches.len() as u128, START_BLOCK)); + branches.push((branches.len() as u128, TEMP_BLOCK)); } to_block } else { @@ -143,7 +146,7 @@ impl<'tcx> MockBlocks<'tcx> { TerminatorKind::Call { func: Operand::Copy(self.dummy_place.clone()), args: vec![], - destination: Some((self.dummy_place.clone(), START_BLOCK)), + destination: Some((self.dummy_place.clone(), TEMP_BLOCK)), cleanup: None, from_hir_call: false, fn_span: DUMMY_SP, @@ -152,16 +155,14 @@ impl<'tcx> MockBlocks<'tcx> { } fn goto(&mut self, some_from_block: Option) -> BasicBlock { - self.add_block_from(some_from_block, TerminatorKind::Goto { target: START_BLOCK }) + self.add_block_from(some_from_block, TerminatorKind::Goto { target: TEMP_BLOCK }) } fn switchint(&mut self, some_from_block: Option) -> BasicBlock { - let move_ = |place: Place<'tcx>| Operand::Move(place); - let discriminant = Place::from(self.new_temp()); let switchint_kind = TerminatorKind::SwitchInt { - discr: move_(discriminant), + discr: Operand::Move(Place::from(self.new_temp())), switch_ty: dummy_ty(), - targets: SwitchTargets::static_if(0, START_BLOCK, START_BLOCK), + targets: SwitchTargets::static_if(0, TEMP_BLOCK, TEMP_BLOCK), }; self.add_block_from(some_from_block, switchint_kind) } From bf6902ca61ae6e91a3ab95bed88426926f245d02 Mon Sep 17 00:00:00 2001 From: Matt Brubeck Date: Fri, 13 Nov 2020 10:23:50 -0800 Subject: [PATCH 26/26] Add BTreeMap::retain and BTreeSet::retain --- library/alloc/src/collections/btree/map.rs | 24 +++++++++++++++++++ .../alloc/src/collections/btree/map/tests.rs | 11 +++++++++ library/alloc/src/collections/btree/set.rs | 24 +++++++++++++++++++ .../alloc/src/collections/btree/set/tests.rs | 11 +++++++++ 4 files changed, 70 insertions(+) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 49122f53d33ad..7151d3763f01f 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -863,6 +863,30 @@ impl BTreeMap { } } + /// Retains only the elements specified by the predicate. + /// + /// In other words, remove all pairs `(k, v)` such that `f(&k, &mut v)` returns `false`. + /// + /// # Examples + /// + /// ``` + /// #![feature(btree_retain)] + /// use std::collections::BTreeMap; + /// + /// let mut map: BTreeMap = (0..8).map(|x| (x, x*10)).collect(); + /// // Keep only the elements with even-numbered keys. + /// map.retain(|&k, _| k % 2 == 0); + /// assert!(map.into_iter().eq(vec![(0, 0), (2, 20), (4, 40), (6, 60)])); + /// ``` + #[inline] + #[unstable(feature = "btree_retain", issue = "79025")] + pub fn retain(&mut self, mut f: F) + where + F: FnMut(&K, &mut V) -> bool, + { + self.drain_filter(|k, v| !f(k, v)); + } + /// Moves all elements from `other` into `Self`, leaving `other` empty. /// /// # Examples diff --git a/library/alloc/src/collections/btree/map/tests.rs b/library/alloc/src/collections/btree/map/tests.rs index dd3ebcccf76a5..11dbb584abdac 100644 --- a/library/alloc/src/collections/btree/map/tests.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -808,6 +808,17 @@ fn test_range_mut() { map.check(); } +#[test] +fn test_retain() { + let mut map: BTreeMap = (0..100).map(|x| (x, x * 10)).collect(); + + map.retain(|&k, _| k % 2 == 0); + assert_eq!(map.len(), 50); + assert_eq!(map[&2], 20); + assert_eq!(map[&4], 40); + assert_eq!(map[&6], 60); +} + mod test_drain_filter { use super::*; diff --git a/library/alloc/src/collections/btree/set.rs b/library/alloc/src/collections/btree/set.rs index 684019f8f5f5e..1a807100653bc 100644 --- a/library/alloc/src/collections/btree/set.rs +++ b/library/alloc/src/collections/btree/set.rs @@ -798,6 +798,30 @@ impl BTreeSet { Recover::take(&mut self.map, value) } + /// Retains only the elements specified by the predicate. + /// + /// In other words, remove all elements `e` such that `f(&e)` returns `false`. + /// + /// # Examples + /// + /// ``` + /// #![feature(btree_retain)] + /// use std::collections::BTreeSet; + /// + /// let xs = [1, 2, 3, 4, 5, 6]; + /// let mut set: BTreeSet = xs.iter().cloned().collect(); + /// // Keep only the even numbers. + /// set.retain(|&k| k % 2 == 0); + /// assert!(set.iter().eq([2, 4, 6].iter())); + /// ``` + #[unstable(feature = "btree_retain", issue = "79025")] + pub fn retain(&mut self, mut f: F) + where + F: FnMut(&T) -> bool, + { + self.drain_filter(|v| !f(v)); + } + /// Moves all elements from `other` into `Self`, leaving `other` empty. /// /// # Examples diff --git a/library/alloc/src/collections/btree/set/tests.rs b/library/alloc/src/collections/btree/set/tests.rs index 52cde8299e418..ef40a048a382e 100644 --- a/library/alloc/src/collections/btree/set/tests.rs +++ b/library/alloc/src/collections/btree/set/tests.rs @@ -324,6 +324,17 @@ fn test_is_subset() { assert_eq!(is_subset(&[99, 100], &large), false); } +#[test] +fn test_retain() { + let xs = [1, 2, 3, 4, 5, 6]; + let mut set: BTreeSet = xs.iter().cloned().collect(); + set.retain(|&k| k % 2 == 0); + assert_eq!(set.len(), 3); + assert!(set.contains(&2)); + assert!(set.contains(&4)); + assert!(set.contains(&6)); +} + #[test] fn test_drain_filter() { let mut x: BTreeSet<_> = [1].iter().copied().collect();