From 740f8db85572aef58d0734fc60bc2b54862ebbb0 Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Wed, 19 Jun 2019 23:15:19 +0300 Subject: [PATCH 01/32] Add FIXME-s that some types should be transparent --- src/libstd/ffi/c_str.rs | 6 ++++++ src/libstd/ffi/os_str.rs | 6 ++++++ src/libstd/path.rs | 12 ++++++++++++ src/libstd/sys_common/os_str_bytes.rs | 6 ++++++ 4 files changed, 30 insertions(+) diff --git a/src/libstd/ffi/c_str.rs b/src/libstd/ffi/c_str.rs index 5c6c43017cf64..f7ad62f4e9a8a 100644 --- a/src/libstd/ffi/c_str.rs +++ b/src/libstd/ffi/c_str.rs @@ -195,6 +195,12 @@ pub struct CString { /// [`from_ptr`]: #method.from_ptr #[derive(Hash)] #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `fn from` in `impl From<&CStr> for Box` current implementation relies +// on `CStr` being layout-compatible with `[u8]`. +// When attribute privacy is implemented, `CStr` should be annotated as `#[repr(transparent)]`. +// Anyway, `CStr` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct CStr { // FIXME: this should not be represented with a DST slice but rather with // just a raw `c_char` along with some form of marker to make diff --git a/src/libstd/ffi/os_str.rs b/src/libstd/ffi/os_str.rs index c7c5849a00fa0..b57f80c2e002b 100644 --- a/src/libstd/ffi/os_str.rs +++ b/src/libstd/ffi/os_str.rs @@ -97,6 +97,12 @@ pub struct OsString { /// [`String`]: ../string/struct.String.html /// [conversions]: index.html#conversions #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `OsStr::from_inner` current implementation relies +// on `OsStr` being layout-compatible with `Slice`. +// When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`. +// Anyway, `OsStr` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct OsStr { inner: Slice } diff --git a/src/libstd/path.rs b/src/libstd/path.rs index 126bc3754dabc..9f7a081403da3 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -1123,6 +1123,12 @@ impl FusedIterator for Ancestors<'_> {} /// Which method works best depends on what kind of situation you're in. #[derive(Clone)] #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `PathBuf::as_mut_vec` current implementation relies +// on `PathBuf` being layout-compatible with `Vec`. +// When attribute privacy is implemented, `PathBuf` should be annotated as `#[repr(transparent)]`. +// Anyway, `PathBuf` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct PathBuf { inner: OsString, } @@ -1745,6 +1751,12 @@ impl AsRef for PathBuf { /// assert_eq!(extension, Some(OsStr::new("txt"))); /// ``` #[stable(feature = "rust1", since = "1.0.0")] +// FIXME: +// `Path::new` current implementation relies +// on `Path` being layout-compatible with `OsStr`. +// When attribute privacy is implemented, `Path` should be annotated as `#[repr(transparent)]`. +// Anyway, `Path` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub struct Path { inner: OsStr, } diff --git a/src/libstd/sys_common/os_str_bytes.rs b/src/libstd/sys_common/os_str_bytes.rs index a4961974d89ab..d734f412bf886 100644 --- a/src/libstd/sys_common/os_str_bytes.rs +++ b/src/libstd/sys_common/os_str_bytes.rs @@ -18,6 +18,12 @@ pub(crate) struct Buf { pub inner: Vec } +// FIXME: +// `Buf::as_slice` current implementation relies +// on `Slice` being layout-compatible with `[u8]`. +// When attribute privacy is implemented, `Slice` should be annotated as `#[repr(transparent)]`. +// Anyway, `Slice` representation and layout are considered implementation detail, are +// not documented and must not be relied upon. pub(crate) struct Slice { pub inner: [u8] } From 03e95ae4127db1b0ae465e8b58383744b7184a70 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2019 13:08:47 +0200 Subject: [PATCH 02/32] Miri shouldn't look at types --- src/librustc_mir/interpret/eval_context.rs | 10 +++++++--- src/librustc_mir/interpret/terminator.rs | 8 +++++--- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 1f23d8c017ccd..6f4227ed34cc4 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -385,15 +385,19 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { local: mir::Local, layout: Option>, ) -> InterpResult<'tcx, TyLayout<'tcx>> { - match frame.locals[local].layout.get() { + // `const_prop` runs into this with an invalid (empty) frame, so we + // have to support that case (mostly by skipping all caching). + match frame.locals.get(local).and_then(|state| state.layout.get()) { None => { let layout = crate::interpret::operand::from_known_layout(layout, || { let local_ty = frame.body.local_decls[local].ty; let local_ty = self.monomorphize_with_substs(local_ty, frame.instance.substs)?; self.layout_of(local_ty) })?; - // Layouts of locals are requested a lot, so we cache them. - frame.locals[local].layout.set(Some(layout)); + if let Some(state) = frame.locals.get(local) { + // Layouts of locals are requested a lot, so we cache them. + state.layout.set(Some(layout)); + } Ok(layout) } Some(layout) => Ok(layout), diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index a0c6fb026dd4b..1d6b48e9da4c4 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -405,9 +405,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } } else { let local = mir::RETURN_PLACE; - let ty = self.frame().body.local_decls[local].ty; - if !self.tcx.is_ty_uninhabited_from_any_module(ty) { - throw_unsup!(FunctionRetMismatch(self.tcx.types.never, ty)) + let callee_layout = self.layout_of_local(self.frame(), local, None)?; + if !callee_layout.abi.is_uninhabited() { + throw_unsup!(FunctionRetMismatch( + self.tcx.types.never, callee_layout.ty + )) } } Ok(()) From 62f1e8a7f12c864f97c49faf4bf49940aac266a6 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2019 14:49:11 +0200 Subject: [PATCH 03/32] fix test --- .../consts/uninhabited-const-issue-61744.rs | 4 +- .../uninhabited-const-issue-61744.stderr | 57 +++++++++++++++++-- 2 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/test/ui/consts/uninhabited-const-issue-61744.rs b/src/test/ui/consts/uninhabited-const-issue-61744.rs index 21fbbf8cfb5a8..4509ebc6338a8 100644 --- a/src/test/ui/consts/uninhabited-const-issue-61744.rs +++ b/src/test/ui/consts/uninhabited-const-issue-61744.rs @@ -1,11 +1,11 @@ // compile-fail pub const unsafe fn fake_type() -> T { - hint_unreachable() + hint_unreachable() //~ ERROR any use of this value will cause an error } pub const unsafe fn hint_unreachable() -> ! { - fake_type() //~ ERROR any use of this value will cause an error + fake_type() } trait Const { diff --git a/src/test/ui/consts/uninhabited-const-issue-61744.stderr b/src/test/ui/consts/uninhabited-const-issue-61744.stderr index e317bdf103c36..f390676fda6d0 100644 --- a/src/test/ui/consts/uninhabited-const-issue-61744.stderr +++ b/src/test/ui/consts/uninhabited-const-issue-61744.stderr @@ -1,11 +1,60 @@ error: any use of this value will cause an error - --> $DIR/uninhabited-const-issue-61744.rs:8:5 + --> $DIR/uninhabited-const-issue-61744.rs:4:5 | -LL | fake_type() - | ^^^^^^^^^^^ +LL | hint_unreachable() + | ^^^^^^^^^^^^^^^^^^ | | - | tried to call a function with return type T passing return place of type ! + | reached the configured maximum number of stack frames | inside call to `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:4:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 + | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:8:5 | inside call to `fake_type::` at $DIR/uninhabited-const-issue-61744.rs:12:36 ... LL | const CONSTANT: i32 = unsafe { fake_type() }; From 440a5c810029088649918d738169f8e0bb65bb35 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2019 16:37:40 +0200 Subject: [PATCH 04/32] rename RUST_CTFE_BACKTRACE to RUSTC_CTFE_BACKTRACE --- src/librustc/mir/interpret/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index f53d2ffb6df54..ef0e205184871 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -217,7 +217,7 @@ fn print_backtrace(backtrace: &mut Backtrace) { impl<'tcx> From> for InterpErrorInfo<'tcx> { fn from(kind: InterpError<'tcx>) -> Self { - let backtrace = match env::var("RUST_CTFE_BACKTRACE") { + let backtrace = match env::var("RUSTC_CTFE_BACKTRACE") { // Matching `RUST_BACKTRACE` -- we treat "0" the same as "not present". Ok(ref val) if val != "0" => { let mut backtrace = Backtrace::new_unresolved(); From d809d6ee899154a0ba99e6159ba9d54330ebf0ac Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 10 Aug 2019 16:47:27 +0200 Subject: [PATCH 05/32] Derive Debug for CrateInfo --- src/librustc_codegen_ssa/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc_codegen_ssa/lib.rs b/src/librustc_codegen_ssa/lib.rs index 0e3c3a77b28f4..68640abb0433e 100644 --- a/src/librustc_codegen_ssa/lib.rs +++ b/src/librustc_codegen_ssa/lib.rs @@ -128,6 +128,7 @@ bitflags::bitflags! { } /// Misc info we load from metadata to persist beyond the tcx. +#[derive(Debug)] pub struct CrateInfo { pub panic_runtime: Option, pub compiler_builtins: Option, From 93839c3fb4e3a3c3341595ac8dc2c7f4e39326d2 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 10 Aug 2019 16:31:38 +0000 Subject: [PATCH 06/32] Add an example to show how to insert item to a sorted vec --- src/libcore/slice/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index d5a34ea2bd5a1..29afc43599620 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -1364,6 +1364,20 @@ impl [T] { /// let r = s.binary_search(&1); /// assert!(match r { Ok(1..=4) => true, _ => false, }); /// ``` + /// + /// If you want to insert an item to a sorted vector, while maintaining + /// sort order: + /// + /// ``` + /// let mut s = vec![0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55]; + /// let num = 42; + /// let idx = match s.binary_search(&num) { + /// Ok(idx) => idx, + /// Err(idx) => idx, + /// }; + /// s.insert(idx, num); + /// assert_eq!(s, [0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 42, 55]); + /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn binary_search(&self, x: &T) -> Result where T: Ord From 30ba4bd8e2da15ce8e2fe3c8fbc339ee67cb3241 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 10 Aug 2019 17:16:58 +0000 Subject: [PATCH 07/32] Use Result::unwrap_or_else instead of matching --- src/libcore/slice/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libcore/slice/mod.rs b/src/libcore/slice/mod.rs index 29afc43599620..ce5af13d4ca90 100644 --- a/src/libcore/slice/mod.rs +++ b/src/libcore/slice/mod.rs @@ -1371,10 +1371,7 @@ impl [T] { /// ``` /// let mut s = vec![0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55]; /// let num = 42; - /// let idx = match s.binary_search(&num) { - /// Ok(idx) => idx, - /// Err(idx) => idx, - /// }; + /// let idx = s.binary_search(&num).unwrap_or_else(|x| x); /// s.insert(idx, num); /// assert_eq!(s, [0, 1, 1, 1, 1, 2, 3, 5, 8, 13, 21, 34, 42, 55]); /// ``` From da6fbb18951547da08c2e353838e6b1d47d9b3be Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Aug 2019 23:11:52 +0200 Subject: [PATCH 08/32] add basic lint testing for misuse of mem::zeroed and mem::uninitialized --- src/librustc_lint/builtin.rs | 59 ++++++++++++++++++++ src/librustc_lint/lib.rs | 1 + src/libsyntax_pos/symbol.rs | 3 + src/test/ui/lint/uninitialized-zeroed.rs | 28 ++++++++++ src/test/ui/lint/uninitialized-zeroed.stderr | 44 +++++++++++++++ src/test/ui/panic-uninitialized-zeroed.rs | 2 +- 6 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/lint/uninitialized-zeroed.rs create mode 100644 src/test/ui/lint/uninitialized-zeroed.stderr diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c9153f285fff7..b4155646c891f 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1862,3 +1862,62 @@ impl EarlyLintPass for IncompleteFeatures { }); } } + +declare_lint! { + pub INVALID_VALUE, + Warn, + "an invalid value is being created (such as a NULL reference)" +} + +declare_lint_pass!(InvalidValue => [INVALID_VALUE]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { + fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &hir::Expr) { + + const ZEROED_PATH: &[Symbol] = &[sym::core, sym::mem, sym::zeroed]; + const UININIT_PATH: &[Symbol] = &[sym::core, sym::mem, sym::uninitialized]; + + /// Return `false` only if we are sure this type does *not* + /// allow zero initialization. + fn ty_maybe_allows_zero_init(ty: Ty<'_>) -> bool { + use rustc::ty::TyKind::*; + match ty.sty { + // Primitive types that don't like 0 as a value. + Ref(..) | FnPtr(..) | Never => false, + // Conservative fallback. + _ => true, + } + } + + if let hir::ExprKind::Call(ref path_expr, ref _args) = expr.node { + if let hir::ExprKind::Path(ref qpath) = path_expr.node { + if let Some(def_id) = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() { + if cx.match_def_path(def_id, &ZEROED_PATH) || + cx.match_def_path(def_id, &UININIT_PATH) + { + // This conjures an instance of a type out of nothing, + // using zeroed or uninitialized memory. + // We are extremely conservative with what we warn about. + let conjured_ty = cx.tables.expr_ty(expr); + + if !ty_maybe_allows_zero_init(conjured_ty) { + cx.span_lint( + INVALID_VALUE, + expr.span, + &format!( + "the type `{}` does not permit {}", + conjured_ty, + if cx.match_def_path(def_id, &ZEROED_PATH) { + "zero-initialization" + } else { + "being left uninitialized" + } + ), + ); + } + } + } + } + } + } +} diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 78bc164ba1a0f..3a540fdf4b91f 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -177,6 +177,7 @@ macro_rules! late_lint_mod_passes { UnreachablePub: UnreachablePub, ExplicitOutlivesRequirements: ExplicitOutlivesRequirements, + InvalidValue: InvalidValue, ]); ) } diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index f7e1b983e5446..2d9556233d15f 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -412,6 +412,7 @@ symbols! { match_beginning_vert, match_default_bindings, may_dangle, + mem, member_constraints, message, meta, @@ -695,6 +696,7 @@ symbols! { underscore_imports, underscore_lifetimes, uniform_paths, + uninitialized, universal_impl_trait, unmarked_api, unreachable_code, @@ -726,6 +728,7 @@ symbols! { windows, windows_subsystem, Yield, + zeroed, } } diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs new file mode 100644 index 0000000000000..40b17651e47b2 --- /dev/null +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -0,0 +1,28 @@ +// ignore-tidy-linelength +// This test checks that calling `mem::{uninitialized,zeroed}` with certain types results +// in a lint. + +#![feature(never_type)] +#![allow(deprecated)] +#![deny(invalid_value)] + +use std::mem; + +fn main() { + unsafe { + let _val: ! = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: ! = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: &'static i32 = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: &'static i32 = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: fn() = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: fn() = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + // Some types that should work just fine. + let _val: Option<&'static i32> = mem::zeroed(); + let _val: Option = mem::zeroed(); + let _val: bool = mem::zeroed(); + let _val: i32 = mem::zeroed(); + } +} diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr new file mode 100644 index 0000000000000..c6a47638d38e5 --- /dev/null +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -0,0 +1,44 @@ +error: the type `!` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:15:23 + | +LL | let _val: ! = mem::zeroed(); + | ^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/uninitialized-zeroed.rs:7:9 + | +LL | #![deny(invalid_value)] + | ^^^^^^^^^^^^^ + +error: the type `!` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:16:23 + | +LL | let _val: ! = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: the type `&'static i32` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:21:34 + | +LL | let _val: &'static i32 = mem::zeroed(); + | ^^^^^^^^^^^^^ + +error: the type `&'static i32` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:22:34 + | +LL | let _val: &'static i32 = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: the type `fn()` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:24:26 + | +LL | let _val: fn() = mem::zeroed(); + | ^^^^^^^^^^^^^ + +error: the type `fn()` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:25:26 + | +LL | let _val: fn() = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + diff --git a/src/test/ui/panic-uninitialized-zeroed.rs b/src/test/ui/panic-uninitialized-zeroed.rs index 0c97babd51cd4..b0d6629561803 100644 --- a/src/test/ui/panic-uninitialized-zeroed.rs +++ b/src/test/ui/panic-uninitialized-zeroed.rs @@ -4,7 +4,7 @@ // in a runtime panic. #![feature(never_type)] -#![allow(deprecated)] +#![allow(deprecated, invalid_value)] use std::{mem, panic}; From ca1e94b131eec4795f1a8dcacdb12f574e7a4a74 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 08:42:50 +0200 Subject: [PATCH 09/32] warn for more cases --- src/librustc_lint/builtin.rs | 43 +++++- src/test/ui/lint/uninitialized-zeroed.rs | 32 +++- src/test/ui/lint/uninitialized-zeroed.stderr | 145 +++++++++++++++++-- 3 files changed, 204 insertions(+), 16 deletions(-) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index b4155646c891f..c652c196afd0d 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -23,7 +23,7 @@ use rustc::hir::def::{Res, DefKind}; use rustc::hir::def_id::{DefId, LOCAL_CRATE}; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{self, Ty, TyCtxt, layout::VariantIdx}; use rustc::{lint, util}; use hir::Node; use util::nodemap::HirIdSet; @@ -1879,11 +1879,40 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { /// Return `false` only if we are sure this type does *not* /// allow zero initialization. - fn ty_maybe_allows_zero_init(ty: Ty<'_>) -> bool { + fn ty_maybe_allows_zero_init<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool { use rustc::ty::TyKind::*; match ty.sty { // Primitive types that don't like 0 as a value. Ref(..) | FnPtr(..) | Never => false, + Adt(..) if ty.is_box() => false, + // Recurse for some compound types. + Adt(adt_def, substs) if !adt_def.is_union() => { + match adt_def.variants.len() { + 0 => false, // Uninhabited enum! + 1 => { + // Struct, or enum with exactly one variant. + // Proceed recursively, check all fields. + let variant = &adt_def.variants[VariantIdx::from_u32(0)]; + variant.fields.iter().all(|field| { + ty_maybe_allows_zero_init( + tcx, + field.ty(tcx, substs), + ) + }) + } + _ => true, // Conservative fallback for multi-variant enum. + } + } + Tuple(substs) => { + // Proceed recursively, check all fields. + substs.iter().all(|field| { + ty_maybe_allows_zero_init( + tcx, + field.expect_ty(), + ) + }) + } + // FIXME: Would be nice to also warn for `NonNull`/`NonZero*`. // Conservative fallback. _ => true, } @@ -1900,8 +1929,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { // We are extremely conservative with what we warn about. let conjured_ty = cx.tables.expr_ty(expr); - if !ty_maybe_allows_zero_init(conjured_ty) { - cx.span_lint( + if !ty_maybe_allows_zero_init(cx.tcx, conjured_ty) { + cx.struct_span_lint( INVALID_VALUE, expr.span, &format!( @@ -1913,7 +1942,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { "being left uninitialized" } ), - ); + ) + .note("this means that this code causes undefined behavior \ + when executed") + .help("use `MaybeUninit` instead") + .emit(); } } } diff --git a/src/test/ui/lint/uninitialized-zeroed.rs b/src/test/ui/lint/uninitialized-zeroed.rs index 40b17651e47b2..8f9ca8717bda6 100644 --- a/src/test/ui/lint/uninitialized-zeroed.rs +++ b/src/test/ui/lint/uninitialized-zeroed.rs @@ -6,22 +6,52 @@ #![allow(deprecated)] #![deny(invalid_value)] -use std::mem; +use std::mem::{self, MaybeUninit}; + +enum Void {} + +struct Ref(&'static i32); + +struct Wrap { wrapped: T } + +#[allow(unused)] +fn generic() { + unsafe { + let _val: &'static T = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: &'static T = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: Wrap<&'static T> = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Wrap<&'static T> = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + } +} fn main() { unsafe { let _val: ! = mem::zeroed(); //~ ERROR: does not permit zero-initialization let _val: ! = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: (i32, !) = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: (i32, !) = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + + let _val: Void = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Void = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: &'static i32 = mem::zeroed(); //~ ERROR: does not permit zero-initialization let _val: &'static i32 = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: Ref = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Ref = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: fn() = mem::zeroed(); //~ ERROR: does not permit zero-initialization let _val: fn() = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + let _val: Wrap = mem::zeroed(); //~ ERROR: does not permit zero-initialization + let _val: Wrap = mem::uninitialized(); //~ ERROR: does not permit being left uninitialized + // Some types that should work just fine. let _val: Option<&'static i32> = mem::zeroed(); let _val: Option = mem::zeroed(); + let _val: MaybeUninit<&'static i32> = mem::zeroed(); let _val: bool = mem::zeroed(); let _val: i32 = mem::zeroed(); } diff --git a/src/test/ui/lint/uninitialized-zeroed.stderr b/src/test/ui/lint/uninitialized-zeroed.stderr index c6a47638d38e5..af54b16bd0b24 100644 --- a/src/test/ui/lint/uninitialized-zeroed.stderr +++ b/src/test/ui/lint/uninitialized-zeroed.stderr @@ -1,44 +1,169 @@ -error: the type `!` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:15:23 +error: the type `&'static T` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:20:32 | -LL | let _val: ! = mem::zeroed(); - | ^^^^^^^^^^^^^ +LL | let _val: &'static T = mem::zeroed(); + | ^^^^^^^^^^^^^ | note: lint level defined here --> $DIR/uninitialized-zeroed.rs:7:9 | LL | #![deny(invalid_value)] | ^^^^^^^^^^^^^ + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `&'static T` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:21:32 + | +LL | let _val: &'static T = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap<&'static T>` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:23:38 + | +LL | let _val: Wrap<&'static T> = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap<&'static T>` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:24:38 + | +LL | let _val: Wrap<&'static T> = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `!` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:30:23 + | +LL | let _val: ! = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `!` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:16:23 + --> $DIR/uninitialized-zeroed.rs:31:23 | LL | let _val: ! = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `(i32, !)` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:33:30 + | +LL | let _val: (i32, !) = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `(i32, !)` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:34:30 + | +LL | let _val: (i32, !) = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Void` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:36:26 + | +LL | let _val: Void = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Void` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:37:26 + | +LL | let _val: Void = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `&'static i32` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:21:34 + --> $DIR/uninitialized-zeroed.rs:39:34 | LL | let _val: &'static i32 = mem::zeroed(); | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `&'static i32` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:22:34 + --> $DIR/uninitialized-zeroed.rs:40:34 | LL | let _val: &'static i32 = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Ref` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:42:25 + | +LL | let _val: Ref = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Ref` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:43:25 + | +LL | let _val: Ref = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `fn()` does not permit zero-initialization - --> $DIR/uninitialized-zeroed.rs:24:26 + --> $DIR/uninitialized-zeroed.rs:45:26 | LL | let _val: fn() = mem::zeroed(); | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead error: the type `fn()` does not permit being left uninitialized - --> $DIR/uninitialized-zeroed.rs:25:26 + --> $DIR/uninitialized-zeroed.rs:46:26 | LL | let _val: fn() = mem::uninitialized(); | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap` does not permit zero-initialization + --> $DIR/uninitialized-zeroed.rs:48:32 + | +LL | let _val: Wrap = mem::zeroed(); + | ^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead + +error: the type `Wrap` does not permit being left uninitialized + --> $DIR/uninitialized-zeroed.rs:49:32 + | +LL | let _val: Wrap = mem::uninitialized(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: this means that this code causes undefined behavior when executed + = help: use `MaybeUninit` instead -error: aborting due to 6 previous errors +error: aborting due to 18 previous errors From fbd56131a937ffd2ebaef88c8f9788bd4447d66c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 08:43:42 +0200 Subject: [PATCH 10/32] fix a comment --- src/librustc/ty/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index a10578b0a4390..da7f894e84d0a 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1949,7 +1949,7 @@ pub struct FieldDef { pub struct AdtDef { /// `DefId` of the struct, enum or union item. pub did: DefId, - /// Variants of the ADT. If this is a struct or enum, then there will be a single variant. + /// Variants of the ADT. If this is a struct or union, then there will be a single variant. pub variants: IndexVec, /// Flags of the ADT (e.g. is this a struct? is this non-exhaustive?) flags: AdtFlags, From 8e6fbbec83707fe46c9f406f6ccd0ca75817dd5e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 09:01:53 +0200 Subject: [PATCH 11/32] add tuple_fields convenience method and use it in a few places --- src/librustc/ty/mod.rs | 2 ++ src/librustc/ty/sty.rs | 14 ++++++++++++-- src/librustc/ty/util.rs | 8 ++++---- src/librustc/ty/walk.rs | 4 ++-- src/librustc_lint/builtin.rs | 9 ++------- src/librustc_mir/shim.rs | 2 +- src/librustc_mir/util/elaborate_drops.rs | 4 ++-- 7 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index da7f894e84d0a..f653f0561776c 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2565,6 +2565,8 @@ impl<'tcx> AdtDef { } impl<'tcx> FieldDef { + /// Returns the type of this field. The `subst` is typically obtained + /// via the second field of `TyKind::AdtDef`. pub fn ty(&self, tcx: TyCtxt<'tcx>, subst: SubstsRef<'tcx>) -> Ty<'tcx> { tcx.type_of(self.did).subst(tcx, subst) } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 064c374de2b4c..129ea9b5b674a 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -171,6 +171,7 @@ pub enum TyKind<'tcx> { Never, /// A tuple type. For example, `(i32, bool)`. + /// Use `TyS::tuple_fields` to iterate over the field types. Tuple(SubstsRef<'tcx>), /// The projection of an associated type. For example, @@ -1723,8 +1724,8 @@ impl<'tcx> TyS<'tcx> { }) }) } - ty::Tuple(tys) => tys.iter().any(|ty| { - ty.expect_ty().conservative_is_privately_uninhabited(tcx) + ty::Tuple(..) => self.tuple_fields().any(|ty| { + ty.conservative_is_privately_uninhabited(tcx) }), ty::Array(ty, len) => { match len.try_eval_usize(tcx, ParamEnv::empty()) { @@ -2103,6 +2104,15 @@ impl<'tcx> TyS<'tcx> { } } + /// Iterates over tuple fields. + /// Panics when called on anything but a tuple. + pub fn tuple_fields(&self) -> impl DoubleEndedIterator> { + match self.sty { + Tuple(substs) => substs.iter().map(|field| field.expect_ty()), + _ => bug!("tuple_fields called on non-tuple"), + } + } + /// If the type contains variants, returns the valid range of variant indices. /// FIXME This requires the optimized MIR in the case of generators. #[inline] diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index c3ecc04b12e01..96e16efd1300a 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -845,15 +845,15 @@ impl<'tcx> ty::TyS<'tcx> { ty: Ty<'tcx>, ) -> Representability { match ty.sty { - Tuple(ref ts) => { + Tuple(..) => { // Find non representable - fold_repr(ts.iter().map(|ty| { + fold_repr(ty.tuple_fields().map(|ty| { is_type_structurally_recursive( tcx, sp, seen, representable_cache, - ty.expect_ty(), + ty, ) })) } @@ -1095,7 +1095,7 @@ fn needs_drop_raw<'tcx>(tcx: TyCtxt<'tcx>, query: ty::ParamEnvAnd<'tcx, Ty<'tcx> // state transformation pass ty::Generator(..) => true, - ty::Tuple(ref tys) => tys.iter().map(|k| k.expect_ty()).any(needs_drop), + ty::Tuple(..) => ty.tuple_fields().any(needs_drop), // unions don't have destructors because of the child types, // only if they manually implement `Drop` (handled above). diff --git a/src/librustc/ty/walk.rs b/src/librustc/ty/walk.rs index c74511cf0fdda..8c3110792a8b4 100644 --- a/src/librustc/ty/walk.rs +++ b/src/librustc/ty/walk.rs @@ -119,8 +119,8 @@ fn push_subtypes<'tcx>(stack: &mut TypeWalkerStack<'tcx>, parent_ty: Ty<'tcx>) { ty::GeneratorWitness(ts) => { stack.extend(ts.skip_binder().iter().cloned().rev()); } - ty::Tuple(ts) => { - stack.extend(ts.iter().map(|k| k.expect_ty()).rev()); + ty::Tuple(..) => { + stack.extend(parent_ty.tuple_fields().rev()); } ty::FnDef(_, substs) => { stack.extend(substs.types().rev()); diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index c652c196afd0d..5c2a86f8f3feb 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1903,14 +1903,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { _ => true, // Conservative fallback for multi-variant enum. } } - Tuple(substs) => { + Tuple(..) => { // Proceed recursively, check all fields. - substs.iter().all(|field| { - ty_maybe_allows_zero_init( - tcx, - field.expect_ty(), - ) - }) + ty.tuple_fields().all(|field| ty_maybe_allows_zero_init(tcx, field)) } // FIXME: Would be nice to also warn for `NonNull`/`NonZero*`. // Conservative fallback. diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 42945c79ddf0e..33447eba7492a 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -324,7 +324,7 @@ fn build_clone_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, self_ty: Ty<'tcx>) - substs.upvar_tys(def_id, tcx) ) } - ty::Tuple(tys) => builder.tuple_like_shim(dest, src, tys.iter().map(|k| k.expect_ty())), + ty::Tuple(..) => builder.tuple_like_shim(dest, src, self_ty.tuple_fields()), _ => { bug!("clone shim for `{:?}` which is not `Copy` and is not an aggregate", self_ty) } diff --git a/src/librustc_mir/util/elaborate_drops.rs b/src/librustc_mir/util/elaborate_drops.rs index d17dcaafc04f5..52fd645e38e22 100644 --- a/src/librustc_mir/util/elaborate_drops.rs +++ b/src/librustc_mir/util/elaborate_drops.rs @@ -804,8 +804,8 @@ where let tys : Vec<_> = substs.upvar_tys(def_id, self.tcx()).collect(); self.open_drop_for_tuple(&tys) } - ty::Tuple(tys) => { - let tys: Vec<_> = tys.iter().map(|k| k.expect_ty()).collect(); + ty::Tuple(..) => { + let tys: Vec<_> = ty.tuple_fields().collect(); self.open_drop_for_tuple(&tys) } ty::Adt(def, substs) => { From 4b062a175fa3b9613995eea7d026edbbb3be715e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 10:09:42 +0200 Subject: [PATCH 12/32] note a FIXME --- src/librustc/ty/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index f653f0561776c..b104bbf466a36 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1843,6 +1843,7 @@ pub struct VariantDef { /// Flags of the variant (e.g. is field list non-exhaustive)? flags: VariantFlags, /// Recovered? + // FIXME: Needs proper doc. Recovered whom from what? pub recovered: bool, } From 3972d05fec01cd46c207f6e98aca5ee20102e17a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 11:52:40 +0200 Subject: [PATCH 13/32] proper doc comment for 'recovered' field of variant Curtesy of petrochenkov --- src/librustc/ty/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index b104bbf466a36..9d563e290de96 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1842,8 +1842,8 @@ pub struct VariantDef { pub ctor_kind: CtorKind, /// Flags of the variant (e.g. is field list non-exhaustive)? flags: VariantFlags, - /// Recovered? - // FIXME: Needs proper doc. Recovered whom from what? + /// Variant is obtained as part of recovering from a syntactic error. + /// May be incomplete or bogus. pub recovered: bool, } From c5a63566d6d8d70687410b41b6464bcef3ef89f3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 11:59:23 +0200 Subject: [PATCH 14/32] allow the lint if a few UB-demonstrating doc tests --- src/libcore/mem/maybe_uninit.rs | 3 +++ src/libcore/mem/mod.rs | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index 8d064de6f4751..1bbea02e0c7c9 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -13,6 +13,7 @@ use crate::mem::ManuallyDrop; /// ever gets used to access memory: /// /// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem::{self, MaybeUninit}; /// /// let x: &i32 = unsafe { mem::zeroed() }; // undefined behavior! @@ -27,6 +28,7 @@ use crate::mem::ManuallyDrop; /// always be `true` or `false`. Hence, creating an uninitialized `bool` is undefined behavior: /// /// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem::{self, MaybeUninit}; /// /// let b: bool = unsafe { mem::uninitialized() }; // undefined behavior! @@ -40,6 +42,7 @@ use crate::mem::ManuallyDrop; /// which otherwise can hold any *fixed* bit pattern: /// /// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem::{self, MaybeUninit}; /// /// let x: i32 = unsafe { mem::uninitialized() }; // undefined behavior! diff --git a/src/libcore/mem/mod.rs b/src/libcore/mem/mod.rs index 86dae985fdb00..16cfe0f7683a8 100644 --- a/src/libcore/mem/mod.rs +++ b/src/libcore/mem/mod.rs @@ -445,7 +445,8 @@ pub const fn needs_drop() -> bool { /// /// *Incorrect* usage of this function: initializing a reference with zero. /// -/// ```no_run +/// ```rust,no_run +/// # #![allow(invalid_value)] /// use std::mem; /// /// let _x: &i32 = unsafe { mem::zeroed() }; // Undefined behavior! From 5c77a17d182fd05f06eaf899281b2eda49047e91 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 7 Aug 2019 16:23:11 +0200 Subject: [PATCH 15/32] note down some more future plans --- src/librustc_lint/builtin.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 5c2a86f8f3feb..13ec27aa1ab3f 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -1908,6 +1908,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InvalidValue { ty.tuple_fields().all(|field| ty_maybe_allows_zero_init(tcx, field)) } // FIXME: Would be nice to also warn for `NonNull`/`NonZero*`. + // FIXME: *Only for `mem::uninitialized`*, we could also warn for `bool`, + // `char`, and any multivariant enum. // Conservative fallback. _ => true, } From 09307474c28eacf0d971ef95ecab0a2186a18c3b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 11 Aug 2019 12:06:12 +0200 Subject: [PATCH 16/32] update clippy --- src/tools/clippy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy b/src/tools/clippy index b041511b5fcd3..72da1015d6d91 160000 --- a/src/tools/clippy +++ b/src/tools/clippy @@ -1 +1 @@ -Subproject commit b041511b5fcd386c4ae74a30b60a5081f8717fbe +Subproject commit 72da1015d6d918fe1b29170acbf486d30e0c2695 From 65ea7b794718306c7b2945f63f3dd01919fc73c0 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:25:50 -0400 Subject: [PATCH 17/32] rustdoc: Replace HirVec with slices in doctree --- src/librustdoc/doctree.rs | 42 +++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/librustdoc/doctree.rs b/src/librustdoc/doctree.rs index 90dcf1be76c0d..6e453561f6da2 100644 --- a/src/librustdoc/doctree.rs +++ b/src/librustdoc/doctree.rs @@ -13,7 +13,7 @@ use rustc::hir::ptr::P; pub struct Module<'hir> { pub name: Option, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub where_outer: Span, pub where_inner: Span, pub extern_crates: Vec>, @@ -41,7 +41,7 @@ pub struct Module<'hir> { impl Module<'hir> { pub fn new( name: Option, - attrs: &'hir hir::HirVec, + attrs: &'hir [ast::Attribute], vis: &'hir hir::Visibility, ) -> Module<'hir> { Module { @@ -89,7 +89,7 @@ pub struct Struct<'hir> { pub struct_type: StructType, pub name: Name, pub generics: &'hir hir::Generics, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub fields: &'hir [hir::StructField], pub whence: Span, } @@ -100,7 +100,7 @@ pub struct Union<'hir> { pub struct_type: StructType, pub name: Name, pub generics: &'hir hir::Generics, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub fields: &'hir [hir::StructField], pub whence: Span, } @@ -109,7 +109,7 @@ pub struct Enum<'hir> { pub vis: &'hir hir::Visibility, pub variants: Vec>, pub generics: &'hir hir::Generics, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub whence: Span, pub name: Name, @@ -118,14 +118,14 @@ pub struct Enum<'hir> { pub struct Variant<'hir> { pub name: Name, pub id: hir::HirId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub def: &'hir hir::VariantData, pub whence: Span, } pub struct Function<'hir> { pub decl: &'hir hir::FnDecl, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub name: Name, pub vis: &'hir hir::Visibility, @@ -140,7 +140,7 @@ pub struct Typedef<'hir> { pub gen: &'hir hir::Generics, pub name: Name, pub id: hir::HirId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub vis: &'hir hir::Visibility, } @@ -149,7 +149,7 @@ pub struct OpaqueTy<'hir> { pub opaque_ty: &'hir hir::OpaqueTy, pub name: Name, pub id: hir::HirId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub vis: &'hir hir::Visibility, } @@ -160,7 +160,7 @@ pub struct Static<'hir> { pub mutability: hir::Mutability, pub expr: hir::BodyId, pub name: Name, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub vis: &'hir hir::Visibility, pub id: hir::HirId, pub whence: Span, @@ -170,7 +170,7 @@ pub struct Constant<'hir> { pub type_: &'hir P, pub expr: hir::BodyId, pub name: Name, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub vis: &'hir hir::Visibility, pub id: hir::HirId, pub whence: Span, @@ -182,8 +182,8 @@ pub struct Trait<'hir> { pub name: Name, pub items: Vec<&'hir hir::TraitItem>, pub generics: &'hir hir::Generics, - pub bounds: &'hir hir::HirVec, - pub attrs: &'hir hir::HirVec, + pub bounds: &'hir [hir::GenericBound], + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub whence: Span, pub vis: &'hir hir::Visibility, @@ -192,8 +192,8 @@ pub struct Trait<'hir> { pub struct TraitAlias<'hir> { pub name: Name, pub generics: &'hir hir::Generics, - pub bounds: &'hir hir::HirVec, - pub attrs: &'hir hir::HirVec, + pub bounds: &'hir [hir::GenericBound], + pub attrs: &'hir [ast::Attribute], pub id: hir::HirId, pub whence: Span, pub vis: &'hir hir::Visibility, @@ -208,7 +208,7 @@ pub struct Impl<'hir> { pub trait_: &'hir Option, pub for_: &'hir P, pub items: Vec<&'hir hir::ImplItem>, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub vis: &'hir hir::Visibility, pub id: hir::HirId, @@ -219,7 +219,7 @@ pub struct ForeignItem<'hir> { pub id: hir::HirId, pub name: Name, pub kind: &'hir hir::ForeignItemKind, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, } @@ -229,7 +229,7 @@ pub struct Macro<'hir> { pub name: Name, pub hid: hir::HirId, pub def_id: hir::def_id::DefId, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, pub matchers: hir::HirVec, pub imported_from: Option, @@ -240,7 +240,7 @@ pub struct ExternCrate<'hir> { pub cnum: CrateNum, pub path: Option, pub vis: &'hir hir::Visibility, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, } @@ -248,7 +248,7 @@ pub struct Import<'hir> { pub name: Name, pub id: hir::HirId, pub vis: &'hir hir::Visibility, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub path: &'hir hir::Path, pub glob: bool, pub whence: Span, @@ -259,7 +259,7 @@ pub struct ProcMacro<'hir> { pub id: hir::HirId, pub kind: MacroKind, pub helpers: Vec, - pub attrs: &'hir hir::HirVec, + pub attrs: &'hir [ast::Attribute], pub whence: Span, } From 19c85a8f8a3f82bfbc92f3ecd23167c0dbd66b55 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:37:31 -0400 Subject: [PATCH 18/32] Move def_id_to_path to use site in visit_ast --- src/librustdoc/clean/mod.rs | 20 +------------------- src/librustdoc/visit_ast.rs | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 6f33bdd7f4d2f..0c38e68a764f4 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -39,7 +39,7 @@ use std::fmt; use std::hash::{Hash, Hasher}; use std::default::Default; use std::{mem, slice, vec}; -use std::iter::{FromIterator, once}; +use std::iter::FromIterator; use std::rc::Rc; use std::cell::RefCell; use std::sync::Arc; @@ -4398,24 +4398,6 @@ impl Clean for hir::TypeBindingKind { } } -pub fn def_id_to_path( - cx: &DocContext<'_>, - did: DefId, - name: Option -) -> Vec { - let crate_name = name.unwrap_or_else(|| cx.tcx.crate_name(did.krate).to_string()); - let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { - // extern blocks have an empty name - let s = elem.data.to_string(); - if !s.is_empty() { - Some(s) - } else { - None - } - }); - once(crate_name).chain(relative).collect() -} - pub fn enter_impl_trait(cx: &DocContext<'_>, f: F) -> R where F: FnOnce() -> R, diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index f01b9eeb30f23..878f1a476be9c 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -15,9 +15,26 @@ use syntax_pos::{self, Span}; use std::mem; use crate::core; -use crate::clean::{self, AttributesExt, NestedAttributesExt, def_id_to_path}; +use crate::clean::{self, AttributesExt, NestedAttributesExt}; use crate::doctree::*; +fn def_id_to_path( + cx: &core::DocContext<'_>, + did: DefId, + name: Option +) -> Vec { + let crate_name = name.unwrap_or_else(|| cx.tcx.crate_name(did.krate).to_string()); + let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { + // extern blocks have an empty name + let s = elem.data.to_string(); + if !s.is_empty() { + Some(s) + } else { + None + } + }); + std::iter::once(crate_name).chain(relative).collect() +} // Also, is there some reason that this doesn't use the 'visit' // framework from syntax?. From 00d7bc7688c31e22e3f27d3e225087b1f8ea694b Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:44:23 -0400 Subject: [PATCH 19/32] Remove crate_name from DocContext tcx.crate_name is the appropriate way to retrieve the crate name. --- src/librustdoc/clean/inline.rs | 5 +---- src/librustdoc/core.rs | 5 +---- src/librustdoc/visit_ast.rs | 11 ++++++----- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index d2a6dcf19bcca..884cdef3c771f 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -163,10 +163,7 @@ pub fn load_attrs<'hir>(cx: &DocContext<'hir>, did: DefId) -> Attrs<'hir> { /// These names are used later on by HTML rendering to generate things like /// source links back to the original item. pub fn record_extern_fqn(cx: &DocContext<'_>, did: DefId, kind: clean::TypeKind) { - let mut crate_name = cx.tcx.crate_name(did.krate).to_string(); - if did.is_local() { - crate_name = cx.crate_name.clone().unwrap_or(crate_name); - } + let crate_name = cx.tcx.crate_name(did.krate).to_string(); let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { // extern blocks have an empty name diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 5138e4a23a47e..869bec6cb88a5 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -46,8 +46,6 @@ pub struct DocContext<'tcx> { pub tcx: TyCtxt<'tcx>, pub resolver: Rc>, - /// The stack of module NodeIds up till this point - pub crate_name: Option, pub cstore: Lrc, /// Later on moved into `html::render::CACHE_KEY` pub renderinfo: RefCell, @@ -332,7 +330,7 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt file_loader: None, diagnostic_output: DiagnosticOutput::Default, stderr: None, - crate_name: crate_name.clone(), + crate_name, lint_caps, }; @@ -372,7 +370,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let ctxt = DocContext { tcx, resolver, - crate_name, cstore: compiler.cstore().clone(), external_traits: Default::default(), active_extern_traits: Default::default(), diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 878f1a476be9c..5fad9038104de 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -6,6 +6,7 @@ use rustc::hir::def::{Res, DefKind}; use rustc::hir::def_id::{DefId, LOCAL_CRATE}; use rustc::middle::privacy::AccessLevel; use rustc::util::nodemap::{FxHashSet, FxHashMap}; +use rustc::ty::TyCtxt; use syntax::ast; use syntax::ext::base::MacroKind; use syntax::source_map::Spanned; @@ -18,13 +19,13 @@ use crate::core; use crate::clean::{self, AttributesExt, NestedAttributesExt}; use crate::doctree::*; +// FIXME: Should this be replaced with tcx.def_path_str? fn def_id_to_path( - cx: &core::DocContext<'_>, + tcx: TyCtxt<'_>, did: DefId, - name: Option ) -> Vec { - let crate_name = name.unwrap_or_else(|| cx.tcx.crate_name(did.krate).to_string()); - let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| { + let crate_name = tcx.crate_name(did.krate).to_string(); + let relative = tcx.def_path(did).data.into_iter().filter_map(|elem| { // extern blocks have an empty name let s = elem.data.to_string(); if !s.is_empty() { @@ -68,7 +69,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // We can't use the entry API, as that keeps the mutable borrow of `self` active // when we try to use `cx`. if self.exact_paths.get(&did).is_none() { - let path = def_id_to_path(self.cx, did, self.cx.crate_name.clone()); + let path = def_id_to_path(self.cx.tcx, did); self.exact_paths.insert(did, path); } } From 8f80a8d7d564b17fe3831c4b5e5404d93a013bf5 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 13:52:45 -0400 Subject: [PATCH 20/32] Use entry API in store_path --- src/librustdoc/visit_ast.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 5fad9038104de..35b6d9972da06 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -66,12 +66,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } fn store_path(&mut self, did: DefId) { - // We can't use the entry API, as that keeps the mutable borrow of `self` active - // when we try to use `cx`. - if self.exact_paths.get(&did).is_none() { - let path = def_id_to_path(self.cx.tcx, did); - self.exact_paths.insert(did, path); - } + let tcx = self.cx.tcx; + self.exact_paths.entry(did).or_insert_with(|| def_id_to_path(tcx, did)); } pub fn visit(mut self, krate: &'tcx hir::Crate) -> Module<'tcx> { From c57481001ea81665b1ca23368b710a0435d28653 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 15:20:21 -0400 Subject: [PATCH 21/32] Remove ReentrantMutex This drops the parking_lot dependency; the ReentrantMutex type appeared to be unused (at least, no compilation failures occurred). This is technically a possible change in behavior of its users, as lock() would wait on other threads releasing their guards, but since we didn't actually remove any threading or such in this code, it appears that we never used that behavior (the behavior change is only noticeable if the type previously was used in two threads, in a single thread ReentrantMutex is useless). --- Cargo.lock | 1 - src/librustdoc/Cargo.toml | 1 - src/librustdoc/clean/inline.rs | 6 ++---- src/librustdoc/clean/mod.rs | 4 +--- src/librustdoc/core.rs | 3 +-- src/librustdoc/fold.rs | 8 ++++---- src/librustdoc/html/render.rs | 2 +- 7 files changed, 9 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 52cfa2cb1f80e..ab6731e4d433d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3252,7 +3252,6 @@ name = "rustdoc" version = "0.0.0" dependencies = [ "minifier 0.0.33 (registry+https://github.com/rust-lang/crates.io-index)", - "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "pulldown-cmark 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-rayon 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "tempfile 3.0.5 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index 334dc74c6c8f7..0eb8b73016d10 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -13,4 +13,3 @@ pulldown-cmark = { version = "0.5.3", default-features = false } minifier = "0.0.33" rayon = { version = "0.2.0", package = "rustc-rayon" } tempfile = "3" -parking_lot = "0.7" diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 884cdef3c771f..a8336607f7ae9 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -574,8 +574,7 @@ pub fn record_extern_trait(cx: &DocContext<'_>, did: DefId) { } { - let external_traits = cx.external_traits.lock(); - if external_traits.borrow().contains_key(&did) || + if cx.external_traits.borrow().contains_key(&did) || cx.active_extern_traits.borrow().contains(&did) { return; @@ -588,8 +587,7 @@ pub fn record_extern_trait(cx: &DocContext<'_>, did: DefId) { let trait_ = build_external_trait(cx, did); { - let external_traits = cx.external_traits.lock(); - external_traits.borrow_mut().insert(did, trait_); + cx.external_traits.borrow_mut().insert(did, trait_); } cx.active_extern_traits.borrow_mut().remove_item(&did); } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 0c38e68a764f4..ee76bf35cab91 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -45,8 +45,6 @@ use std::cell::RefCell; use std::sync::Arc; use std::u32; -use parking_lot::ReentrantMutex; - use crate::core::{self, DocContext}; use crate::doctree; use crate::html::render::{cache, ExternalLocation}; @@ -133,7 +131,7 @@ pub struct Crate { pub primitives: Vec<(DefId, PrimitiveType, Attributes)>, // These are later on moved into `CACHEKEY`, leaving the map empty. // Only here so that they can be filtered through the rustdoc passes. - pub external_traits: Arc>>>, + pub external_traits: Arc>>, pub masked_crates: FxHashSet, } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 869bec6cb88a5..6b524e1206f33 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -22,7 +22,6 @@ use syntax::json::JsonEmitter; use syntax::symbol::sym; use errors; use errors::emitter::{Emitter, EmitterWriter}; -use parking_lot::ReentrantMutex; use std::cell::RefCell; use std::mem; @@ -50,7 +49,7 @@ pub struct DocContext<'tcx> { /// Later on moved into `html::render::CACHE_KEY` pub renderinfo: RefCell, /// Later on moved through `clean::Crate` into `html::render::CACHE_KEY` - pub external_traits: Arc>>>, + pub external_traits: Arc>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. pub active_extern_traits: RefCell>, diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index cfa22bc27b758..5482239c7ce28 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -105,12 +105,12 @@ pub trait DocFolder : Sized { c.module = c.module.take().and_then(|module| self.fold_item(module)); { - let guard = c.external_traits.lock(); - let traits = guard.replace(Default::default()); - guard.borrow_mut().extend(traits.into_iter().map(|(k, mut v)| { + let mut guard = c.external_traits.borrow_mut(); + let external_traits = std::mem::replace(&mut *guard, Default::default()); + *guard = external_traits.into_iter().map(|(k, mut v)| { v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect(); (k, v) - })); + }).collect(); } c } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index b1cee0182052b..211f1e325b9ec 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -659,7 +659,7 @@ pub fn run(mut krate: clean::Crate, crate_version: krate.version.take(), orphan_impl_items: Vec::new(), orphan_trait_impls: Vec::new(), - traits: krate.external_traits.lock().replace(Default::default()), + traits: krate.external_traits.replace(Default::default()), deref_trait_did, deref_mut_trait_did, owned_box_did, From 6be2857a6cc551f63cd79d9d59322fbb2ce9103d Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 16:19:25 -0400 Subject: [PATCH 22/32] Replace Arc with Rc around external_traits --- src/librustdoc/clean/mod.rs | 2 +- src/librustdoc/core.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ee76bf35cab91..af4fe62bcbc31 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -131,7 +131,7 @@ pub struct Crate { pub primitives: Vec<(DefId, PrimitiveType, Attributes)>, // These are later on moved into `CACHEKEY`, leaving the map empty. // Only here so that they can be filtered through the rustdoc passes. - pub external_traits: Arc>>, + pub external_traits: Rc>>, pub masked_crates: FxHashSet, } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 6b524e1206f33..a14ddc706d5d1 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -26,7 +26,6 @@ use errors::emitter::{Emitter, EmitterWriter}; use std::cell::RefCell; use std::mem; use rustc_data_structures::sync::{self, Lrc}; -use std::sync::Arc; use std::rc::Rc; use crate::config::{Options as RustdocOptions, RenderOptions}; @@ -49,7 +48,7 @@ pub struct DocContext<'tcx> { /// Later on moved into `html::render::CACHE_KEY` pub renderinfo: RefCell, /// Later on moved through `clean::Crate` into `html::render::CACHE_KEY` - pub external_traits: Arc>>, + pub external_traits: Rc>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. pub active_extern_traits: RefCell>, From 00319519bba1fa1c2036aef70607428da9519155 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 16:43:39 -0400 Subject: [PATCH 23/32] Store typed Passes --- src/librustdoc/clean/mod.rs | 2 + src/librustdoc/config.rs | 16 +++---- src/librustdoc/core.rs | 31 +++++++------- src/librustdoc/html/render.rs | 14 ++---- src/librustdoc/lib.rs | 7 +-- src/librustdoc/passes/collapse_docs.rs | 4 +- src/librustdoc/passes/mod.rs | 59 +++++++++++++------------- 7 files changed, 65 insertions(+), 68 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index af4fe62bcbc31..b281505956d6a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -133,6 +133,7 @@ pub struct Crate { // Only here so that they can be filtered through the rustdoc passes. pub external_traits: Rc>>, pub masked_crates: FxHashSet, + pub collapsed: bool, } impl Clean for hir::Crate { @@ -221,6 +222,7 @@ impl Clean for hir::Crate { primitives, external_traits: cx.external_traits.clone(), masked_crates, + collapsed: false, } } } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index db90bb4524dcf..2be67d707fe14 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -220,22 +220,22 @@ impl Options { println!("{:>20} - {}", pass.name, pass.description); } println!("\nDefault passes for rustdoc:"); - for &name in passes::DEFAULT_PASSES { - println!("{:>20}", name); + for pass in passes::DEFAULT_PASSES { + println!("{:>20}", pass.name); } println!("\nPasses run with `--document-private-items`:"); - for &name in passes::DEFAULT_PRIVATE_PASSES { - println!("{:>20}", name); + for pass in passes::DEFAULT_PRIVATE_PASSES { + println!("{:>20}", pass.name); } if nightly_options::is_nightly_build() { println!("\nPasses run with `--show-coverage`:"); - for &name in passes::DEFAULT_COVERAGE_PASSES { - println!("{:>20}", name); + for pass in passes::DEFAULT_COVERAGE_PASSES { + println!("{:>20}", pass.name); } println!("\nPasses run with `--show-coverage --document-private-items`:"); - for &name in passes::PRIVATE_COVERAGE_PASSES { - println!("{:>20}", name); + for pass in passes::PRIVATE_COVERAGE_PASSES { + println!("{:>20}", pass.name); } } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index a14ddc706d5d1..adbe4b469e8d0 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -223,7 +223,7 @@ pub fn new_handler(error_format: ErrorOutputType, ) } -pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOptions, Vec) { +pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOptions) { // Parse, resolve, and typecheck the given crate. let RustdocOptions { @@ -427,8 +427,8 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt }, _ => continue, }; - for p in value.as_str().split_whitespace() { - sink.push(p.to_string()); + for name in value.as_str().split_whitespace() { + sink.push(name.to_string()); } } @@ -439,25 +439,26 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt } } - let mut passes: Vec = - passes::defaults(default_passes).iter().map(|p| p.to_string()).collect(); - passes.extend(manual_passes); + let passes = passes::defaults(default_passes).iter().chain(manual_passes.into_iter() + .flat_map(|name| { + if let Some(pass) = passes::find_pass(&name) { + Some(pass) + } else { + error!("unknown pass {}, skipping", name); + None + } + })); info!("Executing passes"); - for pass_name in &passes { - match passes::find_pass(pass_name).map(|p| p.pass) { - Some(pass) => { - debug!("running pass {}", pass_name); - krate = pass(krate, &ctxt); - } - None => error!("unknown pass {}, skipping", *pass_name), - } + for pass in passes { + debug!("running pass {}", pass.name); + krate = (pass.pass)(krate, &ctxt); } ctxt.sess().abort_if_errors(); - (krate, ctxt.renderinfo.into_inner(), render_options, passes) + (krate, ctxt.renderinfo.into_inner(), render_options) }) }) } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 211f1e325b9ec..eb88c72da9eeb 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -185,8 +185,8 @@ struct SharedContext { pub include_sources: bool, /// The local file sources we've emitted and their respective url-paths. pub local_sources: FxHashMap, - /// All the passes that were run on this crate. - pub passes: FxHashSet, + /// Whether the collapsed pass ran + pub collapsed: bool, /// The base-URL of the issue tracker for when an item has been tagged with /// an issue number. pub issue_tracker_base_url: Option, @@ -229,15 +229,10 @@ impl SharedContext { } impl SharedContext { - /// Returns `true` if the `collapse-docs` pass was run on this crate. - pub fn was_collapsed(&self) -> bool { - self.passes.contains("collapse-docs") - } - /// Based on whether the `collapse-docs` pass was run, return either the `doc_value` or the /// `collapsed_doc_value` of the given item. pub fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option> { - if self.was_collapsed() { + if self.collapsed { item.collapsed_doc_value().map(|s| s.into()) } else { item.doc_value().map(|s| s.into()) @@ -526,7 +521,6 @@ pub fn initial_ids() -> Vec { /// Generates the documentation for `crate` into the directory `dst` pub fn run(mut krate: clean::Crate, options: RenderOptions, - passes: FxHashSet, renderinfo: RenderInfo, diag: &errors::Handler, edition: Edition) -> Result<(), Error> { @@ -557,8 +551,8 @@ pub fn run(mut krate: clean::Crate, }; let mut errors = Arc::new(ErrorStorage::new()); let mut scx = SharedContext { + collapsed: krate.collapsed, src_root, - passes, include_sources: true, local_sources: Default::default(), issue_tracker_base_url: None, diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 799b5d923ae89..e63a76614bc99 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -80,7 +80,6 @@ struct Output { krate: clean::Crate, renderinfo: html::render::RenderInfo, renderopts: config::RenderOptions, - passes: Vec, } pub fn main() { @@ -419,14 +418,13 @@ fn main_options(options: config::Options) -> i32 { return rustc_driver::EXIT_SUCCESS; } - let Output { krate, passes, renderinfo, renderopts } = out; + let Output { krate, renderinfo, renderopts } = out; info!("going to format"); let (error_format, treat_err_as_bug, ui_testing, edition) = diag_opts; let diag = core::new_handler(error_format, None, treat_err_as_bug, ui_testing); match html::render::run( krate, renderopts, - passes.into_iter().collect(), renderinfo, &diag, edition, @@ -459,7 +457,7 @@ where R: 'static + Send, let result = rustc_driver::report_ices_to_stderr_if_any(move || { let crate_name = options.crate_name.clone(); let crate_version = options.crate_version.clone(); - let (mut krate, renderinfo, renderopts, passes) = core::run_core(options); + let (mut krate, renderinfo, renderopts) = core::run_core(options); info!("finished with rustc"); @@ -473,7 +471,6 @@ where R: 'static + Send, krate: krate, renderinfo: renderinfo, renderopts, - passes: passes })).unwrap(); }); diff --git a/src/librustdoc/passes/collapse_docs.rs b/src/librustdoc/passes/collapse_docs.rs index 144ff226c4283..31288345ce57b 100644 --- a/src/librustdoc/passes/collapse_docs.rs +++ b/src/librustdoc/passes/collapse_docs.rs @@ -30,7 +30,9 @@ impl DocFragment { } pub fn collapse_docs(krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate { - Collapser.fold_crate(krate) + let mut krate = Collapser.fold_crate(krate); + krate.collapsed = true; + krate } struct Collapser; diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 4e10b46bc8a6f..641a6df221446 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -57,8 +57,9 @@ pub struct Pass { pub description: &'static str, } + /// The full list of passes. -pub const PASSES: &'static [Pass] = &[ +pub const PASSES: &[Pass] = &[ CHECK_PRIVATE_ITEMS_DOC_TESTS, STRIP_HIDDEN, UNINDENT_COMMENTS, @@ -73,43 +74,43 @@ pub const PASSES: &'static [Pass] = &[ ]; /// The list of passes run by default. -pub const DEFAULT_PASSES: &[&str] = &[ - "collect-trait-impls", - "collapse-docs", - "unindent-comments", - "check-private-items-doc-tests", - "strip-hidden", - "strip-private", - "collect-intra-doc-links", - "check-code-block-syntax", - "propagate-doc-cfg", +pub const DEFAULT_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + COLLAPSE_DOCS, + UNINDENT_COMMENTS, + CHECK_PRIVATE_ITEMS_DOC_TESTS, + STRIP_HIDDEN, + STRIP_PRIVATE, + COLLECT_INTRA_DOC_LINKS, + CHECK_CODE_BLOCK_SYNTAX, + PROPAGATE_DOC_CFG, ]; /// The list of default passes run with `--document-private-items` is passed to rustdoc. -pub const DEFAULT_PRIVATE_PASSES: &[&str] = &[ - "collect-trait-impls", - "collapse-docs", - "unindent-comments", - "check-private-items-doc-tests", - "strip-priv-imports", - "collect-intra-doc-links", - "check-code-block-syntax", - "propagate-doc-cfg", +pub const DEFAULT_PRIVATE_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + COLLAPSE_DOCS, + UNINDENT_COMMENTS, + CHECK_PRIVATE_ITEMS_DOC_TESTS, + STRIP_PRIV_IMPORTS, + COLLECT_INTRA_DOC_LINKS, + CHECK_CODE_BLOCK_SYNTAX, + PROPAGATE_DOC_CFG, ]; /// The list of default passes run when `--doc-coverage` is passed to rustdoc. -pub const DEFAULT_COVERAGE_PASSES: &'static [&'static str] = &[ - "collect-trait-impls", - "strip-hidden", - "strip-private", - "calculate-doc-coverage", +pub const DEFAULT_COVERAGE_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + STRIP_HIDDEN, + STRIP_PRIVATE, + CALCULATE_DOC_COVERAGE, ]; /// The list of default passes run when `--doc-coverage --document-private-items` is passed to /// rustdoc. -pub const PRIVATE_COVERAGE_PASSES: &'static [&'static str] = &[ - "collect-trait-impls", - "calculate-doc-coverage", +pub const PRIVATE_COVERAGE_PASSES: &[Pass] = &[ + COLLECT_TRAIT_IMPLS, + CALCULATE_DOC_COVERAGE, ]; /// A shorthand way to refer to which set of passes to use, based on the presence of @@ -124,7 +125,7 @@ pub enum DefaultPassOption { } /// Returns the given default set of passes. -pub fn defaults(default_set: DefaultPassOption) -> &'static [&'static str] { +pub fn defaults(default_set: DefaultPassOption) -> &'static [Pass] { match default_set { DefaultPassOption::Default => DEFAULT_PASSES, DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES, From 03474801512806591c17d4477d98b883d74ed455 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 16:59:21 -0400 Subject: [PATCH 24/32] Don't store all traits in DocContext This is already a query so we're just needlessly copying the data around. --- src/librustdoc/clean/blanket_impl.rs | 3 ++- src/librustdoc/core.rs | 5 +---- src/librustdoc/passes/collect_trait_impls.rs | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs index 64cffaec2eaf3..490d4107c51ab 100644 --- a/src/librustdoc/clean/blanket_impl.rs +++ b/src/librustdoc/clean/blanket_impl.rs @@ -3,6 +3,7 @@ use rustc::traits; use rustc::ty::ToPredicate; use rustc::ty::subst::Subst; use rustc::infer::InferOk; +use rustc::hir::def_id::LOCAL_CRATE; use syntax_pos::DUMMY_SP; use super::*; @@ -27,7 +28,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> { debug!("get_blanket_impls({:?})", ty); let mut impls = Vec::new(); - for &trait_def_id in self.cx.all_traits.iter() { + for &trait_def_id in self.cx.tcx.all_traits(LOCAL_CRATE).iter() { if !self.cx.renderinfo.borrow().access_levels.is_public(trait_def_id) || self.cx.generated_synthetics .borrow_mut() diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index adbe4b469e8d0..c7695fbd8d261 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -68,7 +68,6 @@ pub struct DocContext<'tcx> { /// Auto-trait or blanket impls processed so far, as `(self_ty, trait_def_id)`. // FIXME(eddyb) make this a `ty::TraitRef<'tcx>` set. pub generated_synthetics: RefCell, DefId)>>, - pub all_traits: Vec, pub auto_traits: Vec, } @@ -364,7 +363,6 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt let mut renderinfo = RenderInfo::default(); renderinfo.access_levels = access_levels; - let all_traits = tcx.all_traits(LOCAL_CRATE).to_vec(); let ctxt = DocContext { tcx, resolver, @@ -379,10 +377,9 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt fake_def_ids: Default::default(), all_fake_def_ids: Default::default(), generated_synthetics: Default::default(), - auto_traits: all_traits.iter().cloned().filter(|trait_def_id| { + auto_traits: tcx.all_traits(LOCAL_CRATE).iter().cloned().filter(|trait_def_id| { tcx.trait_is_auto(*trait_def_id) }).collect(), - all_traits, }; debug!("crate: {:?}", tcx.hir().krate()); diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index cd488b9df78d2..86e4e9fd95637 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -4,7 +4,7 @@ use crate::fold::DocFolder; use super::Pass; use rustc::util::nodemap::FxHashSet; -use rustc::hir::def_id::DefId; +use rustc::hir::def_id::{LOCAL_CRATE, DefId}; use syntax::symbol::sym; pub const COLLECT_TRAIT_IMPLS: Pass = Pass { @@ -116,7 +116,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate { // `tcx.crates()` doesn't include the local crate, and `tcx.all_trait_implementations` // doesn't work with it anyway, so pull them from the HIR map instead - for &trait_did in cx.all_traits.iter() { + for &trait_did in cx.tcx.all_traits(LOCAL_CRATE).iter() { for &impl_node in cx.tcx.hir().trait_impls(trait_did) { let impl_did = cx.tcx.hir().local_def_id(impl_node); inline::build_impl(cx, impl_did, None, &mut new_items); From eea2f879afbaad07c562ba572482f402bc6c0006 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 17:10:13 -0400 Subject: [PATCH 25/32] Use a HashSet instead of Vec --- src/librustdoc/clean/inline.rs | 8 +++----- src/librustdoc/core.rs | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index a8336607f7ae9..6f93c95edef08 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -581,13 +581,11 @@ pub fn record_extern_trait(cx: &DocContext<'_>, did: DefId) { } } - cx.active_extern_traits.borrow_mut().push(did); + cx.active_extern_traits.borrow_mut().insert(did); debug!("record_extern_trait: {:?}", did); let trait_ = build_external_trait(cx, did); - { - cx.external_traits.borrow_mut().insert(did, trait_); - } - cx.active_extern_traits.borrow_mut().remove_item(&did); + cx.external_traits.borrow_mut().insert(did, trait_); + cx.active_extern_traits.borrow_mut().remove(&did); } diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c7695fbd8d261..87381f224d0bb 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -51,7 +51,7 @@ pub struct DocContext<'tcx> { pub external_traits: Rc>>, /// Used while populating `external_traits` to ensure we don't process the same trait twice at /// the same time. - pub active_extern_traits: RefCell>, + pub active_extern_traits: RefCell>, // The current set of type and lifetime substitutions, // for expanding type aliases at the HIR level: From ade8b02828de9653e6aca122f1a0f6d8c48ad29b Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 17:16:01 -0400 Subject: [PATCH 26/32] Remove unnecessary channel --- src/librustdoc/lib.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index e63a76614bc99..e30b35937db9f 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -44,7 +44,6 @@ use std::default::Default; use std::env; use std::panic; use std::process; -use std::sync::mpsc::channel; use rustc::session::{early_warn, early_error}; use rustc::session::config::{ErrorOutputType, RustcOptGroup}; @@ -452,8 +451,6 @@ where R: 'static + Send, // First, parse the crate and extract all relevant information. info!("starting to run rustc"); - let (tx, rx) = channel(); - let result = rustc_driver::report_ices_to_stderr_if_any(move || { let crate_name = options.crate_name.clone(); let crate_version = options.crate_version.clone(); @@ -467,15 +464,15 @@ where R: 'static + Send, krate.version = crate_version; - tx.send(f(Output { + f(Output { krate: krate, renderinfo: renderinfo, renderopts, - })).unwrap(); + }) }); match result { - Ok(()) => rx.recv().unwrap(), + Ok(output) => output, Err(_) => panic::resume_unwind(Box::new(errors::FatalErrorMarker)), } } From dbad77ffdd59e54b3f496cfbdb7909a6bbd03031 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:07:07 -0400 Subject: [PATCH 27/32] Remove thread-local for playground config --- src/librustdoc/config.rs | 2 +- src/librustdoc/externalfiles.rs | 8 +- src/librustdoc/html/markdown.rs | 203 +++++++++++++----------- src/librustdoc/html/render.rs | 29 ++-- src/librustdoc/markdown.rs | 11 +- src/tools/error_index_generator/main.rs | 11 +- 6 files changed, 147 insertions(+), 117 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 2be67d707fe14..98ab957ecbb38 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -378,7 +378,7 @@ impl Options { &matches.opt_strs("html-after-content"), &matches.opt_strs("markdown-before-content"), &matches.opt_strs("markdown-after-content"), - &diag, &mut id_map, edition) { + &diag, &mut id_map, edition, &None) { Some(eh) => eh, None => return Err(3), }; diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index d604ba11d4186..d920b7c4c9169 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -4,7 +4,7 @@ use std::str; use errors; use crate::syntax::feature_gate::UnstableFeatures; use crate::syntax::edition::Edition; -use crate::html::markdown::{IdMap, ErrorCodes, Markdown}; +use crate::html::markdown::{IdMap, ErrorCodes, Markdown, Playground}; use std::cell::RefCell; @@ -24,7 +24,7 @@ pub struct ExternalHtml { impl ExternalHtml { pub fn load(in_header: &[String], before_content: &[String], after_content: &[String], md_before_content: &[String], md_after_content: &[String], diag: &errors::Handler, - id_map: &mut IdMap, edition: Edition) + id_map: &mut IdMap, edition: Edition, playground: &Option) -> Option { let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); load_external_files(in_header, diag) @@ -36,7 +36,7 @@ impl ExternalHtml { load_external_files(md_before_content, diag) .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), - codes, edition)))) + codes, edition, playground)))) ) .and_then(|(ih, bc)| load_external_files(after_content, diag) @@ -46,7 +46,7 @@ impl ExternalHtml { load_external_files(md_after_content, diag) .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), - codes, edition)))) + codes, edition, playground)))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index ef52ce62875c7..73233a2289ccd 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -17,7 +17,7 @@ //! let s = "My *markdown* _text_"; //! let mut id_map = IdMap::new(); //! let html = format!("{}", Markdown(s, &[], RefCell::new(&mut id_map), -//! ErrorCodes::Yes, Edition::Edition2015)); +//! ErrorCodes::Yes, Edition::Edition2015, None)); //! // ... something using html //! ``` @@ -59,6 +59,7 @@ pub struct Markdown<'a>( pub ErrorCodes, /// Default edition to use when parsing doctests (to add a `fn main`). pub Edition, + pub &'a Option, ); /// A tuple struct like `Markdown` that renders the markdown with a table of contents. pub struct MarkdownWithToc<'a>( @@ -66,9 +67,16 @@ pub struct MarkdownWithToc<'a>( pub RefCell<&'a mut IdMap>, pub ErrorCodes, pub Edition, + pub &'a Option, ); /// A tuple struct like `Markdown` that renders the markdown escaping HTML tags. -pub struct MarkdownHtml<'a>(pub &'a str, pub RefCell<&'a mut IdMap>, pub ErrorCodes, pub Edition); +pub struct MarkdownHtml<'a>( + pub &'a str, + pub RefCell<&'a mut IdMap>, + pub ErrorCodes, + pub Edition, + pub &'a Option, +); /// A tuple struct like `Markdown` that renders only the first paragraph. pub struct MarkdownSummaryLine<'a>(pub &'a str, pub &'a [(String, String)]); @@ -155,30 +163,39 @@ fn slugify(c: char) -> Option { } } -// Information about the playground if a URL has been specified, containing an -// optional crate name and the URL. -thread_local!(pub static PLAYGROUND: RefCell, String)>> = { - RefCell::new(None) -}); +#[derive(Clone, Debug)] +pub struct Playground { + pub crate_name: Option, + pub url: String, +} /// Adds syntax highlighting and playground Run buttons to Rust code blocks. -struct CodeBlocks<'a, I: Iterator>> { +struct CodeBlocks<'p, 'a, I: Iterator>> { inner: I, check_error_codes: ErrorCodes, edition: Edition, + // Information about the playground if a URL has been specified, containing an + // optional crate name and the URL. + playground: &'p Option, } -impl<'a, I: Iterator>> CodeBlocks<'a, I> { - fn new(iter: I, error_codes: ErrorCodes, edition: Edition) -> Self { +impl<'p, 'a, I: Iterator>> CodeBlocks<'p, 'a, I> { + fn new( + iter: I, + error_codes: ErrorCodes, + edition: Edition, + playground: &'p Option, + ) -> Self { CodeBlocks { inner: iter, check_error_codes: error_codes, edition, + playground, } } } -impl<'a, I: Iterator>> Iterator for CodeBlocks<'a, I> { +impl<'a, I: Iterator>> Iterator for CodeBlocks<'_, 'a, I> { type Item = Event<'a>; fn next(&mut self) -> Option { @@ -213,86 +230,86 @@ impl<'a, I: Iterator>> Iterator for CodeBlocks<'a, I> { } let lines = origtext.lines().filter_map(|l| map_line(l).for_html()); let text = lines.collect::>>().join("\n"); - PLAYGROUND.with(|play| { - // insert newline to clearly separate it from the - // previous block so we can shorten the html output - let mut s = String::from("\n"); - let playground_button = play.borrow().as_ref().and_then(|&(ref krate, ref url)| { - if url.is_empty() { - return None; - } - let test = origtext.lines() - .map(|l| map_line(l).for_code()) - .collect::>>().join("\n"); - let krate = krate.as_ref().map(|s| &**s); - let (test, _) = test::make_test(&test, krate, false, - &Default::default(), edition); - let channel = if test.contains("#![feature(") { - "&version=nightly" - } else { - "" - }; - - let edition_string = format!("&edition={}", edition); - - // These characters don't need to be escaped in a URI. - // FIXME: use a library function for percent encoding. - fn dont_escape(c: u8) -> bool { - (b'a' <= c && c <= b'z') || - (b'A' <= c && c <= b'Z') || - (b'0' <= c && c <= b'9') || - c == b'-' || c == b'_' || c == b'.' || - c == b'~' || c == b'!' || c == b'\'' || - c == b'(' || c == b')' || c == b'*' - } - let mut test_escaped = String::new(); - for b in test.bytes() { - if dont_escape(b) { - test_escaped.push(char::from(b)); - } else { - write!(test_escaped, "%{:02X}", b).unwrap(); - } - } - Some(format!( - r#"Run"#, - url, test_escaped, channel, edition_string - )) - }); - - let tooltip = if ignore { - Some(("This example is not tested".to_owned(), "ignore")) - } else if compile_fail { - Some(("This example deliberately fails to compile".to_owned(), "compile_fail")) - } else if explicit_edition { - Some((format!("This code runs with edition {}", edition), "edition")) + // insert newline to clearly separate it from the + // previous block so we can shorten the html output + let mut s = String::from("\n"); + let playground_button = self.playground.as_ref().and_then(|playground| { + let krate = &playground.crate_name; + let url = &playground.url; + if url.is_empty() { + return None; + } + let test = origtext.lines() + .map(|l| map_line(l).for_code()) + .collect::>>().join("\n"); + let krate = krate.as_ref().map(|s| &**s); + let (test, _) = test::make_test(&test, krate, false, + &Default::default(), edition); + let channel = if test.contains("#![feature(") { + "&version=nightly" } else { - None + "" }; - if let Some((s1, s2)) = tooltip { - s.push_str(&highlight::render_with_highlighting( - &text, - Some(&format!("rust-example-rendered{}", - if ignore { " ignore" } - else if compile_fail { " compile_fail" } - else if explicit_edition { " edition " } - else { "" })), - playground_button.as_ref().map(String::as_str), - Some((s1.as_str(), s2)))); - Some(Event::Html(s.into())) - } else { - s.push_str(&highlight::render_with_highlighting( - &text, - Some(&format!("rust-example-rendered{}", - if ignore { " ignore" } - else if compile_fail { " compile_fail" } - else if explicit_edition { " edition " } - else { "" })), - playground_button.as_ref().map(String::as_str), - None)); - Some(Event::Html(s.into())) + let edition_string = format!("&edition={}", edition); + + // These characters don't need to be escaped in a URI. + // FIXME: use a library function for percent encoding. + fn dont_escape(c: u8) -> bool { + (b'a' <= c && c <= b'z') || + (b'A' <= c && c <= b'Z') || + (b'0' <= c && c <= b'9') || + c == b'-' || c == b'_' || c == b'.' || + c == b'~' || c == b'!' || c == b'\'' || + c == b'(' || c == b')' || c == b'*' } - }) + let mut test_escaped = String::new(); + for b in test.bytes() { + if dont_escape(b) { + test_escaped.push(char::from(b)); + } else { + write!(test_escaped, "%{:02X}", b).unwrap(); + } + } + Some(format!( + r#"Run"#, + url, test_escaped, channel, edition_string + )) + }); + + let tooltip = if ignore { + Some(("This example is not tested".to_owned(), "ignore")) + } else if compile_fail { + Some(("This example deliberately fails to compile".to_owned(), "compile_fail")) + } else if explicit_edition { + Some((format!("This code runs with edition {}", edition), "edition")) + } else { + None + }; + + if let Some((s1, s2)) = tooltip { + s.push_str(&highlight::render_with_highlighting( + &text, + Some(&format!("rust-example-rendered{}", + if ignore { " ignore" } + else if compile_fail { " compile_fail" } + else if explicit_edition { " edition " } + else { "" })), + playground_button.as_ref().map(String::as_str), + Some((s1.as_str(), s2)))); + Some(Event::Html(s.into())) + } else { + s.push_str(&highlight::render_with_highlighting( + &text, + Some(&format!("rust-example-rendered{}", + if ignore { " ignore" } + else if compile_fail { " compile_fail" } + else if explicit_edition { " edition " } + else { "" })), + playground_button.as_ref().map(String::as_str), + None)); + Some(Event::Html(s.into())) + } } } @@ -676,7 +693,7 @@ impl LangString { impl<'a> fmt::Display for Markdown<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let Markdown(md, links, ref ids, codes, edition) = *self; + let Markdown(md, links, ref ids, codes, edition, playground) = *self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case @@ -695,7 +712,7 @@ impl<'a> fmt::Display for Markdown<'a> { let p = HeadingLinks::new(p, None, &mut ids); let p = LinkReplacer::new(p, links); - let p = CodeBlocks::new(p, codes, edition); + let p = CodeBlocks::new(p, codes, edition, playground); let p = Footnotes::new(p); html::push_html(&mut s, p); @@ -705,7 +722,7 @@ impl<'a> fmt::Display for Markdown<'a> { impl<'a> fmt::Display for MarkdownWithToc<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownWithToc(md, ref ids, codes, edition) = *self; + let MarkdownWithToc(md, ref ids, codes, edition, playground) = *self; let mut ids = ids.borrow_mut(); let p = Parser::new_ext(md, opts()); @@ -716,7 +733,7 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { { let p = HeadingLinks::new(p, Some(&mut toc), &mut ids); - let p = CodeBlocks::new(p, codes, edition); + let p = CodeBlocks::new(p, codes, edition, playground); let p = Footnotes::new(p); html::push_html(&mut s, p); } @@ -729,7 +746,7 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { impl<'a> fmt::Display for MarkdownHtml<'a> { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownHtml(md, ref ids, codes, edition) = *self; + let MarkdownHtml(md, ref ids, codes, edition, playground) = *self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case @@ -745,7 +762,7 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { let mut s = String::with_capacity(md.len() * 3 / 2); let p = HeadingLinks::new(p, None, &mut ids); - let p = CodeBlocks::new(p, codes, edition); + let p = CodeBlocks::new(p, codes, edition, playground); let p = Footnotes::new(p); html::push_html(&mut s, p); diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index eb88c72da9eeb..8a7dfebdbbc1a 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -170,6 +170,7 @@ struct Context { /// The map used to ensure all generated 'id=' attributes are unique. id_map: Rc>, pub shared: Arc, + playground: Option, } struct SharedContext { @@ -574,9 +575,11 @@ pub fn run(mut krate: clean::Crate, }; // If user passed in `--playground-url` arg, we fill in crate name here + let mut playground = None; if let Some(url) = playground_url { - markdown::PLAYGROUND.with(|slot| { - *slot.borrow_mut() = Some((Some(krate.name.clone()), url)); + playground = Some(markdown::Playground { + crate_name: Some(krate.name.clone()), + url, }); } @@ -592,9 +595,9 @@ pub fn run(mut krate: clean::Crate, scx.layout.logo = s.to_string(); } (sym::html_playground_url, Some(s)) => { - markdown::PLAYGROUND.with(|slot| { - let name = krate.name.clone(); - *slot.borrow_mut() = Some((Some(name), s.to_string())); + playground = Some(markdown::Playground { + crate_name: Some(krate.name.clone()), + url: s.to_string(), }); } (sym::issue_tracker_base_url, Some(s)) => { @@ -618,6 +621,7 @@ pub fn run(mut krate: clean::Crate, edition, id_map: Rc::new(RefCell::new(id_map)), shared: Arc::new(scx), + playground, }; // Crawl the crate to build various caches used for the output @@ -2592,7 +2596,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>, if is_hidden { " hidden" } else { "" }, prefix, Markdown(md_text, &links, RefCell::new(&mut ids), - cx.codes, cx.edition)) + cx.codes, cx.edition, &cx.playground)) } fn document_short( @@ -2957,7 +2961,8 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { if let Some(note) = note { let mut ids = cx.id_map.borrow_mut(); - let html = MarkdownHtml(¬e, RefCell::new(&mut ids), error_codes, cx.edition); + let html = MarkdownHtml( + ¬e, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground); message.push_str(&format!(": {}", html)); } stability.push(format!("
{}
", message)); @@ -3006,7 +3011,13 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { message = format!( "
{}{}
", message, - MarkdownHtml(&unstable_reason, RefCell::new(&mut ids), error_codes, cx.edition) + MarkdownHtml( + &unstable_reason, + RefCell::new(&mut ids), + error_codes, + cx.edition, + &cx.playground, + ) ); } @@ -4237,7 +4248,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), - cx.codes, cx.edition))?; + cx.codes, cx.edition, &cx.playground))?; } } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index 50a647f244db5..b7dd6c30f09f9 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -60,9 +60,10 @@ pub fn render( }; let playground_url = options.markdown_playground_url .or(options.playground_url); - if let Some(playground) = playground_url { - markdown::PLAYGROUND.with(|s| { *s.borrow_mut() = Some((None, playground)); }); - } + let playground = playground_url.map(|url| markdown::Playground { + crate_name: None, + url, + }); let mut out = match File::create(&output) { Err(e) => { @@ -82,9 +83,9 @@ pub fn render( let mut ids = IdMap::new(); let error_codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); let text = if !options.markdown_no_toc { - MarkdownWithToc(text, RefCell::new(&mut ids), error_codes, edition).to_string() + MarkdownWithToc(text, RefCell::new(&mut ids), error_codes, edition, &playground).to_string() } else { - Markdown(text, &[], RefCell::new(&mut ids), error_codes, edition).to_string() + Markdown(text, &[], RefCell::new(&mut ids), error_codes, edition, &playground).to_string() }; let err = write!( diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index c31a5069e4673..33987b0b54213 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -16,7 +16,7 @@ use std::cell::RefCell; use syntax::edition::DEFAULT_EDITION; use syntax::diagnostics::metadata::{get_metadata_dir, ErrorMetadataMap, ErrorMetadata}; -use rustdoc::html::markdown::{Markdown, IdMap, ErrorCodes, PLAYGROUND}; +use rustdoc::html::markdown::{Markdown, IdMap, ErrorCodes, Playground}; use rustc_serialize::json; enum OutputFormat { @@ -95,9 +95,13 @@ impl Formatter for HTMLFormatter { match info.description { Some(ref desc) => { let mut id_map = self.0.borrow_mut(); + let playground = Playground { + crate_name: None, + url: String::from("https://play.rust-lang.org/"), + }; write!(output, "{}", Markdown(desc, &[], RefCell::new(&mut id_map), - ErrorCodes::Yes, DEFAULT_EDITION))? + ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)))? }, None => write!(output, "

No description.

\n")?, } @@ -260,9 +264,6 @@ fn parse_args() -> (OutputFormat, PathBuf) { fn main() { env_logger::init(); - PLAYGROUND.with(|slot| { - *slot.borrow_mut() = Some((None, String::from("https://play.rust-lang.org/"))); - }); let (format, dst) = parse_args(); let result = syntax::with_default_globals(move || { main_with_result(format, &dst) From c250b5fd033ebd8257cca8ee537e752355a151c3 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:27:17 -0400 Subject: [PATCH 28/32] Remove fmt::Display impls on Markdown structs These impls prevent ergonomic use of the config (e.g., forcing us to use RefCell) despite all usecases for these structs only using their Display impls once. --- src/librustdoc/externalfiles.rs | 4 +- src/librustdoc/html/markdown.rs | 57 +++++++++++-------------- src/librustdoc/html/render.rs | 10 ++--- src/tools/error_index_generator/main.rs | 2 +- 4 files changed, 34 insertions(+), 39 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index d920b7c4c9169..5d953eec31ec3 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -36,7 +36,7 @@ impl ExternalHtml { load_external_files(md_before_content, diag) .map(|m_bc| (ih, format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), - codes, edition, playground)))) + codes, edition, playground).to_string()))) ) .and_then(|(ih, bc)| load_external_files(after_content, diag) @@ -46,7 +46,7 @@ impl ExternalHtml { load_external_files(md_after_content, diag) .map(|m_ac| (ih, bc, format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), - codes, edition, playground)))) + codes, edition, playground).to_string()))) ) .map(|(ih, bc, ac)| ExternalHtml { diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 73233a2289ccd..753832305f9ea 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1,9 +1,6 @@ //! Markdown formatting for rustdoc. //! -//! This module implements markdown formatting through the pulldown-cmark -//! rust-library. This module exposes all of the -//! functionality through a unit struct, `Markdown`, which has an implementation -//! of `fmt::Display`. Example usage: +//! This module implements markdown formatting through the pulldown-cmark library. //! //! ``` //! #![feature(rustc_private)] @@ -16,8 +13,9 @@ //! //! let s = "My *markdown* _text_"; //! let mut id_map = IdMap::new(); -//! let html = format!("{}", Markdown(s, &[], RefCell::new(&mut id_map), -//! ErrorCodes::Yes, Edition::Edition2015, None)); +//! let md = Markdown(s, &[], RefCell::new(&mut id_map), +//! ErrorCodes::Yes, Edition::Edition2015, None); +//! let html = md.to_string(); //! // ... something using html //! ``` @@ -27,7 +25,7 @@ use rustc_data_structures::fx::FxHashMap; use std::cell::RefCell; use std::collections::VecDeque; use std::default::Default; -use std::fmt::{self, Write}; +use std::fmt::Write; use std::borrow::Cow; use std::ops::Range; use std::str; @@ -46,9 +44,8 @@ fn opts() -> Options { Options::ENABLE_TABLES | Options::ENABLE_FOOTNOTES } -/// A tuple struct that has the `fmt::Display` trait implemented. -/// When formatted, this struct will emit the HTML corresponding to the rendered -/// version of the contained markdown string. +/// When `to_string` is called, this struct will emit the HTML corresponding to +/// the rendered version of the contained markdown string. pub struct Markdown<'a>( pub &'a str, /// A list of link replacements. @@ -691,13 +688,13 @@ impl LangString { } } -impl<'a> fmt::Display for Markdown<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let Markdown(md, links, ref ids, codes, edition, playground) = *self; +impl Markdown<'_> { + pub fn to_string(self) -> String { + let Markdown(md, links, ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let replacer = |_: &str, s: &str| { if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) { Some((replace.clone(), s.to_owned())) @@ -716,13 +713,13 @@ impl<'a> fmt::Display for Markdown<'a> { let p = Footnotes::new(p); html::push_html(&mut s, p); - fmt.write_str(&s) + s } } -impl<'a> fmt::Display for MarkdownWithToc<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownWithToc(md, ref ids, codes, edition, playground) = *self; +impl MarkdownWithToc<'_> { + pub fn to_string(self) -> String { + let MarkdownWithToc(md, ref ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); let p = Parser::new_ext(md, opts()); @@ -738,19 +735,17 @@ impl<'a> fmt::Display for MarkdownWithToc<'a> { html::push_html(&mut s, p); } - write!(fmt, "", toc.into_toc())?; - - fmt.write_str(&s) + format!("{}", toc.into_toc(), s) } } -impl<'a> fmt::Display for MarkdownHtml<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownHtml(md, ref ids, codes, edition, playground) = *self; +impl MarkdownHtml<'_> { + pub fn to_string(self) -> String { + let MarkdownHtml(md, ref ids, codes, edition, playground) = self; let mut ids = ids.borrow_mut(); // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let p = Parser::new_ext(md, opts()); // Treat inline HTML as plain text. @@ -766,15 +761,15 @@ impl<'a> fmt::Display for MarkdownHtml<'a> { let p = Footnotes::new(p); html::push_html(&mut s, p); - fmt.write_str(&s) + s } } -impl<'a> fmt::Display for MarkdownSummaryLine<'a> { - fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let MarkdownSummaryLine(md, links) = *self; +impl MarkdownSummaryLine<'_> { + pub fn to_string(self) -> String { + let MarkdownSummaryLine(md, links) = self; // This is actually common enough to special-case - if md.is_empty() { return Ok(()) } + if md.is_empty() { return String::new(); } let replacer = |_: &str, s: &str| { if let Some(&(_, ref replace)) = links.into_iter().find(|link| &*link.0 == s) { @@ -790,7 +785,7 @@ impl<'a> fmt::Display for MarkdownSummaryLine<'a> { html::push_html(&mut s, LinkReplacer::new(SummaryLine::new(p), links)); - fmt.write_str(&s) + s } } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 8a7dfebdbbc1a..fc4e25e3d7ff6 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -2596,7 +2596,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>, if is_hidden { " hidden" } else { "" }, prefix, Markdown(md_text, &links, RefCell::new(&mut ids), - cx.codes, cx.edition, &cx.playground)) + cx.codes, cx.edition, &cx.playground).to_string()) } fn document_short( @@ -2866,7 +2866,7 @@ fn item_module(w: &mut fmt::Formatter<'_>, cx: &Context, ", name = *myitem.name.as_ref().unwrap(), stab_tags = stability_tags(myitem), - docs = MarkdownSummaryLine(doc_value, &myitem.links()), + docs = MarkdownSummaryLine(doc_value, &myitem.links()).to_string(), class = myitem.type_(), add = add, stab = stab.unwrap_or_else(|| String::new()), @@ -2963,7 +2963,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { let mut ids = cx.id_map.borrow_mut(); let html = MarkdownHtml( ¬e, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground); - message.push_str(&format!(": {}", html)); + message.push_str(&format!(": {}", html.to_string())); } stability.push(format!("
{}
", message)); } @@ -3017,7 +3017,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { error_codes, cx.edition, &cx.playground, - ) + ).to_string() ); } @@ -4248,7 +4248,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), - cx.codes, cx.edition, &cx.playground))?; + cx.codes, cx.edition, &cx.playground).to_string())?; } } diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index 33987b0b54213..89b545fb7e413 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -101,7 +101,7 @@ impl Formatter for HTMLFormatter { }; write!(output, "{}", Markdown(desc, &[], RefCell::new(&mut id_map), - ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)))? + ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)).to_string())? }, None => write!(output, "

No description.

\n")?, } From 1aa0964b543e0e21cadee2fed6a7725b594e92be Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:36:04 -0400 Subject: [PATCH 29/32] Drop RefCell from IdMap in markdown rendering --- src/librustdoc/externalfiles.rs | 6 ++---- src/librustdoc/html/markdown.rs | 19 +++++++------------ src/librustdoc/html/markdown/tests.rs | 12 ++++++------ src/librustdoc/html/render.rs | 9 ++++----- src/librustdoc/markdown.rs | 5 ++--- src/tools/error_index_generator/main.rs | 2 +- 6 files changed, 22 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 5d953eec31ec3..8254bc800ca46 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -6,8 +6,6 @@ use crate::syntax::feature_gate::UnstableFeatures; use crate::syntax::edition::Edition; use crate::html::markdown::{IdMap, ErrorCodes, Markdown, Playground}; -use std::cell::RefCell; - #[derive(Clone, Debug)] pub struct ExternalHtml { /// Content that will be included inline in the section of a @@ -35,7 +33,7 @@ impl ExternalHtml { .and_then(|(ih, bc)| load_external_files(md_before_content, diag) .map(|m_bc| (ih, - format!("{}{}", bc, Markdown(&m_bc, &[], RefCell::new(id_map), + format!("{}{}", bc, Markdown(&m_bc, &[], id_map, codes, edition, playground).to_string()))) ) .and_then(|(ih, bc)| @@ -45,7 +43,7 @@ impl ExternalHtml { .and_then(|(ih, bc, ac)| load_external_files(md_after_content, diag) .map(|m_ac| (ih, bc, - format!("{}{}", ac, Markdown(&m_ac, &[], RefCell::new(id_map), + format!("{}{}", ac, Markdown(&m_ac, &[], id_map, codes, edition, playground).to_string()))) ) .map(|(ih, bc, ac)| diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 753832305f9ea..5a7deb651b00d 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -9,12 +9,10 @@ //! //! use syntax::edition::Edition; //! use rustdoc::html::markdown::{IdMap, Markdown, ErrorCodes}; -//! use std::cell::RefCell; //! //! let s = "My *markdown* _text_"; //! let mut id_map = IdMap::new(); -//! let md = Markdown(s, &[], RefCell::new(&mut id_map), -//! ErrorCodes::Yes, Edition::Edition2015, None); +//! let md = Markdown(s, &[], &mut id_map, ErrorCodes::Yes, Edition::Edition2015, &None); //! let html = md.to_string(); //! // ... something using html //! ``` @@ -51,7 +49,7 @@ pub struct Markdown<'a>( /// A list of link replacements. pub &'a [(String, String)], /// The current list of used header IDs. - pub RefCell<&'a mut IdMap>, + pub &'a mut IdMap, /// Whether to allow the use of explicit error codes in doctest lang strings. pub ErrorCodes, /// Default edition to use when parsing doctests (to add a `fn main`). @@ -61,7 +59,7 @@ pub struct Markdown<'a>( /// A tuple struct like `Markdown` that renders the markdown with a table of contents. pub struct MarkdownWithToc<'a>( pub &'a str, - pub RefCell<&'a mut IdMap>, + pub &'a mut IdMap, pub ErrorCodes, pub Edition, pub &'a Option, @@ -69,7 +67,7 @@ pub struct MarkdownWithToc<'a>( /// A tuple struct like `Markdown` that renders the markdown escaping HTML tags. pub struct MarkdownHtml<'a>( pub &'a str, - pub RefCell<&'a mut IdMap>, + pub &'a mut IdMap, pub ErrorCodes, pub Edition, pub &'a Option, @@ -690,8 +688,7 @@ impl LangString { impl Markdown<'_> { pub fn to_string(self) -> String { - let Markdown(md, links, ids, codes, edition, playground) = self; - let mut ids = ids.borrow_mut(); + let Markdown(md, links, mut ids, codes, edition, playground) = self; // This is actually common enough to special-case if md.is_empty() { return String::new(); } @@ -719,8 +716,7 @@ impl Markdown<'_> { impl MarkdownWithToc<'_> { pub fn to_string(self) -> String { - let MarkdownWithToc(md, ref ids, codes, edition, playground) = self; - let mut ids = ids.borrow_mut(); + let MarkdownWithToc(md, mut ids, codes, edition, playground) = self; let p = Parser::new_ext(md, opts()); @@ -741,8 +737,7 @@ impl MarkdownWithToc<'_> { impl MarkdownHtml<'_> { pub fn to_string(self) -> String { - let MarkdownHtml(md, ref ids, codes, edition, playground) = self; - let mut ids = ids.borrow_mut(); + let MarkdownHtml(md, mut ids, codes, edition, playground) = self; // This is actually common enough to special-case if md.is_empty() { return String::new(); } diff --git a/src/librustdoc/html/markdown/tests.rs b/src/librustdoc/html/markdown/tests.rs index 681f363544a61..a95c29038d46f 100644 --- a/src/librustdoc/html/markdown/tests.rs +++ b/src/librustdoc/html/markdown/tests.rs @@ -73,8 +73,8 @@ fn test_lang_string_parse() { fn test_header() { fn t(input: &str, expect: &str) { let mut map = IdMap::new(); - let output = Markdown(input, &[], RefCell::new(&mut map), - ErrorCodes::Yes, DEFAULT_EDITION).to_string(); + let output = Markdown( + input, &[], &mut map, ErrorCodes::Yes, DEFAULT_EDITION, &None).to_string(); assert_eq!(output, expect, "original: {}", input); } @@ -96,8 +96,8 @@ fn test_header() { fn test_header_ids_multiple_blocks() { let mut map = IdMap::new(); fn t(map: &mut IdMap, input: &str, expect: &str) { - let output = Markdown(input, &[], RefCell::new(map), - ErrorCodes::Yes, DEFAULT_EDITION).to_string(); + let output = Markdown(input, &[], map, + ErrorCodes::Yes, DEFAULT_EDITION, &None).to_string(); assert_eq!(output, expect, "original: {}", input); } @@ -134,8 +134,8 @@ fn test_plain_summary_line() { fn test_markdown_html_escape() { fn t(input: &str, expect: &str) { let mut idmap = IdMap::new(); - let output = MarkdownHtml(input, RefCell::new(&mut idmap), - ErrorCodes::Yes, DEFAULT_EDITION).to_string(); + let output = MarkdownHtml(input, &mut idmap, + ErrorCodes::Yes, DEFAULT_EDITION, &None).to_string(); assert_eq!(output, expect, "original: {}", input); } diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index fc4e25e3d7ff6..ea97cea942820 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -2595,7 +2595,7 @@ fn render_markdown(w: &mut fmt::Formatter<'_>, write!(w, "
{}{}
", if is_hidden { " hidden" } else { "" }, prefix, - Markdown(md_text, &links, RefCell::new(&mut ids), + Markdown(md_text, &links, &mut ids, cx.codes, cx.edition, &cx.playground).to_string()) } @@ -2961,8 +2961,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { if let Some(note) = note { let mut ids = cx.id_map.borrow_mut(); - let html = MarkdownHtml( - ¬e, RefCell::new(&mut ids), error_codes, cx.edition, &cx.playground); + let html = MarkdownHtml(¬e, &mut ids, error_codes, cx.edition, &cx.playground); message.push_str(&format!(": {}", html.to_string())); } stability.push(format!("
{}
", message)); @@ -3013,7 +3012,7 @@ fn short_stability(item: &clean::Item, cx: &Context) -> Vec { message, MarkdownHtml( &unstable_reason, - RefCell::new(&mut ids), + &mut ids, error_codes, cx.edition, &cx.playground, @@ -4247,7 +4246,7 @@ fn render_impl(w: &mut fmt::Formatter<'_>, cx: &Context, i: &Impl, link: AssocIt if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) { let mut ids = cx.id_map.borrow_mut(); write!(w, "
{}
", - Markdown(&*dox, &i.impl_item.links(), RefCell::new(&mut ids), + Markdown(&*dox, &i.impl_item.links(), &mut ids, cx.codes, cx.edition, &cx.playground).to_string())?; } } diff --git a/src/librustdoc/markdown.rs b/src/librustdoc/markdown.rs index b7dd6c30f09f9..eaaae3261c728 100644 --- a/src/librustdoc/markdown.rs +++ b/src/librustdoc/markdown.rs @@ -1,7 +1,6 @@ use std::fs::File; use std::io::prelude::*; use std::path::PathBuf; -use std::cell::RefCell; use errors; use testing; @@ -83,9 +82,9 @@ pub fn render( let mut ids = IdMap::new(); let error_codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); let text = if !options.markdown_no_toc { - MarkdownWithToc(text, RefCell::new(&mut ids), error_codes, edition, &playground).to_string() + MarkdownWithToc(text, &mut ids, error_codes, edition, &playground).to_string() } else { - Markdown(text, &[], RefCell::new(&mut ids), error_codes, edition, &playground).to_string() + Markdown(text, &[], &mut ids, error_codes, edition, &playground).to_string() }; let err = write!( diff --git a/src/tools/error_index_generator/main.rs b/src/tools/error_index_generator/main.rs index 89b545fb7e413..a9d1d9997f6ef 100644 --- a/src/tools/error_index_generator/main.rs +++ b/src/tools/error_index_generator/main.rs @@ -100,7 +100,7 @@ impl Formatter for HTMLFormatter { url: String::from("https://play.rust-lang.org/"), }; write!(output, "{}", - Markdown(desc, &[], RefCell::new(&mut id_map), + Markdown(desc, &[], &mut id_map, ErrorCodes::Yes, DEFAULT_EDITION, &Some(playground)).to_string())? }, None => write!(output, "

No description.

\n")?, From 3b8a24d193a3b2d9e7a888338d0c2bb478892ec2 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 10 Aug 2019 18:39:50 -0400 Subject: [PATCH 30/32] Reduce nesting in externalfiles implementation Utilize `?` instead of and_then/map operators --- src/librustdoc/externalfiles.rs | 42 +++++++++++---------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs index 8254bc800ca46..56f1191feed0b 100644 --- a/src/librustdoc/externalfiles.rs +++ b/src/librustdoc/externalfiles.rs @@ -25,34 +25,20 @@ impl ExternalHtml { id_map: &mut IdMap, edition: Edition, playground: &Option) -> Option { let codes = ErrorCodes::from(UnstableFeatures::from_environment().is_nightly_build()); - load_external_files(in_header, diag) - .and_then(|ih| - load_external_files(before_content, diag) - .map(|bc| (ih, bc)) - ) - .and_then(|(ih, bc)| - load_external_files(md_before_content, diag) - .map(|m_bc| (ih, - format!("{}{}", bc, Markdown(&m_bc, &[], id_map, - codes, edition, playground).to_string()))) - ) - .and_then(|(ih, bc)| - load_external_files(after_content, diag) - .map(|ac| (ih, bc, ac)) - ) - .and_then(|(ih, bc, ac)| - load_external_files(md_after_content, diag) - .map(|m_ac| (ih, bc, - format!("{}{}", ac, Markdown(&m_ac, &[], id_map, - codes, edition, playground).to_string()))) - ) - .map(|(ih, bc, ac)| - ExternalHtml { - in_header: ih, - before_content: bc, - after_content: ac, - } - ) + let ih = load_external_files(in_header, diag)?; + let bc = load_external_files(before_content, diag)?; + let m_bc = load_external_files(md_before_content, diag)?; + let bc = format!("{}{}", bc, Markdown(&m_bc, &[], id_map, + codes, edition, playground).to_string()); + let ac = load_external_files(after_content, diag)?; + let m_ac = load_external_files(md_after_content, diag)?; + let ac = format!("{}{}", ac, Markdown(&m_ac, &[], id_map, + codes, edition, playground).to_string()); + Some(ExternalHtml { + in_header: ih, + before_content: bc, + after_content: ac, + }) } } From b204232fd35ff021b4c71fb4d307731014e0345b Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sun, 11 Aug 2019 16:51:36 +0200 Subject: [PATCH 31/32] Derive Debug for NativeLibrary and NativeLibraryKind --- src/librustc/middle/cstore.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index 5a580dfa420b3..d37b2367ae77e 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -87,7 +87,7 @@ pub enum LinkagePreference { RequireStatic, } -#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, +#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable, HashStable)] pub enum NativeLibraryKind { /// native static library (.a archive) @@ -100,7 +100,7 @@ pub enum NativeLibraryKind { NativeUnknown, } -#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)] +#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)] pub struct NativeLibrary { pub kind: NativeLibraryKind, pub name: Option, From 43de341f19e702f6efa2fe30c07dd5099b1a8efb Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sun, 11 Aug 2019 10:54:38 -0400 Subject: [PATCH 32/32] Copy ty::Instance instead of passing by reference ty::Instance is small and Copy, we should not be adding additional indirection. --- src/librustc/ty/layout.rs | 4 ++-- src/librustc_codegen_ssa/mir/block.rs | 6 +++--- src/librustc_mir/interpret/snapshot.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index a9d1fd1fffc92..19c753bc30436 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -2518,7 +2518,7 @@ where + HasTyCtxt<'tcx> + HasParamEnv<'tcx>, { - fn of_instance(cx: &C, instance: &ty::Instance<'tcx>) -> Self; + fn of_instance(cx: &C, instance: ty::Instance<'tcx>) -> Self; fn new(cx: &C, sig: ty::FnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self; fn new_vtable(cx: &C, sig: ty::FnSig<'tcx>, extra_args: &[Ty<'tcx>]) -> Self; fn new_internal( @@ -2538,7 +2538,7 @@ where + HasTyCtxt<'tcx> + HasParamEnv<'tcx>, { - fn of_instance(cx: &C, instance: &ty::Instance<'tcx>) -> Self { + fn of_instance(cx: &C, instance: ty::Instance<'tcx>) -> Self { let sig = instance.fn_sig(cx.tcx()); let sig = cx .tcx() diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 006ebcbdec672..ce98979cc0c64 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -337,7 +337,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { } _ => { (bx.get_fn(drop_fn), - FnType::of_instance(&bx, &drop_fn)) + FnType::of_instance(&bx, drop_fn)) } }; helper.do_call(self, &mut bx, fn_ty, drop_fn, args, @@ -435,7 +435,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Obtain the panic entry point. let def_id = common::langcall(bx.tcx(), Some(span), "", lang_item); let instance = ty::Instance::mono(bx.tcx(), def_id); - let fn_ty = FnType::of_instance(&bx, &instance); + let fn_ty = FnType::of_instance(&bx, instance); let llfn = bx.get_fn(instance); // Codegen the actual panic invoke/call. @@ -552,7 +552,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let def_id = common::langcall(bx.tcx(), Some(span), "", lang_items::PanicFnLangItem); let instance = ty::Instance::mono(bx.tcx(), def_id); - let fn_ty = FnType::of_instance(&bx, &instance); + let fn_ty = FnType::of_instance(&bx, instance); let llfn = bx.get_fn(instance); // Codegen the actual panic invoke/call. diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index fad9fafbb0803..47289064f4d0d 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/src/librustc_mir/interpret/snapshot.rs @@ -304,7 +304,7 @@ impl_stable_hash_for!(enum crate::interpret::eval_context::StackPopCleanup { #[derive(Eq, PartialEq)] struct FrameSnapshot<'a, 'tcx> { - instance: &'a ty::Instance<'tcx>, + instance: ty::Instance<'tcx>, span: Span, return_to_block: &'a StackPopCleanup, return_place: Option>>, @@ -344,7 +344,7 @@ impl<'a, 'mir, 'tcx, Ctx> Snapshot<'a, Ctx> for &'a Frame<'mir, 'tcx> } = self; FrameSnapshot { - instance, + instance: *instance, span: *span, return_to_block, block,