From 57b1e7a428c16a1b692b88cc25ce8b09d8ea503b Mon Sep 17 00:00:00 2001 From: DutchGhost Date: Mon, 16 Mar 2020 10:51:00 +0100 Subject: [PATCH 01/41] Remove the call that makes miri fail --- src/libcore/tests/mem.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libcore/tests/mem.rs b/src/libcore/tests/mem.rs index 8337ab103419f..2329bf97ff5bc 100644 --- a/src/libcore/tests/mem.rs +++ b/src/libcore/tests/mem.rs @@ -142,8 +142,4 @@ fn test_const_forget() { const fn const_forget_box(x: Box) { forget(x); } - - // Call the forget_box at runtime, - // as we can't const-construct a box yet. - const_forget_box(Box::new(0i32)); } From dcc23217b7364056d5a83aaa058993ccd83b6e78 Mon Sep 17 00:00:00 2001 From: DutchGhost Date: Mon, 16 Mar 2020 13:24:59 +0100 Subject: [PATCH 02/41] The const_forget_box was unused, and doesns't add anything to test by itself. --- src/libcore/tests/mem.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/libcore/tests/mem.rs b/src/libcore/tests/mem.rs index 2329bf97ff5bc..1502c9ac3753b 100644 --- a/src/libcore/tests/mem.rs +++ b/src/libcore/tests/mem.rs @@ -134,12 +134,4 @@ fn test_discriminant_send_sync() { fn test_const_forget() { const _: () = forget(0i32); const _: () = forget(Vec::>>::new()); - - // Writing this function signature without const-forget - // triggers compiler errors: - // 1) That we use a non-const fn inside a const fn - // 2) without the forget, it complains about the destructor of Box - const fn const_forget_box(x: Box) { - forget(x); - } } From 0760803c06ff4128405ea22e390625938a15bcb2 Mon Sep 17 00:00:00 2001 From: DutchGhost Date: Mon, 16 Mar 2020 14:45:37 +0100 Subject: [PATCH 03/41] rather than removing const_forget_box, stick an attribute on it and explain it cant be called in ctfe yet --- src/libcore/tests/mem.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libcore/tests/mem.rs b/src/libcore/tests/mem.rs index 1502c9ac3753b..ab6f08fb3ada4 100644 --- a/src/libcore/tests/mem.rs +++ b/src/libcore/tests/mem.rs @@ -134,4 +134,16 @@ fn test_discriminant_send_sync() { fn test_const_forget() { const _: () = forget(0i32); const _: () = forget(Vec::>>::new()); + + // Writing this function signature without const-forget + // triggers compiler errors: + // 1) That we use a non-const fn inside a const fn + // 2) without the forget, it complains about the destructor of Box + // + // FIXME: this method cannot be called in const-eval yet, as Box isn't + // const constructable + #[allow(unused)] + const fn const_forget_box(x: Box) { + forget(x); + } } From 1a764a7ef59b9cb2eb31658625a6a7dacc3d819b Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 10 Feb 2020 17:21:50 +0100 Subject: [PATCH 04/41] Add futures scaffolding to libcore --- src/libcore/future/mod.rs | 79 +++++++++++++++++++++++++++++++++++++++ src/libstd/future.rs | 25 +++++++++---- 2 files changed, 97 insertions(+), 7 deletions(-) diff --git a/src/libcore/future/mod.rs b/src/libcore/future/mod.rs index 89ea4713cfdaa..6150f34d8446d 100644 --- a/src/libcore/future/mod.rs +++ b/src/libcore/future/mod.rs @@ -2,6 +2,85 @@ //! Asynchronous values. +#[cfg(not(bootstrap))] +use crate::{ + ops::{Generator, GeneratorState}, + pin::Pin, + ptr::NonNull, + task::{Context, Poll}, +}; + mod future; #[stable(feature = "futures_api", since = "1.36.0")] pub use self::future::Future; + +/// This type is needed because: +/// +/// a) Generators cannot implement `for<'a, 'b> Generator<&'a mut Context<'b>>`, so we need to pass +/// a raw pointer. +/// b) Raw pointers and `NonNull` aren't `Send` or `Sync`, so that would make every single future +/// non-Send/Sync as well, and we don't want that. +/// +/// It also simplifies the HIR lowering of `.await`. +#[doc(hidden)] +#[unstable(feature = "gen_future", issue = "50547")] +#[cfg(not(bootstrap))] +#[derive(Debug, Copy, Clone)] +pub struct ResumeTy(pub NonNull>); + +#[unstable(feature = "gen_future", issue = "50547")] +#[cfg(not(bootstrap))] +unsafe impl Send for ResumeTy {} + +#[unstable(feature = "gen_future", issue = "50547")] +#[cfg(not(bootstrap))] +unsafe impl Sync for ResumeTy {} + +/// Wrap a generator in a future. +/// +/// This function returns a `GenFuture` underneath, but hides it in `impl Trait` to give +/// better error messages (`impl Future` rather than `GenFuture<[closure.....]>`). +// This is `const` to avoid extra errors after we recover from `const async fn` +#[doc(hidden)] +#[unstable(feature = "gen_future", issue = "50547")] +#[cfg(not(bootstrap))] +#[inline] +pub const fn from_generator(gen: T) -> impl Future +where + T: Generator +{ + #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] + struct GenFuture>(T); + + // We rely on the fact that async/await futures are immovable in order to create + // self-referential borrows in the underlying generator. + impl> !Unpin for GenFuture {} + + impl> Future for GenFuture { + type Output = T::Return; + fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + // Safety: Safe because we're !Unpin + !Drop mapping to a ?Unpin value + let gen = unsafe { Pin::map_unchecked_mut(self, |s| &mut s.0) }; + + // Resume the generator, turning the `&mut Context` into a `NonNull` raw pointer. The + // `.await` lowering will safely cast that back to a `&mut Context`. + match gen.resume(ResumeTy(NonNull::from(cx).cast::>())) { + GeneratorState::Yielded(()) => Poll::Pending, + GeneratorState::Complete(x) => Poll::Ready(x), + } + } + } + + GenFuture(gen) +} + +#[doc(hidden)] +#[unstable(feature = "gen_future", issue = "50547")] +#[cfg(not(bootstrap))] +#[inline] +pub unsafe fn poll_with_context(f: Pin<&mut F>, mut cx: ResumeTy) -> Poll +where + F: Future, +{ + F::poll(f, cx.0.as_mut()) +} diff --git a/src/libstd/future.rs b/src/libstd/future.rs index c1ca6771326cb..c0675eeba98da 100644 --- a/src/libstd/future.rs +++ b/src/libstd/future.rs @@ -1,12 +1,14 @@ //! Asynchronous values. -use core::cell::Cell; -use core::marker::Unpin; -use core::ops::{Drop, Generator, GeneratorState}; -use core::option::Option; -use core::pin::Pin; -use core::ptr::NonNull; -use core::task::{Context, Poll}; +#[cfg(bootstrap)] +use core::{ + cell::Cell, + marker::Unpin, + ops::{Drop, Generator, GeneratorState}, + pin::Pin, + ptr::NonNull, + task::{Context, Poll}, +}; #[doc(inline)] #[stable(feature = "futures_api", since = "1.36.0")] @@ -17,6 +19,7 @@ pub use core::future::*; /// This function returns a `GenFuture` underneath, but hides it in `impl Trait` to give /// better error messages (`impl Future` rather than `GenFuture<[closure.....]>`). // This is `const` to avoid extra errors after we recover from `const async fn` +#[cfg(bootstrap)] #[doc(hidden)] #[unstable(feature = "gen_future", issue = "50547")] pub const fn from_generator>(x: T) -> impl Future { @@ -24,6 +27,7 @@ pub const fn from_generator>(x: T) -> impl Future>(T); // We rely on the fact that async/await futures are immovable in order to create // self-referential borrows in the underlying generator. +#[cfg(bootstrap)] impl> !Unpin for GenFuture {} +#[cfg(bootstrap)] #[doc(hidden)] #[unstable(feature = "gen_future", issue = "50547")] impl> Future for GenFuture { @@ -48,12 +54,15 @@ impl> Future for GenFuture { } } +#[cfg(bootstrap)] thread_local! { static TLS_CX: Cell>>> = Cell::new(None); } +#[cfg(bootstrap)] struct SetOnDrop(Option>>); +#[cfg(bootstrap)] impl Drop for SetOnDrop { fn drop(&mut self) { TLS_CX.with(|tls_cx| { @@ -64,6 +73,7 @@ impl Drop for SetOnDrop { // Safety: the returned guard must drop before `cx` is dropped and before // any previous guard is dropped. +#[cfg(bootstrap)] unsafe fn set_task_context(cx: &mut Context<'_>) -> SetOnDrop { // transmute the context's lifetime to 'static so we can store it. let cx = core::mem::transmute::<&mut Context<'_>, &mut Context<'static>>(cx); @@ -71,6 +81,7 @@ unsafe fn set_task_context(cx: &mut Context<'_>) -> SetOnDrop { SetOnDrop(old_cx) } +#[cfg(bootstrap)] #[doc(hidden)] #[unstable(feature = "gen_future", issue = "50547")] /// Polls a future in the current thread-local task waker. From 18adc45a26127be7c38c81b8c6e1caaab3c8132e Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 10 Feb 2020 17:22:29 +0100 Subject: [PATCH 05/41] Improve debug log in MIR type check --- src/librustc_mir/borrow_check/type_check/input_output.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/borrow_check/type_check/input_output.rs b/src/librustc_mir/borrow_check/type_check/input_output.rs index 37cf77b7095c6..f2194c77c8991 100644 --- a/src/librustc_mir/borrow_check/type_check/input_output.rs +++ b/src/librustc_mir/borrow_check/type_check/input_output.rs @@ -64,13 +64,16 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { } }; + debug!( + "equate_inputs_and_outputs: normalized_input_tys = {:?}, local_decls = {:?}", + normalized_input_tys, body.local_decls + ); + // Equate expected input tys with those in the MIR. for (&normalized_input_ty, argument_index) in normalized_input_tys.iter().zip(0..) { // In MIR, argument N is stored in local N+1. let local = Local::new(argument_index + 1); - debug!("equate_inputs_and_outputs: normalized_input_ty = {:?}", normalized_input_ty); - let mir_input_ty = body.local_decls[local].ty; let mir_input_span = body.local_decls[local].source_info.span; self.equate_normalized_input_or_output( From dfcfa170f57695cd1169ae9dc65c0ed629e1a2a3 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 10 Feb 2020 17:23:09 +0100 Subject: [PATCH 06/41] Make async/await lowering use resume arguments --- src/librustc_ast_lowering/expr.rs | 95 +++++++++++++++---- src/librustc_ast_lowering/item.rs | 2 +- src/librustc_ast_lowering/lib.rs | 5 + src/librustc_span/symbol.rs | 3 +- .../async-await/issues/issue-62009-1.stderr | 4 +- 5 files changed, 86 insertions(+), 23 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index a4cbae5196635..104634f4fa0bb 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -10,6 +10,7 @@ use rustc_hir as hir; use rustc_hir::def::Res; use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned}; use rustc_span::symbol::{sym, Symbol}; +use rustc_span::DUMMY_SP; impl<'hir> LoweringContext<'_, 'hir> { fn lower_exprs(&mut self, exprs: &[AstP]) -> &'hir [hir::Expr<'hir>] { @@ -483,14 +484,44 @@ impl<'hir> LoweringContext<'_, 'hir> { Some(ty) => FnRetTy::Ty(ty), None => FnRetTy::Default(span), }; - let ast_decl = FnDecl { inputs: vec![], output }; + + let task_context_id = self.resolver.next_node_id(); + let task_context_hid = self.lower_node_id(task_context_id); + let ast_decl = FnDecl { + inputs: vec![Param { + attrs: AttrVec::new(), + ty: AstP(Ty { + id: self.resolver.next_node_id(), + kind: TyKind::Infer, + span: DUMMY_SP, + }), + pat: AstP(Pat { + id: task_context_id, + kind: PatKind::Ident( + BindingMode::ByValue(Mutability::Mut), + Ident::with_dummy_span(sym::_task_context), + None, + ), + span: DUMMY_SP, + }), + id: self.resolver.next_node_id(), + span: DUMMY_SP, + is_placeholder: false, + }], + output, + }; let decl = self.lower_fn_decl(&ast_decl, None, /* impl trait allowed */ false, None); let body_id = self.lower_fn_body(&ast_decl, |this| { this.generator_kind = Some(hir::GeneratorKind::Async(async_gen_kind)); - body(this) + + let old_ctx = this.task_context; + this.task_context = Some(task_context_hid); + let res = body(this); + this.task_context = old_ctx; + res }); - // `static || -> { body }`: + // `static |task_context| -> { body }`: let generator_kind = hir::ExprKind::Closure( capture_clause, decl, @@ -523,9 +554,10 @@ impl<'hir> LoweringContext<'_, 'hir> { /// ```rust /// match { /// mut pinned => loop { - /// match ::std::future::poll_with_tls_context(unsafe { - /// <::std::pin::Pin>::new_unchecked(&mut pinned) - /// }) { + /// match unsafe { ::std::future::poll_with_context( + /// <::std::pin::Pin>::new_unchecked(&mut pinned), + /// task_context, + /// ) } { /// ::std::task::Poll::Ready(result) => break result, /// ::std::task::Poll::Pending => {} /// } @@ -561,12 +593,23 @@ impl<'hir> LoweringContext<'_, 'hir> { let (pinned_pat, pinned_pat_hid) = self.pat_ident_binding_mode(span, pinned_ident, hir::BindingAnnotation::Mutable); - // ::std::future::poll_with_tls_context(unsafe { - // ::std::pin::Pin::new_unchecked(&mut pinned) - // })` + let task_context_ident = Ident::with_dummy_span(sym::_task_context); + + // unsafe { + // ::std::future::poll_with_context( + // ::std::pin::Pin::new_unchecked(&mut pinned), + // task_context, + // ) + // } let poll_expr = { let pinned = self.expr_ident(span, pinned_ident, pinned_pat_hid); let ref_mut_pinned = self.expr_mut_addr_of(span, pinned); + let task_context = if let Some(task_context_hid) = self.task_context { + self.expr_ident_mut(span, task_context_ident, task_context_hid) + } else { + // Use of `await` outside of an async context, we cannot use `task_context` here. + self.expr_err(span) + }; let pin_ty_id = self.next_id(); let new_unchecked_expr_kind = self.expr_call_std_assoc_fn( pin_ty_id, @@ -575,14 +618,13 @@ impl<'hir> LoweringContext<'_, 'hir> { "new_unchecked", arena_vec![self; ref_mut_pinned], ); - let new_unchecked = - self.arena.alloc(self.expr(span, new_unchecked_expr_kind, ThinVec::new())); - let unsafe_expr = self.expr_unsafe(new_unchecked); - self.expr_call_std_path( + let new_unchecked = self.expr(span, new_unchecked_expr_kind, ThinVec::new()); + let call = self.expr_call_std_path( gen_future_span, - &[sym::future, sym::poll_with_tls_context], - arena_vec![self; unsafe_expr], - ) + &[sym::future, sym::poll_with_context], + arena_vec![self; new_unchecked, task_context], + ); + self.arena.alloc(self.expr_unsafe(call)) }; // `::std::task::Poll::Ready(result) => break result` @@ -629,12 +671,27 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Yield(unit, hir::YieldSource::Await), ThinVec::new(), ); - self.stmt_expr(span, yield_expr) + let yield_expr = self.arena.alloc(yield_expr); + + if let Some(task_context_hid) = self.task_context { + let lhs = self.expr_ident(span, task_context_ident, task_context_hid); + let assign = self.expr( + span, + hir::ExprKind::Assign(lhs, yield_expr, span), + AttrVec::new(), + ); + self.stmt_expr(span, assign) + } else { + // Use of `await` outside of an async context. Return `yield_expr` so that we can + // proceed with type checking. + self.stmt(span, hir::StmtKind::Semi(yield_expr)) + } }; - let loop_block = self.block_all(span, arena_vec![self; inner_match_stmt, yield_stmt], None); + let loop_block = + self.block_all(span, arena_vec![self; inner_match_stmt, yield_stmt], None); - // loop { .. } + // loop { ...; task_context = yield (); } let loop_expr = self.arena.alloc(hir::Expr { hir_id: loop_hir_id, kind: hir::ExprKind::Loop(loop_block, None, hir::LoopSource::Loop), diff --git a/src/librustc_ast_lowering/item.rs b/src/librustc_ast_lowering/item.rs index aab6aa7c35b0e..b1fd3da99ce01 100644 --- a/src/librustc_ast_lowering/item.rs +++ b/src/librustc_ast_lowering/item.rs @@ -809,7 +809,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } /// Construct `ExprKind::Err` for the given `span`. - fn expr_err(&mut self, span: Span) -> hir::Expr<'hir> { + crate fn expr_err(&mut self, span: Span) -> hir::Expr<'hir> { self.expr(span, hir::ExprKind::Err, AttrVec::new()) } diff --git a/src/librustc_ast_lowering/lib.rs b/src/librustc_ast_lowering/lib.rs index 32b0f0db3589d..3c1a82febdae4 100644 --- a/src/librustc_ast_lowering/lib.rs +++ b/src/librustc_ast_lowering/lib.rs @@ -117,6 +117,10 @@ struct LoweringContext<'a, 'hir: 'a> { generator_kind: Option, + /// When inside an `async` context, this is the `HirId` of the + /// `task_context` local bound to the resume argument of the generator. + task_context: Option, + /// Used to get the current `fn`'s def span to point to when using `await` /// outside of an `async fn`. current_item: Option, @@ -295,6 +299,7 @@ pub fn lower_crate<'a, 'hir>( item_local_id_counters: Default::default(), node_id_to_hir_id: IndexVec::new(), generator_kind: None, + task_context: None, current_item: None, lifetimes_to_define: Vec::new(), is_collecting_in_band_lifetimes: false, diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index 19754c83038e2..e7adbaa03eba4 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -544,7 +544,7 @@ symbols! { plugin_registrar, plugins, Poll, - poll_with_tls_context, + poll_with_context, powerpc_target_feature, precise_pointer_size_matching, pref_align_of, @@ -720,6 +720,7 @@ symbols! { target_has_atomic_load_store, target_thread_local, task, + _task_context, tbm_target_feature, termination_trait, termination_trait_test, diff --git a/src/test/ui/async-await/issues/issue-62009-1.stderr b/src/test/ui/async-await/issues/issue-62009-1.stderr index cd6670923c2c6..0624c049048c7 100644 --- a/src/test/ui/async-await/issues/issue-62009-1.stderr +++ b/src/test/ui/async-await/issues/issue-62009-1.stderr @@ -33,10 +33,10 @@ error[E0277]: the trait bound `[closure@$DIR/issue-62009-1.rs:16:5: 16:15]: std: LL | (|_| 2333).await; | ^^^^^^^^^^^^^^^^ the trait `std::future::Future` is not implemented for `[closure@$DIR/issue-62009-1.rs:16:5: 16:15]` | - ::: $SRC_DIR/libstd/future.rs:LL:COL + ::: $SRC_DIR/libcore/future/mod.rs:LL:COL | LL | F: Future, - | ------ required by this bound in `std::future::poll_with_tls_context` + | ------ required by this bound in `std::future::poll_with_context` error: aborting due to 4 previous errors From f79a95a65dea5901cb020b41997fb4d4ca806b18 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 10 Feb 2020 17:35:58 +0100 Subject: [PATCH 07/41] Test that async/await compiles with `#![no_std]` --- src/test/ui/async-await/no-std.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 src/test/ui/async-await/no-std.rs diff --git a/src/test/ui/async-await/no-std.rs b/src/test/ui/async-await/no-std.rs new file mode 100644 index 0000000000000..63e93cdff7e77 --- /dev/null +++ b/src/test/ui/async-await/no-std.rs @@ -0,0 +1,13 @@ +// edition:2018 +// check-pass + +#![no_std] +#![crate_type = "rlib"] + +use core::future::Future; + +async fn a(f: impl Future) { + f.await; +} + +fn main() {} From b7fba973cb4b6547f24d89b901f7ac294c269503 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 10 Feb 2020 18:20:22 +0100 Subject: [PATCH 08/41] Format --- src/libcore/future/mod.rs | 2 +- src/librustc_ast_lowering/expr.rs | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/libcore/future/mod.rs b/src/libcore/future/mod.rs index 6150f34d8446d..9dcb2cea2ea71 100644 --- a/src/libcore/future/mod.rs +++ b/src/libcore/future/mod.rs @@ -47,7 +47,7 @@ unsafe impl Sync for ResumeTy {} #[inline] pub const fn from_generator(gen: T) -> impl Future where - T: Generator + T: Generator, { #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] struct GenFuture>(T); diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 104634f4fa0bb..868b31d668c0c 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -675,11 +675,8 @@ impl<'hir> LoweringContext<'_, 'hir> { if let Some(task_context_hid) = self.task_context { let lhs = self.expr_ident(span, task_context_ident, task_context_hid); - let assign = self.expr( - span, - hir::ExprKind::Assign(lhs, yield_expr, span), - AttrVec::new(), - ); + let assign = + self.expr(span, hir::ExprKind::Assign(lhs, yield_expr, span), AttrVec::new()); self.stmt_expr(span, assign) } else { // Use of `await` outside of an async context. Return `yield_expr` so that we can @@ -688,8 +685,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } }; - let loop_block = - self.block_all(span, arena_vec![self; inner_match_stmt, yield_stmt], None); + let loop_block = self.block_all(span, arena_vec![self; inner_match_stmt, yield_stmt], None); // loop { ...; task_context = yield (); } let loop_expr = self.arena.alloc(hir::Expr { From 37b5bfce7612d1a5d8862fbfc9034aca5df33f88 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 10 Feb 2020 18:59:21 +0100 Subject: [PATCH 09/41] Improve comments in HIR lowering code --- src/librustc_ast_lowering/expr.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 868b31d668c0c..93e32e85efc41 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -561,7 +561,7 @@ impl<'hir> LoweringContext<'_, 'hir> { /// ::std::task::Poll::Ready(result) => break result, /// ::std::task::Poll::Pending => {} /// } - /// yield (); + /// task_context = yield (); /// } /// } /// ``` @@ -664,6 +664,7 @@ impl<'hir> LoweringContext<'_, 'hir> { self.stmt_expr(span, match_expr) }; + // task_context = yield (); let yield_stmt = { let unit = self.expr_unit(span); let yield_expr = self.expr( @@ -687,7 +688,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let loop_block = self.block_all(span, arena_vec![self; inner_match_stmt, yield_stmt], None); - // loop { ...; task_context = yield (); } + // loop { .. } let loop_expr = self.arena.alloc(hir::Expr { hir_id: loop_hir_id, kind: hir::ExprKind::Loop(loop_block, None, hir::LoopSource::Loop), From be62aed2025a19aadadbfa09633ef5d20dbfb668 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 11 Feb 2020 11:51:58 +0100 Subject: [PATCH 10/41] Remove useless derives on `GenFuture` Not sure why these were there, I guess because this type used to kind of be part of public API? --- src/libcore/future/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libcore/future/mod.rs b/src/libcore/future/mod.rs index 9dcb2cea2ea71..e808f9d2f1024 100644 --- a/src/libcore/future/mod.rs +++ b/src/libcore/future/mod.rs @@ -49,7 +49,6 @@ pub const fn from_generator(gen: T) -> impl Future where T: Generator, { - #[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)] struct GenFuture>(T); // We rely on the fact that async/await futures are immovable in order to create From 6450101b6947d0ff891475dad8668b2f1224212b Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 11 Feb 2020 12:03:52 +0100 Subject: [PATCH 11/41] Split up large `FnDecl` expression --- src/librustc_ast_lowering/expr.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 93e32e85efc41..f84efe2ba76f5 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -487,23 +487,22 @@ impl<'hir> LoweringContext<'_, 'hir> { let task_context_id = self.resolver.next_node_id(); let task_context_hid = self.lower_node_id(task_context_id); + + let arg_ty = Ty { id: self.resolver.next_node_id(), kind: TyKind::Infer, span: DUMMY_SP }; + let arg_pat = Pat { + id: task_context_id, + kind: PatKind::Ident( + BindingMode::ByValue(Mutability::Mut), + Ident::with_dummy_span(sym::_task_context), + None, + ), + span: DUMMY_SP, + }; let ast_decl = FnDecl { inputs: vec![Param { attrs: AttrVec::new(), - ty: AstP(Ty { - id: self.resolver.next_node_id(), - kind: TyKind::Infer, - span: DUMMY_SP, - }), - pat: AstP(Pat { - id: task_context_id, - kind: PatKind::Ident( - BindingMode::ByValue(Mutability::Mut), - Ident::with_dummy_span(sym::_task_context), - None, - ), - span: DUMMY_SP, - }), + ty: AstP(arg_ty), + pat: AstP(arg_pat), id: self.resolver.next_node_id(), span: DUMMY_SP, is_placeholder: false, From 50b9d772dbca3cd54c772b67abe2eff24e6abac0 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 3 Mar 2020 16:05:11 +0100 Subject: [PATCH 12/41] Make `ResumeTy` contents private --- src/libcore/future/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/future/mod.rs b/src/libcore/future/mod.rs index e808f9d2f1024..517f8c98c4774 100644 --- a/src/libcore/future/mod.rs +++ b/src/libcore/future/mod.rs @@ -26,7 +26,7 @@ pub use self::future::Future; #[unstable(feature = "gen_future", issue = "50547")] #[cfg(not(bootstrap))] #[derive(Debug, Copy, Clone)] -pub struct ResumeTy(pub NonNull>); +pub struct ResumeTy(NonNull>); #[unstable(feature = "gen_future", issue = "50547")] #[cfg(not(bootstrap))] From e419168bb8256444ab98f220c03545b8824037d4 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 3 Mar 2020 17:04:35 +0100 Subject: [PATCH 13/41] Clarify comment --- src/libcore/future/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/future/mod.rs b/src/libcore/future/mod.rs index 517f8c98c4774..6ee8479b34780 100644 --- a/src/libcore/future/mod.rs +++ b/src/libcore/future/mod.rs @@ -58,7 +58,7 @@ where impl> Future for GenFuture { type Output = T::Return; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - // Safety: Safe because we're !Unpin + !Drop mapping to a ?Unpin value + // Safety: Safe because we're !Unpin + !Drop, and this is just a field projection. let gen = unsafe { Pin::map_unchecked_mut(self, |s| &mut s.0) }; // Resume the generator, turning the `&mut Context` into a `NonNull` raw pointer. The From 78274bc17c671655aaadba2096490fad59de2133 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Wed, 4 Mar 2020 14:53:47 +0100 Subject: [PATCH 14/41] Don't create AST fragments when lowering to HIR --- src/librustc_ast_lowering/expr.rs | 62 ++++++++++++++++--------------- src/librustc_ast_lowering/item.rs | 2 +- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index f84efe2ba76f5..a212f0a71077b 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -10,7 +10,6 @@ use rustc_hir as hir; use rustc_hir::def::Res; use rustc_span::source_map::{respan, DesugaringKind, Span, Spanned}; use rustc_span::symbol::{sym, Symbol}; -use rustc_span::DUMMY_SP; impl<'hir> LoweringContext<'_, 'hir> { fn lower_exprs(&mut self, exprs: &[AstP]) -> &'hir [hir::Expr<'hir>] { @@ -471,6 +470,15 @@ impl<'hir> LoweringContext<'_, 'hir> { } } + /// Lower an `async` construct to a generator that is then wrapped so it implements `Future`. + /// + /// This results in: + /// + /// ```text + /// std::future::from_generator(static move? |_task_context| -> { + /// + /// }) + /// ``` pub(super) fn make_async_expr( &mut self, capture_clause: CaptureBy, @@ -481,46 +489,42 @@ impl<'hir> LoweringContext<'_, 'hir> { body: impl FnOnce(&mut Self) -> hir::Expr<'hir>, ) -> hir::ExprKind<'hir> { let output = match ret_ty { - Some(ty) => FnRetTy::Ty(ty), - None => FnRetTy::Default(span), + Some(ty) => hir::FnRetTy::Return(self.lower_ty(&ty, ImplTraitContext::disallowed())), + None => hir::FnRetTy::DefaultReturn(span), }; - let task_context_id = self.resolver.next_node_id(); - let task_context_hid = self.lower_node_id(task_context_id); + // Resume argument type. We let the compiler infer this to simplify the lowering. It is + // fully constrained by `future::from_generator`. + let input_ty = hir::Ty { hir_id: self.next_id(), kind: hir::TyKind::Infer, span }; - let arg_ty = Ty { id: self.resolver.next_node_id(), kind: TyKind::Infer, span: DUMMY_SP }; - let arg_pat = Pat { - id: task_context_id, - kind: PatKind::Ident( - BindingMode::ByValue(Mutability::Mut), - Ident::with_dummy_span(sym::_task_context), - None, - ), - span: DUMMY_SP, - }; - let ast_decl = FnDecl { - inputs: vec![Param { - attrs: AttrVec::new(), - ty: AstP(arg_ty), - pat: AstP(arg_pat), - id: self.resolver.next_node_id(), - span: DUMMY_SP, - is_placeholder: false, - }], + // The closure/generator `FnDecl` takes a single (resume) argument of type `input_ty`. + let decl = self.arena.alloc(hir::FnDecl { + inputs: arena_vec![self; input_ty], output, - }; - let decl = self.lower_fn_decl(&ast_decl, None, /* impl trait allowed */ false, None); - let body_id = self.lower_fn_body(&ast_decl, |this| { + c_variadic: false, + implicit_self: hir::ImplicitSelfKind::None, + }); + + // Lower the argument pattern/ident. The ident is used again in the `.await` lowering. + let (pat, task_context_hid) = self.pat_ident_binding_mode( + span, + Ident::with_dummy_span(sym::_task_context), + hir::BindingAnnotation::Mutable, + ); + let param = hir::Param { attrs: &[], hir_id: self.next_id(), pat, span }; + let params = arena_vec![self; param]; + + let body_id = self.lower_body(move |this| { this.generator_kind = Some(hir::GeneratorKind::Async(async_gen_kind)); let old_ctx = this.task_context; this.task_context = Some(task_context_hid); let res = body(this); this.task_context = old_ctx; - res + (params, res) }); - // `static |task_context| -> { body }`: + // `static |_task_context| -> { body }`: let generator_kind = hir::ExprKind::Closure( capture_clause, decl, diff --git a/src/librustc_ast_lowering/item.rs b/src/librustc_ast_lowering/item.rs index b1fd3da99ce01..5f056ef6a4d86 100644 --- a/src/librustc_ast_lowering/item.rs +++ b/src/librustc_ast_lowering/item.rs @@ -955,7 +955,7 @@ impl<'hir> LoweringContext<'_, 'hir> { id } - fn lower_body( + pub(super) fn lower_body( &mut self, f: impl FnOnce(&mut Self) -> (&'hir [hir::Param<'hir>], hir::Expr<'hir>), ) -> hir::BodyId { From db0126a7c251f4cca39ad4c50527e88c97190992 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 17 Mar 2020 22:19:11 +0100 Subject: [PATCH 15/41] Add issue reference --- src/libcore/future/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/future/mod.rs b/src/libcore/future/mod.rs index 6ee8479b34780..8dfda7a4a3236 100644 --- a/src/libcore/future/mod.rs +++ b/src/libcore/future/mod.rs @@ -17,7 +17,7 @@ pub use self::future::Future; /// This type is needed because: /// /// a) Generators cannot implement `for<'a, 'b> Generator<&'a mut Context<'b>>`, so we need to pass -/// a raw pointer. +/// a raw pointer (see https://github.com/rust-lang/rust/issues/68923). /// b) Raw pointers and `NonNull` aren't `Send` or `Sync`, so that would make every single future /// non-Send/Sync as well, and we don't want that. /// From a5206f9749faf60f6b4163a526de9ad18241503e Mon Sep 17 00:00:00 2001 From: Waffle Date: Fri, 13 Mar 2020 11:51:55 +0300 Subject: [PATCH 16/41] add `Option::{zip,zip_with}` methods under "option_zip" gate This commit introduces 2 methods - `Option::zip` and `Option::zip_with` with respective signatures: - zip: `(Option, Option) -> Option<(T, U)>` - zip_with: `(Option, Option, (T, U) -> R) -> Option` Both are under the feature gate "option_zip". I'm not sure about the name "zip", maybe we can find a better name for this. (I would prefer `union` for example, but this is a keyword :( ) -------------------------------------------------------------------------------- Recently in a russian rust begginers telegram chat a newbie asked (translated): > Are there any methods for these conversions: > > 1. `(Option, Option) -> Option<(A, B)>` > 2. `Vec> -> Option>` > > ? While second (2.) is clearly `vec.into_iter().collect::>()`, the first one isn't that clear. I couldn't find anything similar in the `core` and I've come to this solution: ```rust let tuple: (Option, Option) = ...; let res: Option<(A, B)> = tuple.0.and_then(|a| tuple.1.map(|b| (a, b))); ``` However this solution isn't "nice" (same for just `match`/`if let`), so I thought that this functionality should be in `core`. --- src/libcore/lib.rs | 1 + src/libcore/option.rs | 59 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/src/libcore/lib.rs b/src/libcore/lib.rs index 5a731766054bd..94fc2fd357a06 100644 --- a/src/libcore/lib.rs +++ b/src/libcore/lib.rs @@ -140,6 +140,7 @@ #![feature(associated_type_bounds)] #![feature(const_type_id)] #![feature(const_caller_location)] +#![feature(option_zip)] #![feature(no_niche)] // rust-lang/rust#68303 #[prelude_import] diff --git a/src/libcore/option.rs b/src/libcore/option.rs index 9b32442371c37..5db92a1b35248 100644 --- a/src/libcore/option.rs +++ b/src/libcore/option.rs @@ -913,6 +913,65 @@ impl Option { pub fn replace(&mut self, value: T) -> Option { mem::replace(self, Some(value)) } + + /// Zips `self` with another `Option`. + /// + /// Returns `Some((_, _))` when both `self` and `other` + /// are `Some(_)`, otherwise return `None`. + /// + /// # Examples + /// + /// ``` + /// #![feature(option_zip)] + /// let x = Some(1); + /// let y = Some("hi"); + /// let z = None::; + /// + /// assert_eq!(x.zip(y), Some((1, "hi"))); + /// assert_eq!(x.zip(z), None); + /// ``` + #[inline] + #[unstable(feature = "option_zip", issue = "none")] + pub fn zip(self, other: Option) -> Option<(T, U)> { + self.zip_with(other, |a, b| (a, b)) + } + + /// Zips `self` and another `Option` with function `f`. + /// + /// Returns `Some(_)` when both `self` and `other` + /// are `Some(_)`, otherwise return `None`. + /// + /// # Examples + /// + /// ``` + /// #![feature(option_zip)] + /// + /// #[derive(Debug, PartialEq)] + /// struct Point { + /// x: f64, + /// y: f64, + /// } + /// + /// impl Point { + /// fn new(x: f64, y: f64) -> Self { + /// Self { x, y } + /// } + /// } + /// + /// let x = Some(17.); + /// let y = Some(42.); + /// + /// assert_eq!(x.zip_with(y, Point::new), Some(Point { x: 17., y: 42. })); + /// assert_eq!(x.zip_with(None, Point::new), None); + /// ``` + #[inline] + #[unstable(feature = "option_zip", issue = "none")] + pub fn zip_with(self, other: Option, f: F) -> Option + where + F: FnOnce(T, U) -> R, + { + Some(f(self?, other?)) + } } impl Option<&T> { From d36d3fa5a6441f13c3888b6895cc7046740b1e3d Mon Sep 17 00:00:00 2001 From: Waffle Date: Wed, 18 Mar 2020 11:02:29 +0300 Subject: [PATCH 17/41] fixes to `Option::{zip,zip_with}` - remove `#[inline]` attributes (see https://github.com/rust-lang/rust/pull/69997#discussion_r393942617) - fill tracking issue in `#[unstable]` attributes - slightly improve the docs --- src/libcore/option.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/libcore/option.rs b/src/libcore/option.rs index 5db92a1b35248..4bec3ec9f6b42 100644 --- a/src/libcore/option.rs +++ b/src/libcore/option.rs @@ -916,8 +916,8 @@ impl Option { /// Zips `self` with another `Option`. /// - /// Returns `Some((_, _))` when both `self` and `other` - /// are `Some(_)`, otherwise return `None`. + /// If `self` is `Some(s)` and other is `Some(o)`, this method returns `Some((s, o))`. + /// Otherwise, `None` is returned. /// /// # Examples /// @@ -930,16 +930,15 @@ impl Option { /// assert_eq!(x.zip(y), Some((1, "hi"))); /// assert_eq!(x.zip(z), None); /// ``` - #[inline] - #[unstable(feature = "option_zip", issue = "none")] + #[unstable(feature = "option_zip", issue = "70086")] pub fn zip(self, other: Option) -> Option<(T, U)> { self.zip_with(other, |a, b| (a, b)) } /// Zips `self` and another `Option` with function `f`. /// - /// Returns `Some(_)` when both `self` and `other` - /// are `Some(_)`, otherwise return `None`. + /// If `self` is `Some(s)` and other is `Some(o)`, this method returns `Some(f(s, o))`. + /// Otherwise, `None` is returned. /// /// # Examples /// @@ -958,14 +957,13 @@ impl Option { /// } /// } /// - /// let x = Some(17.); - /// let y = Some(42.); + /// let x = Some(17.5); + /// let y = Some(42.7); /// - /// assert_eq!(x.zip_with(y, Point::new), Some(Point { x: 17., y: 42. })); + /// assert_eq!(x.zip_with(y, Point::new), Some(Point { x: 17.5, y: 42.7 })); /// assert_eq!(x.zip_with(None, Point::new), None); /// ``` - #[inline] - #[unstable(feature = "option_zip", issue = "none")] + #[unstable(feature = "option_zip", issue = "70086")] pub fn zip_with(self, other: Option, f: F) -> Option where F: FnOnce(T, U) -> R, From 4c363e3e8af35c8e45333b522cb0d7b1a284c665 Mon Sep 17 00:00:00 2001 From: DutchGhost Date: Wed, 18 Mar 2020 21:08:52 +0100 Subject: [PATCH 18/41] Move the const-forget test into ui tests --- src/libcore/tests/mem.rs | 18 ------------------ src/test/ui/consts/const_forget.rs | 22 ++++++++++++++++++++++ 2 files changed, 22 insertions(+), 18 deletions(-) create mode 100644 src/test/ui/consts/const_forget.rs diff --git a/src/libcore/tests/mem.rs b/src/libcore/tests/mem.rs index ab6f08fb3ada4..59588d97787b7 100644 --- a/src/libcore/tests/mem.rs +++ b/src/libcore/tests/mem.rs @@ -129,21 +129,3 @@ fn test_discriminant_send_sync() { is_send_sync::>(); is_send_sync::>(); } - -#[test] -fn test_const_forget() { - const _: () = forget(0i32); - const _: () = forget(Vec::>>::new()); - - // Writing this function signature without const-forget - // triggers compiler errors: - // 1) That we use a non-const fn inside a const fn - // 2) without the forget, it complains about the destructor of Box - // - // FIXME: this method cannot be called in const-eval yet, as Box isn't - // const constructable - #[allow(unused)] - const fn const_forget_box(x: Box) { - forget(x); - } -} diff --git a/src/test/ui/consts/const_forget.rs b/src/test/ui/consts/const_forget.rs new file mode 100644 index 0000000000000..5dcad9be54f92 --- /dev/null +++ b/src/test/ui/consts/const_forget.rs @@ -0,0 +1,22 @@ +// run-pass + +#![feature(const_forget)] + +use std::mem::forget; + +const _: () = forget(0i32); +const _: () = forget(Vec::>>::new()); + +// Writing this function signature without const-forget +// triggers compiler errors: +// 1) That we use a non-const fn inside a const fn +// 2) without the forget, it complains about the destructor of Box +// +// FIXME: this method cannot be called in const-eval yet, as Box isn't +// const constructable +#[allow(unused)] +const fn const_forget_box(b: Box) { + forget(b); +} + +fn main() {} From 4a2d54d07d6b7487d3292f6fd68dbf4a4cdf91ff Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 19 Mar 2020 09:47:48 +0100 Subject: [PATCH 19/41] add delay_span_bug to TransmuteSizeDiff, just to be sure --- src/librustc_mir/interpret/place.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 107cfee5aceb5..3d40b39f61c9c 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -926,6 +926,10 @@ where // most likey we *are* running `typeck` right now. Investigate whether we can bail out // on `typeck_tables().has_errors` at all const eval entry points. debug!("Size mismatch when transmuting!\nsrc: {:#?}\ndest: {:#?}", src, dest); + self.tcx.sess.delay_span_bug( + self.tcx.span, + "size-changing transmute, should have been caught by transmute checking", + ); throw_inval!(TransmuteSizeDiff(src.layout.ty, dest.layout.ty)); } // Unsized copies rely on interpreting `src.meta` with `dest.layout`, we want From 121bffce8106a73b90eb4f68da42d4d7ceca5375 Mon Sep 17 00:00:00 2001 From: Waffle Date: Thu, 19 Mar 2020 22:19:37 +0300 Subject: [PATCH 20/41] make "other" in docs of `Option::{zip,zip_with}` monofont --- src/libcore/option.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/option.rs b/src/libcore/option.rs index 4bec3ec9f6b42..3aab8b1b3337c 100644 --- a/src/libcore/option.rs +++ b/src/libcore/option.rs @@ -916,7 +916,7 @@ impl Option { /// Zips `self` with another `Option`. /// - /// If `self` is `Some(s)` and other is `Some(o)`, this method returns `Some((s, o))`. + /// If `self` is `Some(s)` and `other` is `Some(o)`, this method returns `Some((s, o))`. /// Otherwise, `None` is returned. /// /// # Examples @@ -937,7 +937,7 @@ impl Option { /// Zips `self` and another `Option` with function `f`. /// - /// If `self` is `Some(s)` and other is `Some(o)`, this method returns `Some(f(s, o))`. + /// If `self` is `Some(s)` and `other` is `Some(o)`, this method returns `Some(f(s, o))`. /// Otherwise, `None` is returned. /// /// # Examples From bd6deaa08df8335242ca1e9715ded87e361df347 Mon Sep 17 00:00:00 2001 From: CDirkx Date: Thu, 19 Mar 2020 20:45:47 +0100 Subject: [PATCH 21/41] Derive PartialEq, Eq and Hash for RangeInclusive The manual implementation of PartialEq, Eq and Hash for RangeInclusive was functionally equivalent to a derived implementation. This change removes the manual implementation and adds the respective derives. A side effect of this change is that the derives also add implementations for StructuralPartialEq and StructuralEq, which enables RangeInclusive to be used in const generics. --- src/libcore/ops/range.rs | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/src/libcore/ops/range.rs b/src/libcore/ops/range.rs index 8ffad82b69d7c..7fdb6dda1f97e 100644 --- a/src/libcore/ops/range.rs +++ b/src/libcore/ops/range.rs @@ -330,7 +330,7 @@ impl> RangeTo { /// assert_eq!(arr[1..=3], [ 1,2,3 ]); // RangeInclusive /// ``` #[doc(alias = "..=")] -#[derive(Clone)] // not Copy -- see #27186 +#[derive(Clone, PartialEq, Eq, Hash)] // not Copy -- see #27186 #[stable(feature = "inclusive_range", since = "1.26.0")] pub struct RangeInclusive { // Note that the fields here are not public to allow changing the @@ -350,26 +350,6 @@ pub struct RangeInclusive { pub(crate) exhausted: bool, } -#[stable(feature = "inclusive_range", since = "1.26.0")] -impl PartialEq for RangeInclusive { - #[inline] - fn eq(&self, other: &Self) -> bool { - self.start == other.start && self.end == other.end && self.exhausted == other.exhausted - } -} - -#[stable(feature = "inclusive_range", since = "1.26.0")] -impl Eq for RangeInclusive {} - -#[stable(feature = "inclusive_range", since = "1.26.0")] -impl Hash for RangeInclusive { - fn hash(&self, state: &mut H) { - self.start.hash(state); - self.end.hash(state); - self.exhausted.hash(state); - } -} - impl RangeInclusive { /// Creates a new inclusive range. Equivalent to writing `start..=end`. /// From 0f0f254a9c90c4fe47897554948f82e44033f37a Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 19 Mar 2020 11:40:38 +0000 Subject: [PATCH 22/41] Use erased regions in MIR --- .../interpret/intrinsics/caller_location.rs | 2 +- .../transform/cleanup_post_borrowck.rs | 1 + src/librustc_mir/transform/erase_regions.rs | 63 ------------------- src/librustc_mir/transform/generator.rs | 23 +++---- src/librustc_mir/transform/mod.rs | 8 ++- src/librustc_mir/transform/promote_consts.rs | 8 ++- src/librustc_mir_build/build/mod.rs | 27 +++++--- src/test/mir-opt/array-index-is-temporary.rs | 4 +- src/test/mir-opt/byte_slice.rs | 4 +- .../mir-opt/packed-struct-drop-aligned.rs | 4 +- src/test/mir-opt/retag.rs | 29 +++++---- 11 files changed, 63 insertions(+), 110 deletions(-) delete mode 100644 src/librustc_mir/transform/erase_regions.rs diff --git a/src/librustc_mir/interpret/intrinsics/caller_location.rs b/src/librustc_mir/interpret/intrinsics/caller_location.rs index 566601f0cae28..dc2b0e1b983dc 100644 --- a/src/librustc_mir/interpret/intrinsics/caller_location.rs +++ b/src/librustc_mir/interpret/intrinsics/caller_location.rs @@ -39,7 +39,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let loc_ty = self .tcx .type_of(self.tcx.require_lang_item(PanicLocationLangItem, None)) - .subst(*self.tcx, self.tcx.mk_substs([self.tcx.lifetimes.re_static.into()].iter())); + .subst(*self.tcx, self.tcx.mk_substs([self.tcx.lifetimes.re_erased.into()].iter())); let loc_layout = self.layout_of(loc_ty).unwrap(); let location = self.allocate(loc_layout, MemoryKind::CallerLocation); diff --git a/src/librustc_mir/transform/cleanup_post_borrowck.rs b/src/librustc_mir/transform/cleanup_post_borrowck.rs index 5288b6b370ddb..3d219ac2c01ec 100644 --- a/src/librustc_mir/transform/cleanup_post_borrowck.rs +++ b/src/librustc_mir/transform/cleanup_post_borrowck.rs @@ -32,6 +32,7 @@ impl<'tcx> MirPass<'tcx> for CleanupNonCodegenStatements { fn run_pass(&self, tcx: TyCtxt<'tcx>, _source: MirSource<'tcx>, body: &mut BodyAndCache<'tcx>) { let mut delete = DeleteNonCodegenStatements { tcx }; delete.visit_body(body); + body.user_type_annotations.raw.clear(); } } diff --git a/src/librustc_mir/transform/erase_regions.rs b/src/librustc_mir/transform/erase_regions.rs deleted file mode 100644 index 996b97c03b14e..0000000000000 --- a/src/librustc_mir/transform/erase_regions.rs +++ /dev/null @@ -1,63 +0,0 @@ -//! This pass erases all early-bound regions from the types occurring in the MIR. -//! We want to do this once just before codegen, so codegen does not have to take -//! care erasing regions all over the place. -//! N.B., we do _not_ erase regions of statements that are relevant for -//! "types-as-contracts"-validation, namely, `AcquireValid` and `ReleaseValid`. - -use crate::transform::{MirPass, MirSource}; -use rustc::mir::visit::{MutVisitor, TyContext}; -use rustc::mir::*; -use rustc::ty::subst::SubstsRef; -use rustc::ty::{self, Ty, TyCtxt}; - -struct EraseRegionsVisitor<'tcx> { - tcx: TyCtxt<'tcx>, -} - -impl EraseRegionsVisitor<'tcx> { - pub fn new(tcx: TyCtxt<'tcx>) -> Self { - EraseRegionsVisitor { tcx } - } -} - -impl MutVisitor<'tcx> for EraseRegionsVisitor<'tcx> { - fn tcx(&self) -> TyCtxt<'tcx> { - self.tcx - } - - fn visit_ty(&mut self, ty: &mut Ty<'tcx>, _: TyContext) { - *ty = self.tcx.erase_regions(ty); - } - - fn visit_region(&mut self, region: &mut ty::Region<'tcx>, _: Location) { - *region = self.tcx.lifetimes.re_erased; - } - - fn visit_const(&mut self, constant: &mut &'tcx ty::Const<'tcx>, _: Location) { - *constant = self.tcx.erase_regions(constant); - } - - fn visit_substs(&mut self, substs: &mut SubstsRef<'tcx>, _: Location) { - *substs = self.tcx.erase_regions(substs); - } - - fn process_projection_elem(&mut self, elem: &PlaceElem<'tcx>) -> Option> { - if let PlaceElem::Field(field, ty) = elem { - let new_ty = self.tcx.erase_regions(ty); - - if new_ty != *ty { - return Some(PlaceElem::Field(*field, new_ty)); - } - } - - None - } -} - -pub struct EraseRegions; - -impl<'tcx> MirPass<'tcx> for EraseRegions { - fn run_pass(&self, tcx: TyCtxt<'tcx>, _: MirSource<'tcx>, body: &mut BodyAndCache<'tcx>) { - EraseRegionsVisitor::new(tcx).visit_body(body); - } -} diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index b2906739ff1b1..82c5ac689b568 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -357,18 +357,11 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { } } -fn make_generator_state_argument_indirect<'tcx>( - tcx: TyCtxt<'tcx>, - def_id: DefId, - body: &mut BodyAndCache<'tcx>, -) { +fn make_generator_state_argument_indirect<'tcx>(tcx: TyCtxt<'tcx>, body: &mut BodyAndCache<'tcx>) { let gen_ty = body.local_decls.raw[1].ty; - let region = ty::ReFree(ty::FreeRegion { scope: def_id, bound_region: ty::BoundRegion::BrEnv }); - - let region = tcx.mk_region(region); - - let ref_gen_ty = tcx.mk_ref(region, ty::TypeAndMut { ty: gen_ty, mutbl: hir::Mutability::Mut }); + let ref_gen_ty = + tcx.mk_ref(tcx.lifetimes.re_erased, ty::TypeAndMut { ty: gen_ty, mutbl: Mutability::Mut }); // Replace the by value generator argument body.local_decls.raw[1].ty = ref_gen_ty; @@ -874,7 +867,6 @@ fn elaborate_generator_drops<'tcx>( fn create_generator_drop_shim<'tcx>( tcx: TyCtxt<'tcx>, transform: &TransformVisitor<'tcx>, - def_id: DefId, source: MirSource<'tcx>, gen_ty: Ty<'tcx>, body: &mut BodyAndCache<'tcx>, @@ -912,7 +904,7 @@ fn create_generator_drop_shim<'tcx>( local_info: LocalInfo::Other, }; - make_generator_state_argument_indirect(tcx, def_id, &mut body); + make_generator_state_argument_indirect(tcx, &mut body); // Change the generator argument from &mut to *mut body.local_decls[SELF_ARG] = LocalDecl { @@ -1047,7 +1039,6 @@ fn can_unwind<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool { fn create_generator_resume_function<'tcx>( tcx: TyCtxt<'tcx>, transform: TransformVisitor<'tcx>, - def_id: DefId, source: MirSource<'tcx>, body: &mut BodyAndCache<'tcx>, can_return: bool, @@ -1112,7 +1103,7 @@ fn create_generator_resume_function<'tcx>( insert_switch(body, cases, &transform, TerminatorKind::Unreachable); - make_generator_state_argument_indirect(tcx, def_id, body); + make_generator_state_argument_indirect(tcx, body); make_generator_state_argument_pinned(tcx, body); no_landing_pads(tcx, body); @@ -1332,11 +1323,11 @@ impl<'tcx> MirPass<'tcx> for StateTransform { // Create a copy of our MIR and use it to create the drop shim for the generator let drop_shim = - create_generator_drop_shim(tcx, &transform, def_id, source, gen_ty, body, drop_clean); + create_generator_drop_shim(tcx, &transform, source, gen_ty, body, drop_clean); body.generator_drop = Some(box drop_shim); // Create the Generator::resume function - create_generator_resume_function(tcx, transform, def_id, source, body, can_return); + create_generator_resume_function(tcx, transform, source, body, can_return); } } diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 3eb9d23a32a25..50868434baa32 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -22,7 +22,6 @@ pub mod copy_prop; pub mod deaggregator; pub mod dump_mir; pub mod elaborate_drops; -pub mod erase_regions; pub mod generator; pub mod inline; pub mod instcombine; @@ -296,8 +295,6 @@ fn run_optimization_passes<'tcx>( &simplify::SimplifyCfg::new("elaborate-drops"), // No lifetime analysis based on borrowing can be done from here on out. - // From here on out, regions are gone. - &erase_regions::EraseRegions, // Optimizations begin. &unreachable_prop::UnreachablePropagation, &uninhabited_enum_branching::UninhabitedEnumBranching, @@ -341,6 +338,9 @@ fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &BodyAndCache<'_> { let mut body = body.steal(); run_optimization_passes(tcx, &mut body, def_id, None); body.ensure_predecessors(); + + debug_assert!(!body.has_free_regions(), "Free regions in optimized MIR"); + tcx.arena.alloc(body) } @@ -358,5 +358,7 @@ fn promoted_mir(tcx: TyCtxt<'_>, def_id: DefId) -> &IndexVec Promoter<'a, 'tcx> { ty, val: ty::ConstKind::Unevaluated( def_id, - InternalSubsts::identity_for_item(tcx, def_id), + InternalSubsts::for_item(tcx, def_id, |param, _| { + if let ty::GenericParamDefKind::Lifetime = param.kind { + tcx.lifetimes.re_erased.into() + } else { + tcx.mk_param_from_def(param) + } + }), Some(promoted_id), ), }), diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index 821c4d68c7e8a..ac613adbcc6ca 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -6,7 +6,7 @@ use rustc::middle::lang_items; use rustc::middle::region; use rustc::mir::*; use rustc::ty::subst::Subst; -use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc_attr::{self as attr, UnwindAttr}; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -43,8 +43,7 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { .. }) | Node::TraitItem(hir::TraitItem { - kind: - hir::TraitItemKind::Fn(hir::FnSig { decl, .. }, hir::TraitFn::Provided(body_id)), + kind: hir::TraitItemKind::Fn(hir::FnSig { decl, .. }, hir::TraitFn::Provided(body_id)), .. }) => (*body_id, decl.output.span()), Node::Item(hir::Item { kind: hir::ItemKind::Static(ty, _, body_id), .. }) @@ -128,12 +127,8 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { let ty = if fn_sig.c_variadic && index == fn_sig.inputs().len() { let va_list_did = tcx.require_lang_item(lang_items::VaListTypeLangItem, Some(arg.span)); - let region = tcx.mk_region(ty::ReScope(region::Scope { - id: body.value.hir_id.local_id, - data: region::ScopeData::CallSite, - })); - tcx.type_of(va_list_did).subst(tcx, &[region.into()]) + tcx.type_of(va_list_did).subst(tcx, &[tcx.lifetimes.re_erased.into()]) } else { fn_sig.inputs()[index] }; @@ -189,6 +184,20 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { let mut body = BodyAndCache::new(body); body.ensure_predecessors(); + + // The borrow checker will replace all the regions here with its own + // inference variables. There's no point having non-erased regions here. + // The exception is `body.user_type_annotations`, which is used unmodified + // by borrow checking. + debug_assert!( + !(body.local_decls.has_free_regions() + || body.basic_blocks().has_free_regions() + || body.var_debug_info.has_free_regions() + || body.yield_ty.has_free_regions()), + "Unexpected free regions in MIR: {:?}", + body, + ); + body }) } @@ -209,7 +218,7 @@ fn liberated_closure_env_ty( }; let closure_env_ty = tcx.closure_env_ty(closure_def_id, closure_substs).unwrap(); - tcx.liberate_late_bound_regions(closure_def_id, &closure_env_ty) + tcx.erase_late_bound_regions(&closure_env_ty) } #[derive(Debug, PartialEq, Eq)] diff --git a/src/test/mir-opt/array-index-is-temporary.rs b/src/test/mir-opt/array-index-is-temporary.rs index 096f98bade25a..57f30acb3a643 100644 --- a/src/test/mir-opt/array-index-is-temporary.rs +++ b/src/test/mir-opt/array-index-is-temporary.rs @@ -15,7 +15,7 @@ fn main() { } // END RUST SOURCE -// START rustc.main.EraseRegions.after.mir +// START rustc.main.SimplifyCfg-elaborate-drops.after.mir // bb0: { // ... // _4 = &mut _2; @@ -38,4 +38,4 @@ fn main() { // ... // return; // } -// END rustc.main.EraseRegions.after.mir +// END rustc.main.SimplifyCfg-elaborate-drops.after.mir diff --git a/src/test/mir-opt/byte_slice.rs b/src/test/mir-opt/byte_slice.rs index 7edfa3e1124db..0fb685c3c4e6d 100644 --- a/src/test/mir-opt/byte_slice.rs +++ b/src/test/mir-opt/byte_slice.rs @@ -6,10 +6,10 @@ fn main() { } // END RUST SOURCE -// START rustc.main.EraseRegions.after.mir +// START rustc.main.SimplifyCfg-elaborate-drops.after.mir // ... // _1 = const b"foo"; // ... // _2 = [const 5u8, const 120u8]; // ... -// END rustc.main.EraseRegions.after.mir +// END rustc.main.SimplifyCfg-elaborate-drops.after.mir diff --git a/src/test/mir-opt/packed-struct-drop-aligned.rs b/src/test/mir-opt/packed-struct-drop-aligned.rs index 113f81c441f7c..39b9006017958 100644 --- a/src/test/mir-opt/packed-struct-drop-aligned.rs +++ b/src/test/mir-opt/packed-struct-drop-aligned.rs @@ -15,7 +15,7 @@ impl Drop for Droppy { } // END RUST SOURCE -// START rustc.main.EraseRegions.before.mir +// START rustc.main.SimplifyCfg-elaborate-drops.after.mir // fn main() -> () { // let mut _0: (); // let mut _1: Packed; @@ -56,4 +56,4 @@ impl Drop for Droppy { // drop(_1) -> [return: bb2, unwind: bb1]; // } // } -// END rustc.main.EraseRegions.before.mir +// END rustc.main.SimplifyCfg-elaborate-drops.after.mir diff --git a/src/test/mir-opt/retag.rs b/src/test/mir-opt/retag.rs index 1c88a9e4d5a32..e917441200b8a 100644 --- a/src/test/mir-opt/retag.rs +++ b/src/test/mir-opt/retag.rs @@ -8,8 +8,12 @@ struct Test(i32); impl Test { // Make sure we run the pass on a method, not just on bare functions. - fn foo<'x>(&self, x: &'x mut i32) -> &'x mut i32 { x } - fn foo_shr<'x>(&self, x: &'x i32) -> &'x i32 { x } + fn foo<'x>(&self, x: &'x mut i32) -> &'x mut i32 { + x + } + fn foo_shr<'x>(&self, x: &'x i32) -> &'x i32 { + x + } } impl Drop for Test { @@ -27,7 +31,10 @@ fn main() { } // Also test closures - let c: fn(&i32) -> &i32 = |x: &i32| -> &i32 { let _y = x; x }; + let c: fn(&i32) -> &i32 = |x: &i32| -> &i32 { + let _y = x; + x + }; let _w = c(&x); // need to call `foo_shr` or it doesn't even get generated @@ -38,7 +45,7 @@ fn main() { } // END RUST SOURCE -// START rustc.{{impl}}-foo.EraseRegions.after.mir +// START rustc.{{impl}}-foo.SimplifyCfg-elaborate-drops.after.mir // bb0: { // Retag([fn entry] _1); // Retag([fn entry] _2); @@ -48,8 +55,8 @@ fn main() { // ... // return; // } -// END rustc.{{impl}}-foo.EraseRegions.after.mir -// START rustc.{{impl}}-foo_shr.EraseRegions.after.mir +// END rustc.{{impl}}-foo.SimplifyCfg-elaborate-drops.after.mir +// START rustc.{{impl}}-foo_shr.SimplifyCfg-elaborate-drops.after.mir // bb0: { // Retag([fn entry] _1); // Retag([fn entry] _2); @@ -59,8 +66,8 @@ fn main() { // ... // return; // } -// END rustc.{{impl}}-foo_shr.EraseRegions.after.mir -// START rustc.main.EraseRegions.after.mir +// END rustc.{{impl}}-foo_shr.SimplifyCfg-elaborate-drops.after.mir +// START rustc.main.SimplifyCfg-elaborate-drops.after.mir // fn main() -> () { // ... // bb0: { @@ -96,8 +103,8 @@ fn main() { // // ... // } -// END rustc.main.EraseRegions.after.mir -// START rustc.main-{{closure}}.EraseRegions.after.mir +// END rustc.main.SimplifyCfg-elaborate-drops.after.mir +// START rustc.main-{{closure}}.SimplifyCfg-elaborate-drops.after.mir // fn main::{{closure}}#0(_1: &[closure@main::{{closure}}#0], _2: &i32) -> &i32 { // ... // bb0: { @@ -112,7 +119,7 @@ fn main() { // return; // } // } -// END rustc.main-{{closure}}.EraseRegions.after.mir +// END rustc.main-{{closure}}.SimplifyCfg-elaborate-drops.after.mir // START rustc.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir // fn std::intrinsics::drop_in_place(_1: *mut Test) -> () { // ... From 6570e275b9cfc6448c096e6bc86cddd602c333bd Mon Sep 17 00:00:00 2001 From: CDirkx Date: Thu, 19 Mar 2020 21:58:11 +0100 Subject: [PATCH 23/41] Removed unused `Hasher` import. --- src/libcore/ops/range.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/ops/range.rs b/src/libcore/ops/range.rs index 7fdb6dda1f97e..adee8cea442b4 100644 --- a/src/libcore/ops/range.rs +++ b/src/libcore/ops/range.rs @@ -1,5 +1,5 @@ use crate::fmt; -use crate::hash::{Hash, Hasher}; +use crate::hash::Hash; /// An unbounded range (`..`). /// From fd0e15bbcda4b1674f22e8db5fd81a63d671c996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 20 Mar 2020 00:00:00 +0000 Subject: [PATCH 24/41] Make std::sync::Arc compatible with ThreadSanitizer The memory fences used previously in Arc implementation are not properly understood by ThreadSanitizer as synchronization primitives. This had unfortunate effect where running any non-trivial program compiled with `-Z sanitizer=thread` would result in numerous false positives. Replace acquire fences with acquire loads when using ThreadSanitizer to address the issue. --- src/liballoc/lib.rs | 1 + src/liballoc/sync.rs | 25 +++++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index ffa4176cc7969..d877ac6ac5c79 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -80,6 +80,7 @@ #![feature(box_into_raw_non_null)] #![feature(box_patterns)] #![feature(box_syntax)] +#![feature(cfg_sanitize)] #![feature(cfg_target_has_atomic)] #![feature(coerce_unsized)] #![feature(const_generic_impls_guard)] diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 4a0cf2984edd9..d9b54fb0b177a 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -40,6 +40,23 @@ mod tests; /// necessarily) at _exactly_ `MAX_REFCOUNT + 1` references. const MAX_REFCOUNT: usize = (isize::MAX) as usize; +#[cfg(not(sanitize = "thread"))] +macro_rules! acquire { + ($x:expr) => { + atomic::fence(Acquire) + }; +} + +// ThreadSanitizer does not support memory fences. To avoid false positive +// reports in Arc / Weak implementation use atomic loads for synchronization +// instead. +#[cfg(sanitize = "thread")] +macro_rules! acquire { + ($x:expr) => { + $x.load(Acquire) + }; +} + /// A thread-safe reference-counting pointer. 'Arc' stands for 'Atomically /// Reference Counted'. /// @@ -402,7 +419,7 @@ impl Arc { return Err(this); } - atomic::fence(Acquire); + acquire!(this.inner().strong); unsafe { let elem = ptr::read(&this.ptr.as_ref().data); @@ -739,7 +756,7 @@ impl Arc { ptr::drop_in_place(&mut self.ptr.as_mut().data); if self.inner().weak.fetch_sub(1, Release) == 1 { - atomic::fence(Acquire); + acquire!(self.inner().weak); Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())) } } @@ -1243,7 +1260,7 @@ unsafe impl<#[may_dangle] T: ?Sized> Drop for Arc { // // [1]: (www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html) // [2]: (https://github.com/rust-lang/rust/pull/41714) - atomic::fence(Acquire); + acquire!(self.inner().strong); unsafe { self.drop_slow(); @@ -1701,7 +1718,7 @@ impl Drop for Weak { let inner = if let Some(inner) = self.inner() { inner } else { return }; if inner.weak.fetch_sub(1, Release) == 1 { - atomic::fence(Acquire); + acquire!(inner.weak); unsafe { Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())) } } } From d6f3a433d9847b2c4c6c31f9ea985625b16dded1 Mon Sep 17 00:00:00 2001 From: DutchGhost Date: Fri, 20 Mar 2020 10:36:40 +0100 Subject: [PATCH 25/41] Update const_forget.rs --- src/test/ui/consts/const_forget.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/consts/const_forget.rs b/src/test/ui/consts/const_forget.rs index 5dcad9be54f92..2dcb72a5a09cb 100644 --- a/src/test/ui/consts/const_forget.rs +++ b/src/test/ui/consts/const_forget.rs @@ -1,4 +1,4 @@ -// run-pass +// check-pass #![feature(const_forget)] From 3f6236ea5af2a4cef4e1d540f4e4f507eed5b0da Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Fri, 20 Mar 2020 13:23:24 +0100 Subject: [PATCH 26/41] Fix oudated comment for NamedRegionMap --- src/librustc_resolve/late/lifetimes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_resolve/late/lifetimes.rs b/src/librustc_resolve/late/lifetimes.rs index 51bf1f4843972..bc843fccc4c71 100644 --- a/src/librustc_resolve/late/lifetimes.rs +++ b/src/librustc_resolve/late/lifetimes.rs @@ -133,7 +133,7 @@ impl RegionExt for Region { /// that it corresponds to. /// /// FIXME. This struct gets converted to a `ResolveLifetimes` for -/// actual use. It has the same data, but indexed by `DefIndex`. This +/// actual use. It has the same data, but indexed by `LocalDefId`. This /// is silly. #[derive(Default)] struct NamedRegionMap { From 5444aded02d198e8b82e9025420375b8a68d156e Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 20 Mar 2020 13:24:35 +0100 Subject: [PATCH 27/41] Add tests for #58319 and #65131 --- src/test/ui/issues/issue-58319.rs | 621 ++++++++++++++++++++++++++ src/test/ui/issues/issue-65131.rs | 18 + src/test/ui/issues/issue-65131.stderr | 12 + 3 files changed, 651 insertions(+) create mode 100644 src/test/ui/issues/issue-58319.rs create mode 100644 src/test/ui/issues/issue-65131.rs create mode 100644 src/test/ui/issues/issue-65131.stderr diff --git a/src/test/ui/issues/issue-58319.rs b/src/test/ui/issues/issue-58319.rs new file mode 100644 index 0000000000000..757307d944f18 --- /dev/null +++ b/src/test/ui/issues/issue-58319.rs @@ -0,0 +1,621 @@ +// run-pass +fn main() {} +#[derive(Clone)] +pub struct Little; +#[derive(Clone)] +pub struct Big( + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, + Little, +); diff --git a/src/test/ui/issues/issue-65131.rs b/src/test/ui/issues/issue-65131.rs new file mode 100644 index 0000000000000..8b5345da900aa --- /dev/null +++ b/src/test/ui/issues/issue-65131.rs @@ -0,0 +1,18 @@ +fn get_pair(_a: &mut u32, _b: &mut u32) {} + +macro_rules! x10 { + ($($t:tt)*) => { + $($t)* $($t)* $($t)* $($t)* $($t)* + $($t)* $($t)* $($t)* $($t)* $($t)* + } +} + +#[allow(unused_assignments)] +fn main() { + let mut x = 1; + + get_pair(&mut x, &mut x); + //~^ ERROR: cannot borrow `x` as mutable more than once at a time + + x10! { x10!{ x10!{ if x > 0 { x += 2 } else { x += 1 } } } } +} diff --git a/src/test/ui/issues/issue-65131.stderr b/src/test/ui/issues/issue-65131.stderr new file mode 100644 index 0000000000000..e234e6da552a8 --- /dev/null +++ b/src/test/ui/issues/issue-65131.stderr @@ -0,0 +1,12 @@ +error[E0499]: cannot borrow `x` as mutable more than once at a time + --> $DIR/issue-65131.rs:14:22 + | +LL | get_pair(&mut x, &mut x); + | -------- ------ ^^^^^^ second mutable borrow occurs here + | | | + | | first mutable borrow occurs here + | first borrow later used by call + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0499`. From e61f126fb48f3f21cc019b7dc005b5287cef79e5 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 18 Mar 2020 10:25:46 -0400 Subject: [PATCH 28/41] Replace shared root with optional root This simplifies the node manipulation, as we can (in later commits) always know when traversing nodes that we are not in a shared root. --- src/liballoc/collections/btree/map.rs | 189 +++++++++++++++----------- 1 file changed, 112 insertions(+), 77 deletions(-) diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index 9da324ba2d4f1..478fa8e15268e 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -122,7 +122,7 @@ use UnderflowResult::*; /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub struct BTreeMap { - root: node::Root, + root: Option>, length: usize, } @@ -147,10 +147,11 @@ impl Clone for BTreeMap { { match node.force() { Leaf(leaf) => { - let mut out_tree = BTreeMap { root: node::Root::new_leaf(), length: 0 }; + let mut out_tree = BTreeMap { root: Some(node::Root::new_leaf()), length: 0 }; { - let mut out_node = match out_tree.root.as_mut().force() { + let root = out_tree.root.as_mut().unwrap(); + let mut out_node = match root.as_mut().force() { Leaf(leaf) => leaf, Internal(_) => unreachable!(), }; @@ -170,8 +171,13 @@ impl Clone for BTreeMap { Internal(internal) => { let mut out_tree = clone_subtree(internal.first_edge().descend()); + // Cannot call ensure_root_is_owned() because lacking K: Ord + if out_tree.root.is_none() { + out_tree.root = Some(node::Root::new_leaf()); + } + { - let mut out_node = out_tree.root.push_level(); + let mut out_node = out_tree.root.as_mut().unwrap().push_level(); let mut in_edge = internal.first_edge(); while let Ok(kv) = in_edge.right_kv() { let (k, v) = kv.into_kv(); @@ -190,7 +196,7 @@ impl Clone for BTreeMap { (root, length) }; - out_node.push(k, v, subroot); + out_node.push(k, v, subroot.unwrap_or_else(|| node::Root::new_leaf())); out_tree.length += 1 + sublength; } } @@ -203,9 +209,9 @@ impl Clone for BTreeMap { if self.is_empty() { // Ideally we'd call `BTreeMap::new` here, but that has the `K: // Ord` constraint, which this method lacks. - BTreeMap { root: node::Root::shared_empty_root(), length: 0 } + BTreeMap { root: None, length: 0 } } else { - clone_subtree(self.root.as_ref()) + clone_subtree(self.root.as_ref().unwrap().as_ref()) } } @@ -271,14 +277,14 @@ where type Key = K; fn get(&self, key: &Q) -> Option<&K> { - match search::search_tree(self.root.as_ref(), key) { + match search::search_tree(self.root.as_ref()?.as_ref(), key) { Found(handle) => Some(handle.into_kv().0), GoDown(_) => None, } } fn take(&mut self, key: &Q) -> Option { - match search::search_tree(self.root.as_mut(), key) { + match search::search_tree(self.root.as_mut()?.as_mut(), key) { Found(handle) => Some( OccupiedEntry { handle, length: &mut self.length, _marker: PhantomData } .remove_kv() @@ -290,7 +296,7 @@ where fn replace(&mut self, key: K) -> Option { self.ensure_root_is_owned(); - match search::search_tree::, K, (), K>(self.root.as_mut(), &key) { + match search::search_tree::, K, (), K>(self.root.as_mut()?.as_mut(), &key) { Found(handle) => Some(mem::replace(handle.into_kv_mut().0, key)), GoDown(handle) => { VacantEntry { key, handle, length: &mut self.length, _marker: PhantomData } @@ -344,15 +350,18 @@ pub struct IterMut<'a, K: 'a, V: 'a> { /// [`BTreeMap`]: struct.BTreeMap.html #[stable(feature = "rust1", since = "1.0.0")] pub struct IntoIter { - front: Handle, marker::Edge>, - back: Handle, marker::Edge>, + front: Option, marker::Edge>>, + back: Option, marker::Edge>>, length: usize, } #[stable(feature = "collection_debug", since = "1.17.0")] impl fmt::Debug for IntoIter { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let range = Range { front: self.front.reborrow(), back: self.back.reborrow() }; + let range = Range { + front: self.front.as_ref().map(|f| f.reborrow()), + back: self.back.as_ref().map(|b| b.reborrow()), + }; f.debug_list().entries(range).finish() } } @@ -417,8 +426,8 @@ pub struct ValuesMut<'a, K: 'a, V: 'a> { /// [`BTreeMap`]: struct.BTreeMap.html #[stable(feature = "btree_range", since = "1.17.0")] pub struct Range<'a, K: 'a, V: 'a> { - front: Handle, K, V, marker::Leaf>, marker::Edge>, - back: Handle, K, V, marker::Leaf>, marker::Edge>, + front: Option, K, V, marker::Leaf>, marker::Edge>>, + back: Option, K, V, marker::Leaf>, marker::Edge>>, } #[stable(feature = "collection_debug", since = "1.17.0")] @@ -437,8 +446,8 @@ impl fmt::Debug for Range<'_, K, V> { /// [`BTreeMap`]: struct.BTreeMap.html #[stable(feature = "btree_range", since = "1.17.0")] pub struct RangeMut<'a, K: 'a, V: 'a> { - front: Handle, K, V, marker::Leaf>, marker::Edge>, - back: Handle, K, V, marker::Leaf>, marker::Edge>, + front: Option, K, V, marker::Leaf>, marker::Edge>>, + back: Option, K, V, marker::Leaf>, marker::Edge>>, // Be invariant in `K` and `V` _marker: PhantomData<&'a mut (K, V)>, @@ -447,7 +456,10 @@ pub struct RangeMut<'a, K: 'a, V: 'a> { #[stable(feature = "collection_debug", since = "1.17.0")] impl fmt::Debug for RangeMut<'_, K, V> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let range = Range { front: self.front.reborrow(), back: self.back.reborrow() }; + let range = Range { + front: self.front.as_ref().map(|f| f.reborrow()), + back: self.back.as_ref().map(|b| b.reborrow()), + }; f.debug_list().entries(range).finish() } } @@ -544,7 +556,7 @@ impl BTreeMap { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn new() -> BTreeMap { - BTreeMap { root: node::Root::shared_empty_root(), length: 0 } + BTreeMap { root: None, length: 0 } } /// Clears the map, removing all elements. @@ -589,7 +601,7 @@ impl BTreeMap { K: Borrow, Q: Ord, { - match search::search_tree(self.root.as_ref(), key) { + match search::search_tree(self.root.as_ref()?.as_ref(), key) { Found(handle) => Some(handle.into_kv().1), GoDown(_) => None, } @@ -616,7 +628,7 @@ impl BTreeMap { K: Borrow, Q: Ord, { - match search::search_tree(self.root.as_ref(), k) { + match search::search_tree(self.root.as_ref()?.as_ref(), k) { Found(handle) => Some(handle.into_kv()), GoDown(_) => None, } @@ -645,7 +657,7 @@ impl BTreeMap { T: Ord, K: Borrow, { - let front = self.root.as_ref().first_leaf_edge(); + let front = self.root.as_ref()?.as_ref().first_leaf_edge(); front.right_kv().ok().map(Handle::into_kv) } @@ -674,7 +686,7 @@ impl BTreeMap { T: Ord, K: Borrow, { - let front = self.root.as_mut().first_leaf_edge(); + let front = self.root.as_mut()?.as_mut().first_leaf_edge(); if let Ok(kv) = front.right_kv() { Some(OccupiedEntry { handle: kv.forget_node_type(), @@ -708,7 +720,7 @@ impl BTreeMap { T: Ord, K: Borrow, { - let back = self.root.as_ref().last_leaf_edge(); + let back = self.root.as_ref()?.as_ref().last_leaf_edge(); back.left_kv().ok().map(Handle::into_kv) } @@ -737,7 +749,7 @@ impl BTreeMap { T: Ord, K: Borrow, { - let back = self.root.as_mut().last_leaf_edge(); + let back = self.root.as_mut()?.as_mut().last_leaf_edge(); if let Ok(kv) = back.left_kv() { Some(OccupiedEntry { handle: kv.forget_node_type(), @@ -801,7 +813,7 @@ impl BTreeMap { K: Borrow, Q: Ord, { - match search::search_tree(self.root.as_mut(), key) { + match search::search_tree(self.root.as_mut()?.as_mut(), key) { Found(handle) => Some(handle.into_kv_mut().1), GoDown(_) => None, } @@ -896,7 +908,7 @@ impl BTreeMap { K: Borrow, Q: Ord, { - match search::search_tree(self.root.as_mut(), key) { + match search::search_tree(self.root.as_mut()?.as_mut(), key) { Found(handle) => Some( OccupiedEntry { handle, length: &mut self.length, _marker: PhantomData } .remove_entry(), @@ -992,11 +1004,15 @@ impl BTreeMap { K: Borrow, R: RangeBounds, { - let root1 = self.root.as_ref(); - let root2 = self.root.as_ref(); - let (f, b) = range_search(root1, root2, range); + if let Some(root) = &self.root { + let root1 = root.as_ref(); + let root2 = root.as_ref(); + let (f, b) = range_search(root1, root2, range); - Range { front: f, back: b } + Range { front: Some(f), back: Some(b) } + } else { + Range { front: None, back: None } + } } /// Constructs a mutable double-ended iterator over a sub-range of elements in the map. @@ -1036,11 +1052,15 @@ impl BTreeMap { K: Borrow, R: RangeBounds, { - let root1 = self.root.as_mut(); - let root2 = unsafe { ptr::read(&root1) }; - let (f, b) = range_search(root1, root2, range); + if let Some(root) = &mut self.root { + let root1 = root.as_mut(); + let root2 = unsafe { ptr::read(&root1) }; + let (f, b) = range_search(root1, root2, range); - RangeMut { front: f, back: b, _marker: PhantomData } + RangeMut { front: Some(f), back: Some(b), _marker: PhantomData } + } else { + RangeMut { front: None, back: None, _marker: PhantomData } + } } /// Gets the given key's corresponding entry in the map for in-place manipulation. @@ -1065,7 +1085,7 @@ impl BTreeMap { pub fn entry(&mut self, key: K) -> Entry<'_, K, V> { // FIXME(@porglezomp) Avoid allocating if we don't insert self.ensure_root_is_owned(); - match search::search_tree(self.root.as_mut(), &key) { + match search::search_tree(self.root.as_mut().unwrap().as_mut(), &key) { Found(handle) => { Occupied(OccupiedEntry { handle, length: &mut self.length, _marker: PhantomData }) } @@ -1077,7 +1097,7 @@ impl BTreeMap { fn from_sorted_iter>(&mut self, iter: I) { self.ensure_root_is_owned(); - let mut cur_node = self.root.as_mut().last_leaf_edge().into_node(); + let mut cur_node = self.root.as_mut().unwrap().as_mut().last_leaf_edge().into_node(); // Iterate through all key-value pairs, pushing them into nodes at the right level. for (key, value) in iter { // Try to push key-value pair into the current leaf node. @@ -1126,7 +1146,7 @@ impl BTreeMap { fn fix_right_edge(&mut self) { // Handle underfull nodes, start from the top. - let mut cur_node = self.root.as_mut(); + let mut cur_node = self.root.as_mut().unwrap().as_mut(); while let Internal(internal) = cur_node.force() { // Check if right-most child is underfull. let mut last_edge = internal.last_edge(); @@ -1187,14 +1207,14 @@ impl BTreeMap { let total_num = self.len(); let mut right = Self::new(); - right.root = node::Root::new_leaf(); - for _ in 0..(self.root.as_ref().height()) { - right.root.push_level(); + right.root = Some(node::Root::new_leaf()); + for _ in 0..(self.root.as_ref().unwrap().as_ref().height()) { + right.root.as_mut().unwrap().push_level(); } { - let mut left_node = self.root.as_mut(); - let mut right_node = right.root.as_mut(); + let mut left_node = self.root.as_mut().unwrap().as_mut(); + let mut right_node = right.root.as_mut().unwrap().as_mut(); loop { let mut split_edge = match search::search_node(left_node, key) { @@ -1223,7 +1243,9 @@ impl BTreeMap { self.fix_right_border(); right.fix_left_border(); - if self.root.as_ref().height() < right.root.as_ref().height() { + if self.root.as_ref().unwrap().as_ref().height() + < right.root.as_ref().unwrap().as_ref().height() + { self.recalc_length(); right.length = total_num - self.len(); } else { @@ -1261,19 +1283,19 @@ impl BTreeMap { res } - self.length = dfs(self.root.as_ref()); + self.length = dfs(self.root.as_ref().unwrap().as_ref()); } /// Removes empty levels on the top. fn fix_top(&mut self) { loop { { - let node = self.root.as_ref(); + let node = self.root.as_ref().unwrap().as_ref(); if node.height() == 0 || node.len() > 0 { break; } } - self.root.pop_level(); + self.root.as_mut().unwrap().pop_level(); } } @@ -1281,7 +1303,7 @@ impl BTreeMap { self.fix_top(); { - let mut cur_node = self.root.as_mut(); + let mut cur_node = self.root.as_mut().unwrap().as_mut(); while let Internal(node) = cur_node.force() { let mut last_kv = node.last_kv(); @@ -1307,7 +1329,7 @@ impl BTreeMap { self.fix_top(); { - let mut cur_node = self.root.as_mut(); + let mut cur_node = self.root.as_mut().unwrap().as_mut(); while let Internal(node) = cur_node.force() { let mut first_kv = node.first_kv(); @@ -1329,8 +1351,8 @@ impl BTreeMap { /// If the root node is the shared root node, allocate our own node. fn ensure_root_is_owned(&mut self) { - if self.root.is_shared_root() { - self.root = node::Root::new_leaf(); + if self.root.is_none() { + self.root = Some(node::Root::new_leaf()); } } } @@ -1458,12 +1480,21 @@ impl IntoIterator for BTreeMap { type IntoIter = IntoIter; fn into_iter(self) -> IntoIter { - let root1 = unsafe { ptr::read(&self.root).into_ref() }; - let root2 = unsafe { ptr::read(&self.root).into_ref() }; + if self.root.is_none() { + mem::forget(self); + return IntoIter { front: None, back: None, length: 0 }; + } + + let root1 = unsafe { unwrap_unchecked(ptr::read(&self.root)).into_ref() }; + let root2 = unsafe { unwrap_unchecked(ptr::read(&self.root)).into_ref() }; let len = self.length; mem::forget(self); - IntoIter { front: root1.first_leaf_edge(), back: root2.last_leaf_edge(), length: len } + IntoIter { + front: Some(root1.first_leaf_edge()), + back: Some(root2.last_leaf_edge()), + length: len, + } } } @@ -1480,7 +1511,8 @@ impl Drop for IntoIter { // No need to avoid the shared root, because the tree was definitely not empty. unsafe { - let mut node = ptr::read(&self.0.front).into_node().forget_type(); + let mut node = + unwrap_unchecked(ptr::read(&self.0.front)).into_node().forget_type(); while let Some(parent) = node.deallocate_and_ascend() { node = parent.into_node().forget_type(); } @@ -1495,14 +1527,13 @@ impl Drop for IntoIter { } unsafe { - let mut node = ptr::read(&self.front).into_node().forget_type(); - if node.is_shared_root() { - return; - } - // Most of the nodes have been deallocated while traversing - // but one pile from a leaf up to the root is left standing. - while let Some(parent) = node.deallocate_and_ascend() { - node = parent.into_node().forget_type(); + if let Some(front) = ptr::read(&self.front) { + let mut node = front.into_node().forget_type(); + // Most of the nodes have been deallocated while traversing + // but one pile from a leaf up to the root is left standing. + while let Some(parent) = node.deallocate_and_ascend() { + node = parent.into_node().forget_type(); + } } } } @@ -1517,7 +1548,7 @@ impl Iterator for IntoIter { None } else { self.length -= 1; - Some(unsafe { self.front.next_unchecked() }) + Some(unsafe { self.front.as_mut().unwrap().next_unchecked() }) } } @@ -1533,7 +1564,7 @@ impl DoubleEndedIterator for IntoIter { None } else { self.length -= 1; - Some(unsafe { self.back.next_back_unchecked() }) + Some(unsafe { self.back.as_mut().unwrap().next_back_unchecked() }) } } } @@ -1683,7 +1714,7 @@ impl<'a, K, V> Range<'a, K, V> { } unsafe fn next_unchecked(&mut self) -> (&'a K, &'a V) { - self.front.next_unchecked() + unwrap_unchecked(self.front.as_mut()).next_unchecked() } } @@ -1696,7 +1727,7 @@ impl<'a, K, V> DoubleEndedIterator for Range<'a, K, V> { impl<'a, K, V> Range<'a, K, V> { unsafe fn next_back_unchecked(&mut self) -> (&'a K, &'a V) { - self.back.next_back_unchecked() + unwrap_unchecked(self.back.as_mut()).next_back_unchecked() } } @@ -1734,7 +1765,7 @@ impl<'a, K, V> RangeMut<'a, K, V> { } unsafe fn next_unchecked(&mut self) -> (&'a mut K, &'a mut V) { - self.front.next_unchecked() + unwrap_unchecked(self.front.as_mut()).next_unchecked() } } @@ -1755,7 +1786,7 @@ impl FusedIterator for RangeMut<'_, K, V> {} impl<'a, K, V> RangeMut<'a, K, V> { unsafe fn next_back_unchecked(&mut self) -> (&'a mut K, &'a mut V) { - self.back.next_back_unchecked() + unwrap_unchecked(self.back.as_mut()).next_back_unchecked() } } @@ -1969,8 +2000,8 @@ impl BTreeMap { pub fn iter(&self) -> Iter<'_, K, V> { Iter { range: Range { - front: self.root.as_ref().first_leaf_edge(), - back: self.root.as_ref().last_leaf_edge(), + front: self.root.as_ref().map(|r| r.as_ref().first_leaf_edge()), + back: self.root.as_ref().map(|r| r.as_ref().last_leaf_edge()), }, length: self.length, } @@ -1999,13 +2030,17 @@ impl BTreeMap { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn iter_mut(&mut self) -> IterMut<'_, K, V> { - let root1 = self.root.as_mut(); - let root2 = unsafe { ptr::read(&root1) }; IterMut { - range: RangeMut { - front: root1.first_leaf_edge(), - back: root2.last_leaf_edge(), - _marker: PhantomData, + range: if let Some(root) = &mut self.root { + let root1 = root.as_mut(); + let root2 = unsafe { ptr::read(&root1) }; + RangeMut { + front: Some(root1.first_leaf_edge()), + back: Some(root2.last_leaf_edge()), + _marker: PhantomData, + } + } else { + RangeMut { front: None, back: None, _marker: PhantomData } }, length: self.length, } From 1c44f852df205ae6d363cc9a2b12f8093b3ed342 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 18 Mar 2020 10:28:33 -0400 Subject: [PATCH 29/41] Remove shared root code and assertions from BTree nodes --- src/liballoc/collections/btree/node.rs | 53 +----------------------- src/liballoc/collections/btree/search.rs | 14 +++---- 2 files changed, 8 insertions(+), 59 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 1132ffdaf8005..5530c9593a51a 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -111,21 +111,6 @@ impl LeafNode { } } -impl NodeHeader { - fn is_shared_root(&self) -> bool { - ptr::eq(self, &EMPTY_ROOT_NODE as *const _ as *const _) - } -} - -// We need to implement Sync here in order to make a static instance. -unsafe impl Sync for NodeHeader<(), ()> {} - -// An empty node used as a placeholder for the root node, to avoid allocations. -// We use just a header in order to save space, since no operation on an empty tree will -// ever take a pointer past the first key. -static EMPTY_ROOT_NODE: NodeHeader<(), ()> = - NodeHeader { parent: ptr::null(), parent_idx: MaybeUninit::uninit(), len: 0 }; - /// The underlying representation of internal nodes. As with `LeafNode`s, these should be hidden /// behind `BoxedNode`s to prevent dropping uninitialized keys and values. Any pointer to an /// `InternalNode` can be directly casted to a pointer to the underlying `LeafNode` portion of the @@ -154,8 +139,8 @@ impl InternalNode { } /// A managed, non-null pointer to a node. This is either an owned pointer to -/// `LeafNode`, an owned pointer to `InternalNode`, or a (not owned) -/// pointer to `NodeHeader<(), ()` (more specifically, the pointer to EMPTY_ROOT_NODE). +/// `LeafNode` or an owned pointer to `InternalNode`. +/// /// All of these types have a `NodeHeader` prefix, meaning that they have at /// least the same size as `NodeHeader` and store the same kinds of data at the same /// offsets; and they have a pointer alignment at least as large as `NodeHeader`'s. @@ -196,20 +181,6 @@ unsafe impl Sync for Root {} unsafe impl Send for Root {} impl Root { - /// Whether the instance of `Root` wraps a shared, empty root node. If not, - /// the entire tree is uniquely owned by the owner of the `Root` instance. - pub fn is_shared_root(&self) -> bool { - self.as_ref().is_shared_root() - } - - /// Returns a shared tree, wrapping a shared root node that is eternally empty. - pub fn shared_empty_root() -> Self { - Root { - node: unsafe { BoxedNode::from_ptr(NonNull::from(&EMPTY_ROOT_NODE).cast()) }, - height: 0, - } - } - /// Returns a new owned tree, with its own root node that is initially empty. pub fn new_leaf() -> Self { Root { node: BoxedNode::from_leaf(Box::new(unsafe { LeafNode::new() })), height: 0 } @@ -245,7 +216,6 @@ impl Root { /// Adds a new internal node with a single edge, pointing to the previous root, and make that /// new node the root. This increases the height by 1 and is the opposite of `pop_level`. pub fn push_level(&mut self) -> NodeRef, K, V, marker::Internal> { - debug_assert!(!self.is_shared_root()); let mut new_node = Box::new(unsafe { InternalNode::new() }); new_node.edges[0].write(unsafe { BoxedNode::from_ptr(self.node.as_ptr()) }); @@ -381,7 +351,6 @@ impl NodeRef { /// Unsafe because the node must not be the shared root. For more information, /// see the `NodeRef` comments. unsafe fn as_leaf(&self) -> &LeafNode { - debug_assert!(!self.is_shared_root()); self.node.as_ref() } @@ -389,11 +358,6 @@ impl NodeRef { unsafe { &*(self.node.as_ptr() as *const NodeHeader) } } - /// Returns whether the node is the shared, empty root. - pub fn is_shared_root(&self) -> bool { - self.as_header().is_shared_root() - } - /// Borrows a view into the keys stored in the node. /// Unsafe because the caller must ensure that the node is not the shared root. pub unsafe fn keys(&self) -> &[K] { @@ -464,7 +428,6 @@ impl NodeRef { pub unsafe fn deallocate_and_ascend( self, ) -> Option, marker::Edge>> { - assert!(!self.is_shared_root()); let height = self.height; let node = self.node; let ret = self.ascend().ok(); @@ -527,14 +490,12 @@ impl<'a, K, V, Type> NodeRef, K, V, Type> { impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { /// Unsafe because the caller must ensure that the node is not the shared root. unsafe fn into_key_slice(self) -> &'a [K] { - debug_assert!(!self.is_shared_root()); // We cannot be the shared root, so `as_leaf` is okay. slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().keys), self.len()) } /// Unsafe because the caller must ensure that the node is not the shared root. unsafe fn into_val_slice(self) -> &'a [V] { - debug_assert!(!self.is_shared_root()); // We cannot be the shared root, so `as_leaf` is okay. slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len()) } @@ -555,7 +516,6 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { /// Unsafe because the caller must ensure that the node is not the shared root. unsafe fn into_key_slice_mut(mut self) -> &'a mut [K] { - debug_assert!(!self.is_shared_root()); // We cannot be the shared root, so `as_leaf_mut` is okay. slice::from_raw_parts_mut( MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys), @@ -565,7 +525,6 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { /// Unsafe because the caller must ensure that the node is not the shared root. unsafe fn into_val_slice_mut(mut self) -> &'a mut [V] { - debug_assert!(!self.is_shared_root()); slice::from_raw_parts_mut( MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals), self.len(), @@ -574,7 +533,6 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { /// Unsafe because the caller must ensure that the node is not the shared root. unsafe fn into_slices_mut(mut self) -> (&'a mut [K], &'a mut [V]) { - debug_assert!(!self.is_shared_root()); // We cannot use the getters here, because calling the second one // invalidates the reference returned by the first. // More precisely, it is the call to `len` that is the culprit, @@ -592,7 +550,6 @@ impl<'a, K, V> NodeRef, K, V, marker::Leaf> { /// Adds a key/value pair the end of the node. pub fn push(&mut self, key: K, val: V) { assert!(self.len() < CAPACITY); - debug_assert!(!self.is_shared_root()); let idx = self.len(); @@ -607,7 +564,6 @@ impl<'a, K, V> NodeRef, K, V, marker::Leaf> { /// Adds a key/value pair to the beginning of the node. pub fn push_front(&mut self, key: K, val: V) { assert!(self.len() < CAPACITY); - debug_assert!(!self.is_shared_root()); unsafe { slice_insert(self.keys_mut(), 0, key); @@ -624,7 +580,6 @@ impl<'a, K, V> NodeRef, K, V, marker::Internal> { pub fn push(&mut self, key: K, val: V, edge: Root) { assert!(edge.height == self.height - 1); assert!(self.len() < CAPACITY); - debug_assert!(!self.is_shared_root()); let idx = self.len(); @@ -658,7 +613,6 @@ impl<'a, K, V> NodeRef, K, V, marker::Internal> { pub fn push_front(&mut self, key: K, val: V, edge: Root) { assert!(edge.height == self.height - 1); assert!(self.len() < CAPACITY); - debug_assert!(!self.is_shared_root()); unsafe { slice_insert(self.keys_mut(), 0, key); @@ -904,7 +858,6 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::Edge fn insert_fit(&mut self, key: K, val: V) -> *mut V { // Necessary for correctness, but in a private module debug_assert!(self.node.len() < CAPACITY); - debug_assert!(!self.node.is_shared_root()); unsafe { slice_insert(self.node.keys_mut(), self.idx, key); @@ -1081,7 +1034,6 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::KV> /// - All the key/value pairs to the right of this handle are put into a newly /// allocated node. pub fn split(mut self) -> (NodeRef, K, V, marker::Leaf>, K, V, Root) { - assert!(!self.node.is_shared_root()); unsafe { let mut new_node = Box::new(LeafNode::new()); @@ -1113,7 +1065,6 @@ impl<'a, K, V> Handle, K, V, marker::Leaf>, marker::KV> pub fn remove( mut self, ) -> (Handle, K, V, marker::Leaf>, marker::Edge>, K, V) { - assert!(!self.node.is_shared_root()); unsafe { let k = slice_remove(self.node.keys_mut(), self.idx); let v = slice_remove(self.node.vals_mut(), self.idx); diff --git a/src/liballoc/collections/btree/search.rs b/src/liballoc/collections/btree/search.rs index 2ba5cebbdee74..f9354263864e5 100644 --- a/src/liballoc/collections/btree/search.rs +++ b/src/liballoc/collections/btree/search.rs @@ -72,14 +72,12 @@ where // Using `keys()` is fine here even if BorrowType is mutable, as all we return // is an index -- not a reference. let len = node.len(); - if len > 0 { - let keys = unsafe { node.keys() }; // safe because a non-empty node cannot be the shared root - for (i, k) in keys.iter().enumerate() { - match key.cmp(k.borrow()) { - Ordering::Greater => {} - Ordering::Equal => return (i, true), - Ordering::Less => return (i, false), - } + let keys = unsafe { node.keys() }; // safe because a non-empty node cannot be the shared root + for (i, k) in keys.iter().enumerate() { + match key.cmp(k.borrow()) { + Ordering::Greater => {} + Ordering::Equal => return (i, true), + Ordering::Less => return (i, false), } } (len, false) From 3c04fda751352fe00884782e7adf792a5e2a91c4 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 18 Mar 2020 10:45:35 -0400 Subject: [PATCH 30/41] Make functions dependent only on shared root avoidance safe --- src/liballoc/collections/btree/map.rs | 4 +- src/liballoc/collections/btree/node.rs | 107 ++++++++++++----------- src/liballoc/collections/btree/search.rs | 5 +- 3 files changed, 58 insertions(+), 58 deletions(-) diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index 478fa8e15268e..21f99257d81d0 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -1349,7 +1349,8 @@ impl BTreeMap { self.fix_top(); } - /// If the root node is the shared root node, allocate our own node. + /// If the root node is the empty (non-allocated) root node, allocate our + /// own node. fn ensure_root_is_owned(&mut self) { if self.root.is_none() { self.root = Some(node::Root::new_leaf()); @@ -1509,7 +1510,6 @@ impl Drop for IntoIter { // don't have to care about panics this time (they'll abort). while let Some(_) = self.0.next() {} - // No need to avoid the shared root, because the tree was definitely not empty. unsafe { let mut node = unwrap_unchecked(ptr::read(&self.0.front)).into_node().forget_type(); diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 5530c9593a51a..99f531264ba20 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -169,8 +169,9 @@ impl BoxedNode { } } -/// Either an owned tree or a shared, empty tree. Note that this does not have a destructor, -/// and must be cleaned up manually if it is an owned tree. +/// An owned tree. +/// +/// Note that this does not have a destructor, and must be cleaned up manually. pub struct Root { node: BoxedNode, /// The number of levels below the root node. @@ -278,10 +279,7 @@ impl Root { /// `Leaf`, the `NodeRef` points to a leaf node, when this is `Internal` the /// `NodeRef` points to an internal node, and when this is `LeafOrInternal` the /// `NodeRef` could be pointing to either type of node. -/// Note that in case of a leaf node, this might still be the shared root! -/// Only turn this into a `LeafNode` reference if you know it is not the shared root! -/// Shared references must be dereferenceable *for the entire size of their pointee*, -/// so '&LeafNode` or `&InternalNode` pointing to the shared root is undefined behavior. +/// /// Turning this into a `NodeHeader` reference is always safe. pub struct NodeRef { /// The number of levels below the node. @@ -344,14 +342,15 @@ impl NodeRef { NodeRef { height: self.height, node: self.node, root: self.root, _marker: PhantomData } } - /// Exposes the leaf "portion" of any leaf or internal node that is not the shared root. + /// Exposes the leaf "portion" of any leaf or internal node. /// If the node is a leaf, this function simply opens up its data. /// If the node is an internal node, so not a leaf, it does have all the data a leaf has /// (header, keys and values), and this function exposes that. - /// Unsafe because the node must not be the shared root. For more information, - /// see the `NodeRef` comments. - unsafe fn as_leaf(&self) -> &LeafNode { - self.node.as_ref() + fn as_leaf(&self) -> &LeafNode { + // The node must be valid for at least the LeafNode portion. + // This is not a reference in the NodeRef type because we don't know if + // it should be unique or shared. + unsafe { self.node.as_ref() } } fn as_header(&self) -> &NodeHeader { @@ -359,14 +358,12 @@ impl NodeRef { } /// Borrows a view into the keys stored in the node. - /// Unsafe because the caller must ensure that the node is not the shared root. - pub unsafe fn keys(&self) -> &[K] { + pub fn keys(&self) -> &[K] { self.reborrow().into_key_slice() } /// Borrows a view into the values stored in the node. - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn vals(&self) -> &[V] { + fn vals(&self) -> &[V] { self.reborrow().into_val_slice() } @@ -470,39 +467,37 @@ impl<'a, K, V, Type> NodeRef, K, V, Type> { /// (header, keys and values), and this function exposes that. /// /// Returns a raw ptr to avoid asserting exclusive access to the entire node. - /// This also implies you can invoke this member on the shared root, but the resulting pointer - /// might not be properly aligned and definitely would not allow accessing keys and values. fn as_leaf_mut(&mut self) -> *mut LeafNode { self.node.as_ptr() } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn keys_mut(&mut self) -> &mut [K] { - self.reborrow_mut().into_key_slice_mut() + fn keys_mut(&mut self) -> &mut [K] { + // SAFETY: the caller will not be able to call further methods on self + // until the key slice reference is dropped, as we have unique access + // for the lifetime of the borrow. + unsafe { self.reborrow_mut().into_key_slice_mut() } } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn vals_mut(&mut self) -> &mut [V] { - self.reborrow_mut().into_val_slice_mut() + fn vals_mut(&mut self) -> &mut [V] { + // SAFETY: the caller will not be able to call further methods on self + // until the value slice reference is dropped, as we have unique access + // for the lifetime of the borrow. + unsafe { self.reborrow_mut().into_val_slice_mut() } } } impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn into_key_slice(self) -> &'a [K] { - // We cannot be the shared root, so `as_leaf` is okay. - slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().keys), self.len()) + fn into_key_slice(self) -> &'a [K] { + unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().keys), self.len()) } } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn into_val_slice(self) -> &'a [V] { - // We cannot be the shared root, so `as_leaf` is okay. - slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len()) + fn into_val_slice(self) -> &'a [V] { + unsafe { slice::from_raw_parts(MaybeUninit::first_ptr(&self.as_leaf().vals), self.len()) } } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn into_slices(self) -> (&'a [K], &'a [V]) { - let k = ptr::read(&self); + fn into_slices(self) -> (&'a [K], &'a [V]) { + // SAFETY: equivalent to reborrow() except not requiring Type: 'a + let k = unsafe { ptr::read(&self) }; (k.into_key_slice(), self.into_val_slice()) } } @@ -514,25 +509,27 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { unsafe { &mut *(self.root as *mut Root) } } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn into_key_slice_mut(mut self) -> &'a mut [K] { - // We cannot be the shared root, so `as_leaf_mut` is okay. - slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys), - self.len(), - ) + fn into_key_slice_mut(mut self) -> &'a mut [K] { + // SAFETY: The keys of a node must always be initialized up to length. + unsafe { + slice::from_raw_parts_mut( + MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).keys), + self.len(), + ) + } } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn into_val_slice_mut(mut self) -> &'a mut [V] { - slice::from_raw_parts_mut( - MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals), - self.len(), - ) + fn into_val_slice_mut(mut self) -> &'a mut [V] { + // SAFETY: The values of a node must always be initialized up to length. + unsafe { + slice::from_raw_parts_mut( + MaybeUninit::first_ptr_mut(&mut (*self.as_leaf_mut()).vals), + self.len(), + ) + } } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn into_slices_mut(mut self) -> (&'a mut [K], &'a mut [V]) { + fn into_slices_mut(mut self) -> (&'a mut [K], &'a mut [V]) { // We cannot use the getters here, because calling the second one // invalidates the reference returned by the first. // More precisely, it is the call to `len` that is the culprit, @@ -540,8 +537,13 @@ impl<'a, K: 'a, V: 'a, Type> NodeRef, K, V, Type> { // overlap with the keys (and even the values, for ZST keys). let len = self.len(); let leaf = self.as_leaf_mut(); - let keys = slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).keys), len); - let vals = slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).vals), len); + // SAFETY: The keys and values of a node must always be initialized up to length. + let keys = unsafe { + slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).keys), len) + }; + let vals = unsafe { + slice::from_raw_parts_mut(MaybeUninit::first_ptr_mut(&mut (*leaf).vals), len) + }; (keys, vals) } } @@ -698,8 +700,7 @@ impl<'a, K, V> NodeRef, K, V, marker::LeafOrInternal> { } } - /// Unsafe because the caller must ensure that the node is not the shared root. - unsafe fn into_kv_pointers_mut(mut self) -> (*mut K, *mut V) { + fn into_kv_pointers_mut(mut self) -> (*mut K, *mut V) { (self.keys_mut().as_mut_ptr(), self.vals_mut().as_mut_ptr()) } } diff --git a/src/liballoc/collections/btree/search.rs b/src/liballoc/collections/btree/search.rs index f9354263864e5..4e80f7f21ebff 100644 --- a/src/liballoc/collections/btree/search.rs +++ b/src/liballoc/collections/btree/search.rs @@ -67,12 +67,11 @@ where Q: Ord, K: Borrow, { - // This function is defined over all borrow types (immutable, mutable, owned), - // and may be called on the shared root in each case. + // This function is defined over all borrow types (immutable, mutable, owned). // Using `keys()` is fine here even if BorrowType is mutable, as all we return // is an index -- not a reference. let len = node.len(); - let keys = unsafe { node.keys() }; // safe because a non-empty node cannot be the shared root + let keys = node.keys(); for (i, k) in keys.iter().enumerate() { match key.cmp(k.borrow()) { Ordering::Greater => {} From 54b7c38889ee7bb59f4d6449822fcfdfa3448ddb Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 18 Mar 2020 11:10:21 -0400 Subject: [PATCH 31/41] Drop NodeHeader type from BTree code We no longer have a separate header because the shared root is gone; all code can work solely with leafs now. --- src/liballoc/collections/btree/node.rs | 46 +++----------------------- 1 file changed, 5 insertions(+), 41 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 99f531264ba20..6ebb98c42cd4f 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -44,34 +44,7 @@ const B: usize = 6; pub const MIN_LEN: usize = B - 1; pub const CAPACITY: usize = 2 * B - 1; -/// The underlying representation of leaf nodes. Note that it is often unsafe to actually store -/// these, since only the first `len` keys and values are assumed to be initialized. As such, -/// these should always be put behind pointers, and specifically behind `BoxedNode` in the owned -/// case. -/// -/// We have a separate type for the header and rely on it matching the prefix of `LeafNode`, in -/// order to statically allocate a single dummy node to avoid allocations. This struct is -/// `repr(C)` to prevent them from being reordered. `LeafNode` does not just contain a -/// `NodeHeader` because we do not want unnecessary padding between `len` and the keys. -/// Crucially, `NodeHeader` can be safely transmuted to different K and V. (This is exploited -/// by `as_header`.) -#[repr(C)] -struct NodeHeader { - /// We use `*const` as opposed to `*mut` so as to be covariant in `K` and `V`. - /// This either points to an actual node or is null. - parent: *const InternalNode, - - /// This node's index into the parent node's `edges` array. - /// `*node.parent.edges[node.parent_idx]` should be the same thing as `node`. - /// This is only guaranteed to be initialized when `parent` is non-null. - parent_idx: MaybeUninit, - - /// The number of keys and values this node stores. - /// - /// This next to `parent_idx` to encourage the compiler to join `len` and - /// `parent_idx` into the same 32-bit word, reducing space overhead. - len: u16, -} +/// The underlying representation of leaf nodes. #[repr(C)] struct LeafNode { /// We use `*const` as opposed to `*mut` so as to be covariant in `K` and `V`. @@ -141,10 +114,7 @@ impl InternalNode { /// A managed, non-null pointer to a node. This is either an owned pointer to /// `LeafNode` or an owned pointer to `InternalNode`. /// -/// All of these types have a `NodeHeader` prefix, meaning that they have at -/// least the same size as `NodeHeader` and store the same kinds of data at the same -/// offsets; and they have a pointer alignment at least as large as `NodeHeader`'s. -/// However, `BoxedNode` contains no information as to which of the three types +/// However, `BoxedNode` contains no information as to which of the two types /// of nodes it actually contains, and, partially due to this lack of information, /// has no destructor. struct BoxedNode { @@ -279,8 +249,6 @@ impl Root { /// `Leaf`, the `NodeRef` points to a leaf node, when this is `Internal` the /// `NodeRef` points to an internal node, and when this is `LeafOrInternal` the /// `NodeRef` could be pointing to either type of node. -/// -/// Turning this into a `NodeHeader` reference is always safe. pub struct NodeRef { /// The number of levels below the node. height: usize, @@ -322,7 +290,7 @@ impl NodeRef { /// Note that, despite being safe, calling this function can have the side effect /// of invalidating mutable references that unsafe code has created. pub fn len(&self) -> usize { - self.as_header().len as usize + self.as_leaf().len as usize } /// Returns the height of this node in the whole tree. Zero height denotes the @@ -353,10 +321,6 @@ impl NodeRef { unsafe { self.node.as_ref() } } - fn as_header(&self) -> &NodeHeader { - unsafe { &*(self.node.as_ptr() as *const NodeHeader) } - } - /// Borrows a view into the keys stored in the node. pub fn keys(&self) -> &[K] { self.reborrow().into_key_slice() @@ -377,7 +341,7 @@ impl NodeRef { pub fn ascend( self, ) -> Result, marker::Edge>, Self> { - let parent_as_leaf = self.as_header().parent as *const LeafNode; + let parent_as_leaf = self.as_leaf().parent as *const LeafNode; if let Some(non_zero) = NonNull::new(parent_as_leaf as *mut _) { Ok(Handle { node: NodeRef { @@ -386,7 +350,7 @@ impl NodeRef { root: self.root, _marker: PhantomData, }, - idx: unsafe { usize::from(*self.as_header().parent_idx.as_ptr()) }, + idx: unsafe { usize::from(*self.as_leaf().parent_idx.as_ptr()) }, _marker: PhantomData, }) } else { From 13f6d771bb7f3617e247d9806e1b9472022cdaf4 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 18 Mar 2020 19:00:15 -0400 Subject: [PATCH 32/41] Simplify ensure_root_is_owned callers This makes ensure_root_is_owned return a reference to the (now guaranteed to exist) root, allowing callers to operate on it without going through another unwrap. Unfortunately this is only rarely useful as it's frequently the case that both the length and the root need to be accessed and field-level borrows in methods don't yet exist. --- src/liballoc/collections/btree/map.rs | 28 +++++++++++++-------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/liballoc/collections/btree/map.rs b/src/liballoc/collections/btree/map.rs index 21f99257d81d0..3ba7befc04609 100644 --- a/src/liballoc/collections/btree/map.rs +++ b/src/liballoc/collections/btree/map.rs @@ -170,13 +170,13 @@ impl Clone for BTreeMap { } Internal(internal) => { let mut out_tree = clone_subtree(internal.first_edge().descend()); - - // Cannot call ensure_root_is_owned() because lacking K: Ord - if out_tree.root.is_none() { - out_tree.root = Some(node::Root::new_leaf()); - } + out_tree.ensure_root_is_owned(); { + // Ideally we'd use the return of ensure_root_is_owned + // instead of re-unwrapping here but unfortunately that + // borrows all of out_tree and we need access to the + // length below. let mut out_node = out_tree.root.as_mut().unwrap().push_level(); let mut in_edge = internal.first_edge(); while let Ok(kv) = in_edge.right_kv() { @@ -1207,9 +1207,9 @@ impl BTreeMap { let total_num = self.len(); let mut right = Self::new(); - right.root = Some(node::Root::new_leaf()); + let right_root = right.ensure_root_is_owned(); for _ in 0..(self.root.as_ref().unwrap().as_ref().height()) { - right.root.as_mut().unwrap().push_level(); + right_root.push_level(); } { @@ -1348,14 +1348,6 @@ impl BTreeMap { self.fix_top(); } - - /// If the root node is the empty (non-allocated) root node, allocate our - /// own node. - fn ensure_root_is_owned(&mut self) { - if self.root.is_none() { - self.root = Some(node::Root::new_leaf()); - } - } } #[stable(feature = "rust1", since = "1.0.0")] @@ -2151,6 +2143,12 @@ impl BTreeMap { pub fn is_empty(&self) -> bool { self.len() == 0 } + + /// If the root node is the empty (non-allocated) root node, allocate our + /// own node. + fn ensure_root_is_owned(&mut self) -> &mut node::Root { + self.root.get_or_insert_with(|| node::Root::new_leaf()) + } } impl<'a, K: Ord, V> Entry<'a, K, V> { From 4d85314a0008ec809054854dff55c09913b1e80f Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 20 Mar 2020 09:40:57 -0400 Subject: [PATCH 33/41] Update test commentary for shared root removal --- src/liballoc/tests/btree/map.rs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/liballoc/tests/btree/map.rs b/src/liballoc/tests/btree/map.rs index d05eec19346de..3a3462d546f7a 100644 --- a/src/liballoc/tests/btree/map.rs +++ b/src/liballoc/tests/btree/map.rs @@ -67,7 +67,7 @@ fn test_basic_large() { #[test] fn test_basic_small() { let mut map = BTreeMap::new(); - // Empty, shared root: + // Empty, root is absent (None): assert_eq!(map.remove(&1), None); assert_eq!(map.len(), 0); assert_eq!(map.get(&1), None); @@ -123,7 +123,7 @@ fn test_basic_small() { assert_eq!(map.values().collect::>(), vec![&4]); assert_eq!(map.remove(&2), Some(4)); - // Empty but private root: + // Empty but root is owned (Some(...)): assert_eq!(map.len(), 0); assert_eq!(map.get(&1), None); assert_eq!(map.get_mut(&1), None); @@ -263,13 +263,6 @@ fn test_iter_mut_mutation() { do_test_iter_mut_mutation::(144); } -#[test] -fn test_into_key_slice_with_shared_root_past_bounds() { - let mut map: BTreeMap = BTreeMap::new(); - assert_eq!(map.get(&Align32(1)), None); - assert_eq!(map.get_mut(&Align32(1)), None); -} - #[test] fn test_values_mut() { let mut a = BTreeMap::new(); From 2f7d7c03334924d8d7b3630545cef62038ff0526 Mon Sep 17 00:00:00 2001 From: Kornel Date: Fri, 20 Mar 2020 14:40:35 +0000 Subject: [PATCH 34/41] must_use on split_off --- src/liballoc/vec.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index d1956270f135f..4769091183a37 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -1377,6 +1377,7 @@ impl Vec { /// assert_eq!(vec2, [2, 3]); /// ``` #[inline] + #[must_use = "use `.truncate()` if you don't need the other half"] #[stable(feature = "split_off", since = "1.4.0")] pub fn split_off(&mut self, at: usize) -> Self { assert!(at <= self.len(), "`at` out of bounds"); From 9b9a22cd2e537708152e847263fa2b999649a53c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 16 Mar 2020 23:36:14 +0100 Subject: [PATCH 35/41] can_begin_literal_maybe_minus: `true` on `"-"? lit` NTs. --- src/librustc_ast/token.rs | 17 ++++++++++---- src/librustc_ast/util/literal.rs | 2 +- src/librustc_expand/mbe/macro_parser.rs | 2 +- src/librustc_parse/parser/expr.rs | 1 + src/librustc_parse/parser/item.rs | 2 +- src/librustc_parse/parser/pat.rs | 2 +- .../extern-abi-from-mac-literal-frag.rs | 22 ++++++++++++++++++- ...sue-70050-ntliteral-accepts-negated-lit.rs | 16 ++++++++++++++ 8 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/parser/issue-70050-ntliteral-accepts-negated-lit.rs diff --git a/src/librustc_ast/token.rs b/src/librustc_ast/token.rs index 3fc6444168e24..be5d322ba1677 100644 --- a/src/librustc_ast/token.rs +++ b/src/librustc_ast/token.rs @@ -424,7 +424,7 @@ impl Token { NtExpr(..) | NtBlock(..) | NtLiteral(..) => true, _ => false, }, - _ => self.can_begin_literal_or_bool(), + _ => self.can_begin_literal_maybe_minus(), } } @@ -448,13 +448,22 @@ impl Token { /// Returns `true` if the token is any literal, a minus (which can prefix a literal, /// for example a '-42', or one of the boolean idents). /// - /// Keep this in sync with `Lit::from_token`. - pub fn can_begin_literal_or_bool(&self) -> bool { + /// In other words, would this token be a valid start of `parse_literal_maybe_minus`? + /// + /// Keep this in sync with and `Lit::from_token`, excluding unary negation. + pub fn can_begin_literal_maybe_minus(&self) -> bool { match self.uninterpolate().kind { Literal(..) | BinOp(Minus) => true, Ident(name, false) if name.is_bool_lit() => true, Interpolated(ref nt) => match &**nt { - NtExpr(e) | NtLiteral(e) => matches!(e.kind, ast::ExprKind::Lit(_)), + NtLiteral(_) => true, + NtExpr(e) => match &e.kind { + ast::ExprKind::Lit(_) => true, + ast::ExprKind::Unary(ast::UnOp::Neg, e) => { + matches!(&e.kind, ast::ExprKind::Lit(_)) + } + _ => false, + }, _ => false, }, _ => false, diff --git a/src/librustc_ast/util/literal.rs b/src/librustc_ast/util/literal.rs index d1757394f3a1d..1b17f343a6d67 100644 --- a/src/librustc_ast/util/literal.rs +++ b/src/librustc_ast/util/literal.rs @@ -189,7 +189,7 @@ impl Lit { /// Converts arbitrary token into an AST literal. /// - /// Keep this in sync with `Token::can_begin_literal_or_bool`. + /// Keep this in sync with `Token::can_begin_literal_or_bool` excluding unary negation. pub fn from_token(token: &Token) -> Result { let lit = match token.uninterpolate().kind { token::Ident(name, false) if name.is_bool_lit() => { diff --git a/src/librustc_expand/mbe/macro_parser.rs b/src/librustc_expand/mbe/macro_parser.rs index 3b9158f444519..0d777d88cad3a 100644 --- a/src/librustc_expand/mbe/macro_parser.rs +++ b/src/librustc_expand/mbe/macro_parser.rs @@ -778,7 +778,7 @@ fn may_begin_with(token: &Token, name: Name) -> bool { } sym::ty => token.can_begin_type(), sym::ident => get_macro_ident(token).is_some(), - sym::literal => token.can_begin_literal_or_bool(), + sym::literal => token.can_begin_literal_maybe_minus(), sym::vis => match token.kind { // The follow-set of :vis + "priv" keyword + interpolated token::Comma | token::Ident(..) | token::Interpolated(_) => true, diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index c65e99842c5dd..58ebc6b563707 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -1374,6 +1374,7 @@ impl<'a> Parser<'a> { } /// Matches `'-' lit | lit` (cf. `ast_validation::AstValidator::check_expr_within_pat`). + /// Keep this in sync with `Token::can_begin_literal_maybe_minus`. pub fn parse_literal_maybe_minus(&mut self) -> PResult<'a, P> { maybe_whole_expr!(self); diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 9d70f606f3ef4..7ccf31d8b4faa 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -1509,7 +1509,7 @@ impl<'a> Parser<'a> { }) // `extern ABI fn` || self.check_keyword(kw::Extern) - && self.look_ahead(1, |t| t.can_begin_literal_or_bool()) + && self.look_ahead(1, |t| t.can_begin_literal_maybe_minus()) && self.look_ahead(2, |t| t.is_keyword(kw::Fn)) } diff --git a/src/librustc_parse/parser/pat.rs b/src/librustc_parse/parser/pat.rs index 4585941943b74..5aab0580997c0 100644 --- a/src/librustc_parse/parser/pat.rs +++ b/src/librustc_parse/parser/pat.rs @@ -696,7 +696,7 @@ impl<'a> Parser<'a> { self.look_ahead(dist, |t| { t.is_path_start() // e.g. `MY_CONST`; || t.kind == token::Dot // e.g. `.5` for recovery; - || t.can_begin_literal_or_bool() // e.g. `42`. + || t.can_begin_literal_maybe_minus() // e.g. `42`. || t.is_whole_expr() }) } diff --git a/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs b/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs index cb23f2c808c34..4ecb21d26ab9b 100644 --- a/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs +++ b/src/test/ui/parser/extern-abi-from-mac-literal-frag.rs @@ -1,7 +1,7 @@ // check-pass // In this test we check that the parser accepts an ABI string when it -// comes from a macro `literal` fragment as opposed to a hardcoded string. +// comes from a macro `literal` or `expr` fragment as opposed to a hardcoded string. fn main() {} @@ -17,6 +17,18 @@ macro_rules! abi_from_lit_frag { } } +macro_rules! abi_from_expr_frag { + ($abi:expr) => { + extern $abi { + fn _import(); + } + + extern $abi fn _export() {} + + type _PTR = extern $abi fn(); + }; +} + mod rust { abi_from_lit_frag!("Rust"); } @@ -24,3 +36,11 @@ mod rust { mod c { abi_from_lit_frag!("C"); } + +mod rust_expr { + abi_from_expr_frag!("Rust"); +} + +mod c_expr { + abi_from_expr_frag!("C"); +} diff --git a/src/test/ui/parser/issue-70050-ntliteral-accepts-negated-lit.rs b/src/test/ui/parser/issue-70050-ntliteral-accepts-negated-lit.rs new file mode 100644 index 0000000000000..aca9d9eb0a5b4 --- /dev/null +++ b/src/test/ui/parser/issue-70050-ntliteral-accepts-negated-lit.rs @@ -0,0 +1,16 @@ +// check-pass + +macro_rules! foo { + ($a:literal) => { + bar!($a) + }; +} + +macro_rules! bar { + ($b:literal) => {}; +} + +fn main() { + foo!(-2); + bar!(-2); +} From bce7f6f3a09c0dc2e56c8b3a9f17c8a18cada009 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Wed, 18 Mar 2020 15:29:05 -0400 Subject: [PATCH 36/41] Fix debugger pretty printing of BTrees --- src/etc/gdb_rust_pretty_printing.py | 34 ++++++++++++-------- src/test/debuginfo/pretty-std-collections.rs | 24 ++++++++++---- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/etc/gdb_rust_pretty_printing.py b/src/etc/gdb_rust_pretty_printing.py index 0914c22eb13f0..cae64ef6665bb 100755 --- a/src/etc/gdb_rust_pretty_printing.py +++ b/src/etc/gdb_rust_pretty_printing.py @@ -370,12 +370,17 @@ def to_string(self): ("(len: %i)" % self.__val.get_wrapped_value()['map']['length'])) def children(self): - root = self.__val.get_wrapped_value()['map']['root'] - node_ptr = root['node'] - i = 0 - for child in children_of_node(node_ptr, root['height'], False): - yield (str(i), child) - i = i + 1 + prev_idx = None + innermap = GdbValue(self.__val.get_wrapped_value()['map']) + if innermap.get_wrapped_value()['length'] > 0: + root = GdbValue(innermap.get_wrapped_value()['root']) + type_name = str(root.type.ty.name).replace('core::option::Option<', '')[:-1] + root = root.get_wrapped_value().cast(gdb.lookup_type(type_name)) + node_ptr = root['node'] + i = 0 + for child in children_of_node(node_ptr, root['height'], False): + yield (str(i), child) + i = i + 1 class RustStdBTreeMapPrinter(object): @@ -391,13 +396,16 @@ def to_string(self): ("(len: %i)" % self.__val.get_wrapped_value()['length'])) def children(self): - root = self.__val.get_wrapped_value()['root'] - node_ptr = root['node'] - i = 0 - for child in children_of_node(node_ptr, root['height'], True): - yield (str(i), child[0]) - yield (str(i), child[1]) - i = i + 1 + if self.__val.get_wrapped_value()['length'] > 0: + root = GdbValue(self.__val.get_wrapped_value()['root']) + type_name = str(root.type.ty.name).replace('core::option::Option<', '')[:-1] + root = root.get_wrapped_value().cast(gdb.lookup_type(type_name)) + node_ptr = root['node'] + i = 0 + for child in children_of_node(node_ptr, root['height'], True): + yield (str(i), child[0]) + yield (str(i), child[1]) + i = i + 1 class RustStdStringPrinter(object): diff --git a/src/test/debuginfo/pretty-std-collections.rs b/src/test/debuginfo/pretty-std-collections.rs index f8997fad9a53f..3d2d88a676d0d 100644 --- a/src/test/debuginfo/pretty-std-collections.rs +++ b/src/test/debuginfo/pretty-std-collections.rs @@ -17,35 +17,43 @@ // gdb-command: print btree_set // gdb-check:$1 = BTreeSet(len: 15) = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14} +// gdb-command: print empty_btree_set +// gdb-check:$2 = BTreeSet(len: 0) + // gdb-command: print btree_map -// gdb-check:$2 = BTreeMap(len: 15) = {[0] = 0, [1] = 1, [2] = 2, [3] = 3, [4] = 4, [5] = 5, [6] = 6, [7] = 7, [8] = 8, [9] = 9, [10] = 10, [11] = 11, [12] = 12, [13] = 13, [14] = 14} +// gdb-check:$3 = BTreeMap(len: 15) = {[0] = 0, [1] = 1, [2] = 2, [3] = 3, [4] = 4, [5] = 5, [6] = 6, [7] = 7, [8] = 8, [9] = 9, [10] = 10, [11] = 11, [12] = 12, [13] = 13, [14] = 14} + +// gdb-command: print empty_btree_map +// gdb-check:$4 = BTreeMap(len: 0) // gdb-command: print vec_deque -// gdb-check:$3 = VecDeque(len: 3, cap: 8) = {5, 3, 7} +// gdb-check:$5 = VecDeque(len: 3, cap: 8) = {5, 3, 7} // gdb-command: print vec_deque2 -// gdb-check:$4 = VecDeque(len: 7, cap: 8) = {2, 3, 4, 5, 6, 7, 8} +// gdb-check:$6 = VecDeque(len: 7, cap: 8) = {2, 3, 4, 5, 6, 7, 8} #![allow(unused_variables)] -use std::collections::BTreeSet; use std::collections::BTreeMap; +use std::collections::BTreeSet; use std::collections::VecDeque; - fn main() { - // BTreeSet let mut btree_set = BTreeSet::new(); for i in 0..15 { btree_set.insert(i); } + let mut empty_btree_set: BTreeSet = BTreeSet::new(); + // BTreeMap let mut btree_map = BTreeMap::new(); for i in 0..15 { btree_map.insert(i, i); } + let mut empty_btree_map: BTreeMap = BTreeMap::new(); + // VecDeque let mut vec_deque = VecDeque::new(); vec_deque.push_back(5); @@ -63,4 +71,6 @@ fn main() { zzz(); // #break } -fn zzz() { () } +fn zzz() { + () +} From 32670ddd2822d2ecdcced9f2878704bceafd17c8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 20 Mar 2020 17:03:04 +0100 Subject: [PATCH 37/41] Clean up E0439 explanation --- src/librustc_error_codes/error_codes/E0439.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_error_codes/error_codes/E0439.md b/src/librustc_error_codes/error_codes/E0439.md index e6da2117ac5f2..3e663df866caa 100644 --- a/src/librustc_error_codes/error_codes/E0439.md +++ b/src/librustc_error_codes/error_codes/E0439.md @@ -1,5 +1,6 @@ -The length of the platform-intrinsic function `simd_shuffle` -wasn't specified. Erroneous code example: +The length of the platform-intrinsic function `simd_shuffle` wasn't specified. + +Erroneous code example: ```compile_fail,E0439 #![feature(platform_intrinsics)] From 5930da446562ed51f4a3551cf81525e296bc8665 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 20 Mar 2020 17:05:00 +0100 Subject: [PATCH 38/41] Abi::is_signed: assert that we are a Scalar --- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_target/abi/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 316cf2ee41918..eacffc9dbbdba 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -603,7 +603,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .not_undef() .and_then(|raw_discr| self.force_bits(raw_discr, discr_val.layout.size)) .map_err(|_| err_ub!(InvalidDiscriminant(raw_discr.erase_tag())))?; - let real_discr = if discr_val.layout.ty.is_signed() { + let real_discr = if discr_val.layout.abi.is_signed() { // going from layout tag type to typeck discriminant type // requires first sign extending with the discriminant layout let sexted = sign_extend(bits_discr, discr_val.layout.size) as i128; diff --git a/src/librustc_target/abi/mod.rs b/src/librustc_target/abi/mod.rs index 2f8bbd66c322b..ade8499609cd4 100644 --- a/src/librustc_target/abi/mod.rs +++ b/src/librustc_target/abi/mod.rs @@ -748,7 +748,7 @@ impl Abi { Primitive::Int(_, signed) => signed, _ => false, }, - _ => false, + _ => panic!("`is_signed` on non-scalar ABI {:?}", self), } } From 0d018a57556d5031601721b2386767f8566243e8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 20 Mar 2020 16:02:46 +0100 Subject: [PATCH 39/41] expand_include: set `.directory` to dir of included file. --- src/librustc_builtin_macros/source_util.rs | 13 ++++++++++++- src/test/ui/macros/issue-69838-dir/bar.rs | 3 +++ src/test/ui/macros/issue-69838-dir/included.rs | 3 +++ .../issue-69838-mods-relative-to-included-path.rs | 7 +++++++ 4 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/macros/issue-69838-dir/bar.rs create mode 100644 src/test/ui/macros/issue-69838-dir/included.rs create mode 100644 src/test/ui/macros/issue-69838-mods-relative-to-included-path.rs diff --git a/src/librustc_builtin_macros/source_util.rs b/src/librustc_builtin_macros/source_util.rs index 662bbe6a287a3..718498f04b94e 100644 --- a/src/librustc_builtin_macros/source_util.rs +++ b/src/librustc_builtin_macros/source_util.rs @@ -4,6 +4,7 @@ use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast_pretty::pprust; use rustc_expand::base::{self, *}; +use rustc_expand::module::DirectoryOwnership; use rustc_expand::panictry; use rustc_parse::{self, new_sub_parser_from_file, parser::Parser}; use rustc_session::lint::builtin::INCOMPLETE_INCLUDE; @@ -11,6 +12,7 @@ use rustc_span::symbol::Symbol; use rustc_span::{self, Pos, Span}; use smallvec::SmallVec; +use std::rc::Rc; use rustc_data_structures::sync::Lrc; @@ -101,7 +103,7 @@ pub fn expand_include<'cx>( None => return DummyResult::any(sp), }; // The file will be added to the code map by the parser - let file = match cx.resolve_path(file, sp) { + let mut file = match cx.resolve_path(file, sp) { Ok(f) => f, Err(mut err) => { err.emit(); @@ -110,6 +112,15 @@ pub fn expand_include<'cx>( }; let p = new_sub_parser_from_file(cx.parse_sess(), &file, None, sp); + // If in the included file we have e.g., `mod bar;`, + // then the path of `bar.rs` should be relative to the directory of `file`. + // See https://github.com/rust-lang/rust/pull/69838/files#r395217057 for a discussion. + // `MacroExpander::fully_expand_fragment` later restores, so "stack discipline" is maintained. + file.pop(); + cx.current_expansion.directory_ownership = DirectoryOwnership::Owned { relative: None }; + let mod_path = cx.current_expansion.module.mod_path.clone(); + cx.current_expansion.module = Rc::new(ModuleData { mod_path, directory: file }); + struct ExpandResult<'a> { p: Parser<'a>, } diff --git a/src/test/ui/macros/issue-69838-dir/bar.rs b/src/test/ui/macros/issue-69838-dir/bar.rs new file mode 100644 index 0000000000000..ec12f8c5cb442 --- /dev/null +++ b/src/test/ui/macros/issue-69838-dir/bar.rs @@ -0,0 +1,3 @@ +// ignore-test -- this is an auxiliary file as part of another test. + +pub fn i_am_in_bar() {} diff --git a/src/test/ui/macros/issue-69838-dir/included.rs b/src/test/ui/macros/issue-69838-dir/included.rs new file mode 100644 index 0000000000000..9900b8fd5092c --- /dev/null +++ b/src/test/ui/macros/issue-69838-dir/included.rs @@ -0,0 +1,3 @@ +// ignore-test -- this is an auxiliary file as part of another test. + +pub mod bar; diff --git a/src/test/ui/macros/issue-69838-mods-relative-to-included-path.rs b/src/test/ui/macros/issue-69838-mods-relative-to-included-path.rs new file mode 100644 index 0000000000000..2a4e97f0ef5f1 --- /dev/null +++ b/src/test/ui/macros/issue-69838-mods-relative-to-included-path.rs @@ -0,0 +1,7 @@ +// check-pass + +include!("issue-69838-dir/included.rs"); + +fn main() { + bar::i_am_in_bar(); +} From 951a3661adf95b58a351b1ee10fe508e37dc17a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Thu, 19 Mar 2020 22:09:26 +0100 Subject: [PATCH 40/41] remove redundant import (clippy::single_component_path_imports) remove redundant format!() call (clippy::useless_format) don't use ok() before calling expect() (clippy::ok_expect) --- src/librustc_lint/types.rs | 2 -- src/librustc_typeck/impl_wf_check/min_specialization.rs | 2 +- src/librustdoc/passes/calculate_doc_coverage.rs | 1 - 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index 4949c93d45eed..fcd50001cb3a9 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -273,7 +273,6 @@ fn lint_int_literal<'a, 'tcx>( cx.sess() .source_map() .span_to_snippet(lit.span) - .ok() .expect("must get snippet from literal"), t.name_str(), min, @@ -338,7 +337,6 @@ fn lint_uint_literal<'a, 'tcx>( cx.sess() .source_map() .span_to_snippet(lit.span) - .ok() .expect("must get snippet from literal"), t.name_str(), min, diff --git a/src/librustc_typeck/impl_wf_check/min_specialization.rs b/src/librustc_typeck/impl_wf_check/min_specialization.rs index cae8837611846..ef94500f5c444 100644 --- a/src/librustc_typeck/impl_wf_check/min_specialization.rs +++ b/src/librustc_typeck/impl_wf_check/min_specialization.rs @@ -270,7 +270,7 @@ fn check_static_lifetimes<'tcx>( span: Span, ) { if tcx.any_free_region_meets(parent_substs, |r| *r == ty::ReStatic) { - tcx.sess.struct_span_err(span, &format!("cannot specialize on `'static` lifetime")).emit(); + tcx.sess.struct_span_err(span, "cannot specialize on `'static` lifetime").emit(); } } diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index f48224512ba4f..98300385c8fb8 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -8,7 +8,6 @@ use rustc_ast::attr; use rustc_span::symbol::sym; use rustc_span::FileName; use serde::Serialize; -use serde_json; use std::collections::BTreeMap; use std::ops; From ad00e9188766b8accdce93b264ed8b13aa12a820 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20Kr=C3=BCger?= Date: Fri, 20 Mar 2020 15:03:11 +0100 Subject: [PATCH 41/41] remove redundant returns (clippy::needless_return) --- src/libcore/hint.rs | 2 +- src/librustc/lint.rs | 2 +- src/librustc/middle/region.rs | 4 ++-- src/librustc/ty/context.rs | 4 ++-- src/librustc/ty/relate.rs | 2 +- src/librustc/ty/sty.rs | 6 ++--- src/librustc/ty/subst.rs | 2 +- .../deriving/decodable.rs | 4 ++-- .../deriving/default.rs | 4 ++-- .../deriving/encodable.rs | 4 ++-- .../deriving/generic/mod.rs | 1 - src/librustc_builtin_macros/format_foreign.rs | 4 ++-- src/librustc_codegen_llvm/back/archive.rs | 2 +- src/librustc_codegen_llvm/back/bytecode.rs | 4 ++-- src/librustc_codegen_llvm/common.rs | 10 +++------ src/librustc_codegen_llvm/context.rs | 2 +- .../debuginfo/metadata.rs | 22 +++++++++---------- src/librustc_codegen_llvm/debuginfo/mod.rs | 2 +- src/librustc_codegen_llvm/debuginfo/utils.rs | 4 +--- src/librustc_codegen_llvm/llvm/archive_ro.rs | 4 ++-- src/librustc_codegen_ssa/back/command.rs | 2 +- src/librustc_codegen_ssa/base.rs | 4 ++-- .../graph/dominators/mod.rs | 4 ++-- src/librustc_driver/lib.rs | 2 +- .../assert_module_sources.rs | 2 +- src/librustc_incremental/persist/save.rs | 1 - src/librustc_infer/infer/equate.rs | 2 +- .../nice_region_error/different_lifetimes.rs | 2 +- src/librustc_infer/infer/higher_ranked/mod.rs | 4 ++-- .../infer/lexical_region_resolve/mod.rs | 8 +++---- src/librustc_infer/infer/nll_relate/mod.rs | 4 ++-- .../infer/region_constraints/mod.rs | 2 +- src/librustc_lexer/src/lib.rs | 6 ++--- src/librustc_lint/builtin.rs | 2 +- src/librustc_lint/unused.rs | 2 +- src/librustc_metadata/creader.rs | 2 +- src/librustc_metadata/foreign_modules.rs | 2 +- src/librustc_metadata/link_args.rs | 2 +- src/librustc_metadata/locator.rs | 2 +- src/librustc_metadata/native_libs.rs | 2 +- .../rmeta/decoder/cstore_impl.rs | 2 +- src/librustc_mir/borrow_check/borrow_set.rs | 2 +- .../borrow_check/diagnostics/region_name.rs | 2 +- src/librustc_mir/const_eval/machine.rs | 2 +- src/librustc_mir/dataflow/mod.rs | 4 ++-- src/librustc_mir/interpret/operator.rs | 6 ++--- src/librustc_mir/interpret/place.rs | 4 +--- src/librustc_mir/interpret/terminator.rs | 2 +- src/librustc_mir/monomorphize/collector.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 2 +- src/librustc_mir_build/build/mod.rs | 5 ++--- src/librustc_mir_build/hair/cx/block.rs | 2 +- src/librustc_parse/lib.rs | 2 +- src/librustc_parse/parser/expr.rs | 4 ++-- src/librustc_parse/parser/item.rs | 8 +++---- src/librustc_parse/parser/pat.rs | 2 +- src/librustc_parse/parser/stmt.rs | 2 +- src/librustc_passes/liveness.rs | 2 +- src/librustc_passes/loops.rs | 2 +- src/librustc_passes/reachable.rs | 4 +--- src/librustc_passes/stability.rs | 2 +- src/librustc_privacy/lib.rs | 4 ++-- src/librustc_resolve/def_collector.rs | 2 +- src/librustc_resolve/imports.rs | 2 +- src/librustc_resolve/late/diagnostics.rs | 4 ++-- src/librustc_resolve/macros.rs | 2 +- src/librustc_save_analysis/lib.rs | 4 ++-- src/librustc_session/config.rs | 2 +- src/librustc_span/caching_source_map_view.rs | 6 +---- src/librustc_span/source_map.rs | 16 ++++++-------- .../traits/auto_trait.rs | 18 +++++++-------- .../traits/coherence.rs | 4 ++-- .../error_reporting/on_unimplemented.rs | 6 +---- .../traits/error_reporting/suggestions.rs | 2 +- .../traits/query/normalize.rs | 4 ++-- src/librustc_trait_selection/traits/wf.rs | 2 +- src/librustc_traits/lowering/environment.rs | 2 +- src/librustc_ty/needs_drop.rs | 2 +- src/librustc_typeck/astconv.rs | 6 ++--- src/librustc_typeck/check/mod.rs | 4 ++-- .../coherence/inherent_impls.rs | 5 +---- src/librustc_typeck/collect/type_of.rs | 2 +- src/libstd/backtrace.rs | 2 +- src/libstd/sys_common/backtrace.rs | 2 +- 84 files changed, 142 insertions(+), 170 deletions(-) diff --git a/src/libcore/hint.rs b/src/libcore/hint.rs index f4fb9ab1757cd..6cbd26a78de72 100644 --- a/src/libcore/hint.rs +++ b/src/libcore/hint.rs @@ -114,6 +114,6 @@ pub fn black_box(dummy: T) -> T { // more than we want, but it's so far good enough. unsafe { asm!("" : : "r"(&dummy)); - return dummy; + dummy } } diff --git a/src/librustc/lint.rs b/src/librustc/lint.rs index d4d01a716db97..4dd276d2e032c 100644 --- a/src/librustc/lint.rs +++ b/src/librustc/lint.rs @@ -85,7 +85,7 @@ impl LintLevelSets { level = cmp::min(*driver_level, level); } - return (level, src); + (level, src) } pub fn get_lint_id_level( diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 2735c4afca2c8..1a63dc9dcf977 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -467,7 +467,7 @@ impl<'tcx> ScopeTree { } debug!("temporary_scope({:?}) = None", expr_id); - return None; + None } /// Returns the lifetime of the variable `id`. @@ -498,7 +498,7 @@ impl<'tcx> ScopeTree { debug!("is_subscope_of({:?}, {:?})=true", subscope, superscope); - return true; + true } /// Returns the ID of the innermost containing body. diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 742d57fb58a51..0e3776f32e0e8 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1447,11 +1447,11 @@ impl<'tcx> TyCtxt<'tcx> { _ => return None, }; - return Some(FreeRegionInfo { + Some(FreeRegionInfo { def_id: suitable_region_binding_scope, boundregion: bound_region, is_impl_item, - }); + }) } pub fn return_type_impl_trait(&self, scope_def_id: DefId) -> Option<(Ty<'tcx>, Span)> { diff --git a/src/librustc/ty/relate.rs b/src/librustc/ty/relate.rs index fb4184a9fb347..872e06e1176dc 100644 --- a/src/librustc/ty/relate.rs +++ b/src/librustc/ty/relate.rs @@ -440,7 +440,7 @@ pub fn super_relate_tys>( (Some(sz_a_val), Some(sz_b_val)) => Err(TypeError::FixedArraySize( expected_found(relation, &sz_a_val, &sz_b_val), )), - _ => return Err(err), + _ => Err(err), } } } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index e265a2f8257fb..42cd2f52cb3ad 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -1612,7 +1612,7 @@ impl<'tcx> PolyExistentialProjection<'tcx> { } pub fn item_def_id(&self) -> DefId { - return self.skip_binder().item_def_id; + self.skip_binder().item_def_id } } @@ -2000,8 +2000,8 @@ impl<'tcx> TyS<'tcx> { #[inline] pub fn is_unsafe_ptr(&self) -> bool { match self.kind { - RawPtr(_) => return true, - _ => return false, + RawPtr(_) => true, + _ => false, } } diff --git a/src/librustc/ty/subst.rs b/src/librustc/ty/subst.rs index a005581283550..a3acc14856e1f 100644 --- a/src/librustc/ty/subst.rs +++ b/src/librustc/ty/subst.rs @@ -524,7 +524,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for SubstFolder<'a, 'tcx> { self.root_ty = None; } - return t1; + t1 } fn fold_const(&mut self, c: &'tcx ty::Const<'tcx>) -> &'tcx ty::Const<'tcx> { diff --git a/src/librustc_builtin_macros/deriving/decodable.rs b/src/librustc_builtin_macros/deriving/decodable.rs index ac5d08ba62d0d..64a810bdcf687 100644 --- a/src/librustc_builtin_macros/deriving/decodable.rs +++ b/src/librustc_builtin_macros/deriving/decodable.rs @@ -87,7 +87,7 @@ fn decodable_substructure( let blkarg = cx.ident_of("_d", trait_span); let blkdecoder = cx.expr_ident(trait_span, blkarg); - return match *substr.fields { + match *substr.fields { StaticStruct(_, ref summary) => { let nfields = match *summary { Unnamed(ref fields, _) => fields.len(), @@ -178,7 +178,7 @@ fn decodable_substructure( ) } _ => cx.bug("expected StaticEnum or StaticStruct in derive(Decodable)"), - }; + } } /// Creates a decoder for a single enum variant/struct: diff --git a/src/librustc_builtin_macros/deriving/default.rs b/src/librustc_builtin_macros/deriving/default.rs index cb85a0b1a10cc..27d5263320041 100644 --- a/src/librustc_builtin_macros/deriving/default.rs +++ b/src/librustc_builtin_macros/deriving/default.rs @@ -53,7 +53,7 @@ fn default_substructure( let default_ident = cx.std_path(&[kw::Default, sym::Default, kw::Default]); let default_call = |span| cx.expr_call_global(span, default_ident.clone(), Vec::new()); - return match *substr.fields { + match *substr.fields { StaticStruct(_, ref summary) => match *summary { Unnamed(ref fields, is_tuple) => { if !is_tuple { @@ -83,5 +83,5 @@ fn default_substructure( DummyResult::raw_expr(trait_span, true) } _ => cx.span_bug(trait_span, "method in `derive(Default)`"), - }; + } } diff --git a/src/librustc_builtin_macros/deriving/encodable.rs b/src/librustc_builtin_macros/deriving/encodable.rs index 9073085381ac1..54926ec3fd502 100644 --- a/src/librustc_builtin_macros/deriving/encodable.rs +++ b/src/librustc_builtin_macros/deriving/encodable.rs @@ -173,7 +173,7 @@ fn encodable_substructure( ], )); - return match *substr.fields { + match *substr.fields { Struct(_, ref fields) => { let emit_struct_field = cx.ident_of("emit_struct_field", trait_span); let mut stmts = Vec::new(); @@ -283,5 +283,5 @@ fn encodable_substructure( } _ => cx.bug("expected Struct or EnumMatching in derive(Encodable)"), - }; + } } diff --git a/src/librustc_builtin_macros/deriving/generic/mod.rs b/src/librustc_builtin_macros/deriving/generic/mod.rs index 84ed6e96aafc8..ee32e914acba4 100644 --- a/src/librustc_builtin_macros/deriving/generic/mod.rs +++ b/src/librustc_builtin_macros/deriving/generic/mod.rs @@ -489,7 +489,6 @@ impl<'a> TraitDef<'a> { // set earlier; see // librustc_expand/expand.rs:MacroExpander::fully_expand_fragment() // librustc_expand/base.rs:Annotatable::derive_allowed() - return; } } } diff --git a/src/librustc_builtin_macros/format_foreign.rs b/src/librustc_builtin_macros/format_foreign.rs index cc3c403450e04..e6a87e4d82586 100644 --- a/src/librustc_builtin_macros/format_foreign.rs +++ b/src/librustc_builtin_macros/format_foreign.rs @@ -359,7 +359,7 @@ pub mod printf { // // Note: `move` used to capture copies of the cursors as they are *now*. let fallback = move || { - return Some(( + Some(( Substitution::Format(Format { span: start.slice_between(next).unwrap(), parameter: None, @@ -371,7 +371,7 @@ pub mod printf { position: InnerSpan::new(start.at, next.at), }), next.slice_after(), - )); + )) }; // Next parsing state. diff --git a/src/librustc_codegen_llvm/back/archive.rs b/src/librustc_codegen_llvm/back/archive.rs index 239ca57ba4143..f1fe40d919eeb 100644 --- a/src/librustc_codegen_llvm/back/archive.rs +++ b/src/librustc_codegen_llvm/back/archive.rs @@ -146,7 +146,7 @@ impl<'a> ArchiveBuilder<'a> for LlvmArchiveBuilder<'a> { } // ok, don't skip this - return false; + false }) } diff --git a/src/librustc_codegen_llvm/back/bytecode.rs b/src/librustc_codegen_llvm/back/bytecode.rs index db29556e70ccc..0c8ce39132abb 100644 --- a/src/librustc_codegen_llvm/back/bytecode.rs +++ b/src/librustc_codegen_llvm/back/bytecode.rs @@ -83,7 +83,7 @@ pub fn encode(identifier: &str, bytecode: &[u8]) -> Vec { encoded.push(0); } - return encoded; + encoded } pub struct DecodedBytecode<'a> { @@ -132,7 +132,7 @@ impl<'a> DecodedBytecode<'a> { pub fn bytecode(&self) -> Vec { let mut data = Vec::new(); DeflateDecoder::new(self.encoded_bytecode).read_to_end(&mut data).unwrap(); - return data; + data } pub fn identifier(&self) -> &'a str { diff --git a/src/librustc_codegen_llvm/common.rs b/src/librustc_codegen_llvm/common.rs index 609ddfc1d3a80..f72060868128c 100644 --- a/src/librustc_codegen_llvm/common.rs +++ b/src/librustc_codegen_llvm/common.rs @@ -96,15 +96,11 @@ impl BackendTypes for CodegenCx<'ll, 'tcx> { impl CodegenCx<'ll, 'tcx> { pub fn const_array(&self, ty: &'ll Type, elts: &[&'ll Value]) -> &'ll Value { - unsafe { - return llvm::LLVMConstArray(ty, elts.as_ptr(), elts.len() as c_uint); - } + unsafe { llvm::LLVMConstArray(ty, elts.as_ptr(), elts.len() as c_uint) } } pub fn const_vector(&self, elts: &[&'ll Value]) -> &'ll Value { - unsafe { - return llvm::LLVMConstVector(elts.as_ptr(), elts.len() as c_uint); - } + unsafe { llvm::LLVMConstVector(elts.as_ptr(), elts.len() as c_uint) } } pub fn const_bytes(&self, bytes: &[u8]) -> &'ll Value { @@ -330,7 +326,7 @@ pub fn val_ty(v: &Value) -> &Type { pub fn bytes_in_context(llcx: &'ll llvm::Context, bytes: &[u8]) -> &'ll Value { unsafe { let ptr = bytes.as_ptr() as *const c_char; - return llvm::LLVMConstStringInContext(llcx, ptr, bytes.len() as c_uint, True); + llvm::LLVMConstStringInContext(llcx, ptr, bytes.len() as c_uint, True) } } diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 7b1526e9da154..4427997c2732d 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -800,7 +800,7 @@ impl CodegenCx<'b, 'tcx> { ifn!("llvm.dbg.declare", fn(self.type_metadata(), self.type_metadata()) -> void); ifn!("llvm.dbg.value", fn(self.type_metadata(), t_i64, self.type_metadata()) -> void); } - return None; + None } } diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 6a7ed4e1dc384..f35220cc6666a 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -203,7 +203,7 @@ impl TypeMap<'ll, 'tcx> { let key = self.unique_id_interner.intern(&unique_type_id); self.type_to_unique_id.insert(type_, UniqueTypeId(key)); - return UniqueTypeId(key); + UniqueTypeId(key) } /// Gets the `UniqueTypeId` for an enum variant. Enum variants are not really @@ -314,7 +314,7 @@ impl RecursiveTypeDescription<'ll, 'tcx> { member_holding_stub, member_descriptions, ); - return MetadataCreationResult::new(metadata_stub, true); + MetadataCreationResult::new(metadata_stub, true) } } } @@ -364,7 +364,7 @@ fn fixed_vec_metadata( ) }; - return MetadataCreationResult::new(metadata, false); + MetadataCreationResult::new(metadata, false) } fn vec_slice_metadata( @@ -445,7 +445,7 @@ fn subroutine_type_metadata( return_if_metadata_created_in_meantime!(cx, unique_type_id); - return MetadataCreationResult::new( + MetadataCreationResult::new( unsafe { llvm::LLVMRustDIBuilderCreateSubroutineType( DIB(cx), @@ -454,7 +454,7 @@ fn subroutine_type_metadata( ) }, false, - ); + ) } // FIXME(1563): This is all a bit of a hack because 'trait pointer' is an ill- @@ -781,7 +781,7 @@ fn file_metadata_raw( let key = (file_name, directory); match debug_context(cx).created_files.borrow_mut().entry(key) { - Entry::Occupied(o) => return o.get(), + Entry::Occupied(o) => o.get(), Entry::Vacant(v) => { let (file_name, directory) = v.key(); debug!("file_metadata: file_name: {:?}, directory: {:?}", file_name, directory); @@ -831,7 +831,7 @@ fn basic_type_metadata(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll DIType { ) }; - return ty_metadata; + ty_metadata } fn foreign_type_metadata( @@ -1273,11 +1273,11 @@ fn prepare_union_metadata( fn use_enum_fallback(cx: &CodegenCx<'_, '_>) -> bool { // On MSVC we have to use the fallback mode, because LLVM doesn't // lower variant parts to PDB. - return cx.sess().target.target.options.is_like_msvc + cx.sess().target.target.options.is_like_msvc // LLVM version 7 did not release with an important bug fix; // but the required patch is in the LLVM 8. Rust LLVM reports // 8 as well. - || llvm_util::get_major_version() < 8; + || llvm_util::get_major_version() < 8 } // FIXME(eddyb) maybe precompute this? Right now it's computed once @@ -2075,7 +2075,7 @@ fn prepare_enum_metadata( } }; - return create_and_register_recursive_type_forward_declaration( + create_and_register_recursive_type_forward_declaration( cx, enum_type, unique_type_id, @@ -2088,7 +2088,7 @@ fn prepare_enum_metadata( containing_scope, span, }), - ); + ) } /// Creates debug information for a composite type, that is, anything that diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index 85decff35b9e0..41829d4ee4256 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -444,7 +444,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { vec![] }; - return create_DIArray(DIB(cx), &template_params[..]); + create_DIArray(DIB(cx), &template_params[..]) } fn get_parameter_names(cx: &CodegenCx<'_, '_>, generics: &ty::Generics) -> Vec { diff --git a/src/librustc_codegen_llvm/debuginfo/utils.rs b/src/librustc_codegen_llvm/debuginfo/utils.rs index bef40decdf3ab..b42d760a77345 100644 --- a/src/librustc_codegen_llvm/debuginfo/utils.rs +++ b/src/librustc_codegen_llvm/debuginfo/utils.rs @@ -24,9 +24,7 @@ pub fn is_node_local_to_unit(cx: &CodegenCx<'_, '_>, def_id: DefId) -> bool { #[allow(non_snake_case)] pub fn create_DIArray(builder: &DIBuilder<'ll>, arr: &[Option<&'ll DIDescriptor>]) -> &'ll DIArray { - return unsafe { - llvm::LLVMRustDIBuilderGetOrCreateArray(builder, arr.as_ptr(), arr.len() as u32) - }; + unsafe { llvm::LLVMRustDIBuilderGetOrCreateArray(builder, arr.as_ptr(), arr.len() as u32) } } #[inline] diff --git a/src/librustc_codegen_llvm/llvm/archive_ro.rs b/src/librustc_codegen_llvm/llvm/archive_ro.rs index ab9df4162472c..64db4f7462df8 100644 --- a/src/librustc_codegen_llvm/llvm/archive_ro.rs +++ b/src/librustc_codegen_llvm/llvm/archive_ro.rs @@ -27,13 +27,13 @@ impl ArchiveRO { /// If this archive is used with a mutable method, then an error will be /// raised. pub fn open(dst: &Path) -> Result { - return unsafe { + unsafe { let s = path_to_c_string(dst); let ar = super::LLVMRustOpenArchive(s.as_ptr()).ok_or_else(|| { super::last_error().unwrap_or_else(|| "failed to open archive".to_owned()) })?; Ok(ArchiveRO { raw: ar }) - }; + } } pub fn iter(&self) -> Iter<'_> { diff --git a/src/librustc_codegen_ssa/back/command.rs b/src/librustc_codegen_ssa/back/command.rs index 30b055b313149..0208bb73abdbe 100644 --- a/src/librustc_codegen_ssa/back/command.rs +++ b/src/librustc_codegen_ssa/back/command.rs @@ -119,7 +119,7 @@ impl Command { for k in &self.env_remove { ret.env_remove(k); } - return ret; + ret } // extensions diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs index e57cae30b7795..5fd16cb121fda 100644 --- a/src/librustc_codegen_ssa/base.rs +++ b/src/librustc_codegen_ssa/base.rs @@ -852,7 +852,7 @@ impl CrateInfo { info.missing_lang_items.insert(cnum, missing); } - return info; + info } } @@ -887,7 +887,7 @@ pub fn provide_both(providers: &mut Providers<'_>) { } } } - return tcx.sess.opts.optimize; + tcx.sess.opts.optimize }; providers.dllimport_foreign_items = |tcx, krate| { diff --git a/src/librustc_data_structures/graph/dominators/mod.rs b/src/librustc_data_structures/graph/dominators/mod.rs index 5283bd78a3029..a7f9340dead88 100644 --- a/src/librustc_data_structures/graph/dominators/mod.rs +++ b/src/librustc_data_structures/graph/dominators/mod.rs @@ -125,9 +125,9 @@ impl<'dom, Node: Idx> Iterator for Iter<'dom, Node> { } else { self.node = Some(dom); } - return Some(node); + Some(node) } else { - return None; + None } } } diff --git a/src/librustc_driver/lib.rs b/src/librustc_driver/lib.rs index 34f0c182499db..e3e076e769f5d 100644 --- a/src/librustc_driver/lib.rs +++ b/src/librustc_driver/lib.rs @@ -752,7 +752,7 @@ impl RustcDefaultCalls { PrintRequest::NativeStaticLibs => {} } } - return Compilation::Stop; + Compilation::Stop } } diff --git a/src/librustc_incremental/assert_module_sources.rs b/src/librustc_incremental/assert_module_sources.rs index 54d7e0ece5031..c5446116f4c50 100644 --- a/src/librustc_incremental/assert_module_sources.rs +++ b/src/librustc_incremental/assert_module_sources.rs @@ -175,6 +175,6 @@ impl AssertModuleSource<'tcx> { return true; } debug!("check_config: no match found"); - return false; + false } } diff --git a/src/librustc_incremental/persist/save.rs b/src/librustc_incremental/persist/save.rs index b465a11c99c06..ba586d0cfba04 100644 --- a/src/librustc_incremental/persist/save.rs +++ b/src/librustc_incremental/persist/save.rs @@ -132,7 +132,6 @@ where } Err(err) => { sess.err(&format!("failed to write dep-graph to `{}`: {}", path_buf.display(), err)); - return; } } } diff --git a/src/librustc_infer/infer/equate.rs b/src/librustc_infer/infer/equate.rs index bb0c124a1892d..8f8fc4f137b73 100644 --- a/src/librustc_infer/infer/equate.rs +++ b/src/librustc_infer/infer/equate.rs @@ -136,7 +136,7 @@ impl TypeRelation<'tcx> for Equate<'combine, 'infcx, 'tcx> { } else { // Fast path for the common case. self.relate(a.skip_binder(), b.skip_binder())?; - return Ok(a.clone()); + Ok(a.clone()) } } } diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/different_lifetimes.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/different_lifetimes.rs index 50b324c72278e..689323ce48346 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/different_lifetimes.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/different_lifetimes.rs @@ -142,6 +142,6 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { .span_label(span_2, String::new()) .span_label(span, span_label) .emit(); - return Some(ErrorReported); + Some(ErrorReported) } } diff --git a/src/librustc_infer/infer/higher_ranked/mod.rs b/src/librustc_infer/infer/higher_ranked/mod.rs index 105b987f85e96..6a5a1c46d4caf 100644 --- a/src/librustc_infer/infer/higher_ranked/mod.rs +++ b/src/librustc_infer/infer/higher_ranked/mod.rs @@ -30,7 +30,7 @@ impl<'a, 'tcx> CombineFields<'a, 'tcx> { let span = self.trace.cause.span; - return self.infcx.commit_if_ok(|snapshot| { + self.infcx.commit_if_ok(|snapshot| { // First, we instantiate each bound region in the supertype with a // fresh placeholder region. let (b_prime, placeholder_map) = self.infcx.replace_bound_vars_with_placeholders(b); @@ -53,7 +53,7 @@ impl<'a, 'tcx> CombineFields<'a, 'tcx> { debug!("higher_ranked_sub: OK result={:?}", result); Ok(ty::Binder::bind(result)) - }); + }) } } diff --git a/src/librustc_infer/infer/lexical_region_resolve/mod.rs b/src/librustc_infer/infer/lexical_region_resolve/mod.rs index 3af10e850d534..821b9f72c0b20 100644 --- a/src/librustc_infer/infer/lexical_region_resolve/mod.rs +++ b/src/librustc_infer/infer/lexical_region_resolve/mod.rs @@ -452,12 +452,10 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { debug!("Expanding value of {:?} from {:?} to {:?}", b_vid, cur_region, lub); *b_data = VarValue::Value(lub); - return true; + true } - VarValue::ErrorValue => { - return false; - } + VarValue::ErrorValue => false, } } @@ -804,7 +802,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { } } - return graph; + graph } fn collect_error_for_expanding_node( diff --git a/src/librustc_infer/infer/nll_relate/mod.rs b/src/librustc_infer/infer/nll_relate/mod.rs index 50bea300c5064..c194e968013eb 100644 --- a/src/librustc_infer/infer/nll_relate/mod.rs +++ b/src/librustc_infer/infer/nll_relate/mod.rs @@ -877,7 +877,7 @@ where // If sub-roots are equal, then `for_vid` and // `vid` are related via subtyping. debug!("TypeGeneralizer::tys: occurs check failed"); - return Err(TypeError::Mismatch); + Err(TypeError::Mismatch) } else { match variables.probe(vid) { TypeVariableValue::Known { value: u } => { @@ -898,7 +898,7 @@ where let u = self.tcx().mk_ty_var(new_var_id); debug!("generalize: replacing original vid={:?} with new={:?}", vid, u); - return Ok(u); + Ok(u) } } } diff --git a/src/librustc_infer/infer/region_constraints/mod.rs b/src/librustc_infer/infer/region_constraints/mod.rs index 868b95043796b..38475b02e5db8 100644 --- a/src/librustc_infer/infer/region_constraints/mod.rs +++ b/src/librustc_infer/infer/region_constraints/mod.rs @@ -505,7 +505,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> { self.undo_log.push(AddVar(vid)); } debug!("created new region variable {:?} in {:?} with origin {:?}", vid, universe, origin); - return vid; + vid } /// Returns the universe for the given variable. diff --git a/src/librustc_lexer/src/lib.rs b/src/librustc_lexer/src/lib.rs index 25334461a113b..d3ac58a49c8d5 100644 --- a/src/librustc_lexer/src/lib.rs +++ b/src/librustc_lexer/src/lib.rs @@ -527,10 +527,10 @@ impl Cursor<'_> { if self.first() == '\'' { self.bump(); let kind = Char { terminated: true }; - return Literal { kind, suffix_start: self.len_consumed() }; + Literal { kind, suffix_start: self.len_consumed() } + } else { + Lifetime { starts_with_number } } - - return Lifetime { starts_with_number }; } fn single_quoted_string(&mut self) -> bool { diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 408031028b102..88f2284cd6154 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -269,7 +269,7 @@ impl EarlyLintPass for UnsafeCode { }) } - _ => return, + _ => {} } } diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 229740615f707..b5826d6a5efa6 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -543,7 +543,7 @@ impl EarlyLintPass for UnusedParens { // Do not lint on `(..)` as that will result in the other arms being useless. Paren(_) // The other cases do not contain sub-patterns. - | Wild | Rest | Lit(..) | MacCall(..) | Range(..) | Ident(.., None) | Path(..) => return, + | Wild | Rest | Lit(..) | MacCall(..) | Range(..) | Ident(.., None) | Path(..) => {}, // These are list-like patterns; parens can always be removed. TupleStruct(_, ps) | Tuple(ps) | Slice(ps) | Or(ps) => for p in ps { self.check_unused_parens_pat(cx, p, false, false); diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index f20cdfcba15ca..9b6e427abc1fd 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -264,7 +264,7 @@ impl<'a> CrateLoader<'a> { ret = Some(cnum); } }); - return ret; + ret } fn verify_no_symbol_conflicts(&self, span: Span, root: &CrateRoot<'_>) { diff --git a/src/librustc_metadata/foreign_modules.rs b/src/librustc_metadata/foreign_modules.rs index fc988ec15cee9..60b8239a82155 100644 --- a/src/librustc_metadata/foreign_modules.rs +++ b/src/librustc_metadata/foreign_modules.rs @@ -6,7 +6,7 @@ use rustc_hir::itemlikevisit::ItemLikeVisitor; crate fn collect(tcx: TyCtxt<'_>) -> Vec { let mut collector = Collector { tcx, modules: Vec::new() }; tcx.hir().krate().visit_all_item_likes(&mut collector); - return collector.modules; + collector.modules } struct Collector<'tcx> { diff --git a/src/librustc_metadata/link_args.rs b/src/librustc_metadata/link_args.rs index 13668b2423fdd..56b26efe5bf1e 100644 --- a/src/librustc_metadata/link_args.rs +++ b/src/librustc_metadata/link_args.rs @@ -16,7 +16,7 @@ crate fn collect(tcx: TyCtxt<'_>) -> Vec { } } - return collector.args; + collector.args } struct Collector { diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index 1ede629e7ef7d..2f9be599ba94b 100644 --- a/src/librustc_metadata/locator.rs +++ b/src/librustc_metadata/locator.rs @@ -949,7 +949,7 @@ fn get_metadata_section( let start = Instant::now(); let ret = get_metadata_section_imp(target, flavor, filename, loader); info!("reading {:?} => {:?}", filename.file_name().unwrap(), start.elapsed()); - return ret; + ret } /// A trivial wrapper for `Mmap` that implements `StableDeref`. diff --git a/src/librustc_metadata/native_libs.rs b/src/librustc_metadata/native_libs.rs index 64bbf393ba0f1..19d2d620f58a7 100644 --- a/src/librustc_metadata/native_libs.rs +++ b/src/librustc_metadata/native_libs.rs @@ -15,7 +15,7 @@ crate fn collect(tcx: TyCtxt<'_>) -> Vec { let mut collector = Collector { tcx, libs: Vec::new() }; tcx.hir().krate().visit_all_item_likes(&mut collector); collector.process_command_line(); - return collector.libs; + collector.libs } crate fn relevant_lib(sess: &Session, lib: &NativeLibrary) -> bool { diff --git a/src/librustc_metadata/rmeta/decoder/cstore_impl.rs b/src/librustc_metadata/rmeta/decoder/cstore_impl.rs index cc2bd51f92f3e..ca75daf1aa9b2 100644 --- a/src/librustc_metadata/rmeta/decoder/cstore_impl.rs +++ b/src/librustc_metadata/rmeta/decoder/cstore_impl.rs @@ -170,7 +170,7 @@ provide! { <'tcx> tcx, def_id, other, cdata, .iter() .filter_map(|&(exported_symbol, export_level)| { if let ExportedSymbol::NonGeneric(def_id) = exported_symbol { - return Some((def_id, export_level)) + Some((def_id, export_level)) } else { None } diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index 9d5cf3ec4bec0..9f4f0ce5620b5 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -273,7 +273,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { assert_eq!(borrow_data.borrowed_place, *place); } - return self.super_rvalue(rvalue, location); + self.super_rvalue(rvalue, location) } } diff --git a/src/librustc_mir/borrow_check/diagnostics/region_name.rs b/src/librustc_mir/borrow_check/diagnostics/region_name.rs index 7103fc596c922..d1d0ba215e08e 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_name.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_name.rs @@ -500,7 +500,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> { } } - return None; + None } /// We've found an enum/struct/union type with the substitutions diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index 28889486c383b..d81aae6523a45 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -56,7 +56,7 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> { self.return_to_block(ret.map(|r| r.1))?; self.dump_place(*dest); - return Ok(true); + Ok(true) } /// "Intercept" a function call to a panic-related function diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index dd0f9ff75b9fe..c98a5e84729ab 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -122,7 +122,7 @@ pub(crate) fn has_rustc_mir_with(attrs: &[ast::Attribute], name: Symbol) -> Opti } } } - return None; + None } pub struct MoveDataParamEnv<'tcx> { @@ -171,7 +171,7 @@ where return None; } } - return None; + None }; let print_preflow_to = name_found(tcx.sess, attributes, sym::borrowck_graphviz_preflow); diff --git a/src/librustc_mir/interpret/operator.rs b/src/librustc_mir/interpret/operator.rs index f2ee5e047a88e..76a5aecb9db62 100644 --- a/src/librustc_mir/interpret/operator.rs +++ b/src/librustc_mir/interpret/operator.rs @@ -64,7 +64,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ge => l >= r, _ => bug!("Invalid operation on char: {:?}", bin_op), }; - return (Scalar::from_bool(res), false, self.tcx.types.bool); + (Scalar::from_bool(res), false, self.tcx.types.bool) } fn binary_bool_op( @@ -87,7 +87,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { BitXor => l ^ r, _ => bug!("Invalid operation on bool: {:?}", bin_op), }; - return (Scalar::from_bool(res), false, self.tcx.types.bool); + (Scalar::from_bool(res), false, self.tcx.types.bool) } fn binary_float_op>>( @@ -113,7 +113,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Rem => ((l % r).value.into(), ty), _ => bug!("invalid float op: `{:?}`", bin_op), }; - return (val, false, ty); + (val, false, ty) } fn binary_int_op( diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 5313446c253c8..933e74ee9ed7e 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -212,9 +212,7 @@ impl<'tcx, Tag> MPlaceTy<'tcx, Tag> { if self.layout.is_unsized() { // We need to consult `meta` metadata match self.layout.ty.kind { - ty::Slice(..) | ty::Str => { - return self.mplace.meta.unwrap_meta().to_machine_usize(cx); - } + ty::Slice(..) | ty::Str => self.mplace.meta.unwrap_meta().to_machine_usize(cx), _ => bug!("len not supported on unsized type {:?}", self.layout.ty), } } else { diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 22a081a9c8e0b..a9e45a032a6be 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -240,7 +240,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match instance.def { ty::InstanceDef::Intrinsic(..) => { assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic); - return M::call_intrinsic(self, span, instance, args, ret, unwind); + M::call_intrinsic(self, span, instance, args, ret, unwind) } ty::InstanceDef::VtableShim(..) | ty::InstanceDef::ReifyShim(..) diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 4dd037d93ce9b..a592e8d9c05fe 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -751,7 +751,7 @@ fn should_monomorphize_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: &Instance<'tcx bug!("cannot create local mono-item for {:?}", def_id) } - return true; + true } /// For a given pair of source and target type that occur in an unsizing coercion, diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index ca23c44f64668..43876380c840e 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -483,7 +483,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { err.span_label(source_info.span, format!("{:?}", panic)); err.emit() }); - return None; + None } fn check_unary_op( diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index 821c4d68c7e8a..f35b50d484bff 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -43,8 +43,7 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { .. }) | Node::TraitItem(hir::TraitItem { - kind: - hir::TraitItemKind::Fn(hir::FnSig { decl, .. }, hir::TraitFn::Provided(body_id)), + kind: hir::TraitItemKind::Fn(hir::FnSig { decl, .. }, hir::TraitFn::Provided(body_id)), .. }) => (*body_id, decl.output.span()), Node::Item(hir::Item { kind: hir::ItemKind::Static(ty, _, body_id), .. }) @@ -368,7 +367,7 @@ impl BlockContext { } } - return None; + None } /// Looks at the topmost frame on the BlockContext and reports diff --git a/src/librustc_mir_build/hair/cx/block.rs b/src/librustc_mir_build/hair/cx/block.rs index 8d7225c8c7b51..07a9d91cd746d 100644 --- a/src/librustc_mir_build/hair/cx/block.rs +++ b/src/librustc_mir_build/hair/cx/block.rs @@ -98,7 +98,7 @@ fn mirror_stmts<'a, 'tcx>( } } } - return result; + result } crate fn to_expr_ref<'a, 'tcx>( diff --git a/src/librustc_parse/lib.rs b/src/librustc_parse/lib.rs index c31cc1b4c9f00..58db7d286e7e6 100644 --- a/src/librustc_parse/lib.rs +++ b/src/librustc_parse/lib.rs @@ -320,7 +320,7 @@ pub fn nt_to_tokenstream(nt: &Nonterminal, sess: &ParseSess, span: Span) -> Toke going with stringified version" ); } - return tokens_for_real; + tokens_for_real } fn prepend_attrs( diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs index c65e99842c5dd..5dac461441d64 100644 --- a/src/librustc_parse/parser/expr.rs +++ b/src/librustc_parse/parser/expr.rs @@ -996,7 +996,7 @@ impl<'a> Parser<'a> { let expr = self.mk_expr(lo.to(self.prev_token.span), ExprKind::Lit(literal), attrs); self.maybe_recover_from_bad_qpath(expr, true) } - None => return Err(self.expected_expression_found()), + None => Err(self.expected_expression_found()), } } @@ -1713,7 +1713,7 @@ impl<'a> Parser<'a> { } let hi = self.token.span; self.bump(); - return Ok(self.mk_expr(lo.to(hi), ExprKind::Match(scrutinee, arms), attrs)); + Ok(self.mk_expr(lo.to(hi), ExprKind::Match(scrutinee, arms), attrs)) } pub(super) fn parse_arm(&mut self) -> PResult<'a, Arm> { diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 9d70f606f3ef4..2b1799f5c4885 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -314,7 +314,7 @@ impl<'a> Parser<'a> { " struct ".into(), Applicability::MaybeIncorrect, // speculative ); - return Err(err); + Err(err) } else if self.look_ahead(1, |t| *t == token::OpenDelim(token::Paren)) { let ident = self.parse_ident().unwrap(); self.bump(); // `(` @@ -362,7 +362,7 @@ impl<'a> Parser<'a> { ); } } - return Err(err); + Err(err) } else if self.look_ahead(1, |t| *t == token::Lt) { let ident = self.parse_ident().unwrap(); self.eat_to_tokens(&[&token::Gt]); @@ -384,7 +384,7 @@ impl<'a> Parser<'a> { Applicability::MachineApplicable, ); } - return Err(err); + Err(err) } else { Ok(()) } @@ -910,7 +910,7 @@ impl<'a> Parser<'a> { let span = self.sess.source_map().def_span(span); let msg = format!("{} is not supported in {}", kind.descr(), ctx); self.struct_span_err(span, &msg).emit(); - return None; + None } fn error_on_foreign_const(&self, span: Span, ident: Ident) { diff --git a/src/librustc_parse/parser/pat.rs b/src/librustc_parse/parser/pat.rs index 4585941943b74..6b987ff1cce68 100644 --- a/src/librustc_parse/parser/pat.rs +++ b/src/librustc_parse/parser/pat.rs @@ -918,7 +918,7 @@ impl<'a> Parser<'a> { } err.emit(); } - return Ok((fields, etc)); + Ok((fields, etc)) } /// Recover on `...` as if it were `..` to avoid further errors. diff --git a/src/librustc_parse/parser/stmt.rs b/src/librustc_parse/parser/stmt.rs index d40597d8fcb0c..d43f5d67113a1 100644 --- a/src/librustc_parse/parser/stmt.rs +++ b/src/librustc_parse/parser/stmt.rs @@ -278,7 +278,7 @@ impl<'a> Parser<'a> { _ => {} } e.span_label(sp, "expected `{`"); - return Err(e); + Err(e) } /// Parses a block. Inner attributes are allowed. diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index 70b106f5d2332..bf577d26b0fed 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -864,7 +864,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { first_merge, any_changed ); - return any_changed; + any_changed } // Indicates that a local variable was *defined*; we know that no diff --git a/src/librustc_passes/loops.rs b/src/librustc_passes/loops.rs index 1daef45a1f591..fa2afae469c04 100644 --- a/src/librustc_passes/loops.rs +++ b/src/librustc_passes/loops.rs @@ -222,7 +222,7 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> { return true; } } - return false; + false } fn emit_unlabled_cf_in_while_condition(&mut self, span: Span, cf_type: &str) { struct_span_err!( diff --git a/src/librustc_passes/reachable.rs b/src/librustc_passes/reachable.rs index 835e7cfb62816..1e9c6c91d385f 100644 --- a/src/librustc_passes/reachable.rs +++ b/src/librustc_passes/reachable.rs @@ -30,9 +30,7 @@ fn item_might_be_inlined(tcx: TyCtxt<'tcx>, item: &hir::Item<'_>, attrs: Codegen } match item.kind { - hir::ItemKind::Fn(ref sig, ..) if sig.header.is_const() => { - return true; - } + hir::ItemKind::Fn(ref sig, ..) if sig.header.is_const() => true, hir::ItemKind::Impl { .. } | hir::ItemKind::Fn(..) => { let generics = tcx.generics_of(tcx.hir().local_def_id(item.hir_id)); generics.requires_monomorphization(tcx) diff --git a/src/librustc_passes/stability.rs b/src/librustc_passes/stability.rs index 11311a3e8aa68..8fa5a4fbc61f4 100644 --- a/src/librustc_passes/stability.rs +++ b/src/librustc_passes/stability.rs @@ -465,7 +465,7 @@ fn new_index(tcx: TyCtxt<'tcx>) -> Index<'tcx> { |v| intravisit::walk_crate(v, krate), ); } - return index; + index } /// Cross-references the feature names of unstable APIs with enabled diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index c8c8c2299305b..a3510737b7edc 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1423,7 +1423,7 @@ impl<'a, 'tcx> ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { Some(_) | None => false, } } else { - return false; + false } } @@ -1837,7 +1837,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'tcx> { && self.tcx.is_private_dep(item_id.krate); log::debug!("leaks_private_dep(item_id={:?})={}", item_id, ret); - return ret; + ret } } diff --git a/src/librustc_resolve/def_collector.rs b/src/librustc_resolve/def_collector.rs index 505cd331a2509..0dee997f2ed96 100644 --- a/src/librustc_resolve/def_collector.rs +++ b/src/librustc_resolve/def_collector.rs @@ -200,7 +200,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { fn visit_pat(&mut self, pat: &'a Pat) { match pat.kind { - PatKind::MacCall(..) => return self.visit_macro_invoc(pat.id), + PatKind::MacCall(..) => self.visit_macro_invoc(pat.id), _ => visit::walk_pat(self, pat), } } diff --git a/src/librustc_resolve/imports.rs b/src/librustc_resolve/imports.rs index 663e61ad2add4..95597e8ebf1e9 100644 --- a/src/librustc_resolve/imports.rs +++ b/src/librustc_resolve/imports.rs @@ -1108,7 +1108,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> { match binding.kind { // Never suggest the name that has binding error // i.e., the name that cannot be previously resolved - NameBindingKind::Res(Res::Err, _) => return None, + NameBindingKind::Res(Res::Err, _) => None, _ => Some(&i.name), } } diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 41380b2a4b78a..e1256551e24da 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -380,7 +380,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { _ => (), } }; - return has_self_arg; + has_self_arg } fn followed_by_brace(&self, span: Span) -> (bool, Option<(Span, String)>) { @@ -430,7 +430,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { break; } } - return (followed_by_brace, closing_brace); + (followed_by_brace, closing_brace) } /// Provides context-dependent help for errors reported by the `smart_resolve_path_fragment` diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 6f2e0bce3acaf..166fc48b44c4c 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -83,7 +83,7 @@ fn sub_namespace_match(candidate: Option, requirement: Option Symbol { if path.segments.len() == 1 { - return path.segments[0].ident.name; + path.segments[0].ident.name } else { let mut path_str = String::with_capacity(64); for (i, segment) in path.segments.iter().enumerate() { diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index 98d81c6252242..59084f1904545 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -534,7 +534,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> { let variant = &def.non_enum_variant(); filter!(self.span_utils, ident.span); let span = self.span_from_span(ident.span); - return Some(Data::RefData(Ref { + Some(Data::RefData(Ref { kind: RefKind::Variable, span, ref_id: self @@ -542,7 +542,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> { .find_field_index(ident, variant) .map(|index| id_from_def_id(variant.fields[index].did)) .unwrap_or_else(|| null_id()), - })); + })) } ty::Tuple(..) => None, _ => { diff --git a/src/librustc_session/config.rs b/src/librustc_session/config.rs index c273e7fdbf916..f16e4ca93d80a 100644 --- a/src/librustc_session/config.rs +++ b/src/librustc_session/config.rs @@ -1140,7 +1140,7 @@ pub fn parse_error_format( _ => {} } - return error_format; + error_format } fn parse_crate_edition(matches: &getopts::Matches) -> Edition { diff --git a/src/librustc_span/caching_source_map_view.rs b/src/librustc_span/caching_source_map_view.rs index d6725160a5d02..68b0bd1a57418 100644 --- a/src/librustc_span/caching_source_map_view.rs +++ b/src/librustc_span/caching_source_map_view.rs @@ -99,10 +99,6 @@ impl<'sm> CachingSourceMapView<'sm> { cache_entry.line_end = line_bounds.1; cache_entry.time_stamp = self.time_stamp; - return Some(( - cache_entry.file.clone(), - cache_entry.line_number, - pos - cache_entry.line_start, - )); + Some((cache_entry.file.clone(), cache_entry.line_number, pos - cache_entry.line_start)) } } diff --git a/src/librustc_span/source_map.rs b/src/librustc_span/source_map.rs index 7dd9e2f6316b4..39eb318a785e7 100644 --- a/src/librustc_span/source_map.rs +++ b/src/librustc_span/source_map.rs @@ -368,7 +368,7 @@ impl SourceMap { // If there is a doctest offset, applies it to the line. pub fn doctest_offset_line(&self, file: &FileName, orig: usize) -> usize { - return match file { + match file { FileName::DocTest(_, offset) => { return if *offset >= 0 { orig + *offset as usize @@ -377,7 +377,7 @@ impl SourceMap { }; } _ => orig, - }; + } } /// Looks up source information about a `BytePos`. @@ -569,10 +569,10 @@ impl SourceMap { let local_end = self.lookup_byte_offset(sp.hi()); if local_begin.sf.start_pos != local_end.sf.start_pos { - return Err(SpanSnippetError::DistinctSources(DistinctSources { + Err(SpanSnippetError::DistinctSources(DistinctSources { begin: (local_begin.sf.name.clone(), local_begin.sf.start_pos), end: (local_end.sf.name.clone(), local_end.sf.start_pos), - })); + })) } else { self.ensure_source_file_source_present(local_begin.sf.clone()); @@ -590,13 +590,11 @@ impl SourceMap { } if let Some(ref src) = local_begin.sf.src { - return extract_source(src, start_index, end_index); + extract_source(src, start_index, end_index) } else if let Some(src) = local_begin.sf.external_src.borrow().get_source() { - return extract_source(src, start_index, end_index); + extract_source(src, start_index, end_index) } else { - return Err(SpanSnippetError::SourceNotAvailable { - filename: local_begin.sf.name.clone(), - }); + Err(SpanSnippetError::SourceNotAvailable { filename: local_begin.sf.name.clone() }) } } } diff --git a/src/librustc_trait_selection/traits/auto_trait.rs b/src/librustc_trait_selection/traits/auto_trait.rs index d221d6886e9fb..bd980e6eb8b8a 100644 --- a/src/librustc_trait_selection/traits/auto_trait.rs +++ b/src/librustc_trait_selection/traits/auto_trait.rs @@ -113,7 +113,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { return AutoTraitResult::ExplicitImpl; } - return tcx.infer_ctxt().enter(|infcx| { + tcx.infer_ctxt().enter(|infcx| { let mut fresh_preds = FxHashSet::default(); // Due to the way projections are handled by SelectionContext, we need to run @@ -219,8 +219,8 @@ impl<'tcx> AutoTraitFinder<'tcx> { let info = AutoTraitInfo { full_user_env, region_data, vid_to_region }; - return AutoTraitResult::PositiveImpl(auto_trait_callback(&infcx, info)); - }); + AutoTraitResult::PositiveImpl(auto_trait_callback(&infcx, info)) + }) } } @@ -384,7 +384,7 @@ impl AutoTraitFinder<'tcx> { ty, trait_did, new_env, final_user_env ); - return Some((new_env, final_user_env)); + Some((new_env, final_user_env)) } /// This method is designed to work around the following issue: @@ -492,7 +492,7 @@ impl AutoTraitFinder<'tcx> { } _ => {} } - return true; + true }); if should_add_new { @@ -591,15 +591,15 @@ impl AutoTraitFinder<'tcx> { } fn is_param_no_infer(&self, substs: SubstsRef<'_>) -> bool { - return self.is_of_param(substs.type_at(0)) && !substs.types().any(|t| t.has_infer_types()); + self.is_of_param(substs.type_at(0)) && !substs.types().any(|t| t.has_infer_types()) } pub fn is_of_param(&self, ty: Ty<'_>) -> bool { - return match ty.kind { + match ty.kind { ty::Param(_) => true, ty::Projection(p) => self.is_of_param(p.self_ty()), _ => false, - }; + } } fn is_self_referential_projection(&self, p: ty::PolyProjectionPredicate<'_>) -> bool { @@ -804,7 +804,7 @@ impl AutoTraitFinder<'tcx> { _ => panic!("Unexpected predicate {:?} {:?}", ty, predicate), }; } - return true; + true } pub fn clean_pred( diff --git a/src/librustc_trait_selection/traits/coherence.rs b/src/librustc_trait_selection/traits/coherence.rs index 5f542e7e13be5..dc13af99fec6c 100644 --- a/src/librustc_trait_selection/traits/coherence.rs +++ b/src/librustc_trait_selection/traits/coherence.rs @@ -221,10 +221,10 @@ pub fn trait_ref_is_knowable<'tcx>( // we are an owner. if orphan_check_trait_ref(tcx, trait_ref, InCrate::Local).is_ok() { debug!("trait_ref_is_knowable: orphan check passed"); - return None; + None } else { debug!("trait_ref_is_knowable: nonlocal, nonfundamental, unowned"); - return Some(Conflict::Upstream); + Some(Conflict::Upstream) } } diff --git a/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs b/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs index 3d0dd73f03c18..18b2ca8983720 100644 --- a/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs +++ b/src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs @@ -111,11 +111,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }), hir::Node::Expr(hir::Expr { .. }) => { let parent_hid = hir.get_parent_node(hir_id); - if parent_hid != hir_id { - return self.describe_enclosure(parent_hid); - } else { - None - } + if parent_hid != hir_id { self.describe_enclosure(parent_hid) } else { None } } _ => None, } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 0523a2019861c..522a8084cdc0d 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -351,7 +351,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // Different to previous arm because one is `&hir::Local` and the other // is `P`. Some(hir::Node::Local(local)) => get_name(err, &local.pat.kind), - _ => return None, + _ => None, } } diff --git a/src/librustc_trait_selection/traits/query/normalize.rs b/src/librustc_trait_selection/traits/query/normalize.rs index adec2ddb25322..99412fafcfa8d 100644 --- a/src/librustc_trait_selection/traits/query/normalize.rs +++ b/src/librustc_trait_selection/traits/query/normalize.rs @@ -169,12 +169,12 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> { debug!("QueryNormalizer: result = {:#?}", result); debug!("QueryNormalizer: obligations = {:#?}", obligations); self.obligations.extend(obligations); - return result.normalized_ty; + result.normalized_ty } Err(_) => { self.error = true; - return ty; + ty } } } diff --git a/src/librustc_trait_selection/traits/wf.rs b/src/librustc_trait_selection/traits/wf.rs index b69c5bdce2abc..5f40c1cefca45 100644 --- a/src/librustc_trait_selection/traits/wf.rs +++ b/src/librustc_trait_selection/traits/wf.rs @@ -595,7 +595,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { } // if we made it through that loop above, we made progress! - return true; + true } fn nominal_obligations( diff --git a/src/librustc_traits/lowering/environment.rs b/src/librustc_traits/lowering/environment.rs index 69d0bd0929687..ed6259d457361 100644 --- a/src/librustc_traits/lowering/environment.rs +++ b/src/librustc_traits/lowering/environment.rs @@ -146,7 +146,7 @@ crate fn program_clauses_for_env<'tcx>( debug!("program_clauses_for_env: closure = {:#?}", closure); - return tcx.mk_clauses(closure.into_iter()); + tcx.mk_clauses(closure.into_iter()) } crate fn environment(tcx: TyCtxt<'_>, def_id: DefId) -> Environment<'_> { diff --git a/src/librustc_ty/needs_drop.rs b/src/librustc_ty/needs_drop.rs index 0f71246c73759..3b72da23bafae 100644 --- a/src/librustc_ty/needs_drop.rs +++ b/src/librustc_ty/needs_drop.rs @@ -132,7 +132,7 @@ where } } - return None; + None } } diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 9a8d161572bcf..3ee6d5df7356b 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -2128,7 +2128,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { return Err(ErrorReported); } } - return Ok(bound); + Ok(bound) } fn complain_about_assoc_type_not_found( @@ -2709,7 +2709,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { } Res::Err => { self.set_tainted_by_errors(); - return self.tcx().types.err; + self.tcx().types.err } _ => span_bug!(span, "unexpected resolution: {:?}", path.res), } @@ -3047,7 +3047,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { ) .emit(); } - return Some(r); + Some(r) } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 368f64e4d41aa..d194223117195 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1580,7 +1580,7 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: DefId) -> bool { } else { span_bug!(span, "unions must be ty::Adt, but got {:?}", item_type.kind); } - return true; + true } /// Checks that an opaque type does not contain cycles and does not use `Self` or `T::Foo` @@ -2313,7 +2313,7 @@ fn check_representable(tcx: TyCtxt<'_>, sp: Span, item_def_id: DefId) -> bool { } Representability::Representable | Representability::ContainsRecursive => (), } - return true; + true } pub fn check_simd(tcx: TyCtxt<'_>, sp: Span, def_id: DefId) { diff --git a/src/librustc_typeck/coherence/inherent_impls.rs b/src/librustc_typeck/coherence/inherent_impls.rs index 60e5df68b5842..c6ee9ab60abf3 100644 --- a/src/librustc_typeck/coherence/inherent_impls.rs +++ b/src/librustc_typeck/coherence/inherent_impls.rs @@ -272,9 +272,7 @@ impl ItemLikeVisitor<'v> for InherentCollect<'tcx> { item.span, ); } - ty::Error => { - return; - } + ty::Error => {} _ => { struct_span_err!( self.tcx.sess, @@ -288,7 +286,6 @@ impl ItemLikeVisitor<'v> for InherentCollect<'tcx> { to wrap it instead", ) .emit(); - return; } } } diff --git a/src/librustc_typeck/collect/type_of.rs b/src/librustc_typeck/collect/type_of.rs index 41c205bc11b35..44ef4ebd463d7 100644 --- a/src/librustc_typeck/collect/type_of.rs +++ b/src/librustc_typeck/collect/type_of.rs @@ -290,7 +290,7 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> { DUMMY_SP, &format!("unexpected const parent path {:?}", parent_node,), ); - return tcx.types.err; + tcx.types.err } } diff --git a/src/libstd/backtrace.rs b/src/libstd/backtrace.rs index 34317c7a2ee3d..e10d466030f0b 100644 --- a/src/libstd/backtrace.rs +++ b/src/libstd/backtrace.rs @@ -250,7 +250,7 @@ impl Backtrace { }, }; ENABLED.store(enabled as usize + 1, SeqCst); - return enabled; + enabled } /// Capture a stack backtrace of the current thread. diff --git a/src/libstd/sys_common/backtrace.rs b/src/libstd/sys_common/backtrace.rs index 2c7ba8f8ea1fd..e9b1e86d7ae49 100644 --- a/src/libstd/sys_common/backtrace.rs +++ b/src/libstd/sys_common/backtrace.rs @@ -28,7 +28,7 @@ pub fn lock() -> impl Drop { unsafe { LOCK.lock(); - return Guard; + Guard } }