From eb5b09688696562d08ebb35e34ded723e6e44277 Mon Sep 17 00:00:00 2001 From: Matthieu M Date: Sun, 3 Feb 2019 16:58:29 +0100 Subject: [PATCH 01/49] RangeInclusive internal iteration performance improvement. Specialize Iterator::try_fold and DoubleEndedIterator::try_rfold to improve code generation in all internal iteration scenarios. This changes brings the performance of internal iteration with RangeInclusive on par with the performance of iteration with Range: - Single conditional jump in hot loop, - Unrolling and vectorization, - And even Closed Form substitution. Unfortunately, it only applies to internal iteration. Despite various attempts at stream-lining the implementation of next and next_back, LLVM has stubbornly refused to optimize external iteration appropriately, leaving me with a choice between: - The current implementation, for which Closed Form substitution is performed, but which uses 2 conditional jumps in the hot loop when optimization fail. - An implementation using a "is_done" boolean, which uses 1 conditional jump in the hot loop when optimization fail, allowing unrolling and vectorization, but for which Closed Form substitution fails. In the absence of any conclusive evidence as to which usecase matters most, and with no assurance that the lack of Closed Form substitution is not indicative of other optimizations being foiled, there is no way to pick one implementation over the other, and thus I defer to the statu quo as far as next and next_back are concerned. --- src/libcore/iter/range.rs | 51 ++++++++++++++++++++++++++++++++++++--- src/libcore/ops/range.rs | 2 ++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/libcore/iter/range.rs b/src/libcore/iter/range.rs index 66c09a0ddd0fb..52b0ccd48d476 100644 --- a/src/libcore/iter/range.rs +++ b/src/libcore/iter/range.rs @@ -1,6 +1,6 @@ use convert::TryFrom; use mem; -use ops::{self, Add, Sub}; +use ops::{self, Add, Sub, Try}; use usize; use super::{FusedIterator, TrustedLen}; @@ -368,11 +368,11 @@ impl Iterator for ops::RangeInclusive { Some(Less) => { self.is_empty = Some(false); self.start = plus_n.add_one(); - return Some(plus_n) + return Some(plus_n); } Some(Equal) => { self.is_empty = Some(true); - return Some(plus_n) + return Some(plus_n); } _ => {} } @@ -382,6 +382,29 @@ impl Iterator for ops::RangeInclusive { None } + #[inline] + fn try_fold(&mut self, init: B, mut f: F) -> R + where + Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try + { + self.compute_is_empty(); + + let mut accum = init; + + while self.start < self.end { + let n = self.start.add_one(); + let n = mem::replace(&mut self.start, n); + accum = f(accum, n)?; + } + + if self.start == self.end { + accum = f(accum, self.start.clone())?; + } + + self.is_empty = Some(true); + Try::from_ok(accum) + } + #[inline] fn last(mut self) -> Option { self.next_back() @@ -415,6 +438,28 @@ impl DoubleEndedIterator for ops::RangeInclusive { self.end.clone() }) } + + #[inline] + fn try_rfold(&mut self, init: B, mut f: F) -> R where + Self: Sized, F: FnMut(B, Self::Item) -> R, R: Try + { + self.compute_is_empty(); + + let mut accum = init; + + while self.start < self.end { + let n = self.end.sub_one(); + let n = mem::replace(&mut self.end, n); + accum = f(accum, n)?; + } + + if self.start == self.end { + accum = f(accum, self.start.clone())?; + } + + self.is_empty = Some(true); + Try::from_ok(accum) + } } #[stable(feature = "fused", since = "1.26.0")] diff --git a/src/libcore/ops/range.rs b/src/libcore/ops/range.rs index 815a4cfeed88e..6776ebdc66edc 100644 --- a/src/libcore/ops/range.rs +++ b/src/libcore/ops/range.rs @@ -334,12 +334,14 @@ pub struct RangeInclusive { trait RangeInclusiveEquality: Sized { fn canonicalized_is_empty(range: &RangeInclusive) -> bool; } + impl RangeInclusiveEquality for T { #[inline] default fn canonicalized_is_empty(range: &RangeInclusive) -> bool { range.is_empty.unwrap_or_default() } } + impl RangeInclusiveEquality for T { #[inline] fn canonicalized_is_empty(range: &RangeInclusive) -> bool { From a15916b245b9a9d92ee77e3a995ff6bfd6c502fc Mon Sep 17 00:00:00 2001 From: Clint Frederickson Date: Tue, 5 Feb 2019 13:56:29 -0700 Subject: [PATCH 02/49] [WIP] add better error message for partial move --- src/librustc_mir/borrow_check/error_reporting.rs | 7 +++++-- .../ui/borrowck/borrowck-uninit-field-access.mir.stderr | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index b070031756798..53b486cc6e316 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -130,6 +130,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); let mut is_loop_move = false; + let mut is_partial_move = false; for move_site in &move_site_vec { let move_out = self.move_data.moves[(*move_site).moi]; let moved_place = &self.move_data.move_paths[move_out.path].place; @@ -137,6 +138,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let move_spans = self.move_spans(moved_place, move_out.source); let move_span = move_spans.args_or_use(); + is_partial_move = used_place.is_prefix_of(moved_place); let move_msg = if move_spans.for_closure() { " into closure" } else { @@ -175,8 +177,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.span_label( span, format!( - "value {} here after move", - desired_action.as_verb_in_past_tense() + "value {} here {}", + desired_action.as_verb_in_past_tense(), + if is_partial_move { "after partial move" } else { "after move" }, ), ); } diff --git a/src/test/ui/borrowck/borrowck-uninit-field-access.mir.stderr b/src/test/ui/borrowck/borrowck-uninit-field-access.mir.stderr index bdec94b4f84b0..99cbf64fd5d46 100644 --- a/src/test/ui/borrowck/borrowck-uninit-field-access.mir.stderr +++ b/src/test/ui/borrowck/borrowck-uninit-field-access.mir.stderr @@ -20,7 +20,7 @@ error[E0382]: use of moved value: `line2` LL | let _moved = (line2.origin, line2.middle); | ------------ value moved here LL | line2.consume(); //[ast]~ ERROR use of partially moved value: `line2` [E0382] - | ^^^^^ value used here after move + | ^^^^^ value used here after partial move | = note: move occurs because `line2.middle` has type `Point`, which does not implement the `Copy` trait From d4c52bfb1708bf1c53ea2fc722aa4d9762982e90 Mon Sep 17 00:00:00 2001 From: Clint Frederickson Date: Wed, 6 Feb 2019 08:42:21 -0700 Subject: [PATCH 03/49] error output updated by ./x.py test --stage 1 src/test/ui --incremental --bless --- src/test/ui/borrowck/borrowck-asm.mir.stderr | 4 ++-- .../ui/borrowck/borrowck-describe-lvalue.mir.stderr | 2 +- src/test/ui/borrowck/borrowck-drop-from-guard.stderr | 2 +- src/test/ui/borrowck/borrowck-issue-48962.stderr | 4 ++-- .../borrowck-move-moved-value-into-closure.mir.stderr | 2 +- .../borrowck/borrowck-move-out-from-array.mir.stderr | 4 ++-- src/test/ui/borrowck/borrowck-reinit.stderr | 2 +- ...499-field-mutation-of-moved-out-with-mut.nll.stderr | 6 +++--- .../issue-54499-field-mutation-of-moved-out.nll.stderr | 6 +++--- .../ui/borrowck/two-phase-nonrecv-autoref.nll.stderr | 4 ++-- src/test/ui/issues/issue-29723.stderr | 2 +- src/test/ui/issues/issue-34721.stderr | 2 +- src/test/ui/moves/moves-based-on-type-tuple.stderr | 2 +- src/test/ui/nll/closure-access-spans.stderr | 8 ++++---- src/test/ui/nll/closure-move-spans.stderr | 6 +++--- src/test/ui/nll/decl-macro-illegal-copy.stderr | 2 +- .../issue-21232-partial-init-and-erroneous-use.stderr | 2 +- .../ui/nll/issue-21232-partial-init-and-use.stderr | 10 +++++----- src/test/ui/nll/move-subpaths-moves-root.stderr | 2 +- src/test/ui/try-block/try-block-bad-lifetime.stderr | 2 +- .../ui/try-block/try-block-maybe-bad-lifetime.stderr | 2 +- 21 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/test/ui/borrowck/borrowck-asm.mir.stderr b/src/test/ui/borrowck/borrowck-asm.mir.stderr index 86e4832b3873c..71b4dbe2aa0c9 100644 --- a/src/test/ui/borrowck/borrowck-asm.mir.stderr +++ b/src/test/ui/borrowck/borrowck-asm.mir.stderr @@ -8,7 +8,7 @@ LL | asm!("nop" : : "r"(x)); | - value moved here LL | } LL | let z = x; //[ast]~ ERROR use of moved value: `x` - | ^ value used here after move + | ^ value used here after partial move error[E0503]: cannot use `x` because it was mutably borrowed --> $DIR/borrowck-asm.rs:35:32 @@ -71,7 +71,7 @@ LL | let x = &mut 2; | - move occurs because `x` has type `&mut i32`, which does not implement the `Copy` trait LL | unsafe { LL | asm!("nop" : : "r"(x), "r"(x) ); //[ast]~ ERROR use of moved value - | - ^ value used here after move + | - ^ value used here after partial move | | | value moved here diff --git a/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr b/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr index 279548f870fd0..52cb98333ac2e 100644 --- a/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr +++ b/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr @@ -367,7 +367,7 @@ error[E0382]: use of moved value: `x` LL | drop(x); | - value moved here LL | drop(x); //[ast]~ ERROR use of moved value: `x` - | ^ value used here after move + | ^ value used here after partial move | = note: move occurs because `x` has type `std::vec::Vec`, which does not implement the `Copy` trait diff --git a/src/test/ui/borrowck/borrowck-drop-from-guard.stderr b/src/test/ui/borrowck/borrowck-drop-from-guard.stderr index 07b597f480feb..c2db8b5830f55 100644 --- a/src/test/ui/borrowck/borrowck-drop-from-guard.stderr +++ b/src/test/ui/borrowck/borrowck-drop-from-guard.stderr @@ -8,7 +8,7 @@ LL | Some(_) if { drop(my_str); false } => {} | ------ value moved here LL | Some(_) => {} LL | None => { foo(my_str); } //~ ERROR [E0382] - | ^^^^^^ value used here after move + | ^^^^^^ value used here after partial move error: aborting due to previous error diff --git a/src/test/ui/borrowck/borrowck-issue-48962.stderr b/src/test/ui/borrowck/borrowck-issue-48962.stderr index de4894d5b526b..d0105e311bc69 100644 --- a/src/test/ui/borrowck/borrowck-issue-48962.stderr +++ b/src/test/ui/borrowck/borrowck-issue-48962.stderr @@ -6,7 +6,7 @@ LL | let mut src = &mut node; LL | {src}; | --- value moved here LL | src.next = None; //~ ERROR use of moved value: `src` [E0382] - | ^^^^^^^^ value used here after move + | ^^^^^^^^ value used here after partial move error[E0382]: use of moved value: `src` --> $DIR/borrowck-issue-48962.rs:22:5 @@ -16,7 +16,7 @@ LL | let mut src = &mut (22, 44); LL | {src}; | --- value moved here LL | src.0 = 66; //~ ERROR use of moved value: `src` [E0382] - | ^^^^^^^^^^ value used here after move + | ^^^^^^^^^^ value used here after partial move error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr b/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr index 0789926563ce7..e362fcb7d54cd 100644 --- a/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr +++ b/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr @@ -11,7 +11,7 @@ LL | call_f(move|| { *t + 1 }); LL | call_f(move|| { *t + 1 }); //[ast]~ ERROR capture of moved value | ^^^^^^ - use occurs due to use in closure | | - | value used here after move + | value used here after partial move error: aborting due to previous error diff --git a/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr b/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr index f866ff9e9bae1..e366d7b95c368 100644 --- a/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr +++ b/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr @@ -4,7 +4,7 @@ error[E0382]: use of moved value: `a[..]` LL | let [_, _x] = a; | -- value moved here LL | let [.., _y] = a; //[ast]~ ERROR [E0382] - | ^^ value used here after move + | ^^ value used here after partial move | = note: move occurs because `a[..]` has type `std::boxed::Box`, which does not implement the `Copy` trait @@ -14,7 +14,7 @@ error[E0382]: use of moved value: `a[..]` LL | let [_x, _] = a; | -- value moved here LL | let [_y..] = a; //[ast]~ ERROR [E0382] - | ^^ value used here after move + | ^^ value used here after partial move | = note: move occurs because `a[..]` has type `std::boxed::Box`, which does not implement the `Copy` trait diff --git a/src/test/ui/borrowck/borrowck-reinit.stderr b/src/test/ui/borrowck/borrowck-reinit.stderr index 96f3981ac2fe6..351fbab2d3f09 100644 --- a/src/test/ui/borrowck/borrowck-reinit.stderr +++ b/src/test/ui/borrowck/borrowck-reinit.stderr @@ -17,7 +17,7 @@ LL | let mut x = Box::new(0); LL | drop(x); | - value moved here LL | let _ = (1,x); //~ ERROR use of moved value: `x` (Ast) - | ^ value used here after move + | ^ value used here after partial move error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr index 42aa038170238..bdf99b0906e6f 100644 --- a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr +++ b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr @@ -6,7 +6,7 @@ LL | let mut t: Tuple = (S(0), 0); LL | drop(t); | - value moved here LL | t.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after move + | ^^^^^^^^^^ value partially assigned here after partial move error[E0382]: assign to part of moved value: `u` --> $DIR/issue-54499-field-mutation-of-moved-out-with-mut.rs:34:9 @@ -16,7 +16,7 @@ LL | let mut u: Tpair = Tpair(S(0), 0); LL | drop(u); | - value moved here LL | u.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after move + | ^^^^^^^^^^ value partially assigned here after partial move error[E0382]: assign to part of moved value: `v` --> $DIR/issue-54499-field-mutation-of-moved-out-with-mut.rs:45:9 @@ -26,7 +26,7 @@ LL | let mut v: Spair = Spair { x: S(0), y: 0 }; LL | drop(v); | - value moved here LL | v.x = S(1); - | ^^^^^^^^^^ value partially assigned here after move + | ^^^^^^^^^^ value partially assigned here after partial move error: aborting due to 3 previous errors diff --git a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr index 1184907f307cb..9505f305c6151 100644 --- a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr +++ b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr @@ -15,7 +15,7 @@ LL | let t: Tuple = (S(0), 0); LL | drop(t); | - value moved here LL | t.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after move + | ^^^^^^^^^^ value partially assigned here after partial move error[E0594]: cannot assign to `t.1`, as `t` is not declared as mutable --> $DIR/issue-54499-field-mutation-of-moved-out.rs:27:9 @@ -43,7 +43,7 @@ LL | let u: Tpair = Tpair(S(0), 0); LL | drop(u); | - value moved here LL | u.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after move + | ^^^^^^^^^^ value partially assigned here after partial move error[E0594]: cannot assign to `u.1`, as `u` is not declared as mutable --> $DIR/issue-54499-field-mutation-of-moved-out.rs:42:9 @@ -71,7 +71,7 @@ LL | let v: Spair = Spair { x: S(0), y: 0 }; LL | drop(v); | - value moved here LL | v.x = S(1); - | ^^^^^^^^^^ value partially assigned here after move + | ^^^^^^^^^^ value partially assigned here after partial move error[E0594]: cannot assign to `v.y`, as `v` is not declared as mutable --> $DIR/issue-54499-field-mutation-of-moved-out.rs:57:9 diff --git a/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr b/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr index d026f81b7aad6..39ff1234eb7f9 100644 --- a/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr +++ b/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr @@ -13,7 +13,7 @@ error[E0382]: use of moved value: `*f` LL | fn twice_ten_so i32>(f: Box) { | - consider adding a `Copy` constraint to this type argument LL | f(f(10)); - | - ^ value used here after move + | - ^ value used here after partial move | | | value moved here | @@ -44,7 +44,7 @@ error[E0382]: use of moved value: `*f` --> $DIR/two-phase-nonrecv-autoref.rs:85:11 | LL | f(f(10)); - | - ^ value used here after move + | - ^ value used here after partial move | | | value moved here | diff --git a/src/test/ui/issues/issue-29723.stderr b/src/test/ui/issues/issue-29723.stderr index 7928af5d5a5cd..71bf9cfd0997c 100644 --- a/src/test/ui/issues/issue-29723.stderr +++ b/src/test/ui/issues/issue-29723.stderr @@ -8,7 +8,7 @@ LL | 0 if { drop(s); false } => String::from("oops"), | - value moved here ... LL | s - | ^ value used here after move + | ^ value used here after partial move error: aborting due to previous error diff --git a/src/test/ui/issues/issue-34721.stderr b/src/test/ui/issues/issue-34721.stderr index 2ed7b543e713c..936467fce8277 100644 --- a/src/test/ui/issues/issue-34721.stderr +++ b/src/test/ui/issues/issue-34721.stderr @@ -13,7 +13,7 @@ LL | x.zero() | - value moved here LL | }; LL | x.zero() - | ^ value used here after move + | ^ value used here after partial move error: aborting due to previous error diff --git a/src/test/ui/moves/moves-based-on-type-tuple.stderr b/src/test/ui/moves/moves-based-on-type-tuple.stderr index c49dbdab40210..bbf88959ebd9d 100644 --- a/src/test/ui/moves/moves-based-on-type-tuple.stderr +++ b/src/test/ui/moves/moves-based-on-type-tuple.stderr @@ -14,7 +14,7 @@ error[E0382]: use of moved value: `x` (Mir) LL | fn dup(x: Box) -> Box<(Box,Box)> { | - move occurs because `x` has type `std::boxed::Box`, which does not implement the `Copy` trait LL | box (x, x) - | - ^ value used here after move + | - ^ value used here after partial move | | | value moved here diff --git a/src/test/ui/nll/closure-access-spans.stderr b/src/test/ui/nll/closure-access-spans.stderr index 3ca0aefb592e0..67b0fb422e26f 100644 --- a/src/test/ui/nll/closure-access-spans.stderr +++ b/src/test/ui/nll/closure-access-spans.stderr @@ -66,7 +66,7 @@ LL | let r = x; LL | || x.len(); //~ ERROR | ^^ - borrow occurs due to use in closure | | - | value borrowed here after move + | value borrowed here after partial move error[E0382]: borrow of moved value: `x` --> $DIR/closure-access-spans.rs:42:5 @@ -78,7 +78,7 @@ LL | let r = x; LL | || x = String::new(); //~ ERROR | ^^ - borrow occurs due to use in closure | | - | value borrowed here after move + | value borrowed here after partial move error[E0382]: borrow of moved value: `x` --> $DIR/closure-access-spans.rs:47:5 @@ -90,7 +90,7 @@ LL | let r = x; LL | || *x = String::new(); //~ ERROR | ^^ - borrow occurs due to use in closure | | - | value borrowed here after move + | value borrowed here after partial move error[E0382]: use of moved value: `x` --> $DIR/closure-access-spans.rs:52:5 @@ -102,7 +102,7 @@ LL | let r = x; LL | || x; //~ ERROR | ^^ - use occurs due to use in closure | | - | value used here after move + | value used here after partial move error: aborting due to 9 previous errors diff --git a/src/test/ui/nll/closure-move-spans.stderr b/src/test/ui/nll/closure-move-spans.stderr index 6750c4047601a..53f3a8c82e0b5 100644 --- a/src/test/ui/nll/closure-move-spans.stderr +++ b/src/test/ui/nll/closure-move-spans.stderr @@ -8,7 +8,7 @@ LL | || x; | | | value moved into closure here LL | let y = x; //~ ERROR - | ^ value used here after move + | ^ value used here after partial move error[E0382]: borrow of moved value: `x` --> $DIR/closure-move-spans.rs:12:13 @@ -20,7 +20,7 @@ LL | || x; | | | value moved into closure here LL | let y = &x; //~ ERROR - | ^^ value borrowed here after move + | ^^ value borrowed here after partial move error[E0382]: borrow of moved value: `x` --> $DIR/closure-move-spans.rs:17:13 @@ -32,7 +32,7 @@ LL | || x; | | | value moved into closure here LL | let y = &mut x; //~ ERROR - | ^^^^^^ value borrowed here after move + | ^^^^^^ value borrowed here after partial move error: aborting due to 3 previous errors diff --git a/src/test/ui/nll/decl-macro-illegal-copy.stderr b/src/test/ui/nll/decl-macro-illegal-copy.stderr index 9232ff52393e1..9e1ba3a4e1bcb 100644 --- a/src/test/ui/nll/decl-macro-illegal-copy.stderr +++ b/src/test/ui/nll/decl-macro-illegal-copy.stderr @@ -5,7 +5,7 @@ LL | $wrapper.inner | -------------- value moved here ... LL | wrapper.inner, - | ^^^^^^^^^^^^^ value used here after move + | ^^^^^^^^^^^^^ value used here after partial move | = note: move occurs because `wrapper.inner` has type `NonCopy`, which does not implement the `Copy` trait diff --git a/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr b/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr index 54c728e3d2783..7715c14d7a345 100644 --- a/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr +++ b/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr @@ -18,7 +18,7 @@ LL | let mut d = D { x: 0, s: S{ y: 0, z: 0 } }; LL | drop(d); | - value moved here LL | d.x = 10; - | ^^^^^^^^ value assigned here after move + | ^^^^^^^^ value assigned here after partial move error[E0381]: assign to part of possibly uninitialized variable: `d` --> $DIR/issue-21232-partial-init-and-erroneous-use.rs:49:5 diff --git a/src/test/ui/nll/issue-21232-partial-init-and-use.stderr b/src/test/ui/nll/issue-21232-partial-init-and-use.stderr index 23da533252cb9..ac15d9ba26c4d 100644 --- a/src/test/ui/nll/issue-21232-partial-init-and-use.stderr +++ b/src/test/ui/nll/issue-21232-partial-init-and-use.stderr @@ -18,7 +18,7 @@ LL | let mut s: S = S::new(); drop(s); | | | move occurs because `s` has type `S>`, which does not implement the `Copy` trait LL | s.x = 10; s.y = Box::new(20); - | ^^^^^^^^ value partially assigned here after move + | ^^^^^^^^ value partially assigned here after partial move error[E0382]: assign to part of moved value: `t` --> $DIR/issue-21232-partial-init-and-use.rs:120:5 @@ -28,7 +28,7 @@ LL | let mut t: T = (0, Box::new(0)); drop(t); | | | move occurs because `t` has type `(u32, std::boxed::Box)`, which does not implement the `Copy` trait LL | t.0 = 10; t.1 = Box::new(20); - | ^^^^^^^^ value partially assigned here after move + | ^^^^^^^^ value partially assigned here after partial move error[E0381]: assign to part of possibly uninitialized variable: `s` --> $DIR/issue-21232-partial-init-and-use.rs:127:5 @@ -50,7 +50,7 @@ LL | let mut s: S = S::new(); drop(s); | | | move occurs because `s` has type `S>`, which does not implement the `Copy` trait LL | s.x = 10; - | ^^^^^^^^ value partially assigned here after move + | ^^^^^^^^ value partially assigned here after partial move error[E0382]: assign to part of moved value: `t` --> $DIR/issue-21232-partial-init-and-use.rs:148:5 @@ -60,7 +60,7 @@ LL | let mut t: T = (0, Box::new(0)); drop(t); | | | move occurs because `t` has type `(u32, std::boxed::Box)`, which does not implement the `Copy` trait LL | t.0 = 10; - | ^^^^^^^^ value partially assigned here after move + | ^^^^^^^^ value partially assigned here after partial move error[E0381]: assign to part of possibly uninitialized variable: `s` --> $DIR/issue-21232-partial-init-and-use.rs:155:5 @@ -159,7 +159,7 @@ LL | match c { LL | c2 => { | -- value moved here LL | c.0 = 2; //~ ERROR assign to part of moved value - | ^^^^^^^ value partially assigned here after move + | ^^^^^^^ value partially assigned here after partial move error[E0382]: assign to part of moved value: `c` --> $DIR/issue-21232-partial-init-and-use.rs:269:13 diff --git a/src/test/ui/nll/move-subpaths-moves-root.stderr b/src/test/ui/nll/move-subpaths-moves-root.stderr index 0dd396f855dc5..8b52cc113ccc5 100644 --- a/src/test/ui/nll/move-subpaths-moves-root.stderr +++ b/src/test/ui/nll/move-subpaths-moves-root.stderr @@ -4,7 +4,7 @@ error[E0382]: use of moved value: `x` LL | drop(x.0); | --- value moved here LL | drop(x); //~ ERROR use of moved value - | ^ value used here after move + | ^ value used here after partial move | = note: move occurs because `x.0` has type `std::vec::Vec`, which does not implement the `Copy` trait diff --git a/src/test/ui/try-block/try-block-bad-lifetime.stderr b/src/test/ui/try-block/try-block-bad-lifetime.stderr index b1b925d694ff9..8967e77b5a7cc 100644 --- a/src/test/ui/try-block/try-block-bad-lifetime.stderr +++ b/src/test/ui/try-block/try-block-bad-lifetime.stderr @@ -32,7 +32,7 @@ LL | Err(k) ?; | - value moved here ... LL | ::std::mem::drop(k); //~ ERROR use of moved value: `k` - | ^ value used here after move + | ^ value used here after partial move error[E0506]: cannot assign to `i` because it is borrowed --> $DIR/try-block-bad-lifetime.rs:32:9 diff --git a/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr b/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr index dafbde6a5150b..33561eae46a2a 100644 --- a/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr +++ b/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr @@ -20,7 +20,7 @@ LL | ::std::mem::drop(x); | - value moved here LL | }; LL | println!("{}", x); //~ ERROR borrow of moved value: `x` - | ^ value borrowed here after move + | ^ value borrowed here after partial move error[E0506]: cannot assign to `i` because it is borrowed --> $DIR/try-block-maybe-bad-lifetime.rs:40:9 From bc9bfc7369e3150dfe954089b7957e34b379a95e Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sat, 9 Feb 2019 15:55:30 +0100 Subject: [PATCH 04/49] Implement ffi_returns_twice attribute --- src/librustc/hir/mod.rs | 3 +++ src/librustc_codegen_llvm/attributes.rs | 3 +++ src/librustc_codegen_llvm/llvm/ffi.rs | 1 + src/librustc_typeck/collect.rs | 12 ++++++++++++ src/librustc_typeck/diagnostics.rs | 1 + src/libsyntax/feature_gate.rs | 8 ++++++++ src/rustllvm/RustWrapper.cpp | 2 ++ src/rustllvm/rustllvm.h | 1 + src/test/codegen/ffi-returns-twice.rs | 15 +++++++++++++++ .../feature-gate-ffi_returns_twice.rs | 7 +++++++ .../feature-gate-ffi_returns_twice.stderr | 11 +++++++++++ src/test/ui/ffi_returns_twice.rs | 6 ++++++ src/test/ui/ffi_returns_twice.stderr | 9 +++++++++ 13 files changed, 79 insertions(+) create mode 100644 src/test/codegen/ffi-returns-twice.rs create mode 100644 src/test/ui/feature-gates/feature-gate-ffi_returns_twice.rs create mode 100644 src/test/ui/feature-gates/feature-gate-ffi_returns_twice.stderr create mode 100644 src/test/ui/ffi_returns_twice.rs create mode 100644 src/test/ui/ffi_returns_twice.stderr diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 3e7dd1432e1e3..5d3a1c78ce2bb 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2489,6 +2489,9 @@ bitflags! { /// #[used], indicates that LLVM can't eliminate this function (but the /// linker can!) const USED = 1 << 9; + /// #[ffi_returns_twice], indicates that an extern function can return + /// multiple times + const FFI_RETURNS_TWICE = 1 << 10; } } diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index e6bc7bca46bc9..8848305f791ef 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -223,6 +223,9 @@ pub fn from_fn_attrs( if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::COLD) { Attribute::Cold.apply_llfn(Function, llfn); } + if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::FFI_RETURNS_TWICE) { + Attribute::ReturnsTwice.apply_llfn(Function, llfn); + } if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NAKED) { naked(llfn, true); } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 58bdfc47fcaed..0cd9653bebbba 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -116,6 +116,7 @@ pub enum Attribute { SanitizeMemory = 22, NonLazyBind = 23, OptimizeNone = 24, + ReturnsTwice = 25, } /// LLVMIntPredicate diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 9dc74c5d63a4e..2c1f9495394b7 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2270,6 +2270,18 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR; } else if attr.check_name("unwind") { codegen_fn_attrs.flags |= CodegenFnAttrFlags::UNWIND; + } else if attr.check_name("ffi_returns_twice") { + if tcx.is_foreign_item(id) { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_RETURNS_TWICE; + } else { + // `#[ffi_returns_twice]` is only allowed `extern fn`s + struct_span_err!( + tcx.sess, + attr.span, + E0723, + "`#[ffi_returns_twice]` may only be used on `extern fn`s" + ).emit(); + } } else if attr.check_name("rustc_allocator_nounwind") { codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND; } else if attr.check_name("naked") { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 3ed09dfe99239..606c4058c86bf 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -4720,4 +4720,5 @@ register_diagnostics! { E0698, // type inside generator must be known in this context E0719, // duplicate values for associated type binding E0722, // Malformed #[optimize] attribute + E0723, // `#[ffi_returns_twice]` is only allowed in `extern fn` } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index e7b9a884b5e0c..b29f3a4bb1cb7 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -290,6 +290,9 @@ declare_features! ( // The `repr(i128)` annotation for enums. (active, repr128, "1.16.0", Some(35118), None), + // Allows the use of `#[ffi_returns_twice]` on extern functions. + (active, ffi_returns_twice, "1.34.0", Some(58314), None), + // The `unadjusted` ABI; perma-unstable. // // rustc internal @@ -1124,6 +1127,11 @@ pub const BUILTIN_ATTRIBUTES: &[(&str, AttributeType, AttributeTemplate, Attribu "the `#[naked]` attribute \ is an experimental feature", cfg_fn!(naked_functions))), + ("ffi_returns_twice", Whitelisted, template!(Word), Gated(Stability::Unstable, + "ffi_returns_twice", + "the `#[ffi_returns_twice]` attribute \ + is an experimental feature", + cfg_fn!(ffi_returns_twice))), ("target_feature", Whitelisted, template!(List: r#"enable = "name""#), Ungated), ("export_name", Whitelisted, template!(NameValueStr: "name"), Ungated), ("inline", Whitelisted, template!(Word, List: "always|never"), Ungated), diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index b33165b846339..a00417a362927 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -190,6 +190,8 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { return Attribute::NonLazyBind; case OptimizeNone: return Attribute::OptimizeNone; + case ReturnsTwice: + return Attribute::ReturnsTwice; } report_fatal_error("bad AttributeKind"); } diff --git a/src/rustllvm/rustllvm.h b/src/rustllvm/rustllvm.h index 933266b402526..a9d267cdb3175 100644 --- a/src/rustllvm/rustllvm.h +++ b/src/rustllvm/rustllvm.h @@ -85,6 +85,7 @@ enum LLVMRustAttribute { SanitizeMemory = 22, NonLazyBind = 23, OptimizeNone = 24, + ReturnsTwice = 25, }; typedef struct OpaqueRustString *RustStringRef; diff --git a/src/test/codegen/ffi-returns-twice.rs b/src/test/codegen/ffi-returns-twice.rs new file mode 100644 index 0000000000000..f6648249eb6e2 --- /dev/null +++ b/src/test/codegen/ffi-returns-twice.rs @@ -0,0 +1,15 @@ +// compile-flags: -C no-prepopulate-passes +#![crate_type = "lib"] +#![feature(ffi_returns_twice)] + +extern { + // CHECK-LABEL: @foo() + // CHECK: attributes #1 = { {{.*}}returns_twice{{.*}} } + #[no_mangle] + #[ffi_returns_twice] + pub fn foo(); +} + +pub fn bar() { + unsafe { foo() } +} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_returns_twice.rs b/src/test/ui/feature-gates/feature-gate-ffi_returns_twice.rs new file mode 100644 index 0000000000000..d3df6e5a852b5 --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-ffi_returns_twice.rs @@ -0,0 +1,7 @@ +// ignore-tidy-linelength +#![crate_type = "lib"] + +extern { + #[ffi_returns_twice] //~ ERROR the `#[ffi_returns_twice]` attribute is an experimental feature (see issue #58314) + pub fn foo(); +} diff --git a/src/test/ui/feature-gates/feature-gate-ffi_returns_twice.stderr b/src/test/ui/feature-gates/feature-gate-ffi_returns_twice.stderr new file mode 100644 index 0000000000000..5c111fe78f48a --- /dev/null +++ b/src/test/ui/feature-gates/feature-gate-ffi_returns_twice.stderr @@ -0,0 +1,11 @@ +error[E0658]: the `#[ffi_returns_twice]` attribute is an experimental feature (see issue #58314) + --> $DIR/feature-gate-ffi_returns_twice.rs:5:5 + | +LL | #[ffi_returns_twice] //~ ERROR the `#[ffi_returns_twice]` attribute is an experimental feature (see issue #58314) + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: add #![feature(ffi_returns_twice)] to the crate attributes to enable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/ffi_returns_twice.rs b/src/test/ui/ffi_returns_twice.rs new file mode 100644 index 0000000000000..5abf468febcef --- /dev/null +++ b/src/test/ui/ffi_returns_twice.rs @@ -0,0 +1,6 @@ +// ignore-tidy-linelength +#![feature(ffi_returns_twice)] +#![crate_type = "lib"] + +#[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on `extern fn`s +pub fn foo() {} diff --git a/src/test/ui/ffi_returns_twice.stderr b/src/test/ui/ffi_returns_twice.stderr new file mode 100644 index 0000000000000..4d3ca73af0283 --- /dev/null +++ b/src/test/ui/ffi_returns_twice.stderr @@ -0,0 +1,9 @@ +error[E0723]: `#[ffi_returns_twice]` may only be used on `extern fn`s + --> $DIR/ffi_returns_twice.rs:5:1 + | +LL | #[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on `extern fn`s + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0723`. From 4fed67f94220351ffa60de1dca078c02a7c15734 Mon Sep 17 00:00:00 2001 From: Matthieu M Date: Sat, 9 Feb 2019 18:42:34 +0100 Subject: [PATCH 05/49] Fix exhaustion of inclusive range try_fold and try_rfold --- src/libcore/iter/range.rs | 14 ++++++++++++-- src/libcore/tests/iter.rs | 24 +++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/libcore/iter/range.rs b/src/libcore/iter/range.rs index 52b0ccd48d476..f0ed88c3dfd85 100644 --- a/src/libcore/iter/range.rs +++ b/src/libcore/iter/range.rs @@ -389,6 +389,10 @@ impl Iterator for ops::RangeInclusive { { self.compute_is_empty(); + if self.is_empty() { + return Try::from_ok(init); + } + let mut accum = init; while self.start < self.end { @@ -397,11 +401,12 @@ impl Iterator for ops::RangeInclusive { accum = f(accum, n)?; } + self.is_empty = Some(true); + if self.start == self.end { accum = f(accum, self.start.clone())?; } - self.is_empty = Some(true); Try::from_ok(accum) } @@ -445,6 +450,10 @@ impl DoubleEndedIterator for ops::RangeInclusive { { self.compute_is_empty(); + if self.is_empty() { + return Try::from_ok(init); + } + let mut accum = init; while self.start < self.end { @@ -453,11 +462,12 @@ impl DoubleEndedIterator for ops::RangeInclusive { accum = f(accum, n)?; } + self.is_empty = Some(true); + if self.start == self.end { accum = f(accum, self.start.clone())?; } - self.is_empty = Some(true); Try::from_ok(accum) } } diff --git a/src/libcore/tests/iter.rs b/src/libcore/tests/iter.rs index cf19851c17b35..89e190e074f1a 100644 --- a/src/libcore/tests/iter.rs +++ b/src/libcore/tests/iter.rs @@ -1738,19 +1738,37 @@ fn test_range_inclusive_folds() { assert_eq!((1..=10).sum::(), 55); assert_eq!((1..=10).rev().sum::(), 55); - let mut it = 40..=50; + let mut it = 44..=50; assert_eq!(it.try_fold(0, i8::checked_add), None); - assert_eq!(it, 44..=50); + assert_eq!(it, 47..=50); + assert_eq!(it.try_fold(0, i8::checked_add), None); + assert_eq!(it, 50..=50); + assert_eq!(it.try_fold(0, i8::checked_add), Some(50)); + assert!(it.is_empty()); + assert_eq!(it.try_fold(0, i8::checked_add), Some(0)); + assert!(it.is_empty()); + + let mut it = 40..=47; + assert_eq!(it.try_rfold(0, i8::checked_add), None); + assert_eq!(it, 40..=44); assert_eq!(it.try_rfold(0, i8::checked_add), None); - assert_eq!(it, 44..=47); + assert_eq!(it, 40..=41); + assert_eq!(it.try_rfold(0, i8::checked_add), Some(81)); + assert!(it.is_empty()); + assert_eq!(it.try_rfold(0, i8::checked_add), Some(0)); + assert!(it.is_empty()); let mut it = 10..=20; assert_eq!(it.try_fold(0, |a,b| Some(a+b)), Some(165)); assert!(it.is_empty()); + assert_eq!(it.try_fold(0, |a,b| Some(a+b)), Some(0)); + assert!(it.is_empty()); let mut it = 10..=20; assert_eq!(it.try_rfold(0, |a,b| Some(a+b)), Some(165)); assert!(it.is_empty()); + assert_eq!(it.try_rfold(0, |a,b| Some(a+b)), Some(0)); + assert!(it.is_empty()); } #[test] From 19ee98f52c857d06c39df8902a6d37e07a8ff406 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sat, 9 Feb 2019 22:09:25 +0100 Subject: [PATCH 06/49] Correct error message --- src/librustc_typeck/collect.rs | 2 +- src/librustc_typeck/diagnostics.rs | 2 +- src/libsyntax/feature_gate.rs | 2 +- src/test/ui/ffi_returns_twice.rs | 2 +- src/test/ui/ffi_returns_twice.stderr | 4 ++-- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 2c1f9495394b7..35847ee50b97e 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2279,7 +2279,7 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen tcx.sess, attr.span, E0723, - "`#[ffi_returns_twice]` may only be used on `extern fn`s" + "`#[ffi_returns_twice]` may only be used on foreign functions" ).emit(); } } else if attr.check_name("rustc_allocator_nounwind") { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index 606c4058c86bf..d45570eb2700d 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -4720,5 +4720,5 @@ register_diagnostics! { E0698, // type inside generator must be known in this context E0719, // duplicate values for associated type binding E0722, // Malformed #[optimize] attribute - E0723, // `#[ffi_returns_twice]` is only allowed in `extern fn` + E0723, // `#[ffi_returns_twice]` is only allowed in foreign functions } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index b29f3a4bb1cb7..19d4e1fa57d08 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -290,7 +290,7 @@ declare_features! ( // The `repr(i128)` annotation for enums. (active, repr128, "1.16.0", Some(35118), None), - // Allows the use of `#[ffi_returns_twice]` on extern functions. + // Allows the use of `#[ffi_returns_twice]` on foreign functions. (active, ffi_returns_twice, "1.34.0", Some(58314), None), // The `unadjusted` ABI; perma-unstable. diff --git a/src/test/ui/ffi_returns_twice.rs b/src/test/ui/ffi_returns_twice.rs index 5abf468febcef..93c372e1d83dc 100644 --- a/src/test/ui/ffi_returns_twice.rs +++ b/src/test/ui/ffi_returns_twice.rs @@ -2,5 +2,5 @@ #![feature(ffi_returns_twice)] #![crate_type = "lib"] -#[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on `extern fn`s +#[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on foreign functions pub fn foo() {} diff --git a/src/test/ui/ffi_returns_twice.stderr b/src/test/ui/ffi_returns_twice.stderr index 4d3ca73af0283..5a6026c403dae 100644 --- a/src/test/ui/ffi_returns_twice.stderr +++ b/src/test/ui/ffi_returns_twice.stderr @@ -1,7 +1,7 @@ -error[E0723]: `#[ffi_returns_twice]` may only be used on `extern fn`s +error[E0723]: `#[ffi_returns_twice]` may only be used on foreign functions --> $DIR/ffi_returns_twice.rs:5:1 | -LL | #[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on `extern fn`s +LL | #[ffi_returns_twice] //~ ERROR `#[ffi_returns_twice]` may only be used on foreign functions | ^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error From bf2c5d339cb6f25fa2c7995b9aee697f7846eb6d Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sun, 10 Feb 2019 09:59:07 +0100 Subject: [PATCH 07/49] Use pattern to match attributes --- src/test/codegen/ffi-returns-twice.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/test/codegen/ffi-returns-twice.rs b/src/test/codegen/ffi-returns-twice.rs index f6648249eb6e2..396e8990d6f88 100644 --- a/src/test/codegen/ffi-returns-twice.rs +++ b/src/test/codegen/ffi-returns-twice.rs @@ -2,14 +2,10 @@ #![crate_type = "lib"] #![feature(ffi_returns_twice)] -extern { - // CHECK-LABEL: @foo() - // CHECK: attributes #1 = { {{.*}}returns_twice{{.*}} } - #[no_mangle] - #[ffi_returns_twice] - pub fn foo(); -} +pub fn bar() { unsafe { foo() } } -pub fn bar() { - unsafe { foo() } +extern { + #[ffi_returns_twice] pub fn foo(); } +// CHECK: declare void @foo(){{.*}}#1{{.*}} +// CHECK: attributes #1 = { {{.*}}returns_twice{{.*}} } From d0fddd3ac2432db409881af514ebfc85117f8887 Mon Sep 17 00:00:00 2001 From: gnzlbg Date: Sun, 10 Feb 2019 12:04:23 +0100 Subject: [PATCH 08/49] Fix attribute check --- src/test/codegen/ffi-returns-twice.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/codegen/ffi-returns-twice.rs b/src/test/codegen/ffi-returns-twice.rs index 396e8990d6f88..4db328f1cdfaf 100644 --- a/src/test/codegen/ffi-returns-twice.rs +++ b/src/test/codegen/ffi-returns-twice.rs @@ -5,7 +5,8 @@ pub fn bar() { unsafe { foo() } } extern { + // CHECK-LABEL: declare void @foo() + // CHECK-SAME: [[ATTRS:#[0-9]+]] + // CHECK-DAG: attributes [[ATTRS]] = { {{.*}}returns_twice{{.*}} } #[ffi_returns_twice] pub fn foo(); } -// CHECK: declare void @foo(){{.*}}#1{{.*}} -// CHECK: attributes #1 = { {{.*}}returns_twice{{.*}} } From 4e5eda36978e8868ee75001efb6388aa22326be0 Mon Sep 17 00:00:00 2001 From: Clint Frederickson Date: Tue, 12 Feb 2019 19:35:32 -0700 Subject: [PATCH 09/49] compute is_partial_move outside of the move_site loop for clarity --- src/librustc_mir/borrow_check/error_reporting.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 53b486cc6e316..e42c12125ee3d 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -130,7 +130,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); let mut is_loop_move = false; - let mut is_partial_move = false; + let is_partial_move = move_site_vec.iter().any(|move_site| { + let move_out = self.move_data.moves[(*move_site).moi]; + let moved_place = &self.move_data.move_paths[move_out.path].place; +// dbg!(moved_place); +// dbg!(used_place); +// if used_place != moved_place { + used_place.is_prefix_of(moved_place) +// } else { false } + }); for move_site in &move_site_vec { let move_out = self.move_data.moves[(*move_site).moi]; let moved_place = &self.move_data.move_paths[move_out.path].place; @@ -138,7 +146,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let move_spans = self.move_spans(moved_place, move_out.source); let move_span = move_spans.args_or_use(); - is_partial_move = used_place.is_prefix_of(moved_place); let move_msg = if move_spans.for_closure() { " into closure" } else { From 96fd2181ba97c75789579881fc62b68c8b65b759 Mon Sep 17 00:00:00 2001 From: Clint Frederickson Date: Wed, 13 Feb 2019 08:48:37 -0700 Subject: [PATCH 10/49] check if `used_place` and `moved_place` are equal when determining if the move was partial --- src/librustc_mir/borrow_check/error_reporting.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index e42c12125ee3d..0fa427aea8137 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -133,11 +133,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let is_partial_move = move_site_vec.iter().any(|move_site| { let move_out = self.move_data.moves[(*move_site).moi]; let moved_place = &self.move_data.move_paths[move_out.path].place; -// dbg!(moved_place); -// dbg!(used_place); -// if used_place != moved_place { + if used_place != moved_place { used_place.is_prefix_of(moved_place) -// } else { false } + } else { false } }); for move_site in &move_site_vec { let move_out = self.move_data.moves[(*move_site).moi]; From 755b32027952d078396fb8f885ac306d7688234e Mon Sep 17 00:00:00 2001 From: Clint Frederickson Date: Wed, 13 Feb 2019 09:34:29 -0700 Subject: [PATCH 11/49] simplified conditional --- src/librustc_mir/borrow_check/error_reporting.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 0fa427aea8137..9678c7bb3dc15 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -133,9 +133,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let is_partial_move = move_site_vec.iter().any(|move_site| { let move_out = self.move_data.moves[(*move_site).moi]; let moved_place = &self.move_data.move_paths[move_out.path].place; - if used_place != moved_place { - used_place.is_prefix_of(moved_place) - } else { false } + used_place != moved_place && used_place.is_prefix_of(moved_place) }); for move_site in &move_site_vec { let move_out = self.move_data.moves[(*move_site).moi]; From 283ffcfce71df51f3d1da4d63441de150938cacd Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Mon, 11 Feb 2019 21:09:47 +0000 Subject: [PATCH 12/49] Check the self-type of inherent associated constants --- src/librustc_typeck/check/mod.rs | 26 ++++++++++++------- .../user-annotations/dump-adt-brace-struct.rs | 4 ++- .../dump-adt-brace-struct.stderr | 8 +++--- .../ui/nll/user-annotations/dump-fn-method.rs | 19 ++++++++++---- .../user-annotations/dump-fn-method.stderr | 20 +++++++------- .../inherent-associated-constants.rs | 17 ++++++++++++ .../inherent-associated-constants.stderr | 10 +++++++ 7 files changed, 74 insertions(+), 30 deletions(-) create mode 100644 src/test/ui/nll/user-annotations/inherent-associated-constants.rs create mode 100644 src/test/ui/nll/user-annotations/inherent-associated-constants.stderr diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fb8f608812197..8d027b07188cc 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2236,7 +2236,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { hir_id, def_id, substs, user_self_ty, self.tag(), ); - if !substs.is_noop() { + if Self::can_contain_user_lifetime_bounds((substs, user_self_ty)) { let canonicalized = self.infcx.canonicalize_user_type_annotation( &UserType::TypeOf(def_id, UserSubsts { substs, @@ -2431,15 +2431,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let ty = self.to_ty(ast_ty); debug!("to_ty_saving_user_provided_ty: ty={:?}", ty); - // If the type given by the user has free regions, save it for - // later, since NLL would like to enforce those. Also pass in - // types that involve projections, since those can resolve to - // `'static` bounds (modulo #54940, which hopefully will be - // fixed by the time you see this comment, dear reader, - // although I have my doubts). Also pass in types with inference - // types, because they may be repeated. Other sorts of things - // are already sufficiently enforced with erased regions. =) - if ty.has_free_regions() || ty.has_projections() || ty.has_infer_types() { + if Self::can_contain_user_lifetime_bounds(ty) { let c_ty = self.infcx.canonicalize_response(&UserType::Ty(ty)); debug!("to_ty_saving_user_provided_ty: c_ty={:?}", c_ty); self.tables.borrow_mut().user_provided_types_mut().insert(ast_ty.hir_id, c_ty); @@ -2448,6 +2440,20 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ty } + // If the type given by the user has free regions, save it for later, since + // NLL would like to enforce those. Also pass in types that involve + // projections, since those can resolve to `'static` bounds (modulo #54940, + // which hopefully will be fixed by the time you see this comment, dear + // reader, although I have my doubts). Also pass in types with inference + // types, because they may be repeated. Other sorts of things are already + // sufficiently enforced with erased regions. =) + fn can_contain_user_lifetime_bounds(t: T) -> bool + where + T: TypeFoldable<'tcx> + { + t.has_free_regions() || t.has_projections() || t.has_infer_types() + } + pub fn node_ty(&self, id: hir::HirId) -> Ty<'tcx> { match self.tables.borrow().node_types().get(id) { Some(&t) => t, diff --git a/src/test/ui/nll/user-annotations/dump-adt-brace-struct.rs b/src/test/ui/nll/user-annotations/dump-adt-brace-struct.rs index 5dcd41078c787..45f56836d18b5 100644 --- a/src/test/ui/nll/user-annotations/dump-adt-brace-struct.rs +++ b/src/test/ui/nll/user-annotations/dump-adt-brace-struct.rs @@ -15,5 +15,7 @@ fn main() { SomeStruct::<_> { t: 22 }; // Nothing interesting given, no annotation. - SomeStruct:: { t: 22 }; //~ ERROR [u32] + SomeStruct:: { t: 22 }; // No lifetime bounds given. + + SomeStruct::<&'static u32> { t: &22 }; //~ ERROR [&ReStatic u32] } diff --git a/src/test/ui/nll/user-annotations/dump-adt-brace-struct.stderr b/src/test/ui/nll/user-annotations/dump-adt-brace-struct.stderr index 123c26195d006..6e24da094e0d2 100644 --- a/src/test/ui/nll/user-annotations/dump-adt-brace-struct.stderr +++ b/src/test/ui/nll/user-annotations/dump-adt-brace-struct.stderr @@ -1,8 +1,8 @@ -error: user substs: UserSubsts { substs: [u32], user_self_ty: None } - --> $DIR/dump-adt-brace-struct.rs:18:5 +error: user substs: UserSubsts { substs: [&ReStatic u32], user_self_ty: None } + --> $DIR/dump-adt-brace-struct.rs:20:5 | -LL | SomeStruct:: { t: 22 }; //~ ERROR [u32] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | SomeStruct::<&'static u32> { t: &22 }; //~ ERROR [&ReStatic u32] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/nll/user-annotations/dump-fn-method.rs b/src/test/ui/nll/user-annotations/dump-fn-method.rs index 7551a9474dc08..b689f18c22593 100644 --- a/src/test/ui/nll/user-annotations/dump-fn-method.rs +++ b/src/test/ui/nll/user-annotations/dump-fn-method.rs @@ -11,7 +11,7 @@ trait Bazoom { fn method(&self, arg: T, arg2: U) { } } -impl Bazoom for T { +impl Bazoom for S { } fn foo<'a, T>(_: T) { } @@ -22,20 +22,29 @@ fn main() { let x = foo; x(22); - // Here: `u32` is given. - let x = foo::; //~ ERROR [u32] + // Here: `u32` is given, which doesn't contain any lifetimes, so we don't + // have any annotation. + let x = foo::; x(22); + let x = foo::<&'static u32>; //~ ERROR [&ReStatic u32] + x(&22); + // Here: we only want the `T` to be given, the rest should be variables. // // (`T` refers to the declaration of `Bazoom`) let x = <_ as Bazoom>::method::<_>; //~ ERROR [^0, u32, ^1] x(&22, 44, 66); - // Here: all are given - let x = >::method::; //~ ERROR [u8, u16, u32] + // Here: all are given and definitely contain no lifetimes, so we + // don't have any annotation. + let x = >::method::; x(&22, 44, 66); + // Here: all are given and we have a lifetime. + let x = >::method::; //~ ERROR [u8, &ReStatic u16, u32] + x(&22, &44, 66); + // Here: we want in particular that *only* the method `U` // annotation is given, the rest are variables. // diff --git a/src/test/ui/nll/user-annotations/dump-fn-method.stderr b/src/test/ui/nll/user-annotations/dump-fn-method.stderr index a1a4e43e8a3e9..04ceb8e5f8495 100644 --- a/src/test/ui/nll/user-annotations/dump-fn-method.stderr +++ b/src/test/ui/nll/user-annotations/dump-fn-method.stderr @@ -1,23 +1,23 @@ -error: user substs: UserSubsts { substs: [u32], user_self_ty: None } - --> $DIR/dump-fn-method.rs:26:13 +error: user substs: UserSubsts { substs: [&ReStatic u32], user_self_ty: None } + --> $DIR/dump-fn-method.rs:30:13 | -LL | let x = foo::; //~ ERROR [u32] - | ^^^^^^^^^^ +LL | let x = foo::<&'static u32>; //~ ERROR [&ReStatic u32] + | ^^^^^^^^^^^^^^^^^^^ error: user substs: UserSubsts { substs: [^0, u32, ^1], user_self_ty: None } - --> $DIR/dump-fn-method.rs:32:13 + --> $DIR/dump-fn-method.rs:36:13 | LL | let x = <_ as Bazoom>::method::<_>; //~ ERROR [^0, u32, ^1] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: user substs: UserSubsts { substs: [u8, u16, u32], user_self_ty: None } - --> $DIR/dump-fn-method.rs:36:13 +error: user substs: UserSubsts { substs: [u8, &ReStatic u16, u32], user_self_ty: None } + --> $DIR/dump-fn-method.rs:45:13 | -LL | let x = >::method::; //~ ERROR [u8, u16, u32] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | let x = >::method::; //~ ERROR [u8, &ReStatic u16, u32] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: user substs: UserSubsts { substs: [^0, ^1, u32], user_self_ty: None } - --> $DIR/dump-fn-method.rs:44:5 + --> $DIR/dump-fn-method.rs:53:5 | LL | y.method::(44, 66); //~ ERROR [^0, ^1, u32] | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/nll/user-annotations/inherent-associated-constants.rs b/src/test/ui/nll/user-annotations/inherent-associated-constants.rs new file mode 100644 index 0000000000000..2490187605ac1 --- /dev/null +++ b/src/test/ui/nll/user-annotations/inherent-associated-constants.rs @@ -0,0 +1,17 @@ +#![feature(nll)] + +struct A<'a>(&'a ()); + +impl A<'static> { + const IC: i32 = 10; +} + +fn non_wf_associated_const<'a>(x: i32) { + A::<'a>::IC; //~ ERROR lifetime may not live long enough +} + +fn wf_associated_const<'a>(x: i32) { + A::<'static>::IC; +} + +fn main() {} diff --git a/src/test/ui/nll/user-annotations/inherent-associated-constants.stderr b/src/test/ui/nll/user-annotations/inherent-associated-constants.stderr new file mode 100644 index 0000000000000..785b39ec887a0 --- /dev/null +++ b/src/test/ui/nll/user-annotations/inherent-associated-constants.stderr @@ -0,0 +1,10 @@ +error: lifetime may not live long enough + --> $DIR/inherent-associated-constants.rs:10:5 + | +LL | fn non_wf_associated_const<'a>(x: i32) { + | -- lifetime `'a` defined here +LL | A::<'a>::IC; //~ ERROR lifetime may not live long enough + | ^^^^^^^^^^^ requires that `'a` must outlive `'static` + +error: aborting due to previous error + From 347a42e38770593cb82affc6d3d7413ebae911f0 Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Thu, 14 Feb 2019 12:39:54 +0530 Subject: [PATCH 13/49] SGX target: fix panic = abort --- src/libpanic_abort/lib.rs | 5 +++-- src/libstd/sys/sgx/rwlock.rs | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libpanic_abort/lib.rs b/src/libpanic_abort/lib.rs index edc97cd28a52a..8c20a6ea55ad0 100644 --- a/src/libpanic_abort/lib.rs +++ b/src/libpanic_abort/lib.rs @@ -58,8 +58,9 @@ pub unsafe extern fn __rust_start_panic(_payload: usize) -> u32 { #[cfg(all(target_vendor="fortanix", target_env="sgx"))] unsafe fn abort() -> ! { - extern "C" { pub fn panic_exit() -> !; } - panic_exit(); + // call std::sys::abort_internal + extern "C" { pub fn __rust_abort() -> !; } + __rust_abort(); } } diff --git a/src/libstd/sys/sgx/rwlock.rs b/src/libstd/sys/sgx/rwlock.rs index 33163a556c16d..4dfbe86d14f4d 100644 --- a/src/libstd/sys/sgx/rwlock.rs +++ b/src/libstd/sys/sgx/rwlock.rs @@ -204,6 +204,7 @@ pub unsafe extern "C" fn __rust_print_err(m: *mut u8, s: i32) { } #[no_mangle] +// NB. used by both libunwind and libpanic_abort pub unsafe extern "C" fn __rust_abort() { ::sys::abort_internal(); } From 503e74e96958045025448f90c8da4c7bd1484be9 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Wed, 6 Feb 2019 14:17:22 +0100 Subject: [PATCH 14/49] Fix SECURITY_SQOS_PRESENT missing if security_qos_flags(SECURITY_ANONYMOUS) is set --- src/libstd/sys/windows/fs.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/windows/fs.rs b/src/libstd/sys/windows/fs.rs index 533b8ae9ba2c0..e1f815a525c98 100644 --- a/src/libstd/sys/windows/fs.rs +++ b/src/libstd/sys/windows/fs.rs @@ -191,7 +191,11 @@ impl OpenOptions { pub fn access_mode(&mut self, access_mode: u32) { self.access_mode = Some(access_mode); } pub fn share_mode(&mut self, share_mode: u32) { self.share_mode = share_mode; } pub fn attributes(&mut self, attrs: u32) { self.attributes = attrs; } - pub fn security_qos_flags(&mut self, flags: u32) { self.security_qos_flags = flags; } + pub fn security_qos_flags(&mut self, flags: u32) { + // We have to set `SECURITY_SQOS_PRESENT` here, because one of the valid flags we can + // receive is `SECURITY_ANONYMOUS = 0x0`, which we can't check for later on. + self.security_qos_flags = flags | c::SECURITY_SQOS_PRESENT; + } pub fn security_attributes(&mut self, attrs: c::LPSECURITY_ATTRIBUTES) { self.security_attributes = attrs as usize; } @@ -239,7 +243,6 @@ impl OpenOptions { self.custom_flags | self.attributes | self.security_qos_flags | - if self.security_qos_flags != 0 { c::SECURITY_SQOS_PRESENT } else { 0 } | if self.create_new { c::FILE_FLAG_OPEN_REPARSE_POINT } else { 0 } } } From 9295f49d72521345b83558fc657a270fd0ed72e7 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Wed, 6 Feb 2019 14:20:27 +0100 Subject: [PATCH 15/49] Correct OpenOptions::security_qos_flags documentation --- src/libstd/sys/windows/ext/fs.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/libstd/sys/windows/ext/fs.rs b/src/libstd/sys/windows/ext/fs.rs index 89038da6295f2..22fcb3cf8df89 100644 --- a/src/libstd/sys/windows/ext/fs.rs +++ b/src/libstd/sys/windows/ext/fs.rs @@ -220,13 +220,27 @@ pub trait OpenOptionsExt { /// the specified value (or combines it with `custom_flags` and `attributes` /// to set the `dwFlagsAndAttributes` for [`CreateFile`]). /// - /// By default, `security_qos_flags` is set to `SECURITY_ANONYMOUS`. For - /// information about possible values, see [Impersonation Levels] on the - /// Windows Dev Center site. - /// + /// By default `security_qos_flags` is not set. It should be specified when + /// opening a named pipe, to control to which degree a server process can + /// act on behalf of a client process (security impersonation level). + /// + /// When `security_qos_flags` is not set a malicious program can gain the + /// elevated privileges of a privileged Rust process when it allows opening + /// user-specified paths, by tricking it into opening a named pipe. So + /// arguably `security_qos_flags` should also be set when opening arbitrary + /// paths. However the bits can then conflict with other flags, specifically + /// `FILE_FLAG_OPEN_NO_RECALL`. + /// + /// For information about possible values, see [Impersonation Levels] on the + /// Windows Dev Center site. The `SECURITY_SQOS_PRESENT` flag is set + /// automatically when using this method. + /// # Examples /// /// ```no_run + /// # #[cfg(for_demonstration_only)] + /// extern crate winapi; + /// # mod winapi { pub const SECURITY_IDENTIFICATION: u32 = 0; } /// use std::fs::OpenOptions; /// use std::os::windows::prelude::*; /// @@ -235,9 +249,9 @@ pub trait OpenOptionsExt { /// .create(true) /// /// // Sets the flag value to `SecurityIdentification`. - /// .security_qos_flags(1) + /// .security_qos_flags(winapi::SECURITY_IDENTIFICATION) /// - /// .open("foo.txt"); + /// .open("\\.\pipe\MyPipe"); /// ``` /// /// [`CreateFile`]: https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858.aspx From 235a6b7d065a2fc55ceee323e85b9309b16e84bf Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 12:32:22 +0100 Subject: [PATCH 16/49] Expose const -> op functions that don't allow violiting const eval invariants --- src/librustc_mir/const_eval.rs | 4 +- src/librustc_mir/interpret/operand.rs | 77 +++++++++++------------- src/librustc_mir/transform/const_prop.rs | 2 +- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 7be7f4b439289..a4b2d6d36878d 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -476,7 +476,7 @@ pub fn const_field<'a, 'tcx>( let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); let result = (|| { // get the operand again - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(value), value.ty)?; + let op = ecx.const_to_op(value, None)?; // downcast let down = match variant { None => op, @@ -502,7 +502,7 @@ pub fn const_variant_index<'a, 'tcx>( ) -> EvalResult<'tcx, VariantIdx> { trace!("const_variant_index: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(val), val.ty)?; + let op = ecx.const_to_op(val, None)?; Ok(ecx.read_discriminant(op)?.1) } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 7da907028eebf..4f34ffc128e69 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -12,7 +12,7 @@ use rustc::mir::interpret::{ EvalResult, EvalErrorKind, }; use super::{ - EvalContext, Machine, AllocMap, Allocation, AllocationExtra, + EvalContext, Machine, MemPlace, MPlaceTy, PlaceTy, Place, MemoryKind, }; pub use rustc::mir::interpret::ScalarMaybeUndef; @@ -545,14 +545,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Move(ref place) => self.eval_place_to_op(place, layout)?, - Constant(ref constant) => { - let layout = from_known_layout(layout, || { - let ty = self.monomorphize(mir_op.ty(self.mir(), *self.tcx))?; - self.layout_of(ty) - })?; - let op = self.const_value_to_op(*constant.literal)?; - OpTy { op, layout } - } + Constant(ref constant) => self.lazy_const_to_op(*constant.literal, layout)?, }; trace!("{:?}: {:?}", mir_op, *op); Ok(op) @@ -568,38 +561,55 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. - fn const_value_to_op( + // Used when Miri runs into a constant, and by CTFE. + pub fn lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, - ) -> EvalResult<'tcx, Operand> { - trace!("const_value_to_op: {:?}", val); - let val = match val { + layout: Option>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + trace!("const_to_op: {:?}", val); + match val { ty::LazyConst::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; - return Ok(*OpTy::from(self.const_eval_raw(GlobalId { + return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None, })?)); }, - ty::LazyConst::Evaluated(c) => c, - }; - match val.val { + ty::LazyConst::Evaluated(c) => self.const_to_op(c, layout), + } + } + + // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + pub fn const_to_op( + &self, + val: ty::Const<'tcx>, + layout: Option>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + let layout = from_known_layout(layout, || { + let ty = self.monomorphize(val.ty)?; + self.layout_of(ty) + })?; + let op = match val.val { ConstValue::ByRef(id, alloc, offset) => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. - Ok(Operand::Indirect( + Operand::Indirect( MemPlace::from_ptr(Pointer::new(id, offset), alloc.align) - ).with_default_tag()) + ).with_default_tag() }, ConstValue::Slice(a, b) => - Ok(Operand::Immediate(Immediate::ScalarPair( + Operand::Immediate(Immediate::ScalarPair( a.into(), Scalar::from_uint(b, self.tcx.data_layout.pointer_size).into(), - )).with_default_tag()), + )).with_default_tag(), ConstValue::Scalar(x) => - Ok(Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag()), - } + Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag(), + }; + Ok(OpTy { + op, + layout, + }) } /// Read discriminant, return the runtime value as well as the variant index. @@ -699,23 +709,4 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } }) } - -} - -impl<'a, 'mir, 'tcx, M> EvalContext<'a, 'mir, 'tcx, M> -where - M: Machine<'a, 'mir, 'tcx, PointerTag=()>, - // FIXME: Working around https://github.com/rust-lang/rust/issues/24159 - M::MemoryMap: AllocMap, Allocation<(), M::AllocExtra>)>, - M::AllocExtra: AllocationExtra<(), M::MemoryExtra>, -{ - // FIXME: CTFE should use allocations, then we can remove this. - pub(crate) fn lazy_const_to_op( - &self, - cnst: ty::LazyConst<'tcx>, - ty: ty::Ty<'tcx>, - ) -> EvalResult<'tcx, OpTy<'tcx>> { - let op = self.const_value_to_op(cnst)?; - Ok(OpTy { op, layout: self.layout_of(ty)? }) - } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7da00c4ea0c36..d69a5130b24d0 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -253,7 +253,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.lazy_const_to_op(*c.literal, c.ty) { + match self.ecx.lazy_const_to_op(*c.literal, None) { Ok(op) => { Some((op, c.span)) }, From bd18cc5708328600a94a02444caf27a5fb6ca20c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 12:36:23 +0100 Subject: [PATCH 17/49] Remove an intermediate value from discriminant reading --- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/step.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4f34ffc128e69..a638c008e760f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -508,7 +508,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Evaluate a place with the goal of reading from it. This lets us sometimes // avoid allocations. - fn eval_place_to_op( + pub(super) fn eval_place_to_op( &self, mir_place: &mir::Place<'tcx>, layout: Option>, diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 97ef2b5fa3485..656c13c16d9ed 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -266,8 +266,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } Discriminant(ref place) => { - let place = self.eval_place(place)?; - let discr_val = self.read_discriminant(self.place_to_op(place)?)?.0; + let op = self.eval_place_to_op(place, None)?; + let discr_val = self.read_discriminant(op)?.0; let size = dest.layout.size; self.write_scalar(Scalar::from_uint(discr_val, size), dest)?; } From b2bf37aec075099a8b58c1a52ebca944f318d530 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 13:27:54 +0100 Subject: [PATCH 18/49] Burn some invariants we keep up into code --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/operand.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index a4b2d6d36878d..9eac125d7a434 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -76,7 +76,7 @@ pub fn op_to_const<'tcx>( _ => false, }; let normalized_op = if normalize { - ecx.try_read_immediate(op)? + Ok(*ecx.read_immediate(op).expect("normalization works on validated constants")) } else { match *op { Operand::Indirect(mplace) => Err(mplace), diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index a638c008e760f..0394ad2b0a65f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -269,7 +269,7 @@ pub(super) fn from_known_layout<'tcx>( impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { /// Try reading an immediate in memory; this is interesting particularly for ScalarPair. /// Returns `None` if the layout does not permit loading this as a value. - pub(super) fn try_read_immediate_from_mplace( + fn try_read_immediate_from_mplace( &self, mplace: MPlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Option>> { @@ -323,7 +323,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> /// Note that for a given layout, this operation will either always fail or always /// succeed! Whether it succeeds depends on whether the layout can be represented /// in a `Immediate`, not on which data is stored there currently. - pub(crate) fn try_read_immediate( + pub(super) fn try_read_immediate( &self, src: OpTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Result, MemPlace>> { From f7c493121d4989895dd9c213ed4e877429229b86 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:29:27 +0100 Subject: [PATCH 19/49] Reuse the `Pointer` type instead of passing reassembling it at many use sites --- src/librustc/ich/impls_ty.rs | 2 +- src/librustc/mir/interpret/value.rs | 5 ++--- src/librustc/ty/structural_impls.rs | 4 ++-- src/librustc_codegen_llvm/consts.rs | 2 +- src/librustc_codegen_ssa/mir/operand.rs | 4 ++-- src/librustc_codegen_ssa/mir/place.rs | 4 ++-- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/hair/pattern/_match.rs | 12 +++++------- src/librustc_mir/interpret/operand.rs | 4 ++-- src/librustc_mir/monomorphize/collector.rs | 2 +- src/librustc_typeck/check/mod.rs | 2 +- 11 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 6f04a68a6ed61..04e03d0d95aa2 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -307,7 +307,7 @@ impl_stable_hash_for!( impl<'tcx> for enum mir::interpret::ConstValue<'tcx> [ mir::interpret::ConstValue ] { Scalar(val), Slice(a, b), - ByRef(id, alloc, offset), + ByRef(ptr, alloc), } ); impl_stable_hash_for!(struct crate::mir::interpret::RawConst<'tcx> { diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 5ec7de4308a13..1e5ba2c176bd2 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -31,9 +31,8 @@ pub enum ConstValue<'tcx> { /// it. Slice(Scalar, u64), - /// An allocation together with an offset into the allocation. - /// Invariant: the `AllocId` matches the allocation. - ByRef(AllocId, &'tcx Allocation, Size), + /// An allocation together with a pointer into the allocation. + ByRef(Pointer, &'tcx Allocation), } #[cfg(target_arch = "x86_64")] diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index d09cfa84a1690..0d1dc4ee2032f 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -499,8 +499,8 @@ impl<'a, 'tcx> Lift<'tcx> for ConstValue<'a> { match *self { ConstValue::Scalar(x) => Some(ConstValue::Scalar(x)), ConstValue::Slice(x, y) => Some(ConstValue::Slice(x, y)), - ConstValue::ByRef(x, alloc, z) => Some(ConstValue::ByRef( - x, alloc.lift_to_tcx(tcx)?, z, + ConstValue::ByRef(ptr, alloc) => Some(ConstValue::ByRef( + ptr, alloc.lift_to_tcx(tcx)?, )), } } diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index ca9e2c87be237..26d3bd9124707 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -71,7 +71,7 @@ pub fn codegen_static_initializer( let static_ = cx.tcx.const_eval(param_env.and(cid))?; let alloc = match static_.val { - ConstValue::ByRef(_, alloc, n) if n.bytes() == 0 => alloc, + ConstValue::ByRef(ptr, alloc) if ptr.offset.bytes() == 0 => alloc, _ => bug!("static const eval returned {:#?}", static_), }; Ok((const_alloc_to_llvm(cx, alloc), alloc)) diff --git a/src/librustc_codegen_ssa/mir/operand.rs b/src/librustc_codegen_ssa/mir/operand.rs index 2c6d968bb032a..3cac1befaf4ef 100644 --- a/src/librustc_codegen_ssa/mir/operand.rs +++ b/src/librustc_codegen_ssa/mir/operand.rs @@ -101,8 +101,8 @@ impl<'a, 'tcx: 'a, V: CodegenObject> OperandRef<'tcx, V> { let b_llval = bx.cx().const_usize(b); OperandValue::Pair(a_llval, b_llval) }, - ConstValue::ByRef(_, alloc, offset) => { - return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, offset))); + ConstValue::ByRef(ptr, alloc) => { + return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, ptr.offset))); }, }; diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index 9d6826d8756b7..1edcbfead2c94 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -417,8 +417,8 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let layout = cx.layout_of(self.monomorphize(&ty)); match bx.tcx().const_eval(param_env.and(cid)) { Ok(val) => match val.val { - mir::interpret::ConstValue::ByRef(_, alloc, offset) => { - bx.cx().from_const_alloc(layout, alloc, offset) + mir::interpret::ConstValue::ByRef(ptr, alloc) => { + bx.cx().from_const_alloc(layout, alloc, ptr.offset) } _ => bug!("promoteds should have an allocation: {:?}", val), }, diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 9eac125d7a434..94b9f0eadd0e7 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -96,7 +96,7 @@ pub fn op_to_const<'tcx>( // FIXME shouldn't it be the case that `mark_static_initialized` has already // interned this? I thought that is the entire point of that `FinishStatic` stuff? let alloc = ecx.tcx.intern_const_alloc(alloc); - ConstValue::ByRef(ptr.alloc_id, alloc, ptr.offset) + ConstValue::ByRef(ptr, alloc) }, Ok(Immediate::Scalar(x)) => ConstValue::Scalar(x.not_undef()?), diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1c7e1aa4d71e0..bf2f0d5d81f47 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -172,7 +172,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const}; use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size}; use rustc::mir::Field; -use rustc::mir::interpret::{ConstValue, Pointer, Scalar}; +use rustc::mir::interpret::{ConstValue, Scalar}; use rustc::util::common::ErrorReported; use syntax::attr::{SignedInt, UnsignedInt}; @@ -214,9 +214,8 @@ impl<'a, 'tcx> LiteralExpander<'a, 'tcx> { match (val, &crty.sty, &rty.sty) { // the easy case, deref a reference (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => ConstValue::ByRef( - p.alloc_id, + p, self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id), - p.offset, ), // unsize array to slice if pattern is array but match value or other patterns are slice (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => { @@ -1428,7 +1427,7 @@ fn slice_pat_covered_by_const<'tcx>( suffix: &[Pattern<'tcx>] ) -> Result { let data: &[u8] = match (const_val.val, &const_val.ty.sty) { - (ConstValue::ByRef(id, alloc, offset), ty::Array(t, n)) => { + (ConstValue::ByRef(ptr, alloc), ty::Array(t, n)) => { if *t != tcx.types.u8 { // FIXME(oli-obk): can't mix const patterns with slice patterns and get // any sort of exhaustiveness/unreachable check yet @@ -1436,7 +1435,6 @@ fn slice_pat_covered_by_const<'tcx>( // are definitely unreachable. return Ok(false); } - let ptr = Pointer::new(id, offset); let n = n.assert_usize(tcx).unwrap(); alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap() }, @@ -1778,8 +1776,8 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( let (opt_ptr, n, ty) = match value.ty.sty { ty::TyKind::Array(t, n) => { match value.val { - ConstValue::ByRef(id, alloc, offset) => ( - Some((Pointer::new(id, offset), alloc)), + ConstValue::ByRef(ptr, alloc) => ( + Some((ptr, alloc)), n.unwrap_usize(cx.tcx), t, ), diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 0394ad2b0a65f..1d6aff749c593 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -591,11 +591,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> self.layout_of(ty) })?; let op = match val.val { - ConstValue::ByRef(id, alloc, offset) => { + ConstValue::ByRef(ptr, alloc) => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. Operand::Indirect( - MemPlace::from_ptr(Pointer::new(id, offset), alloc.align) + MemPlace::from_ptr(ptr, alloc.align) ).with_default_tag() }, ConstValue::Slice(a, b) => diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index a76aa7454cbe4..32c9d5c0c4467 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1257,7 +1257,7 @@ fn collect_const<'a, 'tcx>( ConstValue::Slice(Scalar::Ptr(ptr), _) | ConstValue::Scalar(Scalar::Ptr(ptr)) => collect_miri(tcx, ptr.alloc_id, output), - ConstValue::ByRef(_id, alloc, _offset) => { + ConstValue::ByRef(_ptr, alloc) => { for &((), id) in alloc.relocations.values() { collect_miri(tcx, id, output); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 91e44a1588268..cbc8fde53a263 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1464,7 +1464,7 @@ fn maybe_check_static_with_link_section(tcx: TyCtxt, id: DefId, span: Span) { }; let param_env = ty::ParamEnv::reveal_all(); if let Ok(static_) = tcx.const_eval(param_env.and(cid)) { - let alloc = if let ConstValue::ByRef(_, allocation, _) = static_.val { + let alloc = if let ConstValue::ByRef(_, allocation) = static_.val { allocation } else { bug!("Matching on non-ByRef static") From 7db96a37e7b4b38cdd0354d715cecd56dfdd03b0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:33:54 +0100 Subject: [PATCH 20/49] Reintroduce the invariant comment for clarity --- src/librustc/mir/interpret/value.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 1e5ba2c176bd2..956182fc8b275 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -32,6 +32,7 @@ pub enum ConstValue<'tcx> { Slice(Scalar, u64), /// An allocation together with a pointer into the allocation. + /// Invariant: the pointer's `AllocId` resolves to the allocation. ByRef(Pointer, &'tcx Allocation), } From bee3c670dbf974d097f41447a1214b27bbf9acca Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Feb 2019 14:52:34 +0100 Subject: [PATCH 21/49] Update src/librustc_mir/interpret/operand.rs Co-Authored-By: oli-obk --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 1d6aff749c593..3ec042efb26f8 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -580,7 +580,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + // Used when Miri runs into a constant, and by CTFE. pub fn const_to_op( &self, val: ty::Const<'tcx>, From 525983a2a4ac3029dd9979d924ef444f48a4d7b3 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:05:47 +0100 Subject: [PATCH 22/49] Make validity checking use `MPlaceTy` instead of `OpTy` --- src/librustc_mir/const_eval.rs | 12 +++++------- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/interpret/validity.rs | 23 +++++++++++------------ 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 94b9f0eadd0e7..3c2c0af0bae80 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -523,13 +523,11 @@ fn validate_and_turn_into_const<'a, 'tcx>( let cid = key.value; let ecx = mk_eval_cx(tcx, tcx.def_span(key.value.instance.def_id()), key.param_env); let val = (|| { - let op = ecx.raw_const_to_mplace(constant)?.into(); - // FIXME: Once the visitor infrastructure landed, change validation to - // work directly on `MPlaceTy`. - let mut ref_tracking = RefTracking::new(op); - while let Some((op, path)) = ref_tracking.todo.pop() { + let mplace = ecx.raw_const_to_mplace(constant)?; + let mut ref_tracking = RefTracking::new(mplace); + while let Some((mplace, path)) = ref_tracking.todo.pop() { ecx.validate_operand( - op, + mplace.into(), path, Some(&mut ref_tracking), true, // const mode @@ -538,7 +536,7 @@ fn validate_and_turn_into_const<'a, 'tcx>( // Now that we validated, turn this into a proper constant. let def_id = cid.instance.def.def_id(); let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none(); - op_to_const(&ecx, op, normalize) + op_to_const(&ecx, mplace.into(), normalize) })(); val.map_err(|error| { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index b29e09900f6b1..abc18f1364f87 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -58,7 +58,7 @@ impl<'tcx, Tag> ::std::ops::Deref for PlaceTy<'tcx, Tag> { } /// A MemPlace with its layout. Constructing it is only possible in this module. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] pub struct MPlaceTy<'tcx, Tag=()> { mplace: MemPlace, pub layout: TyLayout<'tcx>, diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 8b97d9ded7449..0163d53f027cf 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -11,7 +11,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, Machine, EvalContext, ValueVisitor, + OpTy, Machine, EvalContext, ValueVisitor, MPlaceTy, }; macro_rules! validation_failure { @@ -74,13 +74,13 @@ pub enum PathElem { } /// State for tracking recursive validation of references -pub struct RefTracking<'tcx, Tag> { - pub seen: FxHashSet<(OpTy<'tcx, Tag>)>, - pub todo: Vec<(OpTy<'tcx, Tag>, Vec)>, +pub struct RefTracking { + pub seen: FxHashSet, + pub todo: Vec<(T, Vec)>, } -impl<'tcx, Tag: Copy+Eq+Hash> RefTracking<'tcx, Tag> { - pub fn new(op: OpTy<'tcx, Tag>) -> Self { +impl<'tcx, T: Copy + Eq + Hash> RefTracking { + pub fn new(op: T) -> Self { let mut ref_tracking = RefTracking { seen: FxHashSet::default(), todo: vec![(op, Vec::new())], @@ -151,7 +151,7 @@ struct ValidityVisitor<'rt, 'a: 'rt, 'mir: 'rt, 'tcx: 'a+'rt+'mir, M: Machine<'a /// starts must not be changed! `visit_fields` and `visit_array` rely on /// this stack discipline. path: Vec, - ref_tracking: Option<&'rt mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&'rt mut RefTracking>>, const_mode: bool, ecx: &'rt EvalContext<'a, 'mir, 'tcx, M>, } @@ -399,16 +399,15 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // before. Proceed recursively even for integer pointers, no // reason to skip them! They are (recursively) valid for some ZST, // but not for others (e.g., `!` is a ZST). - let op = place.into(); - if ref_tracking.seen.insert(op) { - trace!("Recursing below ptr {:#?}", *op); + if ref_tracking.seen.insert(place) { + trace!("Recursing below ptr {:#?}", *place); // We need to clone the path anyway, make sure it gets created // with enough space for the additional `Deref`. let mut new_path = Vec::with_capacity(self.path.len()+1); new_path.clone_from(&self.path); new_path.push(PathElem::Deref); // Remember to come back to this later. - ref_tracking.todo.push((op, new_path)); + ref_tracking.todo.push((place, new_path)); } } } @@ -598,7 +597,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> &self, op: OpTy<'tcx, M::PointerTag>, path: Vec, - ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&mut RefTracking>>, const_mode: bool, ) -> EvalResult<'tcx> { trace!("validate_operand: {:?}, {:?}", *op, op.layout.ty); From 27e438ad948f9e430281e77b0abe3885b64f3bd0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:51:53 +0100 Subject: [PATCH 23/49] Make `may_normalize` explicit in the type system --- src/librustc_mir/const_eval.rs | 72 ++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 3c2c0af0bae80..2f8e3189d12e9 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -21,7 +21,7 @@ use syntax::ast::Mutability; use syntax::source_map::{Span, DUMMY_SP}; use crate::interpret::{self, - PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Operand, Immediate, Scalar, Pointer, + PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Immediate, Scalar, Pointer, RawConst, ConstValue, EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup, Allocation, AllocId, MemoryKind, @@ -62,45 +62,46 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env) } -// FIXME: These two conversion functions are bad hacks. We should just always use allocations. -pub fn op_to_const<'tcx>( +fn mplace_to_const<'tcx>( + ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, + mplace: MPlaceTy<'tcx>, +) -> EvalResult<'tcx, ty::Const<'tcx>> { + let MemPlace { ptr, align, meta } = *mplace; + // extract alloc-offset pair + assert!(meta.is_none()); + let ptr = ptr.to_ptr()?; + let alloc = ecx.memory.get(ptr.alloc_id)?; + assert!(alloc.align >= align); + assert!(alloc.bytes.len() as u64 - ptr.offset.bytes() >= mplace.layout.size.bytes()); + let mut alloc = alloc.clone(); + alloc.align = align; + // FIXME shouldn't it be the case that `mark_static_initialized` has already + // interned this? I thought that is the entire point of that `FinishStatic` stuff? + let alloc = ecx.tcx.intern_const_alloc(alloc); + let val = ConstValue::ByRef(ptr, alloc); + Ok(ty::Const { val, ty: mplace.layout.ty }) +} + +fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, op: OpTy<'tcx>, - may_normalize: bool, ) -> EvalResult<'tcx, ty::Const<'tcx>> { // We do not normalize just any data. Only scalar layout and slices. - let normalize = may_normalize - && match op.layout.abi { - layout::Abi::Scalar(..) => true, - layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), - _ => false, - }; + let normalize = match op.layout.abi { + layout::Abi::Scalar(..) => true, + layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), + _ => false, + }; let normalized_op = if normalize { - Ok(*ecx.read_immediate(op).expect("normalization works on validated constants")) + Err(*ecx.read_immediate(op).expect("normalization works on validated constants")) } else { - match *op { - Operand::Indirect(mplace) => Err(mplace), - Operand::Immediate(val) => Ok(val) - } + op.try_as_mplace() }; let val = match normalized_op { - Err(MemPlace { ptr, align, meta }) => { - // extract alloc-offset pair - assert!(meta.is_none()); - let ptr = ptr.to_ptr()?; - let alloc = ecx.memory.get(ptr.alloc_id)?; - assert!(alloc.align >= align); - assert!(alloc.bytes.len() as u64 - ptr.offset.bytes() >= op.layout.size.bytes()); - let mut alloc = alloc.clone(); - alloc.align = align; - // FIXME shouldn't it be the case that `mark_static_initialized` has already - // interned this? I thought that is the entire point of that `FinishStatic` stuff? - let alloc = ecx.tcx.intern_const_alloc(alloc); - ConstValue::ByRef(ptr, alloc) - }, - Ok(Immediate::Scalar(x)) => + Ok(mplace) => return mplace_to_const(ecx, mplace), + Err(Immediate::Scalar(x)) => ConstValue::Scalar(x.not_undef()?), - Ok(Immediate::ScalarPair(a, b)) => + Err(Immediate::ScalarPair(a, b)) => ConstValue::Slice(a.not_undef()?, b.to_usize(ecx)?), }; Ok(ty::Const { val, ty: op.layout.ty }) @@ -486,7 +487,7 @@ pub fn const_field<'a, 'tcx>( let field = ecx.operand_field(down, field.index() as u64)?; // and finally move back to the const world, always normalizing because // this is not called for statics. - op_to_const(&ecx, field, true) + op_to_const(&ecx, field) })(); result.map_err(|error| { let err = error_to_const_error(&ecx, error); @@ -535,8 +536,11 @@ fn validate_and_turn_into_const<'a, 'tcx>( } // Now that we validated, turn this into a proper constant. let def_id = cid.instance.def.def_id(); - let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none(); - op_to_const(&ecx, mplace.into(), normalize) + if tcx.is_static(def_id).is_some() || cid.promoted.is_some() { + mplace_to_const(&ecx, mplace) + } else { + op_to_const(&ecx, mplace.into()) + } })(); val.map_err(|error| { From 4fdeb2da747231b03f85786f4ef6ce04d88da864 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:54:02 +0100 Subject: [PATCH 24/49] Add `eval` prefix to clarify what the function does --- src/librustc_mir/interpret/operand.rs | 4 ++-- src/librustc_mir/transform/const_prop.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3ec042efb26f8..08f6667039a34 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -545,7 +545,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Move(ref place) => self.eval_place_to_op(place, layout)?, - Constant(ref constant) => self.lazy_const_to_op(*constant.literal, layout)?, + Constant(ref constant) => self.eval_lazy_const_to_op(*constant.literal, layout)?, }; trace!("{:?}: {:?}", mir_op, *op); Ok(op) @@ -562,7 +562,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // Used when Miri runs into a constant, and by CTFE. - pub fn lazy_const_to_op( + pub fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, layout: Option>, diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d69a5130b24d0..1acc7b6e0c57f 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -253,7 +253,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.lazy_const_to_op(*c.literal, None) { + match self.ecx.eval_lazy_const_to_op(*c.literal, None) { Ok(op) => { Some((op, c.span)) }, From 4b085337bbb3434604f90503538e0211eb3bb8f6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 15:05:14 +0100 Subject: [PATCH 25/49] Update docs and visibilities of const to op methods --- src/librustc_mir/interpret/operand.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 08f6667039a34..311c5c6d10be3 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -561,7 +561,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when Miri runs into a constant, and by CTFE. + // Used when Miri runs into a constant, and by const propagation. pub fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, @@ -580,8 +580,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when Miri runs into a constant, and by CTFE. - pub fn const_to_op( + // Used when the miri-engine runs into a constant. + crate fn const_to_op( &self, val: ty::Const<'tcx>, layout: Option>, From 1fe7eb00944c3c41059e16daa7b401bc8b04447c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 15:16:02 +0100 Subject: [PATCH 26/49] Limit the visibility further and expand on a comment --- src/librustc_mir/interpret/operand.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 311c5c6d10be3..de362a7a96a7f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -562,7 +562,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // Used when Miri runs into a constant, and by const propagation. - pub fn eval_lazy_const_to_op( + crate fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, layout: Option>, @@ -580,7 +580,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when the miri-engine runs into a constant. + // Used when the miri-engine runs into a constant and for extracting information from constants + // in patterns via the `const_eval` module crate fn const_to_op( &self, val: ty::Const<'tcx>, From d26bf742db0754893567401e49ae8b016c878a92 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 15 Feb 2019 08:31:44 +1100 Subject: [PATCH 27/49] Change `Token::interpolated_to_tokenstream()`. It is currently a method of `Token`, but it only is valid to call if `self` is a `Token::Interpolated`. This commit eliminates the possibility of misuse by changing it to an associated function that takes a `Nonterminal`, which also simplifies the call sites. This requires splitting out a new function, `nonterminal_to_string`. --- src/librustc/hir/lowering.rs | 10 ++--- src/libsyntax/parse/token.rs | 12 ++---- src/libsyntax/print/pprust.rs | 52 ++++++++++++++------------ src/libsyntax_ext/proc_macro_server.rs | 4 +- 4 files changed, 38 insertions(+), 40 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 84487c40f8745..bbbd38cfed7e4 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1131,12 +1131,12 @@ impl<'a> LoweringContext<'a> { fn lower_token(&mut self, token: Token, span: Span) -> TokenStream { match token { - Token::Interpolated(_) => {} - other => return TokenTree::Token(span, other).into(), + Token::Interpolated(nt) => { + let tts = Token::interpolated_to_tokenstream(&self.sess.parse_sess, nt, span); + self.lower_token_stream(tts) + } + other => TokenTree::Token(span, other).into(), } - - let tts = token.interpolated_to_tokenstream(&self.sess.parse_sess, span); - self.lower_token_stream(tts) } fn lower_arm(&mut self, arm: &Arm) -> hir::Arm { diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index ff7f3e0bfaef3..976eea2bb54b2 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -508,14 +508,8 @@ impl Token { } } - pub fn interpolated_to_tokenstream(&self, sess: &ParseSess, span: Span) - -> TokenStream - { - let nt = match *self { - Token::Interpolated(ref nt) => nt, - _ => panic!("only works on interpolated tokens"), - }; - + pub fn interpolated_to_tokenstream(sess: &ParseSess, nt: Lrc<(Nonterminal, LazyTokenStream)>, + span: Span) -> TokenStream { // An `Interpolated` token means that we have a `Nonterminal` // which is often a parsed AST item. At this point we now need // to convert the parsed AST to an actual token stream, e.g. @@ -558,7 +552,7 @@ impl Token { let tokens_for_real = nt.1.force(|| { // FIXME(#43081): Avoid this pretty-print + reparse hack - let source = pprust::token_to_string(self); + let source = pprust::nonterminal_to_string(&nt.0); let filename = FileName::macro_expansion_source_code(&source); let (tokens, errors) = parse_stream_from_source_str( filename, source, sess, Some(span)); diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index cdf805176a293..0e48e3a5dff2b 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -4,7 +4,7 @@ use crate::ast::{Attribute, MacDelimiter, GenericArg}; use crate::util::parser::{self, AssocOp, Fixity}; use crate::attr; use crate::source_map::{self, SourceMap, Spanned}; -use crate::parse::token::{self, BinOpToken, Token}; +use crate::parse::token::{self, BinOpToken, Nonterminal, Token}; use crate::parse::lexer::comments; use crate::parse::{self, ParseSess}; use crate::print::pp::{self, Breaks}; @@ -257,29 +257,33 @@ pub fn token_to_string(tok: &Token) -> String { token::Comment => "/* */".to_string(), token::Shebang(s) => format!("/* shebang: {}*/", s), - token::Interpolated(ref nt) => match nt.0 { - token::NtExpr(ref e) => expr_to_string(e), - token::NtMeta(ref e) => meta_item_to_string(e), - token::NtTy(ref e) => ty_to_string(e), - token::NtPath(ref e) => path_to_string(e), - token::NtItem(ref e) => item_to_string(e), - token::NtBlock(ref e) => block_to_string(e), - token::NtStmt(ref e) => stmt_to_string(e), - token::NtPat(ref e) => pat_to_string(e), - token::NtIdent(e, false) => ident_to_string(e), - token::NtIdent(e, true) => format!("r#{}", ident_to_string(e)), - token::NtLifetime(e) => ident_to_string(e), - token::NtLiteral(ref e) => expr_to_string(e), - token::NtTT(ref tree) => tt_to_string(tree.clone()), - token::NtArm(ref e) => arm_to_string(e), - token::NtImplItem(ref e) => impl_item_to_string(e), - token::NtTraitItem(ref e) => trait_item_to_string(e), - token::NtGenerics(ref e) => generic_params_to_string(&e.params), - token::NtWhereClause(ref e) => where_clause_to_string(e), - token::NtArg(ref e) => arg_to_string(e), - token::NtVis(ref e) => vis_to_string(e), - token::NtForeignItem(ref e) => foreign_item_to_string(e), - } + token::Interpolated(ref nt) => nonterminal_to_string(&nt.0), + } +} + +pub fn nonterminal_to_string(nt: &Nonterminal) -> String { + match *nt { + token::NtExpr(ref e) => expr_to_string(e), + token::NtMeta(ref e) => meta_item_to_string(e), + token::NtTy(ref e) => ty_to_string(e), + token::NtPath(ref e) => path_to_string(e), + token::NtItem(ref e) => item_to_string(e), + token::NtBlock(ref e) => block_to_string(e), + token::NtStmt(ref e) => stmt_to_string(e), + token::NtPat(ref e) => pat_to_string(e), + token::NtIdent(e, false) => ident_to_string(e), + token::NtIdent(e, true) => format!("r#{}", ident_to_string(e)), + token::NtLifetime(e) => ident_to_string(e), + token::NtLiteral(ref e) => expr_to_string(e), + token::NtTT(ref tree) => tt_to_string(tree.clone()), + token::NtArm(ref e) => arm_to_string(e), + token::NtImplItem(ref e) => impl_item_to_string(e), + token::NtTraitItem(ref e) => trait_item_to_string(e), + token::NtGenerics(ref e) => generic_params_to_string(&e.params), + token::NtWhereClause(ref e) => where_clause_to_string(e), + token::NtArg(ref e) => arg_to_string(e), + token::NtVis(ref e) => vis_to_string(e), + token::NtForeignItem(ref e) => foreign_item_to_string(e), } } diff --git a/src/libsyntax_ext/proc_macro_server.rs b/src/libsyntax_ext/proc_macro_server.rs index fd82dac5ab6d8..60ce65baa48a3 100644 --- a/src/libsyntax_ext/proc_macro_server.rs +++ b/src/libsyntax_ext/proc_macro_server.rs @@ -178,8 +178,8 @@ impl FromInternal<(TreeAndJoint, &'_ ParseSess, &'_ mut Vec)> tt!(Punct::new('#', false)) } - Interpolated(_) => { - let stream = token.interpolated_to_tokenstream(sess, span); + Interpolated(nt) => { + let stream = Token::interpolated_to_tokenstream(sess, nt, span); TokenTree::Group(Group { delimiter: Delimiter::None, stream, From f8801f3bf641f0277087e6621d09f9a6a373b36c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 15 Feb 2019 09:10:02 +1100 Subject: [PATCH 28/49] Remove `LazyTokenStream`. It's present within `Token::Interpolated` as an optimization, so that if a nonterminal is converted to a `TokenStream` multiple times, the first-computed value is saved and reused. But in practice it's not needed. `interpolated_to_tokenstream()` is a cold function: it's only called a few dozen times while compiling rustc itself, and a few hundred times across the entire `rustc-perf` suite. Furthermore, when it is called, it is almost always the first conversion, so no benefit is gained from it. So this commit removes `LazyTokenStream`, along with the now-unnecessary `Token::interpolated()`. As well as a significant simplification, the removal speeds things up slightly, mostly due to not having to `drop` the `LazyTokenStream` instances. --- src/librustc/hir/map/def_collector.rs | 2 +- src/librustc_resolve/build_reduced_graph.rs | 2 +- src/libsyntax/attr/mod.rs | 4 +- src/libsyntax/ext/base.rs | 2 +- src/libsyntax/ext/expand.rs | 5 +- src/libsyntax/ext/tt/macro_parser.rs | 10 +- src/libsyntax/ext/tt/transcribe.rs | 3 +- src/libsyntax/mut_visit.rs | 5 +- src/libsyntax/parse/attr.rs | 4 +- src/libsyntax/parse/parser.rs | 16 +-- src/libsyntax/parse/token.rs | 113 +++++--------------- src/libsyntax/print/pprust.rs | 2 +- src/libsyntax_ext/deriving/custom.rs | 3 +- 13 files changed, 58 insertions(+), 113 deletions(-) diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index 8fe10a85ef380..72aa9570cc2ff 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -339,7 +339,7 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { fn visit_token(&mut self, t: Token) { if let Token::Interpolated(nt) = t { - if let token::NtExpr(ref expr) = nt.0 { + if let token::NtExpr(ref expr) = *nt { if let ExprKind::Mac(..) = expr.node { self.visit_macro_invoc(expr.id); } diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index a82f8df154725..29de5308a3cdb 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -1025,7 +1025,7 @@ impl<'a, 'b> Visitor<'a> for BuildReducedGraphVisitor<'a, 'b> { fn visit_token(&mut self, t: Token) { if let Token::Interpolated(nt) = t { - if let token::NtExpr(ref expr) = nt.0 { + if let token::NtExpr(ref expr) = *nt { if let ast::ExprKind::Mac(..) = expr.node { self.visit_invoc(expr.id); } diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index 420f7426ad786..b5fc850731404 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -517,7 +517,7 @@ impl MetaItem { let span = span.with_hi(segments.last().unwrap().ident.span.hi()); Path { span, segments } } - Some(TokenTree::Token(_, Token::Interpolated(ref nt))) => match nt.0 { + Some(TokenTree::Token(_, Token::Interpolated(nt))) => match *nt { token::Nonterminal::NtIdent(ident, _) => Path::from_ident(ident), token::Nonterminal::NtMeta(ref meta) => return Some(meta.clone()), token::Nonterminal::NtPath(ref path) => path.clone(), @@ -682,7 +682,7 @@ impl LitKind { match token { Token::Ident(ident, false) if ident.name == "true" => Some(LitKind::Bool(true)), Token::Ident(ident, false) if ident.name == "false" => Some(LitKind::Bool(false)), - Token::Interpolated(ref nt) => match nt.0 { + Token::Interpolated(nt) => match *nt { token::NtExpr(ref v) | token::NtLiteral(ref v) => match v.node { ExprKind::Lit(ref lit) => Some(lit.node.clone()), _ => None, diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 5980261593dbf..452cc2f2c65cc 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -266,7 +266,7 @@ impl TTMacroExpander for F impl MutVisitor for AvoidInterpolatedIdents { fn visit_tt(&mut self, tt: &mut tokenstream::TokenTree) { if let tokenstream::TokenTree::Token(_, token::Interpolated(nt)) = tt { - if let token::NtIdent(ident, is_raw) = nt.0 { + if let token::NtIdent(ident, is_raw) = **nt { *tt = tokenstream::TokenTree::Token(ident.span, token::Ident(ident, is_raw)); } diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index d398437d7affc..55a6471e3688d 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -25,6 +25,7 @@ use syntax_pos::{Span, DUMMY_SP, FileName}; use syntax_pos::hygiene::ExpnFormat; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::sync::Lrc; use std::fs; use std::io::ErrorKind; use std::{iter, mem}; @@ -586,14 +587,14 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } AttrProcMacro(ref mac, ..) => { self.gate_proc_macro_attr_item(attr.span, &item); - let item_tok = TokenTree::Token(DUMMY_SP, Token::interpolated(match item { + let item_tok = TokenTree::Token(DUMMY_SP, Token::Interpolated(Lrc::new(match item { Annotatable::Item(item) => token::NtItem(item), Annotatable::TraitItem(item) => token::NtTraitItem(item.into_inner()), Annotatable::ImplItem(item) => token::NtImplItem(item.into_inner()), Annotatable::ForeignItem(item) => token::NtForeignItem(item.into_inner()), Annotatable::Stmt(stmt) => token::NtStmt(stmt.into_inner()), Annotatable::Expr(expr) => token::NtExpr(expr), - })).into(); + }))).into(); let input = self.extract_proc_macro_attr_input(attr.tokens, attr.span); let tok_result = mac.expand(self.cx, attr.span, input, item_tok); let res = self.parse_ast_fragment(tok_result, invoc.fragment_kind, diff --git a/src/libsyntax/ext/tt/macro_parser.rs b/src/libsyntax/ext/tt/macro_parser.rs index 5de1ccec8609e..c2607ed530cf0 100644 --- a/src/libsyntax/ext/tt/macro_parser.rs +++ b/src/libsyntax/ext/tt/macro_parser.rs @@ -829,7 +829,7 @@ fn may_begin_with(name: &str, token: &Token) -> bool { }, "block" => match *token { Token::OpenDelim(token::Brace) => true, - Token::Interpolated(ref nt) => match nt.0 { + Token::Interpolated(ref nt) => match **nt { token::NtItem(_) | token::NtPat(_) | token::NtTy(_) @@ -843,9 +843,9 @@ fn may_begin_with(name: &str, token: &Token) -> bool { }, "path" | "meta" => match *token { Token::ModSep | Token::Ident(..) => true, - Token::Interpolated(ref nt) => match nt.0 { + Token::Interpolated(ref nt) => match **nt { token::NtPath(_) | token::NtMeta(_) => true, - _ => may_be_ident(&nt.0), + _ => may_be_ident(&nt), }, _ => false, }, @@ -862,12 +862,12 @@ fn may_begin_with(name: &str, token: &Token) -> bool { Token::ModSep | // path Token::Lt | // path (UFCS constant) Token::BinOp(token::Shl) => true, // path (double UFCS) - Token::Interpolated(ref nt) => may_be_ident(&nt.0), + Token::Interpolated(ref nt) => may_be_ident(nt), _ => false, }, "lifetime" => match *token { Token::Lifetime(_) => true, - Token::Interpolated(ref nt) => match nt.0 { + Token::Interpolated(ref nt) => match **nt { token::NtLifetime(_) | token::NtTT(_) => true, _ => false, }, diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index b9a50cc6488dd..5c240e237f632 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -149,7 +149,8 @@ pub fn transcribe(cx: &ExtCtxt<'_>, result.push(tt.clone().into()); } else { sp = sp.apply_mark(cx.current_expansion.mark); - let token = TokenTree::Token(sp, Token::interpolated((**nt).clone())); + let token = + TokenTree::Token(sp, Token::Interpolated(Lrc::new((**nt).clone()))); result.push(token.into()); } } else { diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index 1e5eb0992bd1b..59fae789188b0 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -581,9 +581,8 @@ pub fn noop_visit_token(t: &mut Token, vis: &mut T) { token::Ident(id, _is_raw) => vis.visit_ident(id), token::Lifetime(id) => vis.visit_ident(id), token::Interpolated(nt) => { - let nt = Lrc::make_mut(nt); - vis.visit_interpolated(&mut nt.0); - nt.1 = token::LazyTokenStream::new(); + let mut nt = Lrc::make_mut(nt); + vis.visit_interpolated(&mut nt); } _ => {} } diff --git a/src/libsyntax/parse/attr.rs b/src/libsyntax/parse/attr.rs index b36ca0574cb8d..9020c8c6a2dc6 100644 --- a/src/libsyntax/parse/attr.rs +++ b/src/libsyntax/parse/attr.rs @@ -141,7 +141,7 @@ impl<'a> Parser<'a> { /// The delimiters or `=` are still put into the resulting token stream. crate fn parse_meta_item_unrestricted(&mut self) -> PResult<'a, (ast::Path, TokenStream)> { let meta = match self.token { - token::Interpolated(ref nt) => match nt.0 { + token::Interpolated(ref nt) => match **nt { Nonterminal::NtMeta(ref meta) => Some(meta.clone()), _ => None, }, @@ -227,7 +227,7 @@ impl<'a> Parser<'a> { /// meta_item_inner : (meta_item | UNSUFFIXED_LIT) (',' meta_item_inner)? ; pub fn parse_meta_item(&mut self) -> PResult<'a, ast::MetaItem> { let nt_meta = match self.token { - token::Interpolated(ref nt) => match nt.0 { + token::Interpolated(ref nt) => match **nt { token::NtMeta(ref e) => Some(e.clone()), _ => None, }, diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index e22047938e518..559e992ee0d7a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -119,7 +119,7 @@ enum BlockMode { macro_rules! maybe_whole_expr { ($p:expr) => { if let token::Interpolated(nt) = $p.token.clone() { - match nt.0 { + match *nt { token::NtExpr(ref e) | token::NtLiteral(ref e) => { $p.bump(); return Ok((*e).clone()); @@ -146,7 +146,7 @@ macro_rules! maybe_whole_expr { macro_rules! maybe_whole { ($p:expr, $constructor:ident, |$x:ident| $e:expr) => { if let token::Interpolated(nt) = $p.token.clone() { - if let token::$constructor($x) = nt.0.clone() { + if let token::$constructor($x) = (*nt).clone() { $p.bump(); return Ok($e); } @@ -1570,7 +1570,7 @@ impl<'a> Parser<'a> { Some(body) } token::Interpolated(ref nt) => { - match &nt.0 { + match **nt { token::NtBlock(..) => { *at_end = true; let (inner_attrs, body) = self.parse_inner_attrs_and_block()?; @@ -1913,7 +1913,7 @@ impl<'a> Parser<'a> { fn is_named_argument(&mut self) -> bool { let offset = match self.token { - token::Interpolated(ref nt) => match nt.0 { + token::Interpolated(ref nt) => match **nt { token::NtPat(..) => return self.look_ahead(1, |t| t == &token::Colon), _ => 0, } @@ -2099,7 +2099,7 @@ impl<'a> Parser<'a> { /// Matches `token_lit = LIT_INTEGER | ...`. fn parse_lit_token(&mut self) -> PResult<'a, LitKind> { let out = match self.token { - token::Interpolated(ref nt) => match nt.0 { + token::Interpolated(ref nt) => match **nt { token::NtExpr(ref v) | token::NtLiteral(ref v) => match v.node { ExprKind::Lit(ref lit) => { lit.node.clone() } _ => { return self.unexpected_last(&self.token); } @@ -2299,7 +2299,7 @@ impl<'a> Parser<'a> { /// attributes. pub fn parse_path_allowing_meta(&mut self, style: PathStyle) -> PResult<'a, ast::Path> { let meta_ident = match self.token { - token::Interpolated(ref nt) => match nt.0 { + token::Interpolated(ref nt) => match **nt { token::NtMeta(ref meta) => match meta.node { ast::MetaItemKind::Word => Some(meta.ident.clone()), _ => None, @@ -3271,7 +3271,7 @@ impl<'a> Parser<'a> { self.meta_var_span = Some(self.span); // Interpolated identifier and lifetime tokens are replaced with usual identifier // and lifetime tokens, so the former are never encountered during normal parsing. - match nt.0 { + match **nt { token::NtIdent(ident, is_raw) => (token::Ident(ident, is_raw), ident.span), token::NtLifetime(ident) => (token::Lifetime(ident), ident.span), _ => return, @@ -3403,7 +3403,7 @@ impl<'a> Parser<'a> { // can't continue an expression after an ident token::Ident(ident, is_raw) => token::ident_can_begin_expr(ident, is_raw), token::Literal(..) | token::Pound => true, - token::Interpolated(ref nt) => match nt.0 { + token::Interpolated(ref nt) => match **nt { token::NtIdent(..) | token::NtExpr(..) | token::NtBlock(..) | token::NtPath(..) => true, _ => false, diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index 976eea2bb54b2..a1d1a0f51baf0 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -13,16 +13,15 @@ use crate::syntax::parse::parse_stream_from_source_str; use crate::syntax::parse::parser::emit_unclosed_delims; use crate::tokenstream::{self, DelimSpan, TokenStream, TokenTree}; -use serialize::{Decodable, Decoder, Encodable, Encoder}; use syntax_pos::symbol::{self, Symbol}; use syntax_pos::{self, Span, FileName}; use log::info; -use std::{cmp, fmt}; +use std::fmt; use std::mem; #[cfg(target_arch = "x86_64")] use rustc_data_structures::static_assert; -use rustc_data_structures::sync::{Lrc, Lock}; +use rustc_data_structures::sync::Lrc; #[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)] pub enum BinOpToken { @@ -184,9 +183,8 @@ pub enum Token { Ident(ast::Ident, /* is_raw */ bool), Lifetime(ast::Ident), - // The `LazyTokenStream` is a pure function of the `Nonterminal`, - // and so the `LazyTokenStream` can be ignored by Eq, Hash, etc. - Interpolated(Lrc<(Nonterminal, LazyTokenStream)>), + Interpolated(Lrc), + // Can be expanded into several tokens. /// A doc comment. DocComment(ast::Name), @@ -209,10 +207,6 @@ pub enum Token { static_assert!(MEM_SIZE_OF_STATEMENT: mem::size_of::() == 16); impl Token { - pub fn interpolated(nt: Nonterminal) -> Token { - Token::Interpolated(Lrc::new((nt, LazyTokenStream::new()))) - } - /// Recovers a `Token` from an `ast::Ident`. This creates a raw identifier if necessary. pub fn from_ast_ident(ident: ast::Ident) -> Token { Ident(ident, ident.is_raw_guess()) @@ -244,7 +238,7 @@ impl Token { ModSep | // global path Lifetime(..) | // labeled loop Pound => true, // expression attributes - Interpolated(ref nt) => match nt.0 { + Interpolated(ref nt) => match **nt { NtLiteral(..) | NtIdent(..) | NtExpr(..) | @@ -272,7 +266,7 @@ impl Token { Lifetime(..) | // lifetime bound in trait object Lt | BinOp(Shl) | // associated path ModSep => true, // global path - Interpolated(ref nt) => match nt.0 { + Interpolated(ref nt) => match **nt { NtIdent(..) | NtTy(..) | NtPath(..) | NtLifetime(..) => true, _ => false, }, @@ -284,7 +278,7 @@ impl Token { pub fn can_begin_const_arg(&self) -> bool { match self { OpenDelim(Brace) => true, - Interpolated(ref nt) => match nt.0 { + Interpolated(ref nt) => match **nt { NtExpr(..) => true, NtBlock(..) => true, NtLiteral(..) => true, @@ -316,7 +310,7 @@ impl Token { BinOp(Minus) => true, Ident(ident, false) if ident.name == keywords::True.name() => true, Ident(ident, false) if ident.name == keywords::False.name() => true, - Interpolated(ref nt) => match nt.0 { + Interpolated(ref nt) => match **nt { NtLiteral(..) => true, _ => false, }, @@ -328,7 +322,7 @@ impl Token { pub fn ident(&self) -> Option<(ast::Ident, /* is_raw */ bool)> { match *self { Ident(ident, is_raw) => Some((ident, is_raw)), - Interpolated(ref nt) => match nt.0 { + Interpolated(ref nt) => match **nt { NtIdent(ident, is_raw) => Some((ident, is_raw)), _ => None, }, @@ -339,7 +333,7 @@ impl Token { pub fn lifetime(&self) -> Option { match *self { Lifetime(ident) => Some(ident), - Interpolated(ref nt) => match nt.0 { + Interpolated(ref nt) => match **nt { NtLifetime(ident) => Some(ident), _ => None, }, @@ -367,7 +361,7 @@ impl Token { /// Returns `true` if the token is an interpolated path. fn is_path(&self) -> bool { if let Interpolated(ref nt) = *self { - if let NtPath(..) = nt.0 { + if let NtPath(..) = **nt { return true; } } @@ -508,8 +502,8 @@ impl Token { } } - pub fn interpolated_to_tokenstream(sess: &ParseSess, nt: Lrc<(Nonterminal, LazyTokenStream)>, - span: Span) -> TokenStream { + pub fn interpolated_to_tokenstream(sess: &ParseSess, nt: Lrc, span: Span) + -> TokenStream { // An `Interpolated` token means that we have a `Nonterminal` // which is often a parsed AST item. At this point we now need // to convert the parsed AST to an actual token stream, e.g. @@ -524,41 +518,36 @@ impl Token { // stream they came from. Here we attempt to extract these // lossless token streams before we fall back to the // stringification. - let mut tokens = None; - - match nt.0 { + let tokens = match *nt { Nonterminal::NtItem(ref item) => { - tokens = prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span); + prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) } Nonterminal::NtTraitItem(ref item) => { - tokens = prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span); + prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) } Nonterminal::NtImplItem(ref item) => { - tokens = prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span); + prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) } Nonterminal::NtIdent(ident, is_raw) => { let token = Token::Ident(ident, is_raw); - tokens = Some(TokenTree::Token(ident.span, token).into()); + Some(TokenTree::Token(ident.span, token).into()) } Nonterminal::NtLifetime(ident) => { let token = Token::Lifetime(ident); - tokens = Some(TokenTree::Token(ident.span, token).into()); + Some(TokenTree::Token(ident.span, token).into()) } Nonterminal::NtTT(ref tt) => { - tokens = Some(tt.clone().into()); + Some(tt.clone().into()) } - _ => {} - } + _ => None, + }; - let tokens_for_real = nt.1.force(|| { - // FIXME(#43081): Avoid this pretty-print + reparse hack - let source = pprust::nonterminal_to_string(&nt.0); - let filename = FileName::macro_expansion_source_code(&source); - let (tokens, errors) = parse_stream_from_source_str( - filename, source, sess, Some(span)); - emit_unclosed_delims(&errors, &sess.span_diagnostic); - tokens - }); + // FIXME(#43081): Avoid this pretty-print + reparse hack + let source = pprust::nonterminal_to_string(&nt); + let filename = FileName::macro_expansion_source_code(&source); + let (tokens_for_real, errors) = + parse_stream_from_source_str(filename, source, sess, Some(span)); + emit_unclosed_delims(&errors, &sess.span_diagnostic); // During early phases of the compiler the AST could get modified // directly (e.g., attributes added or removed) and the internal cache @@ -734,52 +723,6 @@ crate fn is_op(tok: &Token) -> bool { } } -#[derive(Clone)] -pub struct LazyTokenStream(Lock>); - -impl cmp::Eq for LazyTokenStream {} -impl PartialEq for LazyTokenStream { - fn eq(&self, _other: &LazyTokenStream) -> bool { - true - } -} - -impl fmt::Debug for LazyTokenStream { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - fmt::Debug::fmt(&self.clone().0.into_inner(), f) - } -} - -impl LazyTokenStream { - pub fn new() -> Self { - LazyTokenStream(Lock::new(None)) - } - - fn force TokenStream>(&self, f: F) -> TokenStream { - let mut opt_stream = self.0.lock(); - if opt_stream.is_none() { - *opt_stream = Some(f()); - } - opt_stream.clone().unwrap() - } -} - -impl Encodable for LazyTokenStream { - fn encode(&self, _: &mut S) -> Result<(), S::Error> { - Ok(()) - } -} - -impl Decodable for LazyTokenStream { - fn decode(_: &mut D) -> Result { - Ok(LazyTokenStream::new()) - } -} - -impl ::std::hash::Hash for LazyTokenStream { - fn hash(&self, _hasher: &mut H) {} -} - fn prepend_attrs(sess: &ParseSess, attrs: &[ast::Attribute], tokens: Option<&tokenstream::TokenStream>, diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 0e48e3a5dff2b..dcf9815f6d1ba 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -257,7 +257,7 @@ pub fn token_to_string(tok: &Token) -> String { token::Comment => "/* */".to_string(), token::Shebang(s) => format!("/* shebang: {}*/", s), - token::Interpolated(ref nt) => nonterminal_to_string(&nt.0), + token::Interpolated(ref nt) => nonterminal_to_string(nt), } } diff --git a/src/libsyntax_ext/deriving/custom.rs b/src/libsyntax_ext/deriving/custom.rs index 6aba4d83cd27c..cfc3c931598a1 100644 --- a/src/libsyntax_ext/deriving/custom.rs +++ b/src/libsyntax_ext/deriving/custom.rs @@ -2,6 +2,7 @@ use crate::proc_macro_impl::EXEC_STRATEGY; use crate::proc_macro_server; use errors::FatalError; +use rustc_data_structures::sync::Lrc; use syntax::ast::{self, ItemKind, Attribute, Mac}; use syntax::attr::{mark_used, mark_known}; use syntax::source_map::Span; @@ -65,7 +66,7 @@ impl MultiItemModifier for ProcMacroDerive { // Mark attributes as known, and used. MarkAttrs(&self.attrs).visit_item(&item); - let token = Token::interpolated(token::NtItem(item)); + let token = Token::Interpolated(Lrc::new(token::NtItem(item))); let input = tokenstream::TokenTree::Token(DUMMY_SP, token).into(); let server = proc_macro_server::Rustc::new(ecx); From f0d8fbd283654479c50a2fec94bf2362eed0f189 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 15 Feb 2019 12:36:10 +1100 Subject: [PATCH 29/49] Avoid a `clone()` in `transcribe()`. The current code (expensively) clones the value within an `Rc`. This commit changes things so that the `Rc` itself is (cheaply) cloned instead, avoid some allocations. This requires converting a few `Rc` instances to `Lrc`. --- src/libsyntax/ext/tt/macro_parser.rs | 19 ++++++++++--------- src/libsyntax/ext/tt/transcribe.rs | 3 +-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libsyntax/ext/tt/macro_parser.rs b/src/libsyntax/ext/tt/macro_parser.rs index c2607ed530cf0..fe1cffb092b1c 100644 --- a/src/libsyntax/ext/tt/macro_parser.rs +++ b/src/libsyntax/ext/tt/macro_parser.rs @@ -88,6 +88,7 @@ use smallvec::{smallvec, SmallVec}; use syntax_pos::Span; use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::sync::Lrc; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::mem; use std::ops::{Deref, DerefMut}; @@ -179,7 +180,7 @@ struct MatcherPos<'root, 'tt: 'root> { /// all bound matches from the submatcher into the shared top-level `matches` vector. If `sep` /// and `up` are `Some`, then `matches` is _not_ the shared top-level list. Instead, if one /// wants the shared `matches`, one should use `up.matches`. - matches: Box<[Rc]>, + matches: Box<[Lrc]>, /// The position in `matches` corresponding to the first metavar in this matcher's sequence of /// token trees. In other words, the first metavar in the first token of `top_elts` corresponds /// to `matches[match_lo]`. @@ -218,7 +219,7 @@ struct MatcherPos<'root, 'tt: 'root> { impl<'root, 'tt> MatcherPos<'root, 'tt> { /// Adds `m` as a named match for the `idx`-th metavar. fn push_match(&mut self, idx: usize, m: NamedMatch) { - let matches = Rc::make_mut(&mut self.matches[idx]); + let matches = Lrc::make_mut(&mut self.matches[idx]); matches.push(m); } } @@ -295,11 +296,11 @@ pub fn count_names(ms: &[TokenTree]) -> usize { } /// `len` `Vec`s (initially shared and empty) that will store matches of metavars. -fn create_matches(len: usize) -> Box<[Rc]> { +fn create_matches(len: usize) -> Box<[Lrc]> { if len == 0 { vec![] } else { - let empty_matches = Rc::new(SmallVec::new()); + let empty_matches = Lrc::new(SmallVec::new()); vec![empty_matches; len] }.into_boxed_slice() } @@ -353,8 +354,8 @@ fn initial_matcher_pos<'root, 'tt>(ms: &'tt [TokenTree], open: Span) -> MatcherP /// token tree it was derived from. #[derive(Debug, Clone)] pub enum NamedMatch { - MatchedSeq(Rc, DelimSpan), - MatchedNonterminal(Rc), + MatchedSeq(Lrc, DelimSpan), + MatchedNonterminal(Lrc), } /// Takes a sequence of token trees `ms` representing a matcher which successfully matched input @@ -561,7 +562,7 @@ fn inner_parse_loop<'root, 'tt>( new_item.match_cur += seq.num_captures; new_item.idx += 1; for idx in item.match_cur..item.match_cur + seq.num_captures { - new_item.push_match(idx, MatchedSeq(Rc::new(smallvec![]), sp)); + new_item.push_match(idx, MatchedSeq(Lrc::new(smallvec![]), sp)); } cur_items.push(new_item); } @@ -707,7 +708,7 @@ pub fn parse( let matches = eof_items[0] .matches .iter_mut() - .map(|dv| Rc::make_mut(dv).pop().unwrap()); + .map(|dv| Lrc::make_mut(dv).pop().unwrap()); return nameize(sess, ms, matches); } else if eof_items.len() > 1 { return Error( @@ -780,7 +781,7 @@ pub fn parse( let match_cur = item.match_cur; item.push_match( match_cur, - MatchedNonterminal(Rc::new(parse_nt(&mut parser, span, &ident.as_str()))), + MatchedNonterminal(Lrc::new(parse_nt(&mut parser, span, &ident.as_str()))), ); item.idx += 1; item.match_cur += 1; diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index 5c240e237f632..bd2adb5ac13ba 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -149,8 +149,7 @@ pub fn transcribe(cx: &ExtCtxt<'_>, result.push(tt.clone().into()); } else { sp = sp.apply_mark(cx.current_expansion.mark); - let token = - TokenTree::Token(sp, Token::Interpolated(Lrc::new((**nt).clone()))); + let token = TokenTree::Token(sp, Token::Interpolated(nt.clone())); result.push(token.into()); } } else { From 82ad4f1f45a60995c0955e28bbed3885008e3ee5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 18 Feb 2019 10:06:26 +1100 Subject: [PATCH 30/49] Make `interpolated_to_tokenstream` a method on `Nonterminal`. --- src/librustc/hir/lowering.rs | 2 +- src/libsyntax/parse/token.rs | 163 ++++++++++++------------- src/libsyntax/tokenstream.rs | 4 +- src/libsyntax_ext/proc_macro_server.rs | 2 +- 4 files changed, 85 insertions(+), 86 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index bbbd38cfed7e4..961e892789a81 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1132,7 +1132,7 @@ impl<'a> LoweringContext<'a> { fn lower_token(&mut self, token: Token, span: Span) -> TokenStream { match token { Token::Interpolated(nt) => { - let tts = Token::interpolated_to_tokenstream(&self.sess.parse_sess, nt, span); + let tts = nt.to_tokenstream(&self.sess.parse_sess, span); self.lower_token_stream(tts) } other => TokenTree::Token(span, other).into(), diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index a1d1a0f51baf0..eec422d6266c3 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -86,7 +86,7 @@ impl Lit { } } - // See comments in `interpolated_to_tokenstream` for why we care about + // See comments in `Nonterminal::to_tokenstream` for why we care about // *probably* equal here rather than actual equality fn probably_equal_for_proc_macro(&self, other: &Lit) -> bool { mem::discriminant(self) == mem::discriminant(other) @@ -502,87 +502,7 @@ impl Token { } } - pub fn interpolated_to_tokenstream(sess: &ParseSess, nt: Lrc, span: Span) - -> TokenStream { - // An `Interpolated` token means that we have a `Nonterminal` - // which is often a parsed AST item. At this point we now need - // to convert the parsed AST to an actual token stream, e.g. - // un-parse it basically. - // - // Unfortunately there's not really a great way to do that in a - // guaranteed lossless fashion right now. The fallback here is - // to just stringify the AST node and reparse it, but this loses - // all span information. - // - // As a result, some AST nodes are annotated with the token - // stream they came from. Here we attempt to extract these - // lossless token streams before we fall back to the - // stringification. - let tokens = match *nt { - Nonterminal::NtItem(ref item) => { - prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) - } - Nonterminal::NtTraitItem(ref item) => { - prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) - } - Nonterminal::NtImplItem(ref item) => { - prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) - } - Nonterminal::NtIdent(ident, is_raw) => { - let token = Token::Ident(ident, is_raw); - Some(TokenTree::Token(ident.span, token).into()) - } - Nonterminal::NtLifetime(ident) => { - let token = Token::Lifetime(ident); - Some(TokenTree::Token(ident.span, token).into()) - } - Nonterminal::NtTT(ref tt) => { - Some(tt.clone().into()) - } - _ => None, - }; - - // FIXME(#43081): Avoid this pretty-print + reparse hack - let source = pprust::nonterminal_to_string(&nt); - let filename = FileName::macro_expansion_source_code(&source); - let (tokens_for_real, errors) = - parse_stream_from_source_str(filename, source, sess, Some(span)); - emit_unclosed_delims(&errors, &sess.span_diagnostic); - - // During early phases of the compiler the AST could get modified - // directly (e.g., attributes added or removed) and the internal cache - // of tokens my not be invalidated or updated. Consequently if the - // "lossless" token stream disagrees with our actual stringification - // (which has historically been much more battle-tested) then we go - // with the lossy stream anyway (losing span information). - // - // Note that the comparison isn't `==` here to avoid comparing spans, - // but it *also* is a "probable" equality which is a pretty weird - // definition. We mostly want to catch actual changes to the AST - // like a `#[cfg]` being processed or some weird `macro_rules!` - // expansion. - // - // What we *don't* want to catch is the fact that a user-defined - // literal like `0xf` is stringified as `15`, causing the cached token - // stream to not be literal `==` token-wise (ignoring spans) to the - // token stream we got from stringification. - // - // Instead the "probably equal" check here is "does each token - // recursively have the same discriminant?" We basically don't look at - // the token values here and assume that such fine grained token stream - // modifications, including adding/removing typically non-semantic - // tokens such as extra braces and commas, don't happen. - if let Some(tokens) = tokens { - if tokens.probably_equal_for_proc_macro(&tokens_for_real) { - return tokens - } - info!("cached tokens found, but they're not \"probably equal\", \ - going with stringified version"); - } - return tokens_for_real - } - - // See comments in `interpolated_to_tokenstream` for why we care about + // See comments in `Nonterminal::to_tokenstream` for why we care about // *probably* equal here rather than actual equality crate fn probably_equal_for_proc_macro(&self, other: &Token) -> bool { if mem::discriminant(self) != mem::discriminant(other) { @@ -714,6 +634,85 @@ impl fmt::Debug for Nonterminal { } } +impl Nonterminal { + pub fn to_tokenstream(&self, sess: &ParseSess, span: Span) -> TokenStream { + // A `Nonterminal` is often a parsed AST item. At this point we now + // need to convert the parsed AST to an actual token stream, e.g. + // un-parse it basically. + // + // Unfortunately there's not really a great way to do that in a + // guaranteed lossless fashion right now. The fallback here is to just + // stringify the AST node and reparse it, but this loses all span + // information. + // + // As a result, some AST nodes are annotated with the token stream they + // came from. Here we attempt to extract these lossless token streams + // before we fall back to the stringification. + let tokens = match *self { + Nonterminal::NtItem(ref item) => { + prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) + } + Nonterminal::NtTraitItem(ref item) => { + prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) + } + Nonterminal::NtImplItem(ref item) => { + prepend_attrs(sess, &item.attrs, item.tokens.as_ref(), span) + } + Nonterminal::NtIdent(ident, is_raw) => { + let token = Token::Ident(ident, is_raw); + Some(TokenTree::Token(ident.span, token).into()) + } + Nonterminal::NtLifetime(ident) => { + let token = Token::Lifetime(ident); + Some(TokenTree::Token(ident.span, token).into()) + } + Nonterminal::NtTT(ref tt) => { + Some(tt.clone().into()) + } + _ => None, + }; + + // FIXME(#43081): Avoid this pretty-print + reparse hack + let source = pprust::nonterminal_to_string(self); + let filename = FileName::macro_expansion_source_code(&source); + let (tokens_for_real, errors) = + parse_stream_from_source_str(filename, source, sess, Some(span)); + emit_unclosed_delims(&errors, &sess.span_diagnostic); + + // During early phases of the compiler the AST could get modified + // directly (e.g., attributes added or removed) and the internal cache + // of tokens my not be invalidated or updated. Consequently if the + // "lossless" token stream disagrees with our actual stringification + // (which has historically been much more battle-tested) then we go + // with the lossy stream anyway (losing span information). + // + // Note that the comparison isn't `==` here to avoid comparing spans, + // but it *also* is a "probable" equality which is a pretty weird + // definition. We mostly want to catch actual changes to the AST + // like a `#[cfg]` being processed or some weird `macro_rules!` + // expansion. + // + // What we *don't* want to catch is the fact that a user-defined + // literal like `0xf` is stringified as `15`, causing the cached token + // stream to not be literal `==` token-wise (ignoring spans) to the + // token stream we got from stringification. + // + // Instead the "probably equal" check here is "does each token + // recursively have the same discriminant?" We basically don't look at + // the token values here and assume that such fine grained token stream + // modifications, including adding/removing typically non-semantic + // tokens such as extra braces and commas, don't happen. + if let Some(tokens) = tokens { + if tokens.probably_equal_for_proc_macro(&tokens_for_real) { + return tokens + } + info!("cached tokens found, but they're not \"probably equal\", \ + going with stringified version"); + } + return tokens_for_real + } +} + crate fn is_op(tok: &Token) -> bool { match *tok { OpenDelim(..) | CloseDelim(..) | Literal(..) | DocComment(..) | diff --git a/src/libsyntax/tokenstream.rs b/src/libsyntax/tokenstream.rs index c4f2cffb09708..283679e758b54 100644 --- a/src/libsyntax/tokenstream.rs +++ b/src/libsyntax/tokenstream.rs @@ -72,7 +72,7 @@ impl TokenTree { } } - // See comments in `interpolated_to_tokenstream` for why we care about + // See comments in `Nonterminal::to_tokenstream` for why we care about // *probably* equal here rather than actual equality // // This is otherwise the same as `eq_unspanned`, only recursing with a @@ -310,7 +310,7 @@ impl TokenStream { t1.next().is_none() && t2.next().is_none() } - // See comments in `interpolated_to_tokenstream` for why we care about + // See comments in `Nonterminal::to_tokenstream` for why we care about // *probably* equal here rather than actual equality // // This is otherwise the same as `eq_unspanned`, only recursing with a diff --git a/src/libsyntax_ext/proc_macro_server.rs b/src/libsyntax_ext/proc_macro_server.rs index 60ce65baa48a3..699539b62f515 100644 --- a/src/libsyntax_ext/proc_macro_server.rs +++ b/src/libsyntax_ext/proc_macro_server.rs @@ -179,7 +179,7 @@ impl FromInternal<(TreeAndJoint, &'_ ParseSess, &'_ mut Vec)> } Interpolated(nt) => { - let stream = Token::interpolated_to_tokenstream(sess, nt, span); + let stream = nt.to_tokenstream(sess, span); TokenTree::Group(Group { delimiter: Delimiter::None, stream, From 895a79423bf5298e13a177ee6317f43380d437bc Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 18 Feb 2019 10:17:59 +1100 Subject: [PATCH 31/49] Remove some unnecessary `into()` calls. These are probably leftovers from recent `TokenStream` simplifications. --- src/librustc/hir/lowering.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 961e892789a81..f891e6470ae40 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -1124,7 +1124,7 @@ impl<'a> LoweringContext<'a> { TokenTree::Delimited(span, delim, tts) => TokenTree::Delimited( span, delim, - self.lower_token_stream(tts.into()).into(), + self.lower_token_stream(tts), ).into(), } } From de0554805cf2ec2c9fb01d86797214927f96d2e9 Mon Sep 17 00:00:00 2001 From: Clint Frederickson Date: Mon, 18 Feb 2019 12:33:33 -0700 Subject: [PATCH 32/49] re-blessing error output: ./x.py test src/test/ui --stage 1 --bless --- src/test/ui/borrowck/borrowck-asm.mir.stderr | 4 ++-- .../ui/borrowck/borrowck-describe-lvalue.mir.stderr | 2 +- src/test/ui/borrowck/borrowck-drop-from-guard.stderr | 2 +- src/test/ui/borrowck/borrowck-issue-48962.stderr | 4 ++-- .../borrowck-move-moved-value-into-closure.mir.stderr | 2 +- .../borrowck/borrowck-move-out-from-array.mir.stderr | 4 ++-- src/test/ui/borrowck/borrowck-reinit.stderr | 2 +- ...499-field-mutation-of-moved-out-with-mut.nll.stderr | 6 +++--- .../issue-54499-field-mutation-of-moved-out.nll.stderr | 6 +++--- .../ui/borrowck/two-phase-nonrecv-autoref.nll.stderr | 4 ++-- src/test/ui/issues/issue-29723.stderr | 2 +- src/test/ui/issues/issue-34721.stderr | 2 +- src/test/ui/moves/moves-based-on-type-tuple.stderr | 2 +- src/test/ui/nll/closure-access-spans.stderr | 8 ++++---- src/test/ui/nll/closure-move-spans.stderr | 6 +++--- src/test/ui/nll/decl-macro-illegal-copy.stderr | 2 +- .../issue-21232-partial-init-and-erroneous-use.stderr | 2 +- .../ui/nll/issue-21232-partial-init-and-use.stderr | 10 +++++----- src/test/ui/try-block/try-block-bad-lifetime.stderr | 2 +- .../ui/try-block/try-block-maybe-bad-lifetime.stderr | 2 +- 20 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/test/ui/borrowck/borrowck-asm.mir.stderr b/src/test/ui/borrowck/borrowck-asm.mir.stderr index 71b4dbe2aa0c9..86e4832b3873c 100644 --- a/src/test/ui/borrowck/borrowck-asm.mir.stderr +++ b/src/test/ui/borrowck/borrowck-asm.mir.stderr @@ -8,7 +8,7 @@ LL | asm!("nop" : : "r"(x)); | - value moved here LL | } LL | let z = x; //[ast]~ ERROR use of moved value: `x` - | ^ value used here after partial move + | ^ value used here after move error[E0503]: cannot use `x` because it was mutably borrowed --> $DIR/borrowck-asm.rs:35:32 @@ -71,7 +71,7 @@ LL | let x = &mut 2; | - move occurs because `x` has type `&mut i32`, which does not implement the `Copy` trait LL | unsafe { LL | asm!("nop" : : "r"(x), "r"(x) ); //[ast]~ ERROR use of moved value - | - ^ value used here after partial move + | - ^ value used here after move | | | value moved here diff --git a/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr b/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr index 52cb98333ac2e..279548f870fd0 100644 --- a/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr +++ b/src/test/ui/borrowck/borrowck-describe-lvalue.mir.stderr @@ -367,7 +367,7 @@ error[E0382]: use of moved value: `x` LL | drop(x); | - value moved here LL | drop(x); //[ast]~ ERROR use of moved value: `x` - | ^ value used here after partial move + | ^ value used here after move | = note: move occurs because `x` has type `std::vec::Vec`, which does not implement the `Copy` trait diff --git a/src/test/ui/borrowck/borrowck-drop-from-guard.stderr b/src/test/ui/borrowck/borrowck-drop-from-guard.stderr index c2db8b5830f55..07b597f480feb 100644 --- a/src/test/ui/borrowck/borrowck-drop-from-guard.stderr +++ b/src/test/ui/borrowck/borrowck-drop-from-guard.stderr @@ -8,7 +8,7 @@ LL | Some(_) if { drop(my_str); false } => {} | ------ value moved here LL | Some(_) => {} LL | None => { foo(my_str); } //~ ERROR [E0382] - | ^^^^^^ value used here after partial move + | ^^^^^^ value used here after move error: aborting due to previous error diff --git a/src/test/ui/borrowck/borrowck-issue-48962.stderr b/src/test/ui/borrowck/borrowck-issue-48962.stderr index d0105e311bc69..de4894d5b526b 100644 --- a/src/test/ui/borrowck/borrowck-issue-48962.stderr +++ b/src/test/ui/borrowck/borrowck-issue-48962.stderr @@ -6,7 +6,7 @@ LL | let mut src = &mut node; LL | {src}; | --- value moved here LL | src.next = None; //~ ERROR use of moved value: `src` [E0382] - | ^^^^^^^^ value used here after partial move + | ^^^^^^^^ value used here after move error[E0382]: use of moved value: `src` --> $DIR/borrowck-issue-48962.rs:22:5 @@ -16,7 +16,7 @@ LL | let mut src = &mut (22, 44); LL | {src}; | --- value moved here LL | src.0 = 66; //~ ERROR use of moved value: `src` [E0382] - | ^^^^^^^^^^ value used here after partial move + | ^^^^^^^^^^ value used here after move error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr b/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr index e362fcb7d54cd..0789926563ce7 100644 --- a/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr +++ b/src/test/ui/borrowck/borrowck-move-moved-value-into-closure.mir.stderr @@ -11,7 +11,7 @@ LL | call_f(move|| { *t + 1 }); LL | call_f(move|| { *t + 1 }); //[ast]~ ERROR capture of moved value | ^^^^^^ - use occurs due to use in closure | | - | value used here after partial move + | value used here after move error: aborting due to previous error diff --git a/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr b/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr index e366d7b95c368..f866ff9e9bae1 100644 --- a/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr +++ b/src/test/ui/borrowck/borrowck-move-out-from-array.mir.stderr @@ -4,7 +4,7 @@ error[E0382]: use of moved value: `a[..]` LL | let [_, _x] = a; | -- value moved here LL | let [.., _y] = a; //[ast]~ ERROR [E0382] - | ^^ value used here after partial move + | ^^ value used here after move | = note: move occurs because `a[..]` has type `std::boxed::Box`, which does not implement the `Copy` trait @@ -14,7 +14,7 @@ error[E0382]: use of moved value: `a[..]` LL | let [_x, _] = a; | -- value moved here LL | let [_y..] = a; //[ast]~ ERROR [E0382] - | ^^ value used here after partial move + | ^^ value used here after move | = note: move occurs because `a[..]` has type `std::boxed::Box`, which does not implement the `Copy` trait diff --git a/src/test/ui/borrowck/borrowck-reinit.stderr b/src/test/ui/borrowck/borrowck-reinit.stderr index 351fbab2d3f09..96f3981ac2fe6 100644 --- a/src/test/ui/borrowck/borrowck-reinit.stderr +++ b/src/test/ui/borrowck/borrowck-reinit.stderr @@ -17,7 +17,7 @@ LL | let mut x = Box::new(0); LL | drop(x); | - value moved here LL | let _ = (1,x); //~ ERROR use of moved value: `x` (Ast) - | ^ value used here after partial move + | ^ value used here after move error: aborting due to 2 previous errors diff --git a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr index bdf99b0906e6f..42aa038170238 100644 --- a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr +++ b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out-with-mut.nll.stderr @@ -6,7 +6,7 @@ LL | let mut t: Tuple = (S(0), 0); LL | drop(t); | - value moved here LL | t.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^^^ value partially assigned here after move error[E0382]: assign to part of moved value: `u` --> $DIR/issue-54499-field-mutation-of-moved-out-with-mut.rs:34:9 @@ -16,7 +16,7 @@ LL | let mut u: Tpair = Tpair(S(0), 0); LL | drop(u); | - value moved here LL | u.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^^^ value partially assigned here after move error[E0382]: assign to part of moved value: `v` --> $DIR/issue-54499-field-mutation-of-moved-out-with-mut.rs:45:9 @@ -26,7 +26,7 @@ LL | let mut v: Spair = Spair { x: S(0), y: 0 }; LL | drop(v); | - value moved here LL | v.x = S(1); - | ^^^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^^^ value partially assigned here after move error: aborting due to 3 previous errors diff --git a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr index 9505f305c6151..1184907f307cb 100644 --- a/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr +++ b/src/test/ui/borrowck/issue-54499-field-mutation-of-moved-out.nll.stderr @@ -15,7 +15,7 @@ LL | let t: Tuple = (S(0), 0); LL | drop(t); | - value moved here LL | t.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^^^ value partially assigned here after move error[E0594]: cannot assign to `t.1`, as `t` is not declared as mutable --> $DIR/issue-54499-field-mutation-of-moved-out.rs:27:9 @@ -43,7 +43,7 @@ LL | let u: Tpair = Tpair(S(0), 0); LL | drop(u); | - value moved here LL | u.0 = S(1); - | ^^^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^^^ value partially assigned here after move error[E0594]: cannot assign to `u.1`, as `u` is not declared as mutable --> $DIR/issue-54499-field-mutation-of-moved-out.rs:42:9 @@ -71,7 +71,7 @@ LL | let v: Spair = Spair { x: S(0), y: 0 }; LL | drop(v); | - value moved here LL | v.x = S(1); - | ^^^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^^^ value partially assigned here after move error[E0594]: cannot assign to `v.y`, as `v` is not declared as mutable --> $DIR/issue-54499-field-mutation-of-moved-out.rs:57:9 diff --git a/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr b/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr index 39ff1234eb7f9..d026f81b7aad6 100644 --- a/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr +++ b/src/test/ui/borrowck/two-phase-nonrecv-autoref.nll.stderr @@ -13,7 +13,7 @@ error[E0382]: use of moved value: `*f` LL | fn twice_ten_so i32>(f: Box) { | - consider adding a `Copy` constraint to this type argument LL | f(f(10)); - | - ^ value used here after partial move + | - ^ value used here after move | | | value moved here | @@ -44,7 +44,7 @@ error[E0382]: use of moved value: `*f` --> $DIR/two-phase-nonrecv-autoref.rs:85:11 | LL | f(f(10)); - | - ^ value used here after partial move + | - ^ value used here after move | | | value moved here | diff --git a/src/test/ui/issues/issue-29723.stderr b/src/test/ui/issues/issue-29723.stderr index 71bf9cfd0997c..7928af5d5a5cd 100644 --- a/src/test/ui/issues/issue-29723.stderr +++ b/src/test/ui/issues/issue-29723.stderr @@ -8,7 +8,7 @@ LL | 0 if { drop(s); false } => String::from("oops"), | - value moved here ... LL | s - | ^ value used here after partial move + | ^ value used here after move error: aborting due to previous error diff --git a/src/test/ui/issues/issue-34721.stderr b/src/test/ui/issues/issue-34721.stderr index 936467fce8277..2ed7b543e713c 100644 --- a/src/test/ui/issues/issue-34721.stderr +++ b/src/test/ui/issues/issue-34721.stderr @@ -13,7 +13,7 @@ LL | x.zero() | - value moved here LL | }; LL | x.zero() - | ^ value used here after partial move + | ^ value used here after move error: aborting due to previous error diff --git a/src/test/ui/moves/moves-based-on-type-tuple.stderr b/src/test/ui/moves/moves-based-on-type-tuple.stderr index bbf88959ebd9d..c49dbdab40210 100644 --- a/src/test/ui/moves/moves-based-on-type-tuple.stderr +++ b/src/test/ui/moves/moves-based-on-type-tuple.stderr @@ -14,7 +14,7 @@ error[E0382]: use of moved value: `x` (Mir) LL | fn dup(x: Box) -> Box<(Box,Box)> { | - move occurs because `x` has type `std::boxed::Box`, which does not implement the `Copy` trait LL | box (x, x) - | - ^ value used here after partial move + | - ^ value used here after move | | | value moved here diff --git a/src/test/ui/nll/closure-access-spans.stderr b/src/test/ui/nll/closure-access-spans.stderr index 67b0fb422e26f..3ca0aefb592e0 100644 --- a/src/test/ui/nll/closure-access-spans.stderr +++ b/src/test/ui/nll/closure-access-spans.stderr @@ -66,7 +66,7 @@ LL | let r = x; LL | || x.len(); //~ ERROR | ^^ - borrow occurs due to use in closure | | - | value borrowed here after partial move + | value borrowed here after move error[E0382]: borrow of moved value: `x` --> $DIR/closure-access-spans.rs:42:5 @@ -78,7 +78,7 @@ LL | let r = x; LL | || x = String::new(); //~ ERROR | ^^ - borrow occurs due to use in closure | | - | value borrowed here after partial move + | value borrowed here after move error[E0382]: borrow of moved value: `x` --> $DIR/closure-access-spans.rs:47:5 @@ -90,7 +90,7 @@ LL | let r = x; LL | || *x = String::new(); //~ ERROR | ^^ - borrow occurs due to use in closure | | - | value borrowed here after partial move + | value borrowed here after move error[E0382]: use of moved value: `x` --> $DIR/closure-access-spans.rs:52:5 @@ -102,7 +102,7 @@ LL | let r = x; LL | || x; //~ ERROR | ^^ - use occurs due to use in closure | | - | value used here after partial move + | value used here after move error: aborting due to 9 previous errors diff --git a/src/test/ui/nll/closure-move-spans.stderr b/src/test/ui/nll/closure-move-spans.stderr index 53f3a8c82e0b5..6750c4047601a 100644 --- a/src/test/ui/nll/closure-move-spans.stderr +++ b/src/test/ui/nll/closure-move-spans.stderr @@ -8,7 +8,7 @@ LL | || x; | | | value moved into closure here LL | let y = x; //~ ERROR - | ^ value used here after partial move + | ^ value used here after move error[E0382]: borrow of moved value: `x` --> $DIR/closure-move-spans.rs:12:13 @@ -20,7 +20,7 @@ LL | || x; | | | value moved into closure here LL | let y = &x; //~ ERROR - | ^^ value borrowed here after partial move + | ^^ value borrowed here after move error[E0382]: borrow of moved value: `x` --> $DIR/closure-move-spans.rs:17:13 @@ -32,7 +32,7 @@ LL | || x; | | | value moved into closure here LL | let y = &mut x; //~ ERROR - | ^^^^^^ value borrowed here after partial move + | ^^^^^^ value borrowed here after move error: aborting due to 3 previous errors diff --git a/src/test/ui/nll/decl-macro-illegal-copy.stderr b/src/test/ui/nll/decl-macro-illegal-copy.stderr index 9e1ba3a4e1bcb..9232ff52393e1 100644 --- a/src/test/ui/nll/decl-macro-illegal-copy.stderr +++ b/src/test/ui/nll/decl-macro-illegal-copy.stderr @@ -5,7 +5,7 @@ LL | $wrapper.inner | -------------- value moved here ... LL | wrapper.inner, - | ^^^^^^^^^^^^^ value used here after partial move + | ^^^^^^^^^^^^^ value used here after move | = note: move occurs because `wrapper.inner` has type `NonCopy`, which does not implement the `Copy` trait diff --git a/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr b/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr index 7715c14d7a345..54c728e3d2783 100644 --- a/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr +++ b/src/test/ui/nll/issue-21232-partial-init-and-erroneous-use.stderr @@ -18,7 +18,7 @@ LL | let mut d = D { x: 0, s: S{ y: 0, z: 0 } }; LL | drop(d); | - value moved here LL | d.x = 10; - | ^^^^^^^^ value assigned here after partial move + | ^^^^^^^^ value assigned here after move error[E0381]: assign to part of possibly uninitialized variable: `d` --> $DIR/issue-21232-partial-init-and-erroneous-use.rs:49:5 diff --git a/src/test/ui/nll/issue-21232-partial-init-and-use.stderr b/src/test/ui/nll/issue-21232-partial-init-and-use.stderr index ac15d9ba26c4d..23da533252cb9 100644 --- a/src/test/ui/nll/issue-21232-partial-init-and-use.stderr +++ b/src/test/ui/nll/issue-21232-partial-init-and-use.stderr @@ -18,7 +18,7 @@ LL | let mut s: S = S::new(); drop(s); | | | move occurs because `s` has type `S>`, which does not implement the `Copy` trait LL | s.x = 10; s.y = Box::new(20); - | ^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^ value partially assigned here after move error[E0382]: assign to part of moved value: `t` --> $DIR/issue-21232-partial-init-and-use.rs:120:5 @@ -28,7 +28,7 @@ LL | let mut t: T = (0, Box::new(0)); drop(t); | | | move occurs because `t` has type `(u32, std::boxed::Box)`, which does not implement the `Copy` trait LL | t.0 = 10; t.1 = Box::new(20); - | ^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^ value partially assigned here after move error[E0381]: assign to part of possibly uninitialized variable: `s` --> $DIR/issue-21232-partial-init-and-use.rs:127:5 @@ -50,7 +50,7 @@ LL | let mut s: S = S::new(); drop(s); | | | move occurs because `s` has type `S>`, which does not implement the `Copy` trait LL | s.x = 10; - | ^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^ value partially assigned here after move error[E0382]: assign to part of moved value: `t` --> $DIR/issue-21232-partial-init-and-use.rs:148:5 @@ -60,7 +60,7 @@ LL | let mut t: T = (0, Box::new(0)); drop(t); | | | move occurs because `t` has type `(u32, std::boxed::Box)`, which does not implement the `Copy` trait LL | t.0 = 10; - | ^^^^^^^^ value partially assigned here after partial move + | ^^^^^^^^ value partially assigned here after move error[E0381]: assign to part of possibly uninitialized variable: `s` --> $DIR/issue-21232-partial-init-and-use.rs:155:5 @@ -159,7 +159,7 @@ LL | match c { LL | c2 => { | -- value moved here LL | c.0 = 2; //~ ERROR assign to part of moved value - | ^^^^^^^ value partially assigned here after partial move + | ^^^^^^^ value partially assigned here after move error[E0382]: assign to part of moved value: `c` --> $DIR/issue-21232-partial-init-and-use.rs:269:13 diff --git a/src/test/ui/try-block/try-block-bad-lifetime.stderr b/src/test/ui/try-block/try-block-bad-lifetime.stderr index 8967e77b5a7cc..b1b925d694ff9 100644 --- a/src/test/ui/try-block/try-block-bad-lifetime.stderr +++ b/src/test/ui/try-block/try-block-bad-lifetime.stderr @@ -32,7 +32,7 @@ LL | Err(k) ?; | - value moved here ... LL | ::std::mem::drop(k); //~ ERROR use of moved value: `k` - | ^ value used here after partial move + | ^ value used here after move error[E0506]: cannot assign to `i` because it is borrowed --> $DIR/try-block-bad-lifetime.rs:32:9 diff --git a/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr b/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr index 33561eae46a2a..dafbde6a5150b 100644 --- a/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr +++ b/src/test/ui/try-block/try-block-maybe-bad-lifetime.stderr @@ -20,7 +20,7 @@ LL | ::std::mem::drop(x); | - value moved here LL | }; LL | println!("{}", x); //~ ERROR borrow of moved value: `x` - | ^ value borrowed here after partial move + | ^ value borrowed here after move error[E0506]: cannot assign to `i` because it is borrowed --> $DIR/try-block-maybe-bad-lifetime.rs:40:9 From 06511573f236a07b8ecd7c3e3d5fef1d53e59352 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Mon, 11 Feb 2019 16:51:32 +0100 Subject: [PATCH 33/49] Remove sys::*::Stderr Write implementation --- src/libstd/io/mod.rs | 3 +++ src/libstd/io/stdio.rs | 8 ++++++-- src/libstd/sys/cloudabi/stdio.rs | 13 ------------- src/libstd/sys/redox/stdio.rs | 15 +-------------- src/libstd/sys/sgx/stdio.rs | 13 ------------- src/libstd/sys/unix/stdio.rs | 15 +-------------- src/libstd/sys/wasm/stdio.rs | 11 +---------- src/libstd/sys/windows/stdio.rs | 15 +-------------- 8 files changed, 13 insertions(+), 80 deletions(-) diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index c0570ae60a19c..4fd114884e367 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -286,6 +286,9 @@ pub use self::stdio::{_print, _eprint}; #[doc(no_inline, hidden)] pub use self::stdio::{set_panic, set_print}; +// Used inside the standard library for panic output. +pub(crate) use self::stdio::stderr_raw; + pub mod prelude; mod buffered; mod cursor; diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 4068c0f9c7de5..4fc11db3ff52a 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -32,7 +32,9 @@ struct StdoutRaw(stdio::Stdout); /// /// This handle is not synchronized or buffered in any fashion. Constructed via /// the `std::io::stdio::stderr_raw` function. -struct StderrRaw(stdio::Stderr); +/// +/// Not exposed, but used inside the standard library for panic output. +pub(crate) struct StderrRaw(stdio::Stderr); /// Constructs a new raw handle to the standard input of this process. /// @@ -61,7 +63,9 @@ fn stdout_raw() -> io::Result { stdio::Stdout::new().map(StdoutRaw) } /// /// The returned handle has no external synchronization or buffering layered on /// top. -fn stderr_raw() -> io::Result { stdio::Stderr::new().map(StderrRaw) } +/// +/// Not exposed, but used inside the standard library for panic output. +pub(crate) fn stderr_raw() -> io::Result { stdio::Stderr::new().map(StderrRaw) } impl Read for StdinRaw { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } diff --git a/src/libstd/sys/cloudabi/stdio.rs b/src/libstd/sys/cloudabi/stdio.rs index 2cd3477cd519d..8ec92326dbd8c 100644 --- a/src/libstd/sys/cloudabi/stdio.rs +++ b/src/libstd/sys/cloudabi/stdio.rs @@ -49,19 +49,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(abi::errno::BADF as i32) } diff --git a/src/libstd/sys/redox/stdio.rs b/src/libstd/sys/redox/stdio.rs index e814b0942c14d..66f84e1752752 100644 --- a/src/libstd/sys/redox/stdio.rs +++ b/src/libstd/sys/redox/stdio.rs @@ -47,19 +47,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(::sys::syscall::EBADF as i32) } @@ -67,5 +54,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn panic_output() -> Option { - Stderr::new().ok() + io::stderr_raw().ok() } diff --git a/src/libstd/sys/sgx/stdio.rs b/src/libstd/sys/sgx/stdio.rs index 6f206cd9a51d7..c8efb931d6fe5 100644 --- a/src/libstd/sys/sgx/stdio.rs +++ b/src/libstd/sys/sgx/stdio.rs @@ -46,19 +46,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn is_ebadf(err: &io::Error) -> bool { diff --git a/src/libstd/sys/unix/stdio.rs b/src/libstd/sys/unix/stdio.rs index 715f2eafb2d9b..e23723222be35 100644 --- a/src/libstd/sys/unix/stdio.rs +++ b/src/libstd/sys/unix/stdio.rs @@ -47,19 +47,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(libc::EBADF as i32) } @@ -67,5 +54,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn panic_output() -> Option { - Stderr::new().ok() + io::stderr_raw().ok() } diff --git a/src/libstd/sys/wasm/stdio.rs b/src/libstd/sys/wasm/stdio.rs index 201619b2fb139..015d3f2066042 100644 --- a/src/libstd/sys/wasm/stdio.rs +++ b/src/libstd/sys/wasm/stdio.rs @@ -45,15 +45,6 @@ impl Stderr { } } -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - (&*self).write(data) - } - fn flush(&mut self) -> io::Result<()> { - (&*self).flush() - } -} - pub const STDIN_BUF_SIZE: usize = 0; pub fn is_ebadf(_err: &io::Error) -> bool { @@ -62,7 +53,7 @@ pub fn is_ebadf(_err: &io::Error) -> bool { pub fn panic_output() -> Option { if cfg!(feature = "wasm_syscall") { - Stderr::new().ok() + io::stderr_raw().ok() } else { None } diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 0ea19a855257b..45a12d075bad0 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -165,19 +165,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - impl Output { pub fn handle(&self) -> c::HANDLE { match *self { @@ -216,5 +203,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = 8 * 1024; pub fn panic_output() -> Option { - Stderr::new().ok() + io::stderr_raw().ok() } From cc20ed678e4fc782cbbe55501bb3e300b5430c37 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Thu, 14 Feb 2019 06:29:10 +0100 Subject: [PATCH 34/49] Remove unused Read implementation on sys::Windows::Stdin --- src/libstd/sys/windows/stdio.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 45a12d075bad0..617124c36b30a 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,7 +1,5 @@ #![unstable(issue = "0", feature = "windows_stdio")] -use io::prelude::*; - use cmp; use io::{self, Cursor}; use ptr; @@ -130,13 +128,6 @@ impl Stdin { } } -#[unstable(reason = "not public", issue = "0", feature = "fd_read")] -impl<'a> Read for &'a Stdin { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - (**self).read(buf) - } -} - impl Stdout { pub fn new() -> io::Result { Ok(Stdout) From f411852add58637a3b8628a8f70146106bdc9719 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Thu, 14 Feb 2019 06:35:20 +0100 Subject: [PATCH 35/49] Refactor Windows stdio and remove stdin double buffering --- src/libstd/sys/windows/process.rs | 4 +- src/libstd/sys/windows/stdio.rs | 290 ++++++++++++++++++------------ 2 files changed, 178 insertions(+), 116 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 08a166bd8c504..2527168a968c4 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -252,9 +252,9 @@ impl Stdio { // should still be unavailable so propagate the // INVALID_HANDLE_VALUE. Stdio::Inherit => { - match stdio::get(stdio_id) { + match stdio::get_handle(stdio_id) { Ok(io) => { - let io = Handle::new(io.handle()); + let io = Handle::new(io); let ret = io.duplicate(0, true, c::DUPLICATE_SAME_ACCESS); io.into_raw(); diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 617124c36b30a..ccfe0ced82fbc 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,131 +1,226 @@ #![unstable(issue = "0", feature = "windows_stdio")] +use cell::Cell; use cmp; -use io::{self, Cursor}; +use io; use ptr; use str; -use sync::Mutex; use sys::c; use sys::cvt; use sys::handle::Handle; -pub enum Output { - Console(c::HANDLE), - Pipe(c::HANDLE), -} - +// Don't cache handles but get them fresh for every read/write. This allows us to track changes to +// the value over time (such as if a process calls `SetStdHandle` while it's running). See #40490. pub struct Stdin { - utf8: Mutex>>, + high_surrogate: Cell, } pub struct Stdout; pub struct Stderr; -pub fn get(handle: c::DWORD) -> io::Result { - let handle = unsafe { c::GetStdHandle(handle) }; +// Apparently Windows doesn't handle large reads on stdin or writes to stdout/stderr well (see +// #13304 for details). +// +// From MSDN (2011): "The storage for this buffer is allocated from a shared heap for the +// process that is 64 KB in size. The maximum size of the buffer will depend on heap usage." +// +// We choose the cap at 8 KiB because libuv does the same, and it seems to be acceptable so far. +const MAX_BUFFER_SIZE: usize = 8192; + +// The standard buffer size of BufReader for Stdin should be able to hold 3x more bytes than there +// are `u16`'s in MAX_BUFFER_SIZE. This ensures the read data can always be completely decoded from +// UTF-16 to UTF-8. +pub const STDIN_BUF_SIZE: usize = MAX_BUFFER_SIZE / 2 * 3; + +pub fn get_handle(handle_id: c::DWORD) -> io::Result { + let handle = unsafe { c::GetStdHandle(handle_id) }; if handle == c::INVALID_HANDLE_VALUE { Err(io::Error::last_os_error()) } else if handle.is_null() { Err(io::Error::from_raw_os_error(c::ERROR_INVALID_HANDLE as i32)) } else { - let mut out = 0; - match unsafe { c::GetConsoleMode(handle, &mut out) } { - 0 => Ok(Output::Pipe(handle)), - _ => Ok(Output::Console(handle)), - } + Ok(handle) } } -fn write(handle: c::DWORD, data: &[u8]) -> io::Result { - let handle = match get(handle)? { - Output::Console(c) => c, - Output::Pipe(p) => { - let handle = Handle::new(p); - let ret = handle.write(data); - handle.into_raw(); - return ret - } - }; +fn is_console(handle: c::HANDLE) -> bool { + // `GetConsoleMode` will return false (0) if this is a pipe (we don't care about the reported + // mode). This will only detect Windows Console, not other terminals connected to a pipe like + // MSYS. Which is exactly what we need, as only Windows Console needs a conversion to UTF-16. + let mut mode = 0; + unsafe { c::GetConsoleMode(handle, &mut mode) != 0 } +} - // As with stdin on windows, stdout often can't handle writes of large - // sizes. For an example, see #14940. For this reason, don't try to - // write the entire output buffer on windows. - // - // For some other references, it appears that this problem has been - // encountered by others [1] [2]. We choose the number 8K just because - // libuv does the same. +fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { + let handle = get_handle(handle_id)?; + if !is_console(handle) { + let handle = Handle::new(handle); + let ret = handle.write(data); + handle.into_raw(); // Don't close the handle + return ret; + } + + // As the console is meant for presenting text, we assume bytes of `data` come from a string + // and are encoded as UTF-8, which needs to be encoded as UTF-16. // - // [1]: https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1232 - // [2]: http://www.mail-archive.com/log4net-dev@logging.apache.org/msg00661.html - const OUT_MAX: usize = 8192; - let len = cmp::min(data.len(), OUT_MAX); + // If the data is not valid UTF-8 we write out as many bytes as are valid. + // Only when there are no valid bytes (which will happen on the next call), return an error. + let len = cmp::min(data.len(), MAX_BUFFER_SIZE); let utf8 = match str::from_utf8(&data[..len]) { Ok(s) => s, - Err(ref e) if e.valid_up_to() == 0 => return Err(invalid_encoding()), + Err(ref e) if e.valid_up_to() == 0 => { + return Err(io::Error::new(io::ErrorKind::InvalidData, + "Windows stdio in console mode does not support non-UTF-8 byte sequences; \ + see https://github.com/rust-lang/rust/issues/23344")) + }, Err(e) => str::from_utf8(&data[..e.valid_up_to()]).unwrap(), }; let utf16 = utf8.encode_utf16().collect::>(); + + let mut written = write_u16s(handle, &utf16)?; + + // Figure out how many bytes of as UTF-8 were written away as UTF-16. + if written >= utf16.len() { + Ok(utf8.len()) + } else { + // Make sure we didn't end up writing only half of a surrogate pair (even though the chance + // is tiny). Because it is not possible for user code to re-slice `data` in such a way that + // a missing surrogate can be produced (and also because of the UTF-8 validation above), + // write the missing surrogate out now. + // Buffering it would mean we have to lie about the number of bytes written. + let first_char_remaining = utf16[written]; + if first_char_remaining >= 0xDCEE && first_char_remaining <= 0xDFFF { // low surrogate + // We just hope this works, and give up otherwise + let _ = write_u16s(handle, &utf16[written..written]); + written += 1; + } + // Calculate the number of bytes of `utf8` that were actually written. + let mut count = 0; + for ch in utf16[..written].iter() { + count += match ch { + 0x0000 ..= 0x007F => 1, + 0x0080 ..= 0x07FF => 2, + 0xDCEE ..= 0xDFFF => 1, // Low surrogate. We already counted 3 bytes for the other. + _ => 3, + }; + } + Ok(count) + } +} + +fn write_u16s(handle: c::HANDLE, data: &[u16]) -> io::Result { let mut written = 0; cvt(unsafe { c::WriteConsoleW(handle, - utf16.as_ptr() as c::LPCVOID, - utf16.len() as u32, + data.as_ptr() as c::LPCVOID, + data.len() as u32, &mut written, ptr::null_mut()) })?; - - // FIXME if this only partially writes the utf16 buffer then we need to - // figure out how many bytes of `data` were actually written - assert_eq!(written as usize, utf16.len()); - Ok(utf8.len()) + Ok(written as usize) } impl Stdin { pub fn new() -> io::Result { - Ok(Stdin { - utf8: Mutex::new(Cursor::new(Vec::new())), - }) + Ok(Stdin { high_surrogate: Cell::new(0) }) } pub fn read(&self, buf: &mut [u8]) -> io::Result { - let handle = match get(c::STD_INPUT_HANDLE)? { - Output::Console(c) => c, - Output::Pipe(p) => { - let handle = Handle::new(p); - let ret = handle.read(buf); - handle.into_raw(); - return ret - } + let handle = get_handle(c::STD_INPUT_HANDLE)?; + if !is_console(handle) { + let handle = Handle::new(handle); + let ret = handle.read(buf); + handle.into_raw(); // Don't close the handle + return ret; + } + + if buf.len() == 0 { + return Ok(0); + } else if buf.len() < 4 { + return Err(io::Error::new(io::ErrorKind::InvalidInput, + "Windows stdin in console mode does not support a buffer too small to; \ + guarantee holding one arbitrary UTF-8 character (4 bytes)")) + } + + let mut utf16_buf = [0u16; MAX_BUFFER_SIZE / 2]; + // In the worst case, an UTF-8 string can take 3 bytes for every `u16` of an UTF-16. So + // we can read at most a third of `buf.len()` chars and uphold the guarantee no data gets + // lost. + let amount = cmp::min(buf.len() / 3, utf16_buf.len()); + let read = self.read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount)?; + let utf16 = &utf16_buf[..read]; + + // FIXME: it would be nice if we could directly decode into the buffer instead of doing an + // allocation. + let data = match String::from_utf16(&utf16) { + Ok(utf8) => utf8.into_bytes(), + Err(..) => { + // We can't really do any better than forget all data and return an error. + return Err(io::Error::new(io::ErrorKind::InvalidData, + "Windows stdin in console mode does not support non-UTF-16 input; \ + encountered unpaired surrogate")) + }, }; - let mut utf8 = self.utf8.lock().unwrap(); - // Read more if the buffer is empty - if utf8.position() as usize == utf8.get_ref().len() { - let mut utf16 = vec![0u16; 0x1000]; - let mut num = 0; - let mut input_control = readconsole_input_control(CTRL_Z_MASK); - cvt(unsafe { - c::ReadConsoleW(handle, - utf16.as_mut_ptr() as c::LPVOID, - utf16.len() as u32, - &mut num, - &mut input_control as c::PCONSOLE_READCONSOLE_CONTROL) - })?; - utf16.truncate(num as usize); - // FIXME: what to do about this data that has already been read? - let mut data = match String::from_utf16(&utf16) { - Ok(utf8) => utf8.into_bytes(), - Err(..) => return Err(invalid_encoding()), - }; - if let Some(&last_byte) = data.last() { - if last_byte == CTRL_Z { - data.pop(); - } + buf.copy_from_slice(&data); + Ok(data.len()) + } + + // We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our + // buffer size, and keep it around for the next read hoping to put them together. + // This is a best effort, and may not work if we are not the only reader on Stdin. + pub fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) + -> io::Result + { + // Insert possibly remaining unpaired surrogate from last read. + let mut start = 0; + if self.high_surrogate.get() != 0 { + buf[0] = self.high_surrogate.replace(0); + start = 1; + if amount == 1 { + // Special case: `Stdin::read` guarantees we can always read at least one new `u16` + // and combine it with an unpaired surrogate, because the UTF-8 buffer is at least + // 4 bytes. + amount = 2; + } + } + let mut amount = read_u16s(handle, &mut buf[start..amount])? + start; + + if amount > 0 { + let last_char = buf[amount - 1]; + if last_char >= 0xD800 && last_char <= 0xDBFF { // high surrogate + self.high_surrogate.set(last_char); + amount -= 1; } - *utf8 = Cursor::new(data); } + Ok(amount) + } +} + +fn read_u16s(handle: c::HANDLE, buf: &mut [u16]) -> io::Result { + // Configure the `pInputControl` parameter to not only return on `\r\n` but also Ctrl-Z, the + // traditional DOS method to indicate end of character stream / user input (SUB). + // See #38274 and https://stackoverflow.com/questions/43836040/win-api-readconsole. + const CTRL_Z: u16 = 0x1A; + const CTRL_Z_MASK: c::ULONG = 1 << CTRL_Z; + let mut input_control = c::CONSOLE_READCONSOLE_CONTROL { + nLength: ::mem::size_of::() as c::ULONG, + nInitialChars: 0, + dwCtrlWakeupMask: CTRL_Z_MASK, + dwControlKeyState: 0, + }; + + let mut amount = 0; + cvt(unsafe { + c::ReadConsoleW(handle, + buf.as_mut_ptr() as c::LPVOID, + buf.len() as u32, + &mut amount, + &mut input_control as c::PCONSOLE_READCONSOLE_CONTROL) + })?; - // MemReader shouldn't error here since we just filled it - utf8.read(buf) + if amount > 0 && buf[amount as usize - 1] == CTRL_Z { + amount -= 1; } + Ok(amount as usize) } impl Stdout { @@ -156,43 +251,10 @@ impl Stderr { } } -impl Output { - pub fn handle(&self) -> c::HANDLE { - match *self { - Output::Console(c) => c, - Output::Pipe(c) => c, - } - } -} - -fn invalid_encoding() -> io::Error { - io::Error::new(io::ErrorKind::InvalidData, - "Windows stdio in console mode does not support non-UTF-8 byte sequences; \ - see https://github.com/rust-lang/rust/issues/23344") -} - -fn readconsole_input_control(wakeup_mask: c::ULONG) -> c::CONSOLE_READCONSOLE_CONTROL { - c::CONSOLE_READCONSOLE_CONTROL { - nLength: ::mem::size_of::() as c::ULONG, - nInitialChars: 0, - dwCtrlWakeupMask: wakeup_mask, - dwControlKeyState: 0, - } -} - -const CTRL_Z: u8 = 0x1A; -const CTRL_Z_MASK: c::ULONG = 0x4000000; //1 << 0x1A - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(c::ERROR_INVALID_HANDLE as i32) } -// The default buffer capacity is 64k, but apparently windows -// doesn't like 64k reads on stdin. See #13304 for details, but the -// idea is that on windows we use a slightly smaller buffer that's -// been seen to be acceptable. -pub const STDIN_BUF_SIZE: usize = 8 * 1024; - pub fn panic_output() -> Option { io::stderr_raw().ok() } From 8e219e7eb5ed554e0602476edfbbee5720e5c36e Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Wed, 20 Feb 2019 14:21:15 +0100 Subject: [PATCH 36/49] Turn duration consts into associated consts --- src/libcore/time.rs | 32 ++++++++++++++++---------------- src/libstd/time.rs | 3 --- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/libcore/time.rs b/src/libcore/time.rs index a751965dffab3..62ed8d6f7906a 100644 --- a/src/libcore/time.rs +++ b/src/libcore/time.rs @@ -23,22 +23,6 @@ const MILLIS_PER_SEC: u64 = 1_000; const MICROS_PER_SEC: u64 = 1_000_000; const MAX_NANOS_F64: f64 = ((u64::MAX as u128 + 1)*(NANOS_PER_SEC as u128)) as f64; -/// The duration of one second. -#[unstable(feature = "duration_constants", issue = "57391")] -pub const SECOND: Duration = Duration::from_secs(1); - -/// The duration of one millisecond. -#[unstable(feature = "duration_constants", issue = "57391")] -pub const MILLISECOND: Duration = Duration::from_millis(1); - -/// The duration of one microsecond. -#[unstable(feature = "duration_constants", issue = "57391")] -pub const MICROSECOND: Duration = Duration::from_micros(1); - -/// The duration of one nanosecond. -#[unstable(feature = "duration_constants", issue = "57391")] -pub const NANOSECOND: Duration = Duration::from_nanos(1); - /// A `Duration` type to represent a span of time, typically used for system /// timeouts. /// @@ -75,6 +59,22 @@ pub struct Duration { } impl Duration { + /// The duration of one second. + #[unstable(feature = "duration_constants", issue = "57391")] + pub const SECOND: Duration = Duration::from_secs(1); + + /// The duration of one millisecond. + #[unstable(feature = "duration_constants", issue = "57391")] + pub const MILLISECOND: Duration = Duration::from_millis(1); + + /// The duration of one microsecond. + #[unstable(feature = "duration_constants", issue = "57391")] + pub const MICROSECOND: Duration = Duration::from_micros(1); + + /// The duration of one nanosecond. + #[unstable(feature = "duration_constants", issue = "57391")] + pub const NANOSECOND: Duration = Duration::from_nanos(1); + /// Creates a new `Duration` from the specified number of whole seconds and /// additional nanoseconds. /// diff --git a/src/libstd/time.rs b/src/libstd/time.rs index 507ea395c6c6c..72a5a070233e1 100644 --- a/src/libstd/time.rs +++ b/src/libstd/time.rs @@ -23,9 +23,6 @@ use sys_common::mutex::Mutex; #[stable(feature = "time", since = "1.3.0")] pub use core::time::Duration; -#[unstable(feature = "duration_constants", issue = "57391")] -pub use core::time::{SECOND, MILLISECOND, MICROSECOND, NANOSECOND}; - /// A measurement of a monotonically nondecreasing clock. /// Opaque and useful only with `Duration`. /// From 02fe6a7ba6b0d1733c9abbe40ac85c26bba38403 Mon Sep 17 00:00:00 2001 From: Clint Frederickson Date: Wed, 20 Feb 2019 09:54:10 -0700 Subject: [PATCH 37/49] ./x.py test src/test/ui --stage 1 --bless -i --compare-mode=nll --- .../ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr | 2 +- .../moves-based-on-type-cyclic-types-issue-4821.nll.stderr | 2 +- .../ui/moves/moves-based-on-type-match-bindings.nll.stderr | 2 +- src/test/ui/ref-suggestion.nll.stderr | 2 +- src/test/ui/unsized-locals/borrow-after-move.nll.stderr | 6 +++--- src/test/ui/unsized-locals/double-move.nll.stderr | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/test/ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr b/src/test/ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr index bdec94b4f84b0..99cbf64fd5d46 100644 --- a/src/test/ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr +++ b/src/test/ui/borrowck/borrowck-uninit-field-access.ast.nll.stderr @@ -20,7 +20,7 @@ error[E0382]: use of moved value: `line2` LL | let _moved = (line2.origin, line2.middle); | ------------ value moved here LL | line2.consume(); //[ast]~ ERROR use of partially moved value: `line2` [E0382] - | ^^^^^ value used here after move + | ^^^^^ value used here after partial move | = note: move occurs because `line2.middle` has type `Point`, which does not implement the `Copy` trait diff --git a/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.nll.stderr b/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.nll.stderr index 99550235f6969..5f0d2b42671de 100644 --- a/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.nll.stderr +++ b/src/test/ui/moves/moves-based-on-type-cyclic-types-issue-4821.nll.stderr @@ -5,7 +5,7 @@ LL | Some(right) => consume(right), | ----- value moved here ... LL | consume(node) + r //~ ERROR use of partially moved value: `node` - | ^^^^ value used here after move + | ^^^^ value used here after partial move | = note: move occurs because value has type `std::boxed::Box`, which does not implement the `Copy` trait diff --git a/src/test/ui/moves/moves-based-on-type-match-bindings.nll.stderr b/src/test/ui/moves/moves-based-on-type-match-bindings.nll.stderr index 481c492e16469..6d523fc09c08b 100644 --- a/src/test/ui/moves/moves-based-on-type-match-bindings.nll.stderr +++ b/src/test/ui/moves/moves-based-on-type-match-bindings.nll.stderr @@ -5,7 +5,7 @@ LL | Foo {f} => {} | - value moved here ... LL | touch(&x); //~ ERROR use of partially moved value: `x` - | ^^ value borrowed here after move + | ^^ value borrowed here after partial move | = note: move occurs because `x.f` has type `std::string::String`, which does not implement the `Copy` trait diff --git a/src/test/ui/ref-suggestion.nll.stderr b/src/test/ui/ref-suggestion.nll.stderr index bef8dcca921e6..402f3c77cdb7b 100644 --- a/src/test/ui/ref-suggestion.nll.stderr +++ b/src/test/ui/ref-suggestion.nll.stderr @@ -25,7 +25,7 @@ LL | (Some(y), ()) => {}, | - value moved here ... LL | x; //~ ERROR use of partially moved value - | ^ value used here after move + | ^ value used here after partial move | = note: move occurs because value has type `std::vec::Vec`, which does not implement the `Copy` trait diff --git a/src/test/ui/unsized-locals/borrow-after-move.nll.stderr b/src/test/ui/unsized-locals/borrow-after-move.nll.stderr index 0e6a6f6369a15..010e182674b75 100644 --- a/src/test/ui/unsized-locals/borrow-after-move.nll.stderr +++ b/src/test/ui/unsized-locals/borrow-after-move.nll.stderr @@ -5,7 +5,7 @@ LL | let y = *x; | -- value moved here LL | drop_unsized(y); LL | println!("{}", &x); - | ^^ value borrowed here after move + | ^^ value borrowed here after partial move | = note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait @@ -27,7 +27,7 @@ LL | let y = *x; | -- value moved here LL | y.foo(); LL | println!("{}", &x); - | ^^ value borrowed here after move + | ^^ value borrowed here after partial move | = note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait @@ -48,7 +48,7 @@ error[E0382]: borrow of moved value: `x` LL | x.foo(); | - value moved here LL | println!("{}", &x); - | ^^ value borrowed here after move + | ^^ value borrowed here after partial move | = note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait diff --git a/src/test/ui/unsized-locals/double-move.nll.stderr b/src/test/ui/unsized-locals/double-move.nll.stderr index e40289af5ad3d..c0c3e436f535b 100644 --- a/src/test/ui/unsized-locals/double-move.nll.stderr +++ b/src/test/ui/unsized-locals/double-move.nll.stderr @@ -14,7 +14,7 @@ error[E0382]: use of moved value: `x` LL | let _y = *x; | -- value moved here LL | drop_unsized(x); //~ERROR use of moved value - | ^ value used here after move + | ^ value used here after partial move | = note: move occurs because `*x` has type `str`, which does not implement the `Copy` trait From b09803e8694b3ad83f988e30b4f0a3903ebe2632 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Tue, 19 Feb 2019 15:57:59 +0100 Subject: [PATCH 38/49] Address review comments --- src/libstd/sys/windows/stdio.rs | 58 ++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index ccfe0ced82fbc..6b9b36f6a924b 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,6 +1,7 @@ #![unstable(issue = "0", feature = "windows_stdio")] use cell::Cell; +use char::decode_utf16; use cmp; use io; use ptr; @@ -64,22 +65,27 @@ fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { // // If the data is not valid UTF-8 we write out as many bytes as are valid. // Only when there are no valid bytes (which will happen on the next call), return an error. - let len = cmp::min(data.len(), MAX_BUFFER_SIZE); + let len = cmp::min(data.len(), MAX_BUFFER_SIZE / 2); let utf8 = match str::from_utf8(&data[..len]) { Ok(s) => s, Err(ref e) if e.valid_up_to() == 0 => { return Err(io::Error::new(io::ErrorKind::InvalidData, - "Windows stdio in console mode does not support non-UTF-8 byte sequences; \ - see https://github.com/rust-lang/rust/issues/23344")) + "Windows stdio in console mode does not support writing non-UTF-8 byte sequences")) }, Err(e) => str::from_utf8(&data[..e.valid_up_to()]).unwrap(), }; - let utf16 = utf8.encode_utf16().collect::>(); + let mut utf16 = [0u16; MAX_BUFFER_SIZE / 2]; + let mut len_utf16 = 0; + for (chr, dest) in utf8.encode_utf16().zip(utf16.iter_mut()) { + *dest = chr; + len_utf16 += 1; + } + let utf16 = &utf16[..len_utf16]; let mut written = write_u16s(handle, &utf16)?; // Figure out how many bytes of as UTF-8 were written away as UTF-16. - if written >= utf16.len() { + if written == utf16.len() { Ok(utf8.len()) } else { // Make sure we didn't end up writing only half of a surrogate pair (even though the chance @@ -90,7 +96,7 @@ fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { let first_char_remaining = utf16[written]; if first_char_remaining >= 0xDCEE && first_char_remaining <= 0xDFFF { // low surrogate // We just hope this works, and give up otherwise - let _ = write_u16s(handle, &utf16[written..written]); + let _ = write_u16s(handle, &utf16[written..written+1]); written += 1; } // Calculate the number of bytes of `utf8` that were actually written. @@ -103,6 +109,7 @@ fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { _ => 3, }; } + debug_assert!(String::from_utf16(&utf16[..written]).unwrap() == utf8[..count]); Ok(count) } } @@ -137,7 +144,7 @@ impl Stdin { return Ok(0); } else if buf.len() < 4 { return Err(io::Error::new(io::ErrorKind::InvalidInput, - "Windows stdin in console mode does not support a buffer too small to; \ + "Windows stdin in console mode does not support a buffer too small to \ guarantee holding one arbitrary UTF-8 character (4 bytes)")) } @@ -147,27 +154,14 @@ impl Stdin { // lost. let amount = cmp::min(buf.len() / 3, utf16_buf.len()); let read = self.read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount)?; - let utf16 = &utf16_buf[..read]; - // FIXME: it would be nice if we could directly decode into the buffer instead of doing an - // allocation. - let data = match String::from_utf16(&utf16) { - Ok(utf8) => utf8.into_bytes(), - Err(..) => { - // We can't really do any better than forget all data and return an error. - return Err(io::Error::new(io::ErrorKind::InvalidData, - "Windows stdin in console mode does not support non-UTF-16 input; \ - encountered unpaired surrogate")) - }, - }; - buf.copy_from_slice(&data); - Ok(data.len()) + utf16_to_utf8(&utf16_buf[..read], buf) } // We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our // buffer size, and keep it around for the next read hoping to put them together. // This is a best effort, and may not work if we are not the only reader on Stdin. - pub fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) + fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) -> io::Result { // Insert possibly remaining unpaired surrogate from last read. @@ -223,6 +217,26 @@ fn read_u16s(handle: c::HANDLE, buf: &mut [u16]) -> io::Result { Ok(amount as usize) } +#[allow(unused)] +fn utf16_to_utf8(utf16: &[u16], utf8: &mut [u8]) -> io::Result { + let mut written = 0; + for chr in decode_utf16(utf16.iter().cloned()) { + match chr { + Ok(chr) => { + chr.encode_utf8(&mut utf8[written..]); + written += chr.len_utf8(); + } + Err(_) => { + // We can't really do any better than forget all data and return an error. + return Err(io::Error::new(io::ErrorKind::InvalidData, + "Windows stdin in console mode does not support non-UTF-16 input; \ + encountered unpaired surrogate")) + } + } + } + Ok(written) +} + impl Stdout { pub fn new() -> io::Result { Ok(Stdout) From 6464e32ea97fe0b18f75becc82cba9b19dfe453c Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Wed, 20 Feb 2019 08:18:29 +0100 Subject: [PATCH 39/49] Use standard Read/Write traits in sys::stdio --- src/libstd/sys/cloudabi/stdio.rs | 16 +++++-- src/libstd/sys/redox/stdio.rs | 22 +++++---- src/libstd/sys/sgx/stdio.rs | 22 +++++---- src/libstd/sys/unix/stdio.rs | 22 +++++---- src/libstd/sys/wasm/stdio.rs | 26 ++++++---- src/libstd/sys/windows/stdio.rs | 81 ++++++++++++++++++-------------- 6 files changed, 114 insertions(+), 75 deletions(-) diff --git a/src/libstd/sys/cloudabi/stdio.rs b/src/libstd/sys/cloudabi/stdio.rs index 8ec92326dbd8c..81d79213f615c 100644 --- a/src/libstd/sys/cloudabi/stdio.rs +++ b/src/libstd/sys/cloudabi/stdio.rs @@ -9,8 +9,10 @@ impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, _: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, _buf: &mut [u8]) -> io::Result { Ok(0) } } @@ -19,15 +21,17 @@ impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, _: &[u8]) -> io::Result { +impl io::Write for Stdout { + fn write(&mut self, _buf: &[u8]) -> io::Result { Err(io::Error::new( io::ErrorKind::BrokenPipe, "Stdout is not connected to any output in this environment", )) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } @@ -36,15 +40,17 @@ impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, _: &[u8]) -> io::Result { +impl io::Write for Stderr { + fn write(&mut self, _buf: &[u8]) -> io::Result { Err(io::Error::new( io::ErrorKind::BrokenPipe, "Stderr is not connected to any output in this environment", )) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } diff --git a/src/libstd/sys/redox/stdio.rs b/src/libstd/sys/redox/stdio.rs index 66f84e1752752..b4eb01fd6cc52 100644 --- a/src/libstd/sys/redox/stdio.rs +++ b/src/libstd/sys/redox/stdio.rs @@ -8,10 +8,12 @@ pub struct Stderr(()); impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { let fd = FileDesc::new(0); - let ret = fd.read(data); + let ret = fd.read(buf); fd.into_raw(); ret } @@ -19,30 +21,34 @@ impl Stdin { impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(1); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { cvt(syscall::fsync(1)).and(Ok(())) } } impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(2); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { cvt(syscall::fsync(2)).and(Ok(())) } } diff --git a/src/libstd/sys/sgx/stdio.rs b/src/libstd/sys/sgx/stdio.rs index c8efb931d6fe5..57d66ed9a853c 100644 --- a/src/libstd/sys/sgx/stdio.rs +++ b/src/libstd/sys/sgx/stdio.rs @@ -16,32 +16,38 @@ fn with_std_fd R, R>(fd: abi::Fd, f: F) -> R { impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { - with_std_fd(abi::FD_STDIN, |fd| fd.read(data)) +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + with_std_fd(abi::FD_STDIN, |fd| fd.read(buf)) } } impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - with_std_fd(abi::FD_STDOUT, |fd| fd.write(data)) +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { + with_std_fd(abi::FD_STDOUT, |fd| fd.write(buf)) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { with_std_fd(abi::FD_STDOUT, |fd| fd.flush()) } } impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - with_std_fd(abi::FD_STDERR, |fd| fd.write(data)) +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { + with_std_fd(abi::FD_STDERR, |fd| fd.write(buf)) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { with_std_fd(abi::FD_STDERR, |fd| fd.flush()) } } diff --git a/src/libstd/sys/unix/stdio.rs b/src/libstd/sys/unix/stdio.rs index e23723222be35..82bd2fbf77612 100644 --- a/src/libstd/sys/unix/stdio.rs +++ b/src/libstd/sys/unix/stdio.rs @@ -8,10 +8,12 @@ pub struct Stderr(()); impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { let fd = FileDesc::new(libc::STDIN_FILENO); - let ret = fd.read(data); + let ret = fd.read(buf); fd.into_raw(); // do not close this FD ret } @@ -19,30 +21,34 @@ impl Stdin { impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(libc::STDOUT_FILENO); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); // do not close this FD ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(libc::STDERR_FILENO); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); // do not close this FD ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } diff --git a/src/libstd/sys/wasm/stdio.rs b/src/libstd/sys/wasm/stdio.rs index 015d3f2066042..657655004e9bf 100644 --- a/src/libstd/sys/wasm/stdio.rs +++ b/src/libstd/sys/wasm/stdio.rs @@ -9,9 +9,11 @@ impl Stdin { pub fn new() -> io::Result { Ok(Stdin) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { - Ok(ReadSysCall::perform(0, data)) +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + Ok(ReadSysCall::perform(0, buf)) } } @@ -19,13 +21,15 @@ impl Stdout { pub fn new() -> io::Result { Ok(Stdout) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - WriteSysCall::perform(1, data); - Ok(data.len()) +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { + WriteSysCall::perform(1, buf); + Ok(buf.len()) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } @@ -34,13 +38,15 @@ impl Stderr { pub fn new() -> io::Result { Ok(Stderr) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - WriteSysCall::perform(2, data); - Ok(data.len()) +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { + WriteSysCall::perform(2, buf); + Ok(buf.len()) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 6b9b36f6a924b..5963541a89338 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,6 +1,5 @@ #![unstable(issue = "0", feature = "windows_stdio")] -use cell::Cell; use char::decode_utf16; use cmp; use io; @@ -13,7 +12,7 @@ use sys::handle::Handle; // Don't cache handles but get them fresh for every read/write. This allows us to track changes to // the value over time (such as if a process calls `SetStdHandle` while it's running). See #40490. pub struct Stdin { - high_surrogate: Cell, + surrogate: u16, } pub struct Stdout; pub struct Stderr; @@ -128,10 +127,12 @@ fn write_u16s(handle: c::HANDLE, data: &[u16]) -> io::Result { impl Stdin { pub fn new() -> io::Result { - Ok(Stdin { high_surrogate: Cell::new(0) }) + Ok(Stdin { surrogate: 0 }) } +} - pub fn read(&self, buf: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { let handle = get_handle(c::STD_INPUT_HANDLE)?; if !is_console(handle) { let handle = Handle::new(handle); @@ -153,40 +154,44 @@ impl Stdin { // we can read at most a third of `buf.len()` chars and uphold the guarantee no data gets // lost. let amount = cmp::min(buf.len() / 3, utf16_buf.len()); - let read = self.read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount)?; + let read = read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount, &mut self.surrogate)?; utf16_to_utf8(&utf16_buf[..read], buf) } +} - // We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our - // buffer size, and keep it around for the next read hoping to put them together. - // This is a best effort, and may not work if we are not the only reader on Stdin. - fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) - -> io::Result - { - // Insert possibly remaining unpaired surrogate from last read. - let mut start = 0; - if self.high_surrogate.get() != 0 { - buf[0] = self.high_surrogate.replace(0); - start = 1; - if amount == 1 { - // Special case: `Stdin::read` guarantees we can always read at least one new `u16` - // and combine it with an unpaired surrogate, because the UTF-8 buffer is at least - // 4 bytes. - amount = 2; - } + +// We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our +// buffer size, and keep it around for the next read hoping to put them together. +// This is a best effort, and may not work if we are not the only reader on Stdin. +fn read_u16s_fixup_surrogates(handle: c::HANDLE, + buf: &mut [u16], + mut amount: usize, + surrogate: &mut u16) -> io::Result +{ + // Insert possibly remaining unpaired surrogate from last read. + let mut start = 0; + if *surrogate != 0 { + buf[0] = *surrogate; + *surrogate = 0; + start = 1; + if amount == 1 { + // Special case: `Stdin::read` guarantees we can always read at least one new `u16` + // and combine it with an unpaired surrogate, because the UTF-8 buffer is at least + // 4 bytes. + amount = 2; } - let mut amount = read_u16s(handle, &mut buf[start..amount])? + start; + } + let mut amount = read_u16s(handle, &mut buf[start..amount])? + start; - if amount > 0 { - let last_char = buf[amount - 1]; - if last_char >= 0xD800 && last_char <= 0xDBFF { // high surrogate - self.high_surrogate.set(last_char); - amount -= 1; - } + if amount > 0 { + let last_char = buf[amount - 1]; + if last_char >= 0xD800 && last_char <= 0xDBFF { // high surrogate + *surrogate = last_char; + amount -= 1; } - Ok(amount) } + Ok(amount) } fn read_u16s(handle: c::HANDLE, buf: &mut [u16]) -> io::Result { @@ -241,12 +246,14 @@ impl Stdout { pub fn new() -> io::Result { Ok(Stdout) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - write(c::STD_OUTPUT_HANDLE, data) +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { + write(c::STD_OUTPUT_HANDLE, buf) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } @@ -255,12 +262,14 @@ impl Stderr { pub fn new() -> io::Result { Ok(Stderr) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - write(c::STD_ERROR_HANDLE, data) +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { + write(c::STD_ERROR_HANDLE, buf) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } From f223c03372f198d4fc50960288fc033f9de5da10 Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Wed, 20 Feb 2019 21:58:20 +0100 Subject: [PATCH 40/49] Add examples for duration constants --- src/libcore/time.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/libcore/time.rs b/src/libcore/time.rs index 62ed8d6f7906a..a28f690775be6 100644 --- a/src/libcore/time.rs +++ b/src/libcore/time.rs @@ -60,18 +60,50 @@ pub struct Duration { impl Duration { /// The duration of one second. + /// + /// # Examples + /// + /// ``` + /// use std::time::Duration; + /// + /// assert_eq!(Duration::SECOND, Duration::from_secs(1)); + /// ``` #[unstable(feature = "duration_constants", issue = "57391")] pub const SECOND: Duration = Duration::from_secs(1); /// The duration of one millisecond. + /// + /// # Examples + /// + /// ``` + /// use std::time::Duration; + /// + /// assert_eq!(Duration::MILLISECOND, Duration::from_millis(1)); + /// ``` #[unstable(feature = "duration_constants", issue = "57391")] pub const MILLISECOND: Duration = Duration::from_millis(1); /// The duration of one microsecond. + /// + /// # Examples + /// + /// ``` + /// use std::time::Duration; + /// + /// assert_eq!(Duration::MICROSECOND, Duration::from_micros(1)); + /// ``` #[unstable(feature = "duration_constants", issue = "57391")] pub const MICROSECOND: Duration = Duration::from_micros(1); /// The duration of one nanosecond. + /// + /// # Examples + /// + /// ``` + /// use std::time::Duration; + /// + /// assert_eq!(Duration::NANOSECOND, Duration::from_nanos(1)); + /// ``` #[unstable(feature = "duration_constants", issue = "57391")] pub const NANOSECOND: Duration = Duration::from_nanos(1); From 36f18f2d3a2df4a2ce0925f438631b615e2ab4dc Mon Sep 17 00:00:00 2001 From: Gabriela Alexandra Moldovan Date: Wed, 20 Feb 2019 16:37:52 +0000 Subject: [PATCH 41/49] Allow Self::Module to be mutated. `codegen_allocator` and `write_metadata` mutate the underlying LLVM module. As such, it makes sense for these two functions to receive a mutable reference to the module (as opposed to an immutable one). --- src/librustc_codegen_llvm/lib.rs | 4 ++-- src/librustc_codegen_ssa/base.rs | 8 ++++---- src/librustc_codegen_ssa/traits/backend.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc_codegen_llvm/lib.rs b/src/librustc_codegen_llvm/lib.rs index 9219f42d69235..9a75b8495ded3 100644 --- a/src/librustc_codegen_llvm/lib.rs +++ b/src/librustc_codegen_llvm/lib.rs @@ -120,11 +120,11 @@ impl ExtraBackendMethods for LlvmCodegenBackend { fn write_metadata<'b, 'gcx>( &self, tcx: TyCtxt<'b, 'gcx, 'gcx>, - metadata: &ModuleLlvm + metadata: &mut ModuleLlvm ) -> EncodedMetadata { base::write_metadata(tcx, metadata) } - fn codegen_allocator(&self, tcx: TyCtxt, mods: &ModuleLlvm, kind: AllocatorKind) { + fn codegen_allocator(&self, tcx: TyCtxt, mods: &mut ModuleLlvm, kind: AllocatorKind) { unsafe { allocator::codegen(tcx, mods, kind) } } fn compile_codegen_unit<'a, 'tcx: 'a>( diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs index 7aa75f139d2ae..92f5c39fe5a18 100644 --- a/src/librustc_codegen_ssa/base.rs +++ b/src/librustc_codegen_ssa/base.rs @@ -551,9 +551,9 @@ pub fn codegen_crate( &["crate"], Some("metadata")).as_str() .to_string(); - let metadata_llvm_module = backend.new_metadata(tcx, &metadata_cgu_name); + let mut metadata_llvm_module = backend.new_metadata(tcx, &metadata_cgu_name); let metadata = time(tcx.sess, "write metadata", || { - backend.write_metadata(tcx, &metadata_llvm_module) + backend.write_metadata(tcx, &mut metadata_llvm_module) }); tcx.sess.profiler(|p| p.end_activity(ProfileCategory::Codegen)); @@ -636,9 +636,9 @@ pub fn codegen_crate( &["crate"], Some("allocator")).as_str() .to_string(); - let modules = backend.new_metadata(tcx, &llmod_id); + let mut modules = backend.new_metadata(tcx, &llmod_id); time(tcx.sess, "write allocator module", || { - backend.codegen_allocator(tcx, &modules, kind) + backend.codegen_allocator(tcx, &mut modules, kind) }); Some(ModuleCodegen { diff --git a/src/librustc_codegen_ssa/traits/backend.rs b/src/librustc_codegen_ssa/traits/backend.rs index 73c7614d91393..6f92024ea8af3 100644 --- a/src/librustc_codegen_ssa/traits/backend.rs +++ b/src/librustc_codegen_ssa/traits/backend.rs @@ -36,9 +36,9 @@ pub trait ExtraBackendMethods: CodegenBackend + WriteBackendMethods + Sized + Se fn write_metadata<'b, 'gcx>( &self, tcx: TyCtxt<'b, 'gcx, 'gcx>, - metadata: &Self::Module, + metadata: &mut Self::Module, ) -> EncodedMetadata; - fn codegen_allocator(&self, tcx: TyCtxt, mods: &Self::Module, kind: AllocatorKind); + fn codegen_allocator(&self, tcx: TyCtxt, mods: &mut Self::Module, kind: AllocatorKind); fn compile_codegen_unit<'a, 'tcx: 'a>( &self, tcx: TyCtxt<'a, 'tcx, 'tcx>, From c6d24cd50435f008cc8bf1a00a656b36ca892b5d Mon Sep 17 00:00:00 2001 From: Stjepan Glavina Date: Thu, 21 Feb 2019 09:13:50 +0100 Subject: [PATCH 42/49] Enable feature duration_constants in examples --- src/libcore/time.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcore/time.rs b/src/libcore/time.rs index a28f690775be6..ceeb9ace7e02d 100644 --- a/src/libcore/time.rs +++ b/src/libcore/time.rs @@ -64,6 +64,7 @@ impl Duration { /// # Examples /// /// ``` + /// #![feature(duration_constants)] /// use std::time::Duration; /// /// assert_eq!(Duration::SECOND, Duration::from_secs(1)); @@ -76,6 +77,7 @@ impl Duration { /// # Examples /// /// ``` + /// #![feature(duration_constants)] /// use std::time::Duration; /// /// assert_eq!(Duration::MILLISECOND, Duration::from_millis(1)); @@ -88,6 +90,7 @@ impl Duration { /// # Examples /// /// ``` + /// #![feature(duration_constants)] /// use std::time::Duration; /// /// assert_eq!(Duration::MICROSECOND, Duration::from_micros(1)); @@ -100,6 +103,7 @@ impl Duration { /// # Examples /// /// ``` + /// #![feature(duration_constants)] /// use std::time::Duration; /// /// assert_eq!(Duration::NANOSECOND, Duration::from_nanos(1)); From e5d1fa58f2dd5e86dd91f370313a4be85a39917a Mon Sep 17 00:00:00 2001 From: Gabriela Alexandra Moldovan Date: Thu, 21 Feb 2019 13:38:44 +0000 Subject: [PATCH 43/49] codegen and write_metadata can mutate ModuleLLvm. --- src/librustc_codegen_llvm/allocator.rs | 2 +- src/librustc_codegen_llvm/base.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_codegen_llvm/allocator.rs b/src/librustc_codegen_llvm/allocator.rs index 7430cd3f70961..9787b07ef8cc2 100644 --- a/src/librustc_codegen_llvm/allocator.rs +++ b/src/librustc_codegen_llvm/allocator.rs @@ -9,7 +9,7 @@ use rustc_allocator::{ALLOCATOR_METHODS, AllocatorTy}; use crate::ModuleLlvm; use crate::llvm::{self, False, True}; -pub(crate) unsafe fn codegen(tcx: TyCtxt, mods: &ModuleLlvm, kind: AllocatorKind) { +pub(crate) unsafe fn codegen(tcx: TyCtxt, mods: &mut ModuleLlvm, kind: AllocatorKind) { let llcx = &*mods.llcx; let llmod = mods.llmod(); let usize = match &tcx.sess.target.target.target_pointer_width[..] { diff --git a/src/librustc_codegen_llvm/base.rs b/src/librustc_codegen_llvm/base.rs index 33531bb69485b..7b2e8ec3df6bb 100644 --- a/src/librustc_codegen_llvm/base.rs +++ b/src/librustc_codegen_llvm/base.rs @@ -46,7 +46,7 @@ use crate::value::Value; pub fn write_metadata<'a, 'gcx>( tcx: TyCtxt<'a, 'gcx, 'gcx>, - llvm_module: &ModuleLlvm + llvm_module: &mut ModuleLlvm ) -> EncodedMetadata { use std::io::Write; use flate2::Compression; From 9f58c5fa7cf434dc6b19a961c4ec5a453e6dedcd Mon Sep 17 00:00:00 2001 From: Ingvar Stepanyan Date: Thu, 21 Feb 2019 23:02:34 +0000 Subject: [PATCH 44/49] Optimise vec![false; N] to zero-alloc Nowadays booleans have a well-defined representation, so there is no reason not to optimise their allocation. --- src/liballoc/vec.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index a351d482fedde..44042562a8122 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -1606,6 +1606,7 @@ impl_is_zero!(u64, |x| x == 0); impl_is_zero!(u128, |x| x == 0); impl_is_zero!(usize, |x| x == 0); +impl_is_zero!(bool, |x| x == false); impl_is_zero!(char, |x| x == '\0'); impl_is_zero!(f32, |x: f32| x.to_bits() == 0); From bba0ea2974a76310d26761ecaab74571acfc9bb5 Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Fri, 22 Feb 2019 13:21:42 +0100 Subject: [PATCH 45/49] rustdoc: support methods on primitives in intra-doc links --- src/librustdoc/clean/mod.rs | 6 ++-- .../passes/collect_intra_doc_links.rs | 35 +++++++++++++++++++ src/test/rustdoc/intra-link-prim-methods.rs | 3 ++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 src/test/rustdoc/intra-link-prim-methods.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ea9034ae3a1e0..1bc2d9fc87e4b 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -974,11 +974,13 @@ impl Attributes { "https://doc.rust-lang.org/nightly", }; // This is a primitive so the url is done "by hand". + let tail = fragment.find('#').unwrap_or_else(|| fragment.len()); Some((s.clone(), - format!("{}{}std/primitive.{}.html", + format!("{}{}std/primitive.{}.html{}", url, if !url.ends_with('/') { "/" } else { "" }, - fragment))) + &fragment[..tail], + &fragment[tail..]))) } else { panic!("This isn't a primitive?!"); } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 62b79646f6b08..7c50f9cd4fbce 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1,6 +1,7 @@ use rustc::lint as lint; use rustc::hir; use rustc::hir::def::Def; +use rustc::hir::def_id::DefId; use rustc::ty; use syntax; use syntax::ast::{self, Ident, NodeId}; @@ -126,6 +127,17 @@ impl<'a, 'tcx, 'rcx> LinkCollector<'a, 'tcx, 'rcx> { path = name.clone(); } } + if let Some(prim) = is_primitive(&path, false) { + let did = primitive_impl(cx, &path).ok_or(())?; + return cx.tcx.associated_items(did) + .find(|item| item.ident.name == item_name) + .and_then(|item| match item.kind { + ty::AssociatedKind::Method => Some("method"), + _ => None, + }) + .map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_name)))) + .ok_or(()); + } // FIXME: `with_scope` requires the `NodeId` of a module. let ty = cx.resolver.borrow_mut() @@ -603,3 +615,26 @@ fn is_primitive(path_str: &str, is_val: bool) -> Option { PRIMITIVES.iter().find(|x| x.0 == path_str).map(|x| x.1) } } + +fn primitive_impl(cx: &DocContext, path_str: &str) -> Option { + let tcx = cx.tcx; + match path_str { + "u8" => tcx.lang_items().u8_impl(), + "u16" => tcx.lang_items().u16_impl(), + "u32" => tcx.lang_items().u32_impl(), + "u64" => tcx.lang_items().u64_impl(), + "u128" => tcx.lang_items().u128_impl(), + "usize" => tcx.lang_items().usize_impl(), + "i8" => tcx.lang_items().i8_impl(), + "i16" => tcx.lang_items().i16_impl(), + "i32" => tcx.lang_items().i32_impl(), + "i64" => tcx.lang_items().i64_impl(), + "i128" => tcx.lang_items().i128_impl(), + "isize" => tcx.lang_items().isize_impl(), + "f32" => tcx.lang_items().f32_impl(), + "f64" => tcx.lang_items().f64_impl(), + "str" => tcx.lang_items().str_impl(), + "char" => tcx.lang_items().char_impl(), + _ => None, + } +} diff --git a/src/test/rustdoc/intra-link-prim-methods.rs b/src/test/rustdoc/intra-link-prim-methods.rs new file mode 100644 index 0000000000000..af0426b22c557 --- /dev/null +++ b/src/test/rustdoc/intra-link-prim-methods.rs @@ -0,0 +1,3 @@ +#![deny(intra_doc_link_resolution_failure)] + +//! A [`char`] and its [`char::len_utf8`]. From b5ae4d58883151a977487de86856d9529df9d948 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 22 Feb 2019 13:50:17 +0100 Subject: [PATCH 46/49] Don't generate minification variable if minification disabled --- src/librustdoc/html/render.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index d037154272db6..418289f98ea2e 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -1117,7 +1117,11 @@ themePicker.onblur = handleThemeButtonsBlur; // with rustdoc running in parallel. all_indexes.sort(); let mut w = try_err!(File::create(&dst), &dst); - try_err!(writeln!(&mut w, "var N=null,E=\"\",T=\"t\",U=\"u\",searchIndex={{}};"), &dst); + if options.enable_minification { + try_err!(writeln!(&mut w, "var N=null,E=\"\",T=\"t\",U=\"u\",searchIndex={{}};"), &dst); + } else { + try_err!(writeln!(&mut w, "var searchIndex={{}};"), &dst); + } try_err!(write_minify_replacer(&mut w, &format!("{}\n{}", variables.join(""), all_indexes.join("\n")), options.enable_minification), From e555854f9d738a1a786ca61875390c3aca777a8d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 22 Feb 2019 15:13:07 +0100 Subject: [PATCH 47/49] Make target pointer-width specific variants of (very old) huge-array-simple.rs test. (and now unignore the test since it shouldn't break tests of cross-compiles anymore.) --- src/test/ui/huge-array-simple.rs | 13 +++++++++---- src/test/ui/huge-array-simple.stderr | 4 ++++ 2 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/huge-array-simple.stderr diff --git a/src/test/ui/huge-array-simple.rs b/src/test/ui/huge-array-simple.rs index 5fa874c1fa0d3..8b244a47232fa 100644 --- a/src/test/ui/huge-array-simple.rs +++ b/src/test/ui/huge-array-simple.rs @@ -1,11 +1,16 @@ -// FIXME (#23926): the error output is not consistent between a -// self-hosted and a cross-compiled setup. Skipping for now. - -// ignore-test FIXME(#23926) +// error-pattern: too big for the current architecture +// normalize-stderr-test "; \d+]" -> "; N]" #![allow(exceeding_bitshifts)] +#[cfg(target_pointer_width = "64")] fn main() { let _fat : [u8; (1<<61)+(1<<31)] = [0; (1u64<<61) as usize +(1u64<<31) as usize]; } + +#[cfg(target_pointer_width = "32")] +fn main() { + let _fat : [u8; (1<<31)+(1<<15)] = + [0; (1u32<<31) as usize +(1u32<<15) as usize]; +} diff --git a/src/test/ui/huge-array-simple.stderr b/src/test/ui/huge-array-simple.stderr new file mode 100644 index 0000000000000..3e9c86296cec2 --- /dev/null +++ b/src/test/ui/huge-array-simple.stderr @@ -0,0 +1,4 @@ +error: the type `[u8; N]` is too big for the current architecture + +error: aborting due to previous error + From b72ba0559cab28ab88c9397ece9e5e4b193bdc89 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 22 Feb 2019 16:07:15 +0100 Subject: [PATCH 48/49] Switch from error patterns to `//~ ERROR` markers. AFAICT, we do not have the same const-eval issues that we used to when rust-lang/rust#23926 was filed. (Probably because of the switch to miri for const-evaluation.) --- src/test/ui/consts/const-eval/const-eval-overflow-3.rs | 9 +++++---- .../ui/consts/const-eval/const-eval-overflow-3b.rs | 10 ++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/test/ui/consts/const-eval/const-eval-overflow-3.rs b/src/test/ui/consts/const-eval/const-eval-overflow-3.rs index 95deb04fb01d5..6fd8e9cbc806b 100644 --- a/src/test/ui/consts/const-eval/const-eval-overflow-3.rs +++ b/src/test/ui/consts/const-eval/const-eval-overflow-3.rs @@ -3,11 +3,11 @@ // // This test is checking the count in an array expression. -// FIXME (#23926): the error output is not consistent between a -// self-hosted and a cross-compiled setup; therefore resorting to -// error-pattern for now. -// error-pattern: attempt to add with overflow + + + + #![allow(unused_imports)] @@ -18,6 +18,7 @@ use std::{u8, u16, u32, u64, usize}; const A_I8_I : [u32; (i8::MAX as usize) + 1] = [0; (i8::MAX + 1) as usize]; +//~^ ERROR evaluation of constant value failed fn main() { foo(&A_I8_I[..]); diff --git a/src/test/ui/consts/const-eval/const-eval-overflow-3b.rs b/src/test/ui/consts/const-eval/const-eval-overflow-3b.rs index 5a6be0c9bd38b..d9b06370dff79 100644 --- a/src/test/ui/consts/const-eval/const-eval-overflow-3b.rs +++ b/src/test/ui/consts/const-eval/const-eval-overflow-3b.rs @@ -7,11 +7,11 @@ // types for the left- and right-hand sides of the addition do not // match (as well as overflow). -// FIXME (#23926): the error output is not consistent between a -// self-hosted and a cross-compiled setup; therefore resorting to -// error-pattern for now. -// error-pattern: mismatched types + + + + #![allow(unused_imports)] @@ -22,6 +22,8 @@ use std::{u8, u16, u32, u64, usize}; const A_I8_I : [u32; (i8::MAX as usize) + 1] = [0; (i8::MAX + 1u8) as usize]; +//~^ ERROR mismatched types +//~| ERROR cannot add `u8` to `i8` fn main() { foo(&A_I8_I[..]); From cc1cd8365704fe681caa26a7e4c507fce45e7f1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 22 Feb 2019 11:17:30 -0800 Subject: [PATCH 49/49] Do not underflow after resetting unmatched braces count Fix #58638. --- src/libsyntax/parse/parser.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index e22047938e518..72b201c0d54b8 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1184,8 +1184,10 @@ impl<'a> Parser<'a> { match ate { Some(_) => { // See doc comment for `unmatched_angle_bracket_count`. - self.unmatched_angle_bracket_count -= 1; - debug!("expect_gt: (decrement) count={:?}", self.unmatched_angle_bracket_count); + if self.unmatched_angle_bracket_count > 0 { + self.unmatched_angle_bracket_count -= 1; + debug!("expect_gt: (decrement) count={:?}", self.unmatched_angle_bracket_count); + } Ok(()) }, @@ -2248,8 +2250,10 @@ impl<'a> Parser<'a> { // See doc comment for `unmatched_angle_bracket_count`. self.expect(&token::Gt)?; - self.unmatched_angle_bracket_count -= 1; - debug!("parse_qpath: (decrement) count={:?}", self.unmatched_angle_bracket_count); + if self.unmatched_angle_bracket_count > 0 { + self.unmatched_angle_bracket_count -= 1; + debug!("parse_qpath: (decrement) count={:?}", self.unmatched_angle_bracket_count); + } self.expect(&token::ModSep)?;