From 1825810a898842b4660fd684cea66906b7e32500 Mon Sep 17 00:00:00 2001 From: Soveu Date: Tue, 16 Feb 2021 18:48:42 +0100 Subject: [PATCH 01/20] Vec::dedup optimization --- library/alloc/src/vec/mod.rs | 50 +++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index b6166617789a0..88f9f624aba2d 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1514,15 +1514,53 @@ impl Vec { /// assert_eq!(vec, ["foo", "bar", "baz", "bar"]); /// ``` #[stable(feature = "dedup_by", since = "1.16.0")] - pub fn dedup_by(&mut self, same_bucket: F) + pub fn dedup_by(&mut self, mut same_bucket: F) where F: FnMut(&mut T, &mut T) -> bool, { - let len = { - let (dedup, _) = self.as_mut_slice().partition_dedup_by(same_bucket); - dedup.len() - }; - self.truncate(len); + let len = self.len(); + if len <= 1 { + return; + } + + let ptr = self.as_mut_ptr(); + /* Offset of the element we want to check if it is duplicate */ + let mut read: usize = 1; + /* Offset of the place where we want to place the non-duplicate + * when we find it. */ + let mut write: usize = 1; + + /* Drop items while going through Vec, it should be more efficient than + * doing slice partition_dedup + truncate */ + + /* INVARIANT: len > read >= write > write-1 >= 0 + * SAFETY: Because of the invariant, read_ptr, prev_ptr and write_ptr + * are always in-bounds and read_ptr never aliases prev_ptr */ + unsafe { + while read < len { + let read_ptr = ptr.add(read); + let prev_ptr = ptr.add(write.wrapping_sub(1)); + + if same_bucket(&mut *read_ptr, &mut *prev_ptr) { + /* We have found duplicate, drop it in-place */ + ptr::drop_in_place(read_ptr); + } else { + let write_ptr = ptr.add(write); + + /* Looks like doing just `copy` can be faster than + * conditional `copy_nonoverlapping` */ + ptr::copy(read_ptr, write_ptr, 1); + + /* We have filled that place, so go further */ + write += 1; + } + + read += 1; + } + + /* `write` items are inside vec, rest is already dropped */ + self.set_len(write); + } } /// Appends an element to the back of a collection. From c114894b90971ad7c6743ca5961f276cae1e2b27 Mon Sep 17 00:00:00 2001 From: Soveu Date: Wed, 17 Feb 2021 17:21:12 +0100 Subject: [PATCH 02/20] Vec::dedup optimization - panic gracefully --- library/alloc/src/vec/mod.rs | 81 +++++++++++++++++++++++++++++------- 1 file changed, 65 insertions(+), 16 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index 88f9f624aba2d..e65adb6c77eda 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1523,43 +1523,92 @@ impl Vec { return; } - let ptr = self.as_mut_ptr(); - /* Offset of the element we want to check if it is duplicate */ - let mut read: usize = 1; - /* Offset of the place where we want to place the non-duplicate - * when we find it. */ - let mut write: usize = 1; + /* INVARIANT: vec.len() > read >= write > write-1 >= 0 */ + struct FillGapOnDrop<'a, T, A: core::alloc::Allocator> { + /* Offset of the element we want to check if it is duplicate */ + read: usize, + + /* Offset of the place where we want to place the non-duplicate + * when we find it. */ + write: usize, + + /* The Vec that would need correction if `same_bucket` panicked */ + vec: &'a mut Vec, + } + + impl<'a, T, A: core::alloc::Allocator> Drop for FillGapOnDrop<'a, T, A> { + fn drop(&mut self) { + /* This code gets executed either at the end of `dedup_by` or + * when `same_bucket` panics */ + + /* SAFETY (if finishing successfully): self.read == len, so + * no data is copied and length is set correctly */ + + /* SAFETY (if panicing): invariant guarantees that `read - write` + * and `len - read` never overflow and that the copy is always + * in-bounds. */ + unsafe { + let ptr = self.vec.as_mut_ptr(); + let len = self.vec.len(); + + /* How many items were left when `same_bucket` paniced. + * Basically vec[read..].len() */ + let items_left = len - self.read; + + /* Pointer to first item in vec[write..write+items_left] slice */ + let dropped_ptr = ptr.add(self.write); + /* Pointer to first item in vec[read..] slice */ + let valid_ptr = ptr.add(self.read); + + /* Copy `vec[read..]` to `vec[write..write+items_left]`. + * The slices can overlap, so `copy_nonoverlapping` cannot be used */ + ptr::copy(valid_ptr, dropped_ptr, items_left); + + /* How many items have been already dropped + * Basically vec[read..write].len() */ + let dropped = self.read - self.write; + + self.vec.set_len(len - dropped); + } + } + } + + let mut gap = FillGapOnDrop { read: 1, write: 1, vec: self }; + + let ptr = gap.vec.as_mut_ptr(); /* Drop items while going through Vec, it should be more efficient than * doing slice partition_dedup + truncate */ - /* INVARIANT: len > read >= write > write-1 >= 0 - * SAFETY: Because of the invariant, read_ptr, prev_ptr and write_ptr + /* SAFETY: Because of the invariant, read_ptr, prev_ptr and write_ptr * are always in-bounds and read_ptr never aliases prev_ptr */ unsafe { - while read < len { - let read_ptr = ptr.add(read); - let prev_ptr = ptr.add(write.wrapping_sub(1)); + while gap.read < len { + let read_ptr = ptr.add(gap.read); + let prev_ptr = ptr.add(gap.write.wrapping_sub(1)); if same_bucket(&mut *read_ptr, &mut *prev_ptr) { /* We have found duplicate, drop it in-place */ ptr::drop_in_place(read_ptr); } else { - let write_ptr = ptr.add(write); + let write_ptr = ptr.add(gap.write); /* Looks like doing just `copy` can be faster than * conditional `copy_nonoverlapping` */ ptr::copy(read_ptr, write_ptr, 1); /* We have filled that place, so go further */ - write += 1; + gap.write += 1; } - read += 1; + gap.read += 1; } - /* `write` items are inside vec, rest is already dropped */ - self.set_len(write); + /* Technically we could let `gap` clean up with its Drop, but + * when `same_bucket` is guaranteed to not panic, this bloats a little + * the codegen, so we just do it manually */ + gap.vec.set_len(gap.write); + mem::forget(gap); } } From e292e6e8ce10e5372c0e0433f4b73e768b101e6e Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Tue, 9 Feb 2021 22:14:32 +0100 Subject: [PATCH 03/20] Stabilize `peekable_peek_mut` Resolves #78302 Update peekable.rs Update library/core/src/iter/traits/iterator.rs Co-authored-by: Ashley Mannix --- library/core/src/iter/adapters/peekable.rs | 3 +- library/core/src/iter/traits/iterator.rs | 46 ++++++++++++++++------ library/core/tests/lib.rs | 1 - 3 files changed, 35 insertions(+), 15 deletions(-) diff --git a/library/core/src/iter/adapters/peekable.rs b/library/core/src/iter/adapters/peekable.rs index 43301444e3e2c..dfa59df6dab6c 100644 --- a/library/core/src/iter/adapters/peekable.rs +++ b/library/core/src/iter/adapters/peekable.rs @@ -233,7 +233,6 @@ impl Peekable { /// Basic usage: /// /// ``` - /// #![feature(peekable_peek_mut)] /// let mut iter = [1, 2, 3].iter().peekable(); /// /// // Like with `peek()`, we can see into the future without advancing the iterator. @@ -251,7 +250,7 @@ impl Peekable { /// assert_eq!(iter.collect::>(), vec![&5, &3]); /// ``` #[inline] - #[unstable(feature = "peekable_peek_mut", issue = "78302")] + #[stable(feature = "peekable_peek_mut", since = "1.52.0")] pub fn peek_mut(&mut self) -> Option<&mut I::Item> { let iter = &mut self.iter; self.peeked.get_or_insert_with(|| iter.next()).as_mut() diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index a38b35a5b5c74..e9fba30a344a1 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -937,20 +937,16 @@ pub trait Iterator { Enumerate::new(self) } - /// Creates an iterator which can use [`peek`] to look at the next element of - /// the iterator without consuming it. + /// Creates an iterator which can use the [`peek`] and [`peek_mut`] methods + /// to look at the next element of the iterator without consuming it. See + /// their documentation for more information. /// - /// Adds a [`peek`] method to an iterator. See its documentation for - /// more information. + /// Note that the underlying iterator is still advanced when [`peek`] or + /// [`peek_mut`] are called for the first time: In order to retrieve the + /// next element, [`next`] is called on the underlying iterator, hence any + /// side effects (i.e. anything other than fetching the next value) of + /// the [`next`] method will occur. /// - /// Note that the underlying iterator is still advanced when [`peek`] is - /// called for the first time: In order to retrieve the next element, - /// [`next`] is called on the underlying iterator, hence any side effects (i.e. - /// anything other than fetching the next value) of the [`next`] method - /// will occur. - /// - /// [`peek`]: Peekable::peek - /// [`next`]: Iterator::next /// /// # Examples /// @@ -977,6 +973,32 @@ pub trait Iterator { /// assert_eq!(iter.peek(), None); /// assert_eq!(iter.next(), None); /// ``` + /// + /// Using [`peek_mut`] to mutate the next item without advancing the + /// iterator: + /// + /// ``` + /// let xs = [1, 2, 3]; + /// + /// let mut iter = xs.iter().peekable(); + /// + /// // peek_mut() lets us see into the future + /// assert_eq!(iter.peek_mut(), Some(&mut &1)); + /// assert_eq!(iter.peek_mut(), Some(&mut &1)); + /// assert_eq!(iter.next(), Some(&1)); + /// + /// if let Some(mut p) = iter.peek_mut() { + /// assert_eq!(*p, &2); + /// // put a value into the iterator + /// *p = &1000; + /// } + /// + /// // The value reappears as the iterator continues + /// assert_eq!(iter.collect::>(), vec![&1000, &3]); + /// ``` + /// [`peek`]: Peekable::peek + /// [`peek_mut`]: Peekable::peek_mut + /// [`next`]: Iterator::next #[inline] #[stable(feature = "rust1", since = "1.0.0")] fn peekable(self) -> Peekable diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 12182accc471a..9dc606ff2c11b 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -68,7 +68,6 @@ #![feature(option_result_unwrap_unchecked)] #![feature(result_into_ok_or_err)] #![feature(option_unwrap_none)] -#![feature(peekable_peek_mut)] #![cfg_attr(not(bootstrap), feature(ptr_metadata))] #![feature(once_cell)] #![feature(unsafe_block_in_unsafe_fn)] From 2abab1f688fe0d4a740b216b298f32fbb48b653b Mon Sep 17 00:00:00 2001 From: Soveu Date: Mon, 15 Mar 2021 20:24:35 +0100 Subject: [PATCH 04/20] Vec::dedup optimization - add tests --- library/alloc/tests/lib.rs | 1 + library/alloc/tests/vec.rs | 73 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index dd98f806451d8..fd1612f316bfd 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -21,6 +21,7 @@ #![feature(vecdeque_binary_search)] #![feature(slice_group_by)] #![feature(vec_extend_from_within)] +#![feature(slice_partition_dedup)] use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 2969da58d4268..1d28fc9eebf82 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -2096,3 +2096,76 @@ fn test_extend_from_within() { assert_eq!(v, ["a", "b", "c", "b", "c", "a", "b"]); } + +#[test] +fn test_vec_dedup_by() { + let mut vec: Vec = vec![1, -1, 2, 3, 1, -5, 5, -2, 2]; + + vec.dedup_by(|a, b| a.abs() == b.abs()); + + assert_eq!(vec, [1, 2, 3, 1, -5, -2]); +} + +#[test] +fn test_vec_dedup_empty() { + let mut vec: Vec = Vec::new(); + + vec.dedup(); + + assert_eq!(vec, []); +} + +#[test] +fn test_vec_dedup_one() { + let mut vec = vec![12i32]; + + vec.dedup(); + + assert_eq!(vec, [12]); +} + +#[test] +fn test_vec_dedup_multiple_ident() { + let mut vec = vec![12, 12, 12, 12, 12, 11, 11, 11, 11, 11, 11]; + + vec.dedup(); + + assert_eq!(vec, [12, 11]); +} + +#[test] +fn test_vec_dedup_partialeq() { + #[derive(Debug)] + struct Foo(i32, i32); + + impl PartialEq for Foo { + fn eq(&self, other: &Foo) -> bool { + self.0 == other.0 + } + } + + let mut vec = vec![Foo(0, 1), Foo(0, 5), Foo(1, 7), Foo(1, 9)]; + + vec.dedup(); + assert_eq!(vec, [Foo(0, 1), Foo(1, 7)]); +} + +#[test] +fn test_vec_dedup() { + let mut vec: Vec = Vec::with_capacity(8); + let mut template = vec.clone(); + + for x in 0u8..255u8 { + vec.clear(); + template.clear(); + + let iter = (0..8).map(move |bit| (x >> bit) & 1 == 1); + vec.extend(iter); + template.extend_from_slice(&vec); + + let (dedup, _) = template.partition_dedup(); + vec.dedup(); + + assert_eq!(vec, dedup); + } +} From afdbc9ece176ccac7b1d156efcb397d089d88b5a Mon Sep 17 00:00:00 2001 From: Soveu Date: Mon, 15 Mar 2021 20:36:29 +0100 Subject: [PATCH 05/20] Vec::dedup optimization - finishing polishes --- library/alloc/src/vec/mod.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index e65adb6c77eda..19198d4eeefd9 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1538,13 +1538,9 @@ impl Vec { impl<'a, T, A: core::alloc::Allocator> Drop for FillGapOnDrop<'a, T, A> { fn drop(&mut self) { - /* This code gets executed either at the end of `dedup_by` or - * when `same_bucket` panics */ + /* This code gets executed when `same_bucket` panics */ - /* SAFETY (if finishing successfully): self.read == len, so - * no data is copied and length is set correctly */ - - /* SAFETY (if panicing): invariant guarantees that `read - write` + /* SAFETY: invariant guarantees that `read - write` * and `len - read` never overflow and that the copy is always * in-bounds. */ unsafe { @@ -1553,7 +1549,7 @@ impl Vec { /* How many items were left when `same_bucket` paniced. * Basically vec[read..].len() */ - let items_left = len - self.read; + let items_left = len.wrapping_sub(self.read); /* Pointer to first item in vec[write..write+items_left] slice */ let dropped_ptr = ptr.add(self.write); @@ -1566,7 +1562,7 @@ impl Vec { /* How many items have been already dropped * Basically vec[read..write].len() */ - let dropped = self.read - self.write; + let dropped = self.read.wrapping_sub(self.write); self.vec.set_len(len - dropped); } @@ -1574,7 +1570,6 @@ impl Vec { } let mut gap = FillGapOnDrop { read: 1, write: 1, vec: self }; - let ptr = gap.vec.as_mut_ptr(); /* Drop items while going through Vec, it should be more efficient than @@ -1593,8 +1588,9 @@ impl Vec { } else { let write_ptr = ptr.add(gap.write); - /* Looks like doing just `copy` can be faster than - * conditional `copy_nonoverlapping` */ + /* Because `read_ptr` can be equal to `write_ptr`, we either + * have to use `copy` or conditional `copy_nonoverlapping`. + * Looks like the first option is faster. */ ptr::copy(read_ptr, write_ptr, 1); /* We have filled that place, so go further */ From d6a7c1d47fdf68626b5535ff24bd115e4aba7d71 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 15 Mar 2021 15:54:25 -0400 Subject: [PATCH 06/20] Extend `proc_macro_back_compat` lint to `procedural-masquerade` We now lint on *any* use of `procedural-masquerade` crate. While this crate still exists, its main reverse dependency (`cssparser`) no longer depends on it. Any crates still depending off should stop doing so, as it only exists to support very old Rust versions. If a crate actually needs to support old versions of rustc via `procedural-masquerade`, then they'll just need to accept the warning until we remove it entirely (at the same time as the back-compat hack). The latest version of `procedural-masquerade` does not work with the latest rustc, but trying to check for the version seems like more trouble than it's worth. While working on this, I realized that the `proc-macro-hack` check was never actually doing anything. The corresponding enum variant in `proc-macro-hack` is named `Value` or `Nested` - it has never been called `Input`. Due to a strange Crater issue, the Crater run that tested adding this did *not* end up testing it - some of the crates that would have failed did not actually have their tests checked, making it seem as though the `proc-macro-hack` check was working. The Crater issue is being discussed at https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Nearly.20identical.20Crater.20runs.20processed.20a.20crate.20differently/near/230406661 Despite the `proc-macro-hack` check not actually doing anything, we haven't gotten any reports from users about their build being broken. I went ahead and removed it entirely, since it's clear that no one is being affected by the `proc-macro-hack` regression in practice. --- compiler/rustc_ast/src/token.rs | 27 ------------- compiler/rustc_expand/src/base.rs | 40 +++++++++++++++++++ compiler/rustc_expand/src/proc_macro.rs | 3 +- .../rustc_expand/src/proc_macro_server.rs | 2 +- .../issue-73933-procedural-masquerade.rs | 13 ++++++ .../issue-73933-procedural-masquerade.stderr | 25 ++++++++++++ .../issue-73933-procedural-masquerade.stdout | 22 ++++++++++ 7 files changed, 103 insertions(+), 29 deletions(-) create mode 100644 src/test/ui/proc-macro/issue-73933-procedural-masquerade.rs create mode 100644 src/test/ui/proc-macro/issue-73933-procedural-masquerade.stderr create mode 100644 src/test/ui/proc-macro/issue-73933-procedural-masquerade.stdout diff --git a/compiler/rustc_ast/src/token.rs b/compiler/rustc_ast/src/token.rs index 093f7f2668c46..7e58426d27de4 100644 --- a/compiler/rustc_ast/src/token.rs +++ b/compiler/rustc_ast/src/token.rs @@ -784,33 +784,6 @@ impl Nonterminal { NtTT(tt) => tt.span(), } } - - /// This nonterminal looks like some specific enums from - /// `proc-macro-hack` and `procedural-masquerade` crates. - /// We need to maintain some special pretty-printing behavior for them due to incorrect - /// asserts in old versions of those crates and their wide use in the ecosystem. - /// See issue #73345 for more details. - /// FIXME(#73933): Remove this eventually. - pub fn pretty_printing_compatibility_hack(&self) -> bool { - let item = match self { - NtItem(item) => item, - NtStmt(stmt) => match &stmt.kind { - ast::StmtKind::Item(item) => item, - _ => return false, - }, - _ => return false, - }; - - let name = item.ident.name; - if name == sym::ProceduralMasqueradeDummyType || name == sym::ProcMacroHack { - if let ast::ItemKind::Enum(enum_def, _) = &item.kind { - if let [variant] = &*enum_def.variants { - return variant.ident.name == sym::Input; - } - } - } - false - } } impl PartialEq for Nonterminal { diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index dd93fe8350e42..ce65793051837 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -10,6 +10,8 @@ use rustc_attr::{self as attr, Deprecation, Stability}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::{self, Lrc}; use rustc_errors::{DiagnosticBuilder, ErrorReported}; +use rustc_lint_defs::builtin::PROC_MACRO_BACK_COMPAT; +use rustc_lint_defs::BuiltinLintDiagnostics; use rustc_parse::{self, nt_to_tokenstream, parser, MACRO_ARGUMENTS}; use rustc_session::{parse::ParseSess, Limit, Session}; use rustc_span::def_id::DefId; @@ -1241,3 +1243,41 @@ pub fn get_exprs_from_tts( } Some(es) } + +/// This nonterminal looks like some specific enums from +/// `proc-macro-hack` and `procedural-masquerade` crates. +/// We need to maintain some special pretty-printing behavior for them due to incorrect +/// asserts in old versions of those crates and their wide use in the ecosystem. +/// See issue #73345 for more details. +/// FIXME(#73933): Remove this eventually. +pub(crate) fn pretty_printing_compatibility_hack(nt: &Nonterminal, sess: &ParseSess) -> bool { + let item = match nt { + Nonterminal::NtItem(item) => item, + Nonterminal::NtStmt(stmt) => match &stmt.kind { + ast::StmtKind::Item(item) => item, + _ => return false, + }, + _ => return false, + }; + + let name = item.ident.name; + if name == sym::ProceduralMasqueradeDummyType { + if let ast::ItemKind::Enum(enum_def, _) = &item.kind { + if let [variant] = &*enum_def.variants { + if variant.ident.name == sym::Input { + sess.buffer_lint_with_diagnostic( + &PROC_MACRO_BACK_COMPAT, + item.ident.span, + ast::CRATE_NODE_ID, + "using `procedural-masquerade` crate", + BuiltinLintDiagnostics::ProcMacroBackCompat( + "The `procedural-masquerade` crate has been unnecessary since Rust 1.30.0. \ + Versions of this crate below 0.1.7 will eventually stop compiling.".to_string()) + ); + return true; + } + } + } + } + false +} diff --git a/compiler/rustc_expand/src/proc_macro.rs b/compiler/rustc_expand/src/proc_macro.rs index 8cbaa7c945a81..61b776ff2d280 100644 --- a/compiler/rustc_expand/src/proc_macro.rs +++ b/compiler/rustc_expand/src/proc_macro.rs @@ -90,7 +90,8 @@ impl MultiItemModifier for ProcMacroDerive { } _ => unreachable!(), }; - let input = if item.pretty_printing_compatibility_hack() { + let input = if crate::base::pretty_printing_compatibility_hack(&item, &ecx.sess.parse_sess) + { TokenTree::token(token::Interpolated(Lrc::new(item)), DUMMY_SP).into() } else { nt_to_tokenstream(&item, &ecx.sess.parse_sess, CanSynthesizeMissingTokens::Yes) diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs index 837fad905800a..67edfe19da383 100644 --- a/compiler/rustc_expand/src/proc_macro_server.rs +++ b/compiler/rustc_expand/src/proc_macro_server.rs @@ -187,7 +187,7 @@ impl FromInternal<(TreeAndSpacing, &'_ ParseSess, &'_ mut Vec)> delimiter: Delimiter::None, stream, span: DelimSpan::from_single(span), - flatten: nt.pretty_printing_compatibility_hack(), + flatten: crate::base::pretty_printing_compatibility_hack(&nt, sess), }) } } diff --git a/src/test/ui/proc-macro/issue-73933-procedural-masquerade.rs b/src/test/ui/proc-macro/issue-73933-procedural-masquerade.rs new file mode 100644 index 0000000000000..abc3d2691a307 --- /dev/null +++ b/src/test/ui/proc-macro/issue-73933-procedural-masquerade.rs @@ -0,0 +1,13 @@ +// check-pass +// aux-build:test-macros.rs + +#[macro_use] +extern crate test_macros; + +#[derive(Print)] +enum ProceduralMasqueradeDummyType { //~ WARN using +//~| WARN this was previously + Input +} + +fn main() {} diff --git a/src/test/ui/proc-macro/issue-73933-procedural-masquerade.stderr b/src/test/ui/proc-macro/issue-73933-procedural-masquerade.stderr new file mode 100644 index 0000000000000..0b930705e3510 --- /dev/null +++ b/src/test/ui/proc-macro/issue-73933-procedural-masquerade.stderr @@ -0,0 +1,25 @@ +warning: using `procedural-masquerade` crate + --> $DIR/issue-73933-procedural-masquerade.rs:8:6 + | +LL | enum ProceduralMasqueradeDummyType { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(proc_macro_back_compat)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83125 + = note: The `procedural-masquerade` crate has been unnecessary since Rust 1.30.0. Versions of this crate below 0.1.7 will eventually stop compiling. + +warning: 1 warning emitted + +Future incompatibility report: Future breakage date: None, diagnostic: +warning: using `procedural-masquerade` crate + --> $DIR/issue-73933-procedural-masquerade.rs:8:6 + | +LL | enum ProceduralMasqueradeDummyType { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(proc_macro_back_compat)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #83125 + = note: The `procedural-masquerade` crate has been unnecessary since Rust 1.30.0. Versions of this crate below 0.1.7 will eventually stop compiling. + diff --git a/src/test/ui/proc-macro/issue-73933-procedural-masquerade.stdout b/src/test/ui/proc-macro/issue-73933-procedural-masquerade.stdout new file mode 100644 index 0000000000000..8edd68f8a3b84 --- /dev/null +++ b/src/test/ui/proc-macro/issue-73933-procedural-masquerade.stdout @@ -0,0 +1,22 @@ +PRINT-DERIVE INPUT (DISPLAY): enum ProceduralMasqueradeDummyType { Input, } +PRINT-DERIVE RE-COLLECTED (DISPLAY): enum ProceduralMasqueradeDummyType { Input } +PRINT-DERIVE INPUT (DEBUG): TokenStream [ + Ident { + ident: "enum", + span: #0 bytes(100..104), + }, + Ident { + ident: "ProceduralMasqueradeDummyType", + span: #0 bytes(105..134), + }, + Group { + delimiter: Brace, + stream: TokenStream [ + Ident { + ident: "Input", + span: #0 bytes(186..191), + }, + ], + span: #0 bytes(135..193), + }, +] From 2285f11724e2fa3251c94c9ab7672544099600e2 Mon Sep 17 00:00:00 2001 From: Soveu Date: Mon, 15 Mar 2021 21:26:22 +0100 Subject: [PATCH 07/20] Vec::dedup optimization - add test for panic --- library/alloc/tests/vec.rs | 54 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 1d28fc9eebf82..536fb4af5647d 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -7,6 +7,7 @@ use std::mem::{size_of, swap}; use std::ops::Bound::*; use std::panic::{catch_unwind, AssertUnwindSafe}; use std::rc::Rc; +use std::sync::atomic::{AtomicU32, Ordering}; use std::vec::{Drain, IntoIter}; struct DropCounter<'a> { @@ -2169,3 +2170,56 @@ fn test_vec_dedup() { assert_eq!(vec, dedup); } } + +#[test] +fn test_vec_dedup_panicking() { + #[derive(Debug)] + struct Panic { + drop_counter: &'static AtomicU32, + value: bool, + index: usize, + } + + impl PartialEq for Panic { + fn eq(&self, other: &Self) -> bool { + self.value == other.value + } + } + + impl Drop for Panic { + fn drop(&mut self) { + let x = self.drop_counter.fetch_add(1, Ordering::SeqCst); + assert!(x != 4); + } + } + + static DROP_COUNTER: AtomicU32 = AtomicU32::new(0); + let expected = [ + Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 }, + Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 }, + Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 }, + Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 }, + ]; + let mut vec = vec![ + Panic { drop_counter: &DROP_COUNTER, value: false, index: 0 }, + // these elements get deduplicated + Panic { drop_counter: &DROP_COUNTER, value: false, index: 1 }, + Panic { drop_counter: &DROP_COUNTER, value: false, index: 2 }, + Panic { drop_counter: &DROP_COUNTER, value: false, index: 3 }, + Panic { drop_counter: &DROP_COUNTER, value: false, index: 4 }, + // here it panics + Panic { drop_counter: &DROP_COUNTER, value: false, index: 5 }, + Panic { drop_counter: &DROP_COUNTER, value: true, index: 6 }, + Panic { drop_counter: &DROP_COUNTER, value: true, index: 7 }, + ]; + + let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + vec.dedup(); + })); + + let ok = vec.iter().zip(expected.iter()).all(|(x, y)| x.index == y.index); + + if !ok { + panic!("expected: {:?}\ngot: {:?}\n", expected, vec); + } +} From 1796cc0e6c43f1958d746c94608503c900b3a28e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sat, 13 Mar 2021 00:00:00 +0000 Subject: [PATCH 08/20] Make source-based code coverage compatible with MIR inlining When codegenning code coverage use the instance that coverage data was originally generated for, to ensure basic level of compatibility with MIR inlining. --- .../src/coverageinfo/mapgen.rs | 2 +- .../rustc_codegen_ssa/src/coverageinfo/map.rs | 21 +++++++--- .../rustc_codegen_ssa/src/mir/coverageinfo.rs | 25 +++++++---- .../rustc_codegen_ssa/src/mir/statement.rs | 2 +- .../rustc_mir/src/transform/coverage/query.rs | 41 +++++++++++++++---- compiler/rustc_mir/src/transform/inline.rs | 9 ---- compiler/rustc_session/src/config.rs | 19 --------- 7 files changed, 68 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 444a9d4ba0463..1cea927f50c45 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -254,7 +254,7 @@ fn save_function_record( /// /// 1. The file name of an "Unreachable" function must match the file name of the existing /// codegenned (covered) function to which the unreachable code regions will be added. -/// 2. The function to which the unreachable code regions will be added must not be a genaric +/// 2. The function to which the unreachable code regions will be added must not be a generic /// function (must not have type parameters) because the coverage tools will get confused /// if the codegenned function has more than one instantiation and additional `CodeRegion`s /// attached to only one of those instantiations. diff --git a/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs b/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs index 549b8d41f5130..af6482fdbc24f 100644 --- a/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs +++ b/compiler/rustc_codegen_ssa/src/coverageinfo/map.rs @@ -8,7 +8,7 @@ use rustc_middle::mir::coverage::{ use rustc_middle::ty::Instance; use rustc_middle::ty::TyCtxt; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub struct Expression { lhs: ExpressionOperandId, op: Op, @@ -64,7 +64,9 @@ impl<'tcx> FunctionCoverage<'tcx> { /// Adds a code region to be counted by an injected counter intrinsic. pub fn add_counter(&mut self, id: CounterValueReference, region: CodeRegion) { - self.counters[id].replace(region).expect_none("add_counter called with duplicate `id`"); + if let Some(previous_region) = self.counters[id].replace(region.clone()) { + assert_eq!(previous_region, region, "add_counter: code region for id changed"); + } } /// Both counters and "counter expressions" (or simply, "expressions") can be operands in other @@ -94,9 +96,18 @@ impl<'tcx> FunctionCoverage<'tcx> { expression_id, lhs, op, rhs, region ); let expression_index = self.expression_index(u32::from(expression_id)); - self.expressions[expression_index] - .replace(Expression { lhs, op, rhs, region }) - .expect_none("add_counter_expression called with duplicate `id_descending_from_max`"); + if let Some(previous_expression) = self.expressions[expression_index].replace(Expression { + lhs, + op, + rhs, + region: region.clone(), + }) { + assert_eq!( + previous_expression, + Expression { lhs, op, rhs, region }, + "add_counter_expression: expression for id changed" + ); + } } /// Add a region that will be marked as "unreachable", with a constant "zero counter". diff --git a/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs b/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs index a115d35866638..5ab1baafb57de 100644 --- a/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs @@ -2,27 +2,38 @@ use crate::traits::*; use rustc_middle::mir::coverage::*; use rustc_middle::mir::Coverage; +use rustc_middle::mir::SourceScope; use super::FunctionCx; impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { - pub fn codegen_coverage(&self, bx: &mut Bx, coverage: Coverage) { + pub fn codegen_coverage(&self, bx: &mut Bx, coverage: Coverage, scope: SourceScope) { + // Determine the instance that coverage data was originally generated for. + let scope_data = &self.mir.source_scopes[scope]; + let instance = if let Some((inlined_instance, _)) = scope_data.inlined { + self.monomorphize(inlined_instance) + } else if let Some(inlined_scope) = scope_data.inlined_parent_scope { + self.monomorphize(self.mir.source_scopes[inlined_scope].inlined.unwrap().0) + } else { + self.instance + }; + let Coverage { kind, code_region } = coverage; match kind { CoverageKind::Counter { function_source_hash, id } => { - if bx.set_function_source_hash(self.instance, function_source_hash) { + if bx.set_function_source_hash(instance, function_source_hash) { // If `set_function_source_hash()` returned true, the coverage map is enabled, // so continue adding the counter. if let Some(code_region) = code_region { // Note: Some counters do not have code regions, but may still be referenced // from expressions. In that case, don't add the counter to the coverage map, // but do inject the counter intrinsic. - bx.add_coverage_counter(self.instance, id, code_region); + bx.add_coverage_counter(instance, id, code_region); } - let coverageinfo = bx.tcx().coverageinfo(self.instance.def_id()); + let coverageinfo = bx.tcx().coverageinfo(instance.def_id()); - let fn_name = bx.create_pgo_func_name_var(self.instance); + let fn_name = bx.create_pgo_func_name_var(instance); let hash = bx.const_u64(function_source_hash); let num_counters = bx.const_u32(coverageinfo.num_counters); let index = bx.const_u32(u32::from(id)); @@ -34,11 +45,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } } CoverageKind::Expression { id, lhs, op, rhs } => { - bx.add_coverage_counter_expression(self.instance, id, lhs, op, rhs, code_region); + bx.add_coverage_counter_expression(instance, id, lhs, op, rhs, code_region); } CoverageKind::Unreachable => { bx.add_coverage_unreachable( - self.instance, + instance, code_region.expect("unreachable regions always have code regions"), ); } diff --git a/compiler/rustc_codegen_ssa/src/mir/statement.rs b/compiler/rustc_codegen_ssa/src/mir/statement.rs index 5523e5f2e8604..fe7f6288adb27 100644 --- a/compiler/rustc_codegen_ssa/src/mir/statement.rs +++ b/compiler/rustc_codegen_ssa/src/mir/statement.rs @@ -112,7 +112,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { bx } mir::StatementKind::Coverage(box ref coverage) => { - self.codegen_coverage(&mut bx, coverage.clone()); + self.codegen_coverage(&mut bx, coverage.clone(), statement.source_info.scope); bx } mir::StatementKind::CopyNonOverlapping(box mir::CopyNonOverlapping { diff --git a/compiler/rustc_mir/src/transform/coverage/query.rs b/compiler/rustc_mir/src/transform/coverage/query.rs index 4b455a6a1ba72..de8447f1974e7 100644 --- a/compiler/rustc_mir/src/transform/coverage/query.rs +++ b/compiler/rustc_mir/src/transform/coverage/query.rs @@ -1,8 +1,7 @@ use super::*; use rustc_middle::mir::coverage::*; -use rustc_middle::mir::visit::Visitor; -use rustc_middle::mir::{self, Coverage, CoverageInfo, Location}; +use rustc_middle::mir::{self, Body, Coverage, CoverageInfo}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::def_id::DefId; @@ -85,10 +84,21 @@ impl CoverageVisitor { } } } -} -impl Visitor<'_> for CoverageVisitor { - fn visit_coverage(&mut self, coverage: &Coverage, _location: Location) { + fn visit_body(&mut self, body: &Body<'_>) { + for bb_data in body.basic_blocks().iter() { + for statement in bb_data.statements.iter() { + if let StatementKind::Coverage(box ref coverage) = statement.kind { + if is_inlined(body, statement) { + continue; + } + self.visit_coverage(coverage); + } + } + } + } + + fn visit_coverage(&mut self, coverage: &Coverage) { if self.add_missing_operands { match coverage.kind { CoverageKind::Expression { lhs, rhs, .. } => { @@ -129,10 +139,14 @@ fn coverageinfo_from_mir<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo } fn covered_file_name<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Option { - for bb_data in mir_body(tcx, def_id).basic_blocks().iter() { + let body = mir_body(tcx, def_id); + for bb_data in body.basic_blocks().iter() { for statement in bb_data.statements.iter() { if let StatementKind::Coverage(box ref coverage) = statement.kind { if let Some(code_region) = coverage.code_region.as_ref() { + if is_inlined(body, statement) { + continue; + } return Some(code_region.file_name); } } @@ -151,13 +165,17 @@ fn mir_body<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx mir::Body<'tcx> { } fn covered_code_regions<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Vec<&'tcx CodeRegion> { - mir_body(tcx, def_id) - .basic_blocks() + let body = mir_body(tcx, def_id); + body.basic_blocks() .iter() .map(|data| { data.statements.iter().filter_map(|statement| match statement.kind { StatementKind::Coverage(box ref coverage) => { - coverage.code_region.as_ref() // may be None + if is_inlined(body, statement) { + None + } else { + coverage.code_region.as_ref() // may be None + } } _ => None, }) @@ -165,3 +183,8 @@ fn covered_code_regions<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> Vec<&'tcx Cod .flatten() .collect() } + +fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { + let scope_data = &body.source_scopes[statement.source_info.scope]; + scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some() +} diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index cf85503b3d54b..26e7555a61d23 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -39,15 +39,6 @@ struct CallSite<'tcx> { /// Returns true if MIR inlining is enabled in the current compilation session. crate fn is_enabled(tcx: TyCtxt<'_>) -> bool { - if tcx.sess.opts.debugging_opts.instrument_coverage { - // Since `Inline` happens after `InstrumentCoverage`, the function-specific coverage - // counters can be invalidated, such as by merging coverage counter statements from - // a pre-inlined function into a different function. This kind of change is invalid, - // so inlining must be skipped. Note: This check is performed here so inlining can - // be disabled without preventing other optimizations (regardless of `mir_opt_level`). - return false; - } - if let Some(enabled) = tcx.sess.opts.debugging_opts.inline_mir { return enabled; } diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index c1be90efc7299..75078a1231163 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1937,25 +1937,6 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { } Some(SymbolManglingVersion::V0) => {} } - - if let Some(mir_opt_level) = debugging_opts.mir_opt_level { - if mir_opt_level > 1 { - // Functions inlined during MIR transform can, at best, make it impossible to - // effectively cover inlined functions, and, at worst, break coverage map generation - // during LLVM codegen. For example, function counter IDs are only unique within a - // function. Inlining after these counters are injected can produce duplicate counters, - // resulting in an invalid coverage map (and ICE); so this option combination is not - // allowed. - early_warn( - error_format, - &format!( - "`-Z mir-opt-level={}` (or any level > 1) enables function inlining, which \ - is incompatible with `-Z instrument-coverage`. Inlining will be disabled.", - mir_opt_level, - ), - ); - } - } } if let Ok(graphviz_font) = std::env::var("RUSTC_GRAPHVIZ_FONT") { From 5a9538acb55f0980728a159996fdeff77b845455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sat, 13 Mar 2021 00:00:00 +0000 Subject: [PATCH 09/20] Functions inlined into reachable functions are reachable Consider functions to be reachable for code coverage purposes, either when they reach the code generation directly, or indirectly as inlined part of another function. --- .../src/coverageinfo/mapgen.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 8 ++++++ .../src/monomorphize/partitioning/mod.rs | 25 +++++++++++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 1cea927f50c45..352638aa88ee8 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -284,7 +284,7 @@ fn add_unreachable_coverage<'tcx>( let all_def_ids: DefIdSet = tcx.mir_keys(LOCAL_CRATE).iter().map(|local_def_id| local_def_id.to_def_id()).collect(); - let (codegenned_def_ids, _) = tcx.collect_and_partition_mono_items(LOCAL_CRATE); + let codegenned_def_ids = tcx.codegened_and_inlined_items(LOCAL_CRATE); let mut unreachable_def_ids_by_file: FxHashMap> = FxHashMap::default(); for &non_codegenned_def_id in all_def_ids.difference(codegenned_def_ids) { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index b03b26d64606c..f912653d920ca 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1397,6 +1397,14 @@ rustc_queries! { query is_codegened_item(def_id: DefId) -> bool { desc { |tcx| "determining whether `{}` needs codegen", tcx.def_path_str(def_id) } } + + /// All items participating in code generation together with items inlined into them. + query codegened_and_inlined_items(_: CrateNum) + -> &'tcx DefIdSet { + eval_always + desc { "codegened_and_inlined_items" } + } + query codegen_unit(_: Symbol) -> &'tcx CodegenUnit<'tcx> { desc { "codegen_unit" } } diff --git a/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs b/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs index b68a8104fba72..dc2379fd92b83 100644 --- a/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs +++ b/compiler/rustc_mir/src/monomorphize/partitioning/mod.rs @@ -424,8 +424,33 @@ fn collect_and_partition_mono_items<'tcx>( (tcx.arena.alloc(mono_items), codegen_units) } +fn codegened_and_inlined_items<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> &'tcx DefIdSet { + let (items, cgus) = tcx.collect_and_partition_mono_items(cnum); + let mut visited = DefIdSet::default(); + let mut result = items.clone(); + + for cgu in cgus { + for (item, _) in cgu.items() { + if let MonoItem::Fn(ref instance) = item { + let did = instance.def_id(); + if !visited.insert(did) { + continue; + } + for scope in &tcx.instance_mir(instance.def).source_scopes { + if let Some((ref inlined, _)) = scope.inlined { + result.insert(inlined.def_id()); + } + } + } + } + } + + tcx.arena.alloc(result) +} + pub fn provide(providers: &mut Providers) { providers.collect_and_partition_mono_items = collect_and_partition_mono_items; + providers.codegened_and_inlined_items = codegened_and_inlined_items; providers.is_codegened_item = |tcx, def_id| { let (all_mono_items, _) = tcx.collect_and_partition_mono_items(LOCAL_CRATE); From ad8f9af7cbeae48db0606cb5e39cbc04e5e35789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Sat, 13 Mar 2021 00:00:00 +0000 Subject: [PATCH 10/20] Remove inline-instrument-coverage-fail.rs test case --- .../inline-instrument-coverage-fail.rs | 21 ------------------- .../inline-instrument-coverage-fail.stderr | 2 -- 2 files changed, 23 deletions(-) delete mode 100644 src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs delete mode 100644 src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.stderr diff --git a/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs b/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs deleted file mode 100644 index 8ed7f25d2bbdb..0000000000000 --- a/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs +++ /dev/null @@ -1,21 +0,0 @@ -// Ensures -Zmir-opt-level=3 (specifically, inlining) is not allowed with -Zinstrument-coverage. -// Regression test for issue #80060. -// -// needs-profiler-support -// build-pass -// compile-flags: -Zmir-opt-level=3 -Zinstrument-coverage -#[inline(never)] -fn foo() {} - -pub fn baz() { - bar(); -} - -#[inline(always)] -fn bar() { - foo(); -} - -fn main() { - bar(); -} diff --git a/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.stderr b/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.stderr deleted file mode 100644 index d482afc395d19..0000000000000 --- a/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.stderr +++ /dev/null @@ -1,2 +0,0 @@ -warning: `-Z mir-opt-level=3` (or any level > 1) enables function inlining, which is incompatible with `-Z instrument-coverage`. Inlining will be disabled. - From 4b6cc0c20488a598b97f887cbc747aee61bee7df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Mon, 15 Mar 2021 00:00:00 +0000 Subject: [PATCH 11/20] Add support for compile-flags in coverage tests --- src/test/run-make-fulldeps/coverage-reports/Makefile | 6 +++--- src/test/run-make-fulldeps/coverage-spanview/Makefile | 8 ++------ src/test/run-make-fulldeps/coverage/async.rs | 2 +- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/test/run-make-fulldeps/coverage-reports/Makefile b/src/test/run-make-fulldeps/coverage-reports/Makefile index 31583eaa8fee5..af75ec5e24d13 100644 --- a/src/test/run-make-fulldeps/coverage-reports/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports/Makefile @@ -82,13 +82,13 @@ endif %: $(SOURCEDIR)/lib/%.rs # Compile the test library with coverage instrumentation $(RUSTC) $(SOURCEDIR)/lib/$@.rs \ - $$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/lib/$@.rs && echo "--edition=2018" ) \ + $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/lib/$@.rs) \ --crate-type rlib -Zinstrument-coverage %: $(SOURCEDIR)/%.rs # Compile the test program with coverage instrumentation $(RUSTC) $(SOURCEDIR)/$@.rs \ - $$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/$@.rs && echo "--edition=2018" ) \ + $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \ -L "$(TMPDIR)" -Zinstrument-coverage # Run it in order to generate some profiling data, @@ -107,7 +107,7 @@ endif # Run it through rustdoc as well to cover doctests LLVM_PROFILE_FILE="$(TMPDIR)"/$@-%p.profraw \ $(RUSTDOC) --crate-name workaround_for_79771 --test $(SOURCEDIR)/$@.rs \ - $$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/$@.rs && echo "--edition=2018" ) \ + $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \ -L "$(TMPDIR)" -Zinstrument-coverage \ -Z unstable-options --persist-doctests=$(TMPDIR)/rustdoc-$@ diff --git a/src/test/run-make-fulldeps/coverage-spanview/Makefile b/src/test/run-make-fulldeps/coverage-spanview/Makefile index cd54ac0ed4c75..b0bfa7074db94 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/Makefile +++ b/src/test/run-make-fulldeps/coverage-spanview/Makefile @@ -38,9 +38,7 @@ endif %: $(SOURCEDIR)/lib/%.rs # Compile the test library with coverage instrumentation $(RUSTC) $(SOURCEDIR)/lib/$@.rs \ - $$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/lib/$@.rs && \ - echo "--edition=2018" \ - ) \ + $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/lib/$@.rs) \ --crate-type rlib \ -Ztrim-diagnostic-paths=no \ -Zinstrument-coverage \ @@ -70,9 +68,7 @@ endif %: $(SOURCEDIR)/%.rs # Compile the test program with coverage instrumentation $(RUSTC) $(SOURCEDIR)/$@.rs \ - $$( grep -q '^\/\/ require-rust-edition-2018' $(SOURCEDIR)/$@.rs && \ - echo "--edition=2018" \ - ) \ + $$( sed -nE 's#^// compile-flags:(.*)#\1# p' $(SOURCEDIR)/$@.rs) \ -L "$(TMPDIR)" \ -Ztrim-diagnostic-paths=no \ -Zinstrument-coverage \ diff --git a/src/test/run-make-fulldeps/coverage/async.rs b/src/test/run-make-fulldeps/coverage/async.rs index 5553af92465ca..d5ec32deac125 100644 --- a/src/test/run-make-fulldeps/coverage/async.rs +++ b/src/test/run-make-fulldeps/coverage/async.rs @@ -1,6 +1,6 @@ #![allow(unused_assignments, dead_code)] -// require-rust-edition-2018 +// compile-flags: --edition=2018 async fn c(x: u8) -> u8 { if x == 8 { From 0d84e0b68c78006c4aa6f5ec021d92277be7d3cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 16 Mar 2021 00:00:00 +0000 Subject: [PATCH 12/20] Add test case for -Zinline-mir & -Zinstrument-coverage --- .../expected_show_coverage.async.txt | 2 +- .../expected_show_coverage.inline.txt | 53 +++++ .../expected_show_coverage.uses_crate.txt | 4 +- ....display.-------.InstrumentCoverage.0.html | 161 +++++++++++++++ ...ne.error.-------.InstrumentCoverage.0.html | 79 ++++++++ ...e.length.-------.InstrumentCoverage.0.html | 82 ++++++++ ...ine.main.-------.InstrumentCoverage.0.html | 94 +++++++++ ...ermutate.-------.InstrumentCoverage.0.html | 183 ++++++++++++++++++ ...utations.-------.InstrumentCoverage.0.html | 113 +++++++++++ ...ine.swap.-------.InstrumentCoverage.0.html | 173 +++++++++++++++++ src/test/run-make-fulldeps/coverage/inline.rs | 51 +++++ 11 files changed, 992 insertions(+), 3 deletions(-) create mode 100644 src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.display.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.error.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.length.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.main.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutate.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutations.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.swap.-------.InstrumentCoverage.0.html create mode 100644 src/test/run-make-fulldeps/coverage/inline.rs diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt index 824bddaa40155..3f9403e6f70b8 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt @@ -1,6 +1,6 @@ 1| |#![allow(unused_assignments, dead_code)] 2| | - 3| |// require-rust-edition-2018 + 3| |// compile-flags: --edition=2018 4| | 5| 1|async fn c(x: u8) -> u8 { 6| 1| if x == 8 { diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt new file mode 100644 index 0000000000000..6148d89ed75e1 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.inline.txt @@ -0,0 +1,53 @@ + 1| |// compile-flags: -Zinline-mir + 2| | + 3| |use std::fmt::Display; + 4| | + 5| 1|fn main() { + 6| 1| permutations(&['a', 'b', 'c']); + 7| 1|} + 8| | + 9| |#[inline(always)] + 10| 1|fn permutations(xs: &[T]) { + 11| 1| let mut ys = xs.to_owned(); + 12| 1| permutate(&mut ys, 0); + 13| 1|} + 14| | + 15| 16|fn permutate(xs: &mut [T], k: usize) { + 16| 16| let n = length(xs); + 17| 16| if k == n { + 18| 6| display(xs); + 19| 10| } else if k < n { + 20| 15| for i in k..n { + ^10 + 21| 15| swap(xs, i, k); + 22| 15| permutate(xs, k + 1); + 23| 15| swap(xs, i, k); + 24| 15| } + 25| 0| } else { + 26| 0| error(); + 27| 0| } + 28| 16|} + 29| | + 30| 16|fn length(xs: &[T]) -> usize { + 31| 16| xs.len() + 32| 16|} + 33| | + 34| |#[inline] + 35| 30|fn swap(xs: &mut [T], i: usize, j: usize) { + 36| 30| let t = xs[i]; + 37| 30| xs[i] = xs[j]; + 38| 30| xs[j] = t; + 39| 30|} + 40| | + 41| 6|fn display(xs: &[T]) { + 42| 18| for x in xs { + 43| 18| print!("{}", x); + 44| 18| } + 45| 6| println!(); + 46| 6|} + 47| | + 48| |#[inline(always)] + 49| |fn error() { + 50| | panic!("error"); + 51| |} + diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt index 0c28305dd7725..bc2f673349a67 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.uses_crate.txt @@ -19,12 +19,12 @@ 18| 2| println!("used_only_from_bin_crate_generic_function with {:?}", arg); 19| 2|} ------------------ - | used_crate::used_only_from_bin_crate_generic_function::<&str>: + | used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec>: | 17| 1|pub fn used_only_from_bin_crate_generic_function(arg: T) { | 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); | 19| 1|} ------------------ - | used_crate::used_only_from_bin_crate_generic_function::<&alloc::vec::Vec>: + | used_crate::used_only_from_bin_crate_generic_function::<&str>: | 17| 1|pub fn used_only_from_bin_crate_generic_function(arg: T) { | 18| 1| println!("used_only_from_bin_crate_generic_function with {:?}", arg); | 19| 1|} diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.display.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.display.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..6287516636ea9 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.display.-------.InstrumentCoverage.0.html @@ -0,0 +1,161 @@ + + + + +inline.display - Coverage Spans + + + +
@0,1⦊fn display<T: Display>(xs: &[T]) ⦉@0,1{ + for @6,8,9,10,11⦊x⦉@6,8,9,10,11 in @6,8,9,10,11⦊xs { + print!("{}", x); + }⦉@6,8,9,10,11 + @5,12,13⦊println!(); +}⦉@5,12,13
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.error.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.error.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..bbf19c3e446f9 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.error.-------.InstrumentCoverage.0.html @@ -0,0 +1,79 @@ + + + + +inline.error - Coverage Spans + + + +
@0,1⦊fn error() { + panic!("error"); +}⦉@0,1
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.length.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.length.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..8e8efb6d9f6b1 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.length.-------.InstrumentCoverage.0.html @@ -0,0 +1,82 @@ + + + + +inline.length - Coverage Spans + + + +
@0,1⦊fn length<T>(xs: &[T]) -> usize { + xs.len() +}⦉@0,1
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.main.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.main.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..4ec2e9beede30 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.main.-------.InstrumentCoverage.0.html @@ -0,0 +1,94 @@ + + + + +inline.main - Coverage Spans + + + +
@0,1⦊fn main() { + permutations(&['a', 'b', 'c']); +}⦉@0,1
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutate.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutate.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..fd72973ccd076 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutate.-------.InstrumentCoverage.0.html @@ -0,0 +1,183 @@ + + + + +inline.permutate - Coverage Spans + + + +
@0,1⦊fn permutate<T: Copy + Display>(xs: &mut [T], k: usize) { + let n = length(xs); + if k == n⦉@0,1 @2,4⦊{ + display(xs); + }⦉@2,4 else if @3⦊k < n⦉@3 { + for @12,14,15,16,17,18⦊i⦉@12,14,15,16,17,18 in @5,7⦊k..n⦉@5,7 @12,14,15,16,17,18⦊{ + swap(xs, i, k); + permutate(xs, k + 1); + swap(xs, i, k); + }⦉@12,14,15,16,17,18 + } else @6,19⦊{ + error(); + }⦉@6,19 +}@21⦊‸⦉@21
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutations.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutations.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..4bfd22f3cd903 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.permutations.-------.InstrumentCoverage.0.html @@ -0,0 +1,113 @@ + + + + +inline.permutations - Coverage Spans + + + +
@0,1,2,3,4⦊fn permutations<T: Copy + Display>(xs: &[T]) { + let mut ys = xs.to_owned(); + permutate(&mut ys, 0); +}⦉@0,1,2,3,4
+ + diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.swap.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.swap.-------.InstrumentCoverage.0.html new file mode 100644 index 0000000000000..4c3f63093d30e --- /dev/null +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.inline/inline.swap.-------.InstrumentCoverage.0.html @@ -0,0 +1,173 @@ + + + + +inline.swap - Coverage Spans + + + +
@0,1,2,3,4⦊fn swap<T: Copy>(xs: &mut [T], i: usize, j: usize) { + let t = xs[i]; + xs[i] = xs[j]; + xs[j] = t; +}⦉@0,1,2,3,4
+ + diff --git a/src/test/run-make-fulldeps/coverage/inline.rs b/src/test/run-make-fulldeps/coverage/inline.rs new file mode 100644 index 0000000000000..9cfab9ddbadf2 --- /dev/null +++ b/src/test/run-make-fulldeps/coverage/inline.rs @@ -0,0 +1,51 @@ +// compile-flags: -Zinline-mir + +use std::fmt::Display; + +fn main() { + permutations(&['a', 'b', 'c']); +} + +#[inline(always)] +fn permutations(xs: &[T]) { + let mut ys = xs.to_owned(); + permutate(&mut ys, 0); +} + +fn permutate(xs: &mut [T], k: usize) { + let n = length(xs); + if k == n { + display(xs); + } else if k < n { + for i in k..n { + swap(xs, i, k); + permutate(xs, k + 1); + swap(xs, i, k); + } + } else { + error(); + } +} + +fn length(xs: &[T]) -> usize { + xs.len() +} + +#[inline] +fn swap(xs: &mut [T], i: usize, j: usize) { + let t = xs[i]; + xs[i] = xs[j]; + xs[j] = t; +} + +fn display(xs: &[T]) { + for x in xs { + print!("{}", x); + } + println!(); +} + +#[inline(always)] +fn error() { + panic!("error"); +} From bd2737fc96b8a8a6ad7e1d4546de9c64108a0a0e Mon Sep 17 00:00:00 2001 From: Daniel Silverstone Date: Tue, 16 Mar 2021 10:50:45 +0000 Subject: [PATCH 13/20] ci/docker: Add SDK/NDK level 21 to android docker for 32bit platforms Certain features of Linux (getauxval() and epoll_create1()) are only available in android SDK/NDK levels 18 and 21 respectively. The 32bit platform is currently on level 14 for compatibility with Android 4.0. This patch adds SDK/NDK level 21 to the docker for 32 bit platforms, while leaving the default setup at level 14. With this done, projects such as `rustup` which rely on these dockers can build with modern ecosystem crates such as tokio 1.0, by using the level 21 toolchain, but those which do not need to switch will be unaffected, since the level 14 toolchain remains available. Signed-off-by: Daniel Silverstone --- src/ci/docker/host-x86_64/dist-android/Dockerfile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ci/docker/host-x86_64/dist-android/Dockerfile b/src/ci/docker/host-x86_64/dist-android/Dockerfile index ee727359f3961..2f0496d7dd4ba 100644 --- a/src/ci/docker/host-x86_64/dist-android/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-android/Dockerfile @@ -9,6 +9,8 @@ RUN . /scripts/android-ndk.sh && \ download_ndk android-ndk-r15c-linux-x86_64.zip && \ make_standalone_toolchain arm 14 && \ make_standalone_toolchain x86 14 && \ + make_standalone_toolchain arm 21 && \ + make_standalone_toolchain x86 21 && \ make_standalone_toolchain arm64 21 && \ make_standalone_toolchain x86_64 21 && \ remove_ndk From b0092bc995fa3e6633c3aaa1d0a56006ab7ad1e3 Mon Sep 17 00:00:00 2001 From: Soveu Date: Tue, 16 Mar 2021 14:41:26 +0100 Subject: [PATCH 14/20] Vec::dedup optimization - add benches --- library/alloc/benches/lib.rs | 1 + library/alloc/benches/vec.rs | 89 ++++++++++++++++++++++++++++++++++++ library/alloc/tests/vec.rs | 2 +- 3 files changed, 91 insertions(+), 1 deletion(-) diff --git a/library/alloc/benches/lib.rs b/library/alloc/benches/lib.rs index 32edb86d10119..38a8f65f1695a 100644 --- a/library/alloc/benches/lib.rs +++ b/library/alloc/benches/lib.rs @@ -4,6 +4,7 @@ #![feature(btree_drain_filter)] #![feature(map_first_last)] #![feature(repr_simd)] +#![feature(slice_partition_dedup)] #![feature(test)] extern crate test; diff --git a/library/alloc/benches/vec.rs b/library/alloc/benches/vec.rs index 89893b6209c0a..73eb353f6e7d4 100644 --- a/library/alloc/benches/vec.rs +++ b/library/alloc/benches/vec.rs @@ -671,3 +671,92 @@ fn bench_map_fast(b: &mut Bencher) { let data = black_box([(0, 0); LEN]); b.iter(|| map_fast(&data)); } + +fn random_sorted_fill(mut seed: u32, buf: &mut [u32]) { + let mask = if buf.len() < 8192 { + 0xFF + } else if buf.len() < 200_000 { + 0xFFFF + } else { + 0xFFFF_FFFF + }; + + for item in buf.iter_mut() { + seed ^= seed << 13; + seed ^= seed >> 17; + seed ^= seed << 5; + + *item = seed & mask; + } + + buf.sort(); +} + +fn bench_vec_dedup_old(b: &mut Bencher, sz: usize) { + let mut template = vec![0u32; sz]; + b.bytes = std::mem::size_of_val(template.as_slice()) as u64; + random_sorted_fill(0x43, &mut template); + + let mut vec = template.clone(); + b.iter(|| { + let len = { + let (dedup, _) = vec.partition_dedup(); + dedup.len() + }; + vec.truncate(len); + + black_box(vec.first()); + vec.clear(); + vec.extend_from_slice(&template); + }); +} + +fn bench_vec_dedup_new(b: &mut Bencher, sz: usize) { + let mut template = vec![0u32; sz]; + b.bytes = std::mem::size_of_val(template.as_slice()) as u64; + random_sorted_fill(0x43, &mut template); + + let mut vec = template.clone(); + b.iter(|| { + vec.dedup(); + black_box(vec.first()); + vec.clear(); + vec.extend_from_slice(&template); + }); +} + +#[bench] +fn bench_dedup_old_100(b: &mut Bencher) { + bench_vec_dedup_old(b, 100); +} +#[bench] +fn bench_dedup_new_100(b: &mut Bencher) { + bench_vec_dedup_new(b, 100); +} + +#[bench] +fn bench_dedup_old_1000(b: &mut Bencher) { + bench_vec_dedup_old(b, 1000); +} +#[bench] +fn bench_dedup_new_1000(b: &mut Bencher) { + bench_vec_dedup_new(b, 1000); +} + +#[bench] +fn bench_dedup_old_10000(b: &mut Bencher) { + bench_vec_dedup_old(b, 10000); +} +#[bench] +fn bench_dedup_new_10000(b: &mut Bencher) { + bench_vec_dedup_new(b, 10000); +} + +#[bench] +fn bench_dedup_old_100000(b: &mut Bencher) { + bench_vec_dedup_old(b, 100000); +} +#[bench] +fn bench_dedup_new_100000(b: &mut Bencher) { + bench_vec_dedup_new(b, 100000); +} diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index e4a20dd932451..c142536cd2dfb 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -2267,4 +2267,4 @@ fn test_extend_from_within_panicing_clone() { std::panic::catch_unwind(move || vec.extend_from_within(..)).unwrap_err(); assert_eq!(count.load(Ordering::SeqCst), 4); -} \ No newline at end of file +} From 5bd50ef9be0d234224402ad080e55e4cf3b48546 Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Tue, 16 Mar 2021 17:14:23 +0100 Subject: [PATCH 15/20] Simplify C compilation for Fortanix-SGX target --- .../docker/host-x86_64/dist-various-2/Dockerfile | 10 ++++------ .../x86_64-fortanix-unknown-sgx-clang-wrap.sh | 14 -------------- 2 files changed, 4 insertions(+), 20 deletions(-) delete mode 100755 src/ci/docker/host-x86_64/dist-various-2/x86_64-fortanix-unknown-sgx-clang-wrap.sh diff --git a/src/ci/docker/host-x86_64/dist-various-2/Dockerfile b/src/ci/docker/host-x86_64/dist-various-2/Dockerfile index 7bb3cb1451688..de3a99f34fcfa 100644 --- a/src/ci/docker/host-x86_64/dist-various-2/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-various-2/Dockerfile @@ -45,10 +45,10 @@ ENV \ CC_armv7_unknown_linux_gnueabi=arm-linux-gnueabi-gcc-8 \ CXX_armv7_unknown_linux_gnueabi=arm-linux-gnueabi-g++-8 \ AR_x86_64_fortanix_unknown_sgx=ar \ - CC_x86_64_fortanix_unknown_sgx=x86_64-fortanix-unknown-sgx-clang-11 \ - CFLAGS_x86_64_fortanix_unknown_sgx="-mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ - CXX_x86_64_fortanix_unknown_sgx=x86_64-fortanix-unknown-sgx-clang++-11 \ - CXXFLAGS_x86_64_fortanix_unknown_sgx="-mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ + CC_x86_64_fortanix_unknown_sgx=clang-11 \ + CFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ + CXX_x86_64_fortanix_unknown_sgx=clang++-11 \ + CXXFLAGS_x86_64_fortanix_unknown_sgx="-D__ELF__ -isystem/usr/include/x86_64-linux-gnu -mlvi-hardening -mllvm -x86-experimental-lvi-inline-asm-hardening" \ AR_i686_unknown_freebsd=i686-unknown-freebsd11-ar \ CC_i686_unknown_freebsd=i686-unknown-freebsd11-clang \ CXX_i686_unknown_freebsd=i686-unknown-freebsd11-clang++ \ @@ -71,8 +71,6 @@ COPY host-x86_64/dist-various-2/build-solaris-toolchain.sh /tmp/ RUN /tmp/build-solaris-toolchain.sh x86_64 amd64 solaris-i386 RUN /tmp/build-solaris-toolchain.sh sparcv9 sparcv9 solaris-sparc COPY host-x86_64/dist-various-2/build-x86_64-fortanix-unknown-sgx-toolchain.sh /tmp/ -COPY host-x86_64/dist-various-2/x86_64-fortanix-unknown-sgx-clang-wrap.sh /usr/bin/x86_64-fortanix-unknown-sgx-clang-11 -RUN ln -s /usr/bin/x86_64-fortanix-unknown-sgx-clang-11 /usr/bin/x86_64-fortanix-unknown-sgx-clang++-11 RUN /tmp/build-x86_64-fortanix-unknown-sgx-toolchain.sh COPY host-x86_64/dist-various-2/build-wasi-toolchain.sh /tmp/ diff --git a/src/ci/docker/host-x86_64/dist-various-2/x86_64-fortanix-unknown-sgx-clang-wrap.sh b/src/ci/docker/host-x86_64/dist-various-2/x86_64-fortanix-unknown-sgx-clang-wrap.sh deleted file mode 100755 index c4ff44c37b1e3..0000000000000 --- a/src/ci/docker/host-x86_64/dist-various-2/x86_64-fortanix-unknown-sgx-clang-wrap.sh +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/bash - -args=("$@") - -for i in "${!args[@]}"; do - # x86_64-fortanix-unknown-sgx doesn't have a C sysroot for things like - # stdint.h and the C++ STL. Unlike GCC, clang will not use the host's - # sysroot instead. Force it. - if [ "${args[$i]}" = "--target=x86_64-fortanix-unknown-sgx" ]; then - args[$i]="--target=x86_64-unknown-linux-gnu" - fi -done - -exec "${0/x86_64-fortanix-unknown-sgx-clang/clang}" "${args[@]}" From e3031fe22ae19965967f4fdf8bf46b017902a06a Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 16 Mar 2021 17:33:03 -0400 Subject: [PATCH 16/20] Allow registering tool lints with `register_tool` Previously, there was no way to add a custom tool prefix, even if the tool itself had registered a lint: ``` #![feature(register_tool)] #![register_tool(xyz)] #![warn(xyz::my_lint)] ``` ``` $ rustc unknown-lint.rs --crate-type lib error[E0710]: an unknown tool name found in scoped lint: `xyz::my_lint` --> unknown-lint.rs:3:9 | 3 | #![warn(xyz::my_lint)] | ^^^ ``` This allows opting-in to lints from other tools using `register_tool`. --- compiler/rustc_ast/src/attr/mod.rs | 4 -- compiler/rustc_lint/src/context.rs | 2 +- compiler/rustc_lint/src/levels.rs | 46 +++++++++++++++++----- src/test/ui/lint/register-tool-lint.rs | 11 ++++++ src/test/ui/lint/register-tool-lint.stderr | 27 +++++++++++++ src/test/ui/tool_lints.rs | 6 +-- src/test/ui/tool_lints.stderr | 12 ++++-- src/test/ui/unknown-lint-tool-name.rs | 12 +++--- src/test/ui/unknown-lint-tool-name.stderr | 24 ++++++++--- 9 files changed, 112 insertions(+), 32 deletions(-) create mode 100644 src/test/ui/lint/register-tool-lint.rs create mode 100644 src/test/ui/lint/register-tool-lint.stderr diff --git a/compiler/rustc_ast/src/attr/mod.rs b/compiler/rustc_ast/src/attr/mod.rs index 40b0cefd83aa6..2c5e180f80d49 100644 --- a/compiler/rustc_ast/src/attr/mod.rs +++ b/compiler/rustc_ast/src/attr/mod.rs @@ -33,10 +33,6 @@ impl MarkedAttrs { } } -pub fn is_known_lint_tool(m_item: Ident) -> bool { - [sym::clippy, sym::rustc, sym::rustdoc].contains(&m_item.name) -} - impl NestedMetaItem { /// Returns the `MetaItem` if `self` is a `NestedMetaItem::MetaItem`. pub fn meta_item(&self) -> Option<&MetaItem> { diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 42ead89ca4f85..1e98464f13a82 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/compiler/rustc_lint/src/context.rs @@ -714,7 +714,7 @@ impl<'a> EarlyContext<'a> { sess, krate, lint_store, - builder: LintLevelsBuilder::new(sess, warn_about_weird_lints, lint_store), + builder: LintLevelsBuilder::new(sess, warn_about_weird_lints, lint_store, &krate.attrs), buffered, } } diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index b3bdaf5bdc7d0..360cccb58d54f 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -1,13 +1,12 @@ use crate::context::{CheckLintNameResult, LintStore}; use crate::late::unerased_lint_store; use rustc_ast as ast; -use rustc_ast::attr; use rustc_ast::unwrap_or; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder}; use rustc_hir as hir; -use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; +use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc_hir::{intravisit, HirId}; use rustc_middle::hir::map::Map; use rustc_middle::lint::LevelAndSource; @@ -32,7 +31,8 @@ use std::cmp; fn lint_levels(tcx: TyCtxt<'_>, cnum: CrateNum) -> LintLevelMap { assert_eq!(cnum, LOCAL_CRATE); let store = unerased_lint_store(tcx); - let levels = LintLevelsBuilder::new(tcx.sess, false, &store); + let crate_attrs = tcx.get_attrs(DefId { krate: cnum, index: CRATE_DEF_INDEX }); + let levels = LintLevelsBuilder::new(tcx.sess, false, &store, crate_attrs); let mut builder = LintLevelMapBuilder { levels, tcx, store }; let krate = tcx.hir().krate(); @@ -56,6 +56,7 @@ pub struct LintLevelsBuilder<'s> { cur: u32, warn_about_weird_lints: bool, store: &'s LintStore, + crate_attrs: &'s [ast::Attribute], } pub struct BuilderPush { @@ -64,7 +65,12 @@ pub struct BuilderPush { } impl<'s> LintLevelsBuilder<'s> { - pub fn new(sess: &'s Session, warn_about_weird_lints: bool, store: &'s LintStore) -> Self { + pub fn new( + sess: &'s Session, + warn_about_weird_lints: bool, + store: &'s LintStore, + crate_attrs: &'s [ast::Attribute], + ) -> Self { let mut builder = LintLevelsBuilder { sess, sets: LintLevelSets::new(), @@ -72,6 +78,7 @@ impl<'s> LintLevelsBuilder<'s> { id_to_set: Default::default(), warn_about_weird_lints, store, + crate_attrs, }; builder.process_command_line(sess, store); assert_eq!(builder.sets.list.len(), 1); @@ -304,15 +311,22 @@ impl<'s> LintLevelsBuilder<'s> { }; let tool_name = if meta_item.path.segments.len() > 1 { let tool_ident = meta_item.path.segments[0].ident; - if !attr::is_known_lint_tool(tool_ident) { - struct_span_err!( + if !is_known_lint_tool(tool_ident.name, sess, &self.crate_attrs) { + let mut err = struct_span_err!( sess, tool_ident.span, E0710, - "an unknown tool name found in scoped lint: `{}`", + "unknown tool name `{}` found in scoped lint: `{}`", + tool_ident.name, pprust::path_to_string(&meta_item.path), - ) - .emit(); + ); + if sess.is_nightly_build() { + err.help(&format!( + "add `#![register_tool({})]` to the crate root", + tool_ident.name + )); + } + err.emit(); continue; } @@ -559,6 +573,20 @@ impl<'s> LintLevelsBuilder<'s> { } } +fn is_known_lint_tool(m_item: Symbol, sess: &Session, attrs: &[ast::Attribute]) -> bool { + if [sym::clippy, sym::rustc, sym::rustdoc].contains(&m_item) { + return true; + } + // Look for registered tools + // NOTE: does no error handling; error handling is done by rustc_resolve. + sess.filter_by_name(attrs, sym::register_tool) + .filter_map(|attr| attr.meta_item_list()) + .flat_map(std::convert::identity) + .filter_map(|nested_meta| nested_meta.ident()) + .map(|ident| ident.name) + .any(|name| name == m_item) +} + struct LintLevelMapBuilder<'a, 'tcx> { levels: LintLevelsBuilder<'tcx>, tcx: TyCtxt<'tcx>, diff --git a/src/test/ui/lint/register-tool-lint.rs b/src/test/ui/lint/register-tool-lint.rs new file mode 100644 index 0000000000000..0ba5a37b167cd --- /dev/null +++ b/src/test/ui/lint/register-tool-lint.rs @@ -0,0 +1,11 @@ +#![crate_type = "lib"] +#![feature(register_tool)] +#![register_tool(xyz)] +#![warn(xyz::my_lint)] // this should not error +#![warn(abc::my_lint)] +//~^ ERROR unknown tool name `abc` found in scoped lint +//~| HELP add `#![register_tool(abc)]` +//~| ERROR unknown tool name `abc` +//~| HELP add `#![register_tool(abc)]` +//~| ERROR unknown tool name `abc` +//~| HELP add `#![register_tool(abc)]` diff --git a/src/test/ui/lint/register-tool-lint.stderr b/src/test/ui/lint/register-tool-lint.stderr new file mode 100644 index 0000000000000..750c74cec1eb9 --- /dev/null +++ b/src/test/ui/lint/register-tool-lint.stderr @@ -0,0 +1,27 @@ +error[E0710]: unknown tool name `abc` found in scoped lint: `abc::my_lint` + --> $DIR/register-tool-lint.rs:5:9 + | +LL | #![warn(abc::my_lint)] + | ^^^ + | + = help: add `#![register_tool(abc)]` to the crate root + +error[E0710]: unknown tool name `abc` found in scoped lint: `abc::my_lint` + --> $DIR/register-tool-lint.rs:5:9 + | +LL | #![warn(abc::my_lint)] + | ^^^ + | + = help: add `#![register_tool(abc)]` to the crate root + +error[E0710]: unknown tool name `abc` found in scoped lint: `abc::my_lint` + --> $DIR/register-tool-lint.rs:5:9 + | +LL | #![warn(abc::my_lint)] + | ^^^ + | + = help: add `#![register_tool(abc)]` to the crate root + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0710`. diff --git a/src/test/ui/tool_lints.rs b/src/test/ui/tool_lints.rs index 9c8540eede792..9e4aa7a939acb 100644 --- a/src/test/ui/tool_lints.rs +++ b/src/test/ui/tool_lints.rs @@ -1,5 +1,5 @@ #[warn(foo::bar)] -//~^ ERROR an unknown tool name found in scoped lint: `foo::bar` -//~| ERROR an unknown tool name found in scoped lint: `foo::bar` -//~| ERROR an unknown tool name found in scoped lint: `foo::bar` +//~^ ERROR unknown tool name `foo` found in scoped lint: `foo::bar` +//~| ERROR unknown tool name `foo` found in scoped lint: `foo::bar` +//~| ERROR unknown tool name `foo` found in scoped lint: `foo::bar` fn main() {} diff --git a/src/test/ui/tool_lints.stderr b/src/test/ui/tool_lints.stderr index 1bcd7fd735de8..e06f6ddc1ca3a 100644 --- a/src/test/ui/tool_lints.stderr +++ b/src/test/ui/tool_lints.stderr @@ -1,20 +1,26 @@ -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/tool_lints.rs:1:8 | LL | #[warn(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/tool_lints.rs:1:8 | LL | #[warn(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/tool_lints.rs:1:8 | LL | #[warn(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root error: aborting due to 3 previous errors diff --git a/src/test/ui/unknown-lint-tool-name.rs b/src/test/ui/unknown-lint-tool-name.rs index 182aec34b4781..84ab7c1944ab2 100644 --- a/src/test/ui/unknown-lint-tool-name.rs +++ b/src/test/ui/unknown-lint-tool-name.rs @@ -1,8 +1,8 @@ -#![deny(foo::bar)] //~ ERROR an unknown tool name found in scoped lint: `foo::bar` - //~| ERROR an unknown tool name found in scoped lint: `foo::bar` - //~| ERROR an unknown tool name found in scoped lint: `foo::bar` +#![deny(foo::bar)] //~ ERROR unknown tool name `foo` found in scoped lint: `foo::bar` + //~| ERROR unknown tool name `foo` found in scoped lint: `foo::bar` + //~| ERROR unknown tool name `foo` found in scoped lint: `foo::bar` -#[allow(foo::bar)] //~ ERROR an unknown tool name found in scoped lint: `foo::bar` - //~| ERROR an unknown tool name found in scoped lint: `foo::bar` - //~| ERROR an unknown tool name found in scoped lint: `foo::bar` +#[allow(foo::bar)] //~ ERROR unknown tool name `foo` found in scoped lint: `foo::bar` + //~| ERROR unknown tool name `foo` found in scoped lint: `foo::bar` + //~| ERROR unknown tool name `foo` found in scoped lint: `foo::bar` fn main() {} diff --git a/src/test/ui/unknown-lint-tool-name.stderr b/src/test/ui/unknown-lint-tool-name.stderr index 414816d229cdb..1d145515abf66 100644 --- a/src/test/ui/unknown-lint-tool-name.stderr +++ b/src/test/ui/unknown-lint-tool-name.stderr @@ -1,38 +1,50 @@ -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/unknown-lint-tool-name.rs:1:9 | LL | #![deny(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/unknown-lint-tool-name.rs:5:9 | LL | #[allow(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/unknown-lint-tool-name.rs:1:9 | LL | #![deny(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/unknown-lint-tool-name.rs:5:9 | LL | #[allow(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/unknown-lint-tool-name.rs:1:9 | LL | #![deny(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root -error[E0710]: an unknown tool name found in scoped lint: `foo::bar` +error[E0710]: unknown tool name `foo` found in scoped lint: `foo::bar` --> $DIR/unknown-lint-tool-name.rs:5:9 | LL | #[allow(foo::bar)] | ^^^ + | + = help: add `#![register_tool(foo)]` to the crate root error: aborting due to 6 previous errors From f414c33e5ed47182c95d3859816424a093e0a7a9 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Wed, 17 Mar 2021 11:33:07 +0900 Subject: [PATCH 17/20] Display error details when a `mmap` call fails --- library/std/src/sys/unix/stack_overflow.rs | 5 +++-- library/std/src/sys/unix/thread.rs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/unix/stack_overflow.rs b/library/std/src/sys/unix/stack_overflow.rs index d7bba50c76ab8..2a487fff54ae7 100644 --- a/library/std/src/sys/unix/stack_overflow.rs +++ b/library/std/src/sys/unix/stack_overflow.rs @@ -39,6 +39,7 @@ impl Drop for Handler { ))] mod imp { use super::Handler; + use crate::io; use crate::mem; use crate::ptr; @@ -149,11 +150,11 @@ mod imp { 0, ); if stackp == MAP_FAILED { - panic!("failed to allocate an alternative stack"); + panic!("failed to allocate an alternative stack: {}", io::Error::last_os_error()); } let guard_result = libc::mprotect(stackp, page_size(), PROT_NONE); if guard_result != 0 { - panic!("failed to set up alternative stack guard page"); + panic!("failed to set up alternative stack guard page: {}", io::Error::last_os_error()); } stackp.add(page_size()) } diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index 40c9630751416..01a12dcf5a2af 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -231,6 +231,7 @@ pub mod guard { use libc::{mmap, mprotect}; use libc::{MAP_ANON, MAP_FAILED, MAP_FIXED, MAP_PRIVATE, PROT_NONE, PROT_READ, PROT_WRITE}; + use crate::io; use crate::ops::Range; use crate::sync::atomic::{AtomicUsize, Ordering}; use crate::sys::os; @@ -361,12 +362,12 @@ pub mod guard { 0, ); if result != stackaddr || result == MAP_FAILED { - panic!("failed to allocate a guard page"); + panic!("failed to allocate a guard page: {}", io::Error::last_os_error()); } let result = mprotect(stackaddr, page_size, PROT_NONE); if result != 0 { - panic!("failed to protect the guard page"); + panic!("failed to protect the guard page: {}", io::Error::last_os_error()); } let guardaddr = stackaddr as usize; From 7dc654ce1f7161f918f148018abaa12feb3b0669 Mon Sep 17 00:00:00 2001 From: lukaslueg Date: Wed, 17 Mar 2021 09:09:03 +0100 Subject: [PATCH 18/20] Update library/core/src/iter/traits/iterator.rs Co-authored-by: Yuki Okushi --- library/core/src/iter/traits/iterator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index e9fba30a344a1..af09b41b6b0ee 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -944,7 +944,7 @@ pub trait Iterator { /// Note that the underlying iterator is still advanced when [`peek`] or /// [`peek_mut`] are called for the first time: In order to retrieve the /// next element, [`next`] is called on the underlying iterator, hence any - /// side effects (i.e. anything other than fetching the next value) of + /// side effects (i.e. anything other than fetching the next value) of /// the [`next`] method will occur. /// /// From 8af15db8b8cb7755c402462f3c12108e299afaf3 Mon Sep 17 00:00:00 2001 From: lukaslueg Date: Wed, 17 Mar 2021 09:09:17 +0100 Subject: [PATCH 19/20] Update library/core/src/iter/traits/iterator.rs Co-authored-by: Yuki Okushi --- library/core/src/iter/traits/iterator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index af09b41b6b0ee..1d1e1e65b4839 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -982,7 +982,7 @@ pub trait Iterator { /// /// let mut iter = xs.iter().peekable(); /// - /// // peek_mut() lets us see into the future + /// // `peek_mut()` lets us see into the future /// assert_eq!(iter.peek_mut(), Some(&mut &1)); /// assert_eq!(iter.peek_mut(), Some(&mut &1)); /// assert_eq!(iter.next(), Some(&1)); From ee98c6fea4b96abd8cad6616291657dc05329045 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 17 Mar 2021 13:48:25 +0100 Subject: [PATCH 20/20] Don't show HTML diff if tidy isn't installed for rustdoc tests --- src/tools/compiletest/src/main.rs | 2 +- src/tools/compiletest/src/runtest.rs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index b32a6f08638cc..1d4b5e1247de7 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -44,7 +44,7 @@ fn main() { } if !config.has_tidy && config.mode == Mode::Rustdoc { - eprintln!("warning: `tidy` is not installed; generated diffs will be harder to read"); + eprintln!("warning: `tidy` is not installed; diffs will not be generated"); } log_config(&config); diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 9a82591e5a75a..4f5003529a3cb 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2365,6 +2365,9 @@ impl<'test> TestCx<'test> { } fn compare_to_default_rustdoc(&mut self, out_dir: &Path) { + if !self.config.has_tidy { + return; + } println!("info: generating a diff against nightly rustdoc"); let suffix = @@ -2426,10 +2429,8 @@ impl<'test> TestCx<'test> { } } }; - if self.config.has_tidy { - tidy_dir(out_dir); - tidy_dir(&compare_dir); - } + tidy_dir(out_dir); + tidy_dir(&compare_dir); let pager = { let output = Command::new("git").args(&["config", "--get", "core.pager"]).output().ok();