From 808e4315e8bc7786b43fe4cb2f7b1dd6c2373ea3 Mon Sep 17 00:00:00 2001 From: Masood Malekghassemi Date: Thu, 12 May 2016 18:01:23 -0700 Subject: [PATCH 01/17] Propagate obligations through projection --- src/librustc/traits/project.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index 5c7095beb79..44ec42de8cb 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -207,7 +207,7 @@ fn project_and_unify_type<'cx, 'gcx, 'tcx>( debug!("project_and_unify_type(obligation={:?})", obligation); - let Normalized { value: normalized_ty, obligations } = + let Normalized { value: normalized_ty, mut obligations } = match opt_normalize_projection_type(selcx, obligation.predicate.projection_ty.clone(), obligation.cause.clone(), @@ -224,8 +224,9 @@ fn project_and_unify_type<'cx, 'gcx, 'tcx>( let origin = TypeOrigin::RelateOutputImplTypes(obligation.cause.span); match infcx.eq_types(true, origin, normalized_ty, obligation.predicate.ty) { Ok(InferOk { obligations: inferred_obligations, .. }) => { - // FIXME(#32730) propagate obligations + // FIXME(#32730) once obligations are generated in inference, drop this assertion assert!(inferred_obligations.is_empty()); + obligations.extend(inferred_obligations); Ok(Some(obligations)) }, Err(err) => Err(MismatchedProjectionTypes { err: err }), @@ -710,7 +711,8 @@ fn assemble_candidates_from_predicates<'cx, 'gcx, 'tcx, I>( origin, data_poly_trait_ref, obligation_poly_trait_ref) - // FIXME(#32730) propagate obligations + // FIXME(#32730) once obligations are propagated from unification in + // inference, drop this assertion .map(|InferOk { obligations, .. }| assert!(obligations.is_empty())) .is_ok() }); @@ -1047,8 +1049,8 @@ fn confirm_fn_pointer_candidate<'cx, 'gcx, 'tcx>( fn_pointer_vtable: VtableFnPointerData<'tcx, PredicateObligation<'tcx>>) -> (Ty<'tcx>, Vec>) { - // FIXME(#32730) propagate obligations (fn pointer vtable nested obligations ONLY come from - // unification in inference) + // FIXME(#32730) drop this assertion once obligations are propagated from inference (fn pointer + // vtable nested obligations ONLY come from unification in inference) assert!(fn_pointer_vtable.nested.is_empty()); let fn_type = selcx.infcx().shallow_resolve(fn_pointer_vtable.fn_ty); let sig = fn_type.fn_sig(); @@ -1130,13 +1132,14 @@ fn confirm_param_env_candidate<'cx, 'gcx, 'tcx>( obligation.predicate.item_name); let origin = TypeOrigin::RelateOutputImplTypes(obligation.cause.span); - match infcx.eq_trait_refs(false, - origin, - obligation.predicate.trait_ref.clone(), - projection.projection_ty.trait_ref.clone()) { + let obligations = match infcx.eq_trait_refs(false, + origin, + obligation.predicate.trait_ref.clone(), + projection.projection_ty.trait_ref.clone()) { Ok(InferOk { obligations, .. }) => { - // FIXME(#32730) propagate obligations + // FIXME(#32730) once obligations are generated in inference, remove this assertion assert!(obligations.is_empty()); + obligations } Err(e) => { span_bug!( @@ -1146,9 +1149,9 @@ fn confirm_param_env_candidate<'cx, 'gcx, 'tcx>( projection, e); } - } + }; - (projection.ty, vec!()) + (projection.ty, obligations) } fn confirm_impl_candidate<'cx, 'gcx, 'tcx>( From 9e5574803fcc09b855e33b7a71abb304b9cd6c54 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Mon, 23 May 2016 12:50:34 -0400 Subject: [PATCH 02/17] Update error format for readability. Add spacing header<->snippet and another line between errors --- src/libsyntax/errors/emitter.rs | 2 +- src/libsyntax/errors/snippet/mod.rs | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/errors/emitter.rs b/src/libsyntax/errors/emitter.rs index 7c9985d7d23..2ba003c5eaf 100644 --- a/src/libsyntax/errors/emitter.rs +++ b/src/libsyntax/errors/emitter.rs @@ -238,7 +238,7 @@ impl EmitterWriter { self.first = false; } else { if !self.old_school { - write!(self.dst, "\n")?; + write!(self.dst, "\n\n")?; } } } diff --git a/src/libsyntax/errors/snippet/mod.rs b/src/libsyntax/errors/snippet/mod.rs index 188e676e7df..414da87b749 100644 --- a/src/libsyntax/errors/snippet/mod.rs +++ b/src/libsyntax/errors/snippet/mod.rs @@ -478,6 +478,13 @@ impl FileInfo { }], kind: RenderedLineKind::PrimaryFileName, }); + output.push(RenderedLine { + text: vec![StyledString { + text: "".to_string(), + style: Style::FileNameStyle, + }], + kind: RenderedLineKind::Annotations, + }); } None => { output.push(RenderedLine { From 428099233a80018bc9740a09889391cd97087de4 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Mon, 23 May 2016 14:48:11 -0400 Subject: [PATCH 03/17] Fix #33819 and update ui test --- src/librustc_borrowck/borrowck/mod.rs | 6 +++++- src/test/compile-fail/issue-33819.rs | 9 +++++++++ src/test/ui/mismatched_types/main.stderr | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/issue-33819.rs diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 36222e172b8..819717628d6 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -977,7 +977,11 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { if let Categorization::Local(local_id) = err.cmt.cat { let span = self.tcx.map.span(local_id); if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(span) { - if snippet != "self" { + if snippet.starts_with("ref ") { + db.span_label(span, + &format!("use `{}` here to make mutable", + snippet.replace("ref ", "ref mut "))); + } else if snippet != "self" { db.span_label(span, &format!("use `mut {}` here to make mutable", snippet)); } diff --git a/src/test/compile-fail/issue-33819.rs b/src/test/compile-fail/issue-33819.rs new file mode 100644 index 00000000000..418e66dbd4d --- /dev/null +++ b/src/test/compile-fail/issue-33819.rs @@ -0,0 +1,9 @@ +fn main() { + let mut op = Some(2); + match op { + Some(ref v) => { let a = &mut v; }, + //~^ ERROR:cannot borrow immutable + //~| use `ref mut v` here to make mutable + None => {}, + } +} diff --git a/src/test/ui/mismatched_types/main.stderr b/src/test/ui/mismatched_types/main.stderr index 98bc11752e0..0e68c0d0b13 100644 --- a/src/test/ui/mismatched_types/main.stderr +++ b/src/test/ui/mismatched_types/main.stderr @@ -1,8 +1,10 @@ error: mismatched types [--explain E0308] --> $DIR/main.rs:14:18 + |> 14 |> let x: u32 = ( |> ^ expected u32, found () note: expected type `u32` note: found type `()` + error: aborting due to previous error From 86a62562700a1814b365478f36cf5e577c9a75e4 Mon Sep 17 00:00:00 2001 From: diwic Date: Tue, 24 May 2016 07:28:32 +0200 Subject: [PATCH 04/17] panic.rs: fix docs (recover -> catch_unwind) The current docs are a bit inconsistent. First, change all references of "recover" to "catch_unwind" because the function was renamed. Second, consistently use the term "unwind safe" instead of "panic safe", "exception safe" and "recover safe" (all these terms were used previously). --- src/libstd/panic.rs | 70 ++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/src/libstd/panic.rs b/src/libstd/panic.rs index 3b170dc9c6d..6770eafeac9 100644 --- a/src/libstd/panic.rs +++ b/src/libstd/panic.rs @@ -39,14 +39,14 @@ pub fn take_handler() -> Box { take_hook() } -/// A marker trait which represents "panic safe" types in Rust. +/// A marker trait which represents "unwind safe" types in Rust. /// /// This trait is implemented by default for many types and behaves similarly in /// terms of inference of implementation to the `Send` and `Sync` traits. The -/// purpose of this trait is to encode what types are safe to cross a `recover` -/// boundary with no fear of panic safety. +/// purpose of this trait is to encode what types are safe to cross a `catch_unwind` +/// boundary with no fear of unwind safety. /// -/// ## What is panic safety? +/// ## What is unwind safety? /// /// In Rust a function can "return" early if it either panics or calls a /// function which transitively panics. This sort of control flow is not always @@ -59,62 +59,62 @@ pub fn take_handler() -> Box { /// /// Typically in Rust, it is difficult to perform step (2) because catching a /// panic involves either spawning a thread (which in turns makes it difficult -/// to later witness broken invariants) or using the `recover` function in this +/// to later witness broken invariants) or using the `catch_unwind` function in this /// module. Additionally, even if an invariant is witnessed, it typically isn't a -/// problem in Rust because there's no uninitialized values (like in C or C++). +/// problem in Rust because there are no uninitialized values (like in C or C++). /// /// It is possible, however, for **logical** invariants to be broken in Rust, -/// which can end up causing behavioral bugs. Another key aspect of panic safety +/// which can end up causing behavioral bugs. Another key aspect of unwind safety /// in Rust is that, in the absence of `unsafe` code, a panic cannot lead to /// memory unsafety. /// -/// That was a bit of a whirlwind tour of panic safety, but for more information -/// about panic safety and how it applies to Rust, see an [associated RFC][rfc]. +/// That was a bit of a whirlwind tour of unwind safety, but for more information +/// about unwind safety and how it applies to Rust, see an [associated RFC][rfc]. /// /// [rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1236-stabilize-catch-panic.md /// /// ## What is `UnwindSafe`? /// -/// Now that we've got an idea of what panic safety is in Rust, it's also +/// Now that we've got an idea of what unwind safety is in Rust, it's also /// important to understand what this trait represents. As mentioned above, one -/// way to witness broken invariants is through the `recover` function in this +/// way to witness broken invariants is through the `catch_unwind` function in this /// module as it allows catching a panic and then re-using the environment of /// the closure. /// /// Simply put, a type `T` implements `UnwindSafe` if it cannot easily allow -/// witnessing a broken invariant through the use of `recover` (catching a +/// witnessing a broken invariant through the use of `catch_unwind` (catching a /// panic). This trait is a marker trait, so it is automatically implemented for -/// many types, and it is also structurally composed (e.g. a struct is recover -/// safe if all of its components are recover safe). +/// many types, and it is also structurally composed (e.g. a struct is unwind +/// safe if all of its components are unwind safe). /// /// Note, however, that this is not an unsafe trait, so there is not a succinct /// contract that this trait is providing. Instead it is intended as more of a -/// "speed bump" to alert users of `recover` that broken invariants may be +/// "speed bump" to alert users of `catch_unwind` that broken invariants may be /// witnessed and may need to be accounted for. /// /// ## Who implements `UnwindSafe`? /// /// Types such as `&mut T` and `&RefCell` are examples which are **not** -/// recover safe. The general idea is that any mutable state which can be shared -/// across `recover` is not recover safe by default. This is because it is very -/// easy to witness a broken invariant outside of `recover` as the data is +/// unwind safe. The general idea is that any mutable state which can be shared +/// across `catch_unwind` is not unwind safe by default. This is because it is very +/// easy to witness a broken invariant outside of `catch_unwind` as the data is /// simply accessed as usual. /// -/// Types like `&Mutex`, however, are recover safe because they implement +/// Types like `&Mutex`, however, are unwind safe because they implement /// poisoning by default. They still allow witnessing a broken invariant, but /// they already provide their own "speed bumps" to do so. /// /// ## When should `UnwindSafe` be used? /// /// Is not intended that most types or functions need to worry about this trait. -/// It is only used as a bound on the `recover` function and as mentioned above, +/// It is only used as a bound on the `catch_unwind` function and as mentioned above, /// the lack of `unsafe` means it is mostly an advisory. The `AssertUnwindSafe` /// wrapper struct in this module can be used to force this trait to be -/// implemented for any closed over variables passed to the `recover` function +/// implemented for any closed over variables passed to the `catch_unwind` function /// (more on this below). #[stable(feature = "catch_unwind", since = "1.9.0")] #[rustc_on_unimplemented = "the type {Self} may not be safely transferred \ - across a recover boundary"] + across an unwind boundary"] pub trait UnwindSafe {} /// Deprecated, renamed to UnwindSafe @@ -126,7 +126,7 @@ pub trait RecoverSafe {} impl RecoverSafe for T {} /// A marker trait representing types where a shared reference is considered -/// recover safe. +/// unwind safe. /// /// This trait is namely not implemented by `UnsafeCell`, the root of all /// interior mutability. @@ -136,23 +136,23 @@ impl RecoverSafe for T {} #[stable(feature = "catch_unwind", since = "1.9.0")] #[rustc_on_unimplemented = "the type {Self} contains interior mutability \ and a reference may not be safely transferrable \ - across a recover boundary"] + across a catch_unwind boundary"] pub trait RefUnwindSafe {} -/// A simple wrapper around a type to assert that it is panic safe. +/// A simple wrapper around a type to assert that it is unwind safe. /// -/// When using `recover` it may be the case that some of the closed over -/// variables are not panic safe. For example if `&mut T` is captured the -/// compiler will generate a warning indicating that it is not panic safe. It +/// When using `catch_unwind` it may be the case that some of the closed over +/// variables are not unwind safe. For example if `&mut T` is captured the +/// compiler will generate a warning indicating that it is not unwind safe. It /// may not be the case, however, that this is actually a problem due to the -/// specific usage of `recover` if panic safety is specifically taken into +/// specific usage of `catch_unwind` if unwind safety is specifically taken into /// account. This wrapper struct is useful for a quick and lightweight -/// annotation that a variable is indeed panic safe. +/// annotation that a variable is indeed unwind safe. /// /// # Examples /// /// One way to use `AssertUnwindSafe` is to assert that the entire closure -/// itself is recover safe, bypassing all checks for all variables: +/// itself is unwind safe, bypassing all checks for all variables: /// /// ``` /// use std::panic::{self, AssertUnwindSafe}; @@ -160,7 +160,7 @@ pub trait RefUnwindSafe {} /// let mut variable = 4; /// /// // This code will not compile because the closure captures `&mut variable` -/// // which is not considered panic safe by default. +/// // which is not considered unwind safe by default. /// /// // panic::catch_unwind(|| { /// // variable += 3; @@ -239,7 +239,7 @@ impl UnwindSafe for AssertUnwindSafe {} impl UnwindSafe for AssertRecoverSafe {} // not covered via the Shared impl above b/c the inner contents use -// Cell/AtomicUsize, but the usage here is recover safe so we can lift the +// Cell/AtomicUsize, but the usage here is unwind safe so we can lift the // impl up one level to Arc/Rc itself #[stable(feature = "catch_unwind", since = "1.9.0")] impl UnwindSafe for Rc {} @@ -352,9 +352,9 @@ impl R> FnOnce<()> for AssertRecoverSafe { /// that all captured variables are safe to cross this boundary. The purpose of /// this bound is to encode the concept of [exception safety][rfc] in the type /// system. Most usage of this function should not need to worry about this -/// bound as programs are naturally panic safe without `unsafe` code. If it +/// bound as programs are naturally unwind safe without `unsafe` code. If it /// becomes a problem the associated `AssertUnwindSafe` wrapper type in this -/// module can be used to quickly assert that the usage here is indeed exception +/// module can be used to quickly assert that the usage here is indeed unwind /// safe. /// /// [rfc]: https://github.com/rust-lang/rfcs/blob/master/text/1236-stabilize-catch-panic.md From 9cc8debeb746826604858be86e2f6e5cce29026c Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 24 May 2016 07:40:09 -0400 Subject: [PATCH 05/17] Move issue-26480 cfail to ui test. Fix #33763 --- .../run-make/unicode-input/span_length.rs | 65 +++++++++++++------ .../mismatched_types}/issue-26480.rs | 12 +--- .../ui/mismatched_types/issue-26480.stderr | 17 +++++ 3 files changed, 63 insertions(+), 31 deletions(-) rename src/test/{compile-fail => ui/mismatched_types}/issue-26480.rs (72%) create mode 100644 src/test/ui/mismatched_types/issue-26480.stderr diff --git a/src/test/run-make/unicode-input/span_length.rs b/src/test/run-make/unicode-input/span_length.rs index 3963d20df88..da8769e616c 100644 --- a/src/test/run-make/unicode-input/span_length.rs +++ b/src/test/run-make/unicode-input/span_length.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -#![feature(rand, core)] +#![feature(rand)] use std::fs::File; use std::io::prelude::*; @@ -18,6 +18,11 @@ use std::process::Command; use std::__rand::{thread_rng, Rng}; use std::{char, env}; +pub fn check_old_skool() -> bool { + use std::env; + env::var("RUST_NEW_ERROR_FORMAT").is_err() +} + // creates a file with `fn main() { }` and checks the // compiler emits a span of the appropriate length (for the // "unresolved name" message); currently just using the number of code @@ -65,10 +70,17 @@ fn main() { let err = String::from_utf8_lossy(&result.stderr); - // the span should end the line (e.g no extra ~'s) - let expected_span = format!("^{}\n", repeat("~").take(n - 1) - .collect::()); - assert!(err.contains(&expected_span)); + if check_old_skool() { + // the span should end the line (e.g no extra ~'s) + let expected_span = format!("^{}\n", repeat("~").take(n - 1) + .collect::()); + assert!(err.contains(&expected_span)); + } else { + // the span should end the line (e.g no extra ~'s) + let expected_span = format!("^{}\n", repeat("^").take(n - 1) + .collect::()); + assert!(err.contains(&expected_span)); + } } // Test multi-column characters and tabs @@ -77,9 +89,6 @@ fn main() { r#"extern "路濫狼á́́" fn foo() {{}} extern "路濫狼á́" fn bar() {{}}"#); } - // Extra characters. Every line is preceded by `filename:lineno ` - let offset = main_file.to_str().unwrap().len() + 3; - let result = Command::new("sh") .arg("-c") .arg(format!("{} {}", @@ -91,17 +100,31 @@ fn main() { // Test both the length of the snake and the leading spaces up to it - // First snake is 8 ~s long, with 7 preceding spaces (excluding file name/line offset) - let expected_span = format!("\n{}^{}\n", - repeat(" ").take(offset + 7).collect::(), - repeat("~").take(8).collect::()); - assert!(err.contains(&expected_span)); - // Second snake is only 7 ~s long, with 36 preceding spaces, - // because rustc counts chars() now rather than width(). This - // is because width() functions are to be removed from - // librustc_unicode - let expected_span = format!("\n{}^{}\n", - repeat(" ").take(offset + 36).collect::(), - repeat("~").take(7).collect::()); - assert!(err.contains(&expected_span)); + if check_old_skool() { + // Extra characters. Every line is preceded by `filename:lineno ` + let offset = main_file.to_str().unwrap().len() + 3; + + // First snake is 8 ~s long, with 7 preceding spaces (excluding file name/line offset) + let expected_span = format!("\n{}^{}\n", + repeat(" ").take(offset + 7).collect::(), + repeat("~").take(8).collect::()); + assert!(err.contains(&expected_span)); + // Second snake is only 7 ~s long, with 36 preceding spaces, + // because rustc counts chars() now rather than width(). This + // is because width() functions are to be removed from + // librustc_unicode + let expected_span = format!("\n{}^{}\n", + repeat(" ").take(offset + 36).collect::(), + repeat("~").take(7).collect::()); + assert!(err.contains(&expected_span)); + } else { + let expected_span = format!("\n |>{}{}\n", + repeat(" ").take(8).collect::(), + repeat("^").take(9).collect::()); + assert!(err.contains(&expected_span)); + let expected_span = format!("\n |>{}{}\n", + repeat(" ").take(37).collect::(), + repeat("^").take(8).collect::()); + assert!(err.contains(&expected_span)); + } } diff --git a/src/test/compile-fail/issue-26480.rs b/src/test/ui/mismatched_types/issue-26480.rs similarity index 72% rename from src/test/compile-fail/issue-26480.rs rename to src/test/ui/mismatched_types/issue-26480.rs index 634a4014e11..516d92372e7 100644 --- a/src/test/compile-fail/issue-26480.rs +++ b/src/test/ui/mismatched_types/issue-26480.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// rustc-env:RUST_NEW_ERROR_FORMAT extern { fn write(fildes: i32, buf: *const i8, nbyte: u64) -> i64; } @@ -24,25 +25,16 @@ macro_rules! write { unsafe { write(stdout, $arr.as_ptr() as *const i8, $arr.len() * size_of($arr[0])); - //~^ ERROR mismatched types - //~| expected u64, found usize - //~| expected type - //~| found type } }} } macro_rules! cast { - ($x:expr) => ($x as ()) //~ ERROR non-scalar cast + ($x:expr) => ($x as ()) } fn main() { let hello = ['H', 'e', 'y']; write!(hello); - //~^ NOTE in this expansion of write! - //~| NOTE in this expansion of write! - //~| NOTE in this expansion of write! - cast!(2); - //~^ NOTE in this expansion of cast! } diff --git a/src/test/ui/mismatched_types/issue-26480.stderr b/src/test/ui/mismatched_types/issue-26480.stderr new file mode 100644 index 00000000000..48bb546b382 --- /dev/null +++ b/src/test/ui/mismatched_types/issue-26480.stderr @@ -0,0 +1,17 @@ +error: mismatched types [--explain E0308] + --> $DIR/issue-26480.rs:27:19 + |> +27 |> $arr.len() * size_of($arr[0])); + |> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected u64, found usize +$DIR/issue-26480.rs:38:5: 38:19: note: in this expansion of write! (defined in $DIR/issue-26480.rs) + + +error: non-scalar cast: `_` as `()` + --> $DIR/issue-26480.rs:33:19 + |> +33 |> ($x:expr) => ($x as ()) + |> ^^^^^^^^ +$DIR/issue-26480.rs:39:5: 39:14: note: in this expansion of cast! (defined in $DIR/issue-26480.rs) + + +error: aborting due to 2 previous errors From 00b78d0d6ad5985a29d9b1b001e81dbb1bf624b6 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 24 May 2016 10:42:32 -0400 Subject: [PATCH 06/17] Back to single line between errors. Add header space to secondary files --- src/libsyntax/errors/emitter.rs | 6 +++++- src/libsyntax/errors/snippet/mod.rs | 7 +++++++ src/libsyntax/errors/snippet/test.rs | 14 ++++++++++++++ src/test/ui/mismatched_types/issue-26480.stderr | 2 -- src/test/ui/mismatched_types/main.stderr | 1 - 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/libsyntax/errors/emitter.rs b/src/libsyntax/errors/emitter.rs index 2ba003c5eaf..6b15aa4f92c 100644 --- a/src/libsyntax/errors/emitter.rs +++ b/src/libsyntax/errors/emitter.rs @@ -238,7 +238,7 @@ impl EmitterWriter { self.first = false; } else { if !self.old_school { - write!(self.dst, "\n\n")?; + write!(self.dst, "\n")?; } } } @@ -682,6 +682,7 @@ mod test { println!("r#\"\n{}\"#", str); assert_eq!(str, &r#" --> dummy.txt:11:1 + |> 11 |> e-lä-vän |> ^ "#[1..]); @@ -746,6 +747,7 @@ mod test { let expect_start = &r#" --> dummy.txt:1:6 + |> 1 |> _____aaaaaa____bbbbbb__cccccdd_ |> ^^^^^^ ^^^^^^ ^^^^^^^ "#[1..]; @@ -818,6 +820,7 @@ mod test { let expect0 = &r#" --> dummy.txt:5:1 + |> 5 |> ccccc |> ^ ... @@ -830,6 +833,7 @@ mod test { let expect = &r#" --> dummy.txt:1:1 + |> 1 |> aaaaa |> ^ ... diff --git a/src/libsyntax/errors/snippet/mod.rs b/src/libsyntax/errors/snippet/mod.rs index 414da87b749..2a43a14ddf8 100644 --- a/src/libsyntax/errors/snippet/mod.rs +++ b/src/libsyntax/errors/snippet/mod.rs @@ -494,6 +494,13 @@ impl FileInfo { }], kind: RenderedLineKind::OtherFileName, }); + output.push(RenderedLine { + text: vec![StyledString { + text: "".to_string(), + style: Style::FileNameStyle, + }], + kind: RenderedLineKind::Annotations, + }); } } } diff --git a/src/libsyntax/errors/snippet/test.rs b/src/libsyntax/errors/snippet/test.rs index 62ce3fa9dd5..51fe4572dbc 100644 --- a/src/libsyntax/errors/snippet/test.rs +++ b/src/libsyntax/errors/snippet/test.rs @@ -98,6 +98,7 @@ fn foo() { let text = make_string(&lines); assert_eq!(&text[..], &" --> foo.rs:3:2 + |> 3 |> \tbar; |> \t^^^ "[1..]); @@ -130,6 +131,7 @@ fn foo() { println!("text=\n{}", text); assert_eq!(&text[..], &r#" ::: foo.rs + |> 3 |> vec.push(vec.pop().unwrap()); |> --- --- - previous borrow ends here |> | | @@ -199,12 +201,14 @@ fn bar() { // Note that the `|>` remain aligned across both files: assert_eq!(&text[..], &r#" --> foo.rs:3:14 + |> 3 |> vec.push(vec.pop().unwrap()); |> --- ^^^ - c |> | | |> | b |> a ::: bar.rs + |> 17 |> vec.push(); |> --- - f |> | @@ -249,6 +253,7 @@ fn foo() { println!("text=\n{}", text); assert_eq!(&text[..], &r#" ::: foo.rs + |> 3 |> let name = find_id(&data, 22).unwrap(); |> ---- immutable borrow begins here ... @@ -288,6 +293,7 @@ fn foo() { println!("text=r#\"\n{}\".trim_left()", text); assert_eq!(&text[..], &r#" ::: foo.rs + |> 3 |> vec.push(vec.pop().unwrap()); |> -------- ------ D |> || @@ -324,6 +330,7 @@ fn foo() { println!("text=r#\"\n{}\".trim_left()", text); assert_eq!(&text[..], &r#" ::: foo.rs + |> 3 |> vec.push(vec.pop().unwrap()); |> --- --- - previous borrow ends here |> | | @@ -362,6 +369,7 @@ fn foo() { println!("text=r#\"\n{}\".trim_left()", text); assert_eq!(&text[..], &r#" ::: foo.rs + |> 4 |> let mut vec2 = vec; |> --- `vec` moved here because it has type `collections::vec::Vec` ... @@ -398,6 +406,7 @@ fn foo() { println!("text=&r#\"\n{}\n\"#[1..]", text); assert_eq!(text, &r#" ::: foo.rs + |> 3 |> let mut vec = vec![0, 1, 2]; |> --- --- 4 |> let mut vec2 = vec; @@ -429,6 +438,7 @@ impl SomeTrait for () { println!("r#\"\n{}\"", text); assert_eq!(text, &r#" ::: foo.rs + |> 3 |> fn foo(x: u32) { |> - "#[1..]); @@ -458,6 +468,7 @@ fn span_overlap_label() { println!("r#\"\n{}\"", text); assert_eq!(text, &r#" ::: foo.rs + |> 2 |> fn foo(x: u32) { |> -------------- |> | | @@ -492,6 +503,7 @@ fn span_overlap_label2() { println!("r#\"\n{}\"", text); assert_eq!(text, &r#" ::: foo.rs + |> 2 |> fn foo(x: u32) { |> -------------- |> | | @@ -537,6 +549,7 @@ fn span_overlap_label3() { println!("r#\"\n{}\"", text); assert_eq!(text, &r#" ::: foo.rs + |> 3 |> let closure = || { |> - foo 4 |> inner @@ -577,6 +590,7 @@ fn main() { println!("r#\"\n{}\"", text); assert_eq!(text, &r#" --> foo.rs:11:2 + |> 11 |> } |> - "#[1..]); diff --git a/src/test/ui/mismatched_types/issue-26480.stderr b/src/test/ui/mismatched_types/issue-26480.stderr index 48bb546b382..c00594a59c1 100644 --- a/src/test/ui/mismatched_types/issue-26480.stderr +++ b/src/test/ui/mismatched_types/issue-26480.stderr @@ -5,7 +5,6 @@ error: mismatched types [--explain E0308] |> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected u64, found usize $DIR/issue-26480.rs:38:5: 38:19: note: in this expansion of write! (defined in $DIR/issue-26480.rs) - error: non-scalar cast: `_` as `()` --> $DIR/issue-26480.rs:33:19 |> @@ -13,5 +12,4 @@ error: non-scalar cast: `_` as `()` |> ^^^^^^^^ $DIR/issue-26480.rs:39:5: 39:14: note: in this expansion of cast! (defined in $DIR/issue-26480.rs) - error: aborting due to 2 previous errors diff --git a/src/test/ui/mismatched_types/main.stderr b/src/test/ui/mismatched_types/main.stderr index 0e68c0d0b13..1af332ee5be 100644 --- a/src/test/ui/mismatched_types/main.stderr +++ b/src/test/ui/mismatched_types/main.stderr @@ -6,5 +6,4 @@ error: mismatched types [--explain E0308] note: expected type `u32` note: found type `()` - error: aborting due to previous error From df87a781de531b0578cfe6c9e0a6cb624951f67c Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 24 May 2016 10:57:44 -0400 Subject: [PATCH 07/17] Satisfy tidy --- src/test/compile-fail/issue-33819.rs | 9 +++++++++ src/test/ui/mismatched_types/issue-26480.rs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/compile-fail/issue-33819.rs b/src/test/compile-fail/issue-33819.rs index 418e66dbd4d..9c9677c1e98 100644 --- a/src/test/compile-fail/issue-33819.rs +++ b/src/test/compile-fail/issue-33819.rs @@ -1,3 +1,12 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. fn main() { let mut op = Some(2); match op { diff --git a/src/test/ui/mismatched_types/issue-26480.rs b/src/test/ui/mismatched_types/issue-26480.rs index 516d92372e7..96db31f4b11 100644 --- a/src/test/ui/mismatched_types/issue-26480.rs +++ b/src/test/ui/mismatched_types/issue-26480.rs @@ -30,7 +30,7 @@ macro_rules! write { } macro_rules! cast { - ($x:expr) => ($x as ()) + ($x:expr) => ($x as ()) } fn main() { From 40285ca717f687f8c1df8f28e53fb97392ac6eb5 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 6 Mar 2016 15:54:44 +0300 Subject: [PATCH 08/17] Apply visit_path to import prefixes by default --- src/librustc/hir/intravisit.rs | 15 ++++----------- src/librustc_incremental/calculate_svh.rs | 4 ---- src/libsyntax/visit.rs | 15 ++++----------- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 2e9e433b830..e3074d6ec34 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -274,12 +274,9 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) { visitor.visit_path(path, item.id); } ViewPathList(ref prefix, ref list) => { - if !list.is_empty() { - for item in list { - visitor.visit_path_list_item(prefix, item) - } - } else { - visitor.visit_path(prefix, item.id); + visitor.visit_path(prefix, item.id); + for item in list { + visitor.visit_path_list_item(prefix, item) } } } @@ -413,12 +410,8 @@ pub fn walk_path<'v, V: Visitor<'v>>(visitor: &mut V, path: &'v Path) { } pub fn walk_path_list_item<'v, V: Visitor<'v>>(visitor: &mut V, - prefix: &'v Path, + _prefix: &'v Path, item: &'v PathListItem) { - for segment in &prefix.segments { - visitor.visit_path_segment(prefix.span, segment); - } - walk_opt_name(visitor, item.span, item.node.name()); walk_opt_name(visitor, item.span, item.node.rename()); } diff --git a/src/librustc_incremental/calculate_svh.rs b/src/librustc_incremental/calculate_svh.rs index 24ecce11487..e5b22652d3d 100644 --- a/src/librustc_incremental/calculate_svh.rs +++ b/src/librustc_incremental/calculate_svh.rs @@ -401,10 +401,6 @@ mod svh_visitor { SawPath.hash(self.st); visit::walk_path(self, path) } - fn visit_path_list_item(&mut self, prefix: &'a Path, item: &'a PathListItem) { - SawPath.hash(self.st); visit::walk_path_list_item(self, prefix, item) - } - fn visit_block(&mut self, b: &'a Block) { SawBlock.hash(self.st); visit::walk_block(self, b) } diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index f50a480e5e5..3c56d9e2355 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -247,12 +247,9 @@ pub fn walk_item<'v, V: Visitor<'v>>(visitor: &mut V, item: &'v Item) { visitor.visit_path(path, item.id); } ViewPathList(ref prefix, ref list) => { - if !list.is_empty() { - for item in list { - visitor.visit_path_list_item(prefix, item) - } - } else { - visitor.visit_path(prefix, item.id); + visitor.visit_path(prefix, item.id); + for item in list { + visitor.visit_path_list_item(prefix, item) } } } @@ -382,12 +379,8 @@ pub fn walk_path<'v, V: Visitor<'v>>(visitor: &mut V, path: &'v Path) { } } -pub fn walk_path_list_item<'v, V: Visitor<'v>>(visitor: &mut V, prefix: &'v Path, +pub fn walk_path_list_item<'v, V: Visitor<'v>>(visitor: &mut V, _prefix: &'v Path, item: &'v PathListItem) { - for segment in &prefix.segments { - visitor.visit_path_segment(prefix.span, segment); - } - walk_opt_ident(visitor, item.span, item.node.name()); walk_opt_ident(visitor, item.span, item.node.rename()); } From c209d44c342a664bad5428ff988ee1084c13bed7 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 24 May 2016 14:37:11 +0300 Subject: [PATCH 09/17] refactor autoderef to avoid registering obligations Refactor `FnCtxt::autoderef` to use an external iterator and to not register any obligation from the main autoderef loop, but rather to register them after (and if) the loop successfully completes. Fixes #24819 Fixes #25801 Fixes #27631 Fixes #31258 Fixes #31964 Fixes #32320 Fixes #33515 Fixes #33755 --- src/librustc/traits/mod.rs | 2 +- src/librustc/ty/adjustment.rs | 3 +- src/librustc_typeck/check/autoderef.rs | 210 +++++++++++++++ src/librustc_typeck/check/callee.rs | 18 +- src/librustc_typeck/check/coercion.rs | 32 +-- src/librustc_typeck/check/method/confirm.rs | 100 +++---- src/librustc_typeck/check/method/probe.rs | 33 ++- src/librustc_typeck/check/method/suggest.rs | 63 ++--- src/librustc_typeck/check/mod.rs | 255 ++++-------------- src/test/compile-fail/E0055.rs | 1 - ...rrowck-borrow-overloaded-auto-deref-mut.rs | 2 +- src/test/compile-fail/issue-24819.rs | 21 ++ 12 files changed, 386 insertions(+), 354 deletions(-) create mode 100644 src/librustc_typeck/check/autoderef.rs create mode 100644 src/test/compile-fail/issue-24819.rs diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index c5db2a8a780..c177ec4dbed 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -31,7 +31,7 @@ pub use self::coherence::overlapping_impls; pub use self::coherence::OrphanCheckErr; pub use self::fulfill::{FulfillmentContext, GlobalFulfilledPredicates, RegionObligation}; pub use self::project::{MismatchedProjectionTypes, ProjectionMode}; -pub use self::project::{normalize, Normalized}; +pub use self::project::{normalize, normalize_projection_type, Normalized}; pub use self::object_safety::ObjectSafetyViolation; pub use self::object_safety::MethodViolationCode; pub use self::select::{EvaluationCache, SelectionContext, SelectionCache}; diff --git a/src/librustc/ty/adjustment.rs b/src/librustc/ty/adjustment.rs index 71e49031347..60f2ca6f4d9 100644 --- a/src/librustc/ty/adjustment.rs +++ b/src/librustc/ty/adjustment.rs @@ -235,8 +235,9 @@ impl<'a, 'gcx, 'tcx> ty::TyS<'tcx> { None => { span_bug!( expr_span, - "the {}th autoderef failed: {}", + "the {}th autoderef for {} failed: {}", autoderef, + expr_id, adjusted_ty); } } diff --git a/src/librustc_typeck/check/autoderef.rs b/src/librustc_typeck/check/autoderef.rs new file mode 100644 index 00000000000..9e2b7cd0346 --- /dev/null +++ b/src/librustc_typeck/check/autoderef.rs @@ -0,0 +1,210 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use astconv::AstConv; + +use super::FnCtxt; + +use rustc::traits; +use rustc::ty::{self, Ty, TraitRef}; +use rustc::ty::{ToPredicate, TypeFoldable}; +use rustc::ty::{MethodCall, MethodCallee}; +use rustc::ty::subst::Substs; +use rustc::ty::{LvaluePreference, NoPreference, PreferMutLvalue}; +use rustc::hir; + +use syntax::codemap::Span; +use syntax::parse::token; + +#[derive(Copy, Clone, Debug)] +enum AutoderefKind { + Builtin, + Overloaded +} + +pub struct Autoderef<'a, 'gcx: 'tcx, 'tcx: 'a> { + fcx: &'a FnCtxt<'a, 'gcx, 'tcx>, + steps: Vec<(Ty<'tcx>, AutoderefKind)>, + cur_ty: Ty<'tcx>, + obligations: Vec>, + at_start: bool, + span: Span +} + +impl<'a, 'gcx, 'tcx> Iterator for Autoderef<'a, 'gcx, 'tcx> { + type Item = (Ty<'tcx>, usize); + + fn next(&mut self) -> Option { + let tcx = self.fcx.tcx; + + debug!("autoderef: steps={:?}, cur_ty={:?}", + self.steps, self.cur_ty); + if self.at_start { + self.at_start = false; + debug!("autoderef stage #0 is {:?}", self.cur_ty); + return Some((self.cur_ty, 0)); + } + + if self.steps.len() == tcx.sess.recursion_limit.get() { + // We've reached the recursion limit, error gracefully. + span_err!(tcx.sess, self.span, E0055, + "reached the recursion limit while auto-dereferencing {:?}", + self.cur_ty); + return None; + } + + if self.cur_ty.is_ty_var() { + return None; + } + + // Otherwise, deref if type is derefable: + let (kind, new_ty) = if let Some(mt) = self.cur_ty.builtin_deref(false, NoPreference) { + (AutoderefKind::Builtin, mt.ty) + } else { + match self.overloaded_deref_ty(self.cur_ty) { + Some(ty) => (AutoderefKind::Overloaded, ty), + _ => return None + } + }; + + if new_ty.references_error() { + return None; + } + + self.steps.push((self.cur_ty, kind)); + debug!("autoderef stage #{:?} is {:?} from {:?}", self.steps.len(), + new_ty, (self.cur_ty, kind)); + self.cur_ty = new_ty; + + Some((self.cur_ty, self.steps.len())) + } +} + +impl<'a, 'gcx, 'tcx> Autoderef<'a, 'gcx, 'tcx> { + fn overloaded_deref_ty(&mut self, ty: Ty<'tcx>) -> Option> { + debug!("overloaded_deref_ty({:?})", ty); + + let tcx = self.fcx.tcx(); + + // + let trait_ref = TraitRef { + def_id: match tcx.lang_items.deref_trait() { + Some(f) => f, + None => return None + }, + substs: tcx.mk_substs(Substs::new_trait(vec![], vec![], self.cur_ty)) + }; + + let cause = traits::ObligationCause::misc(self.span, self.fcx.body_id); + + let mut selcx = traits::SelectionContext::new(self.fcx); + let obligation = traits::Obligation::new(cause.clone(), trait_ref.to_predicate()); + if !selcx.evaluate_obligation(&obligation) { + debug!("overloaded_deref_ty: cannot match obligation"); + return None; + } + + let normalized = traits::normalize_projection_type( + &mut selcx, + ty::ProjectionTy { + trait_ref: trait_ref, + item_name: token::intern("Target") + }, + cause, + 0 + ); + + debug!("overloaded_deref_ty({:?}) = {:?}", ty, normalized); + self.obligations.extend(normalized.obligations); + + Some(self.fcx.resolve_type_vars_if_possible(&normalized.value)) + } + + pub fn unambiguous_final_ty(&self) -> Ty<'tcx> { + self.fcx.structurally_resolved_type(self.span, self.cur_ty) + } + + pub fn finalize<'b, I>(self, pref: LvaluePreference, exprs: I) + where I: IntoIterator + { + let methods : Vec<_> = self.steps.iter().map(|&(ty, kind)| { + if let AutoderefKind::Overloaded = kind { + self.fcx.try_overloaded_deref(self.span, None, ty, pref) + } else { + None + } + }).collect(); + + debug!("finalize({:?}) - {:?},{:?}", pref, methods, self.obligations); + + for expr in exprs { + debug!("finalize - finalizing #{} - {:?}", expr.id, expr); + for (n, method) in methods.iter().enumerate() { + if let &Some(method) = method { + let method_call = MethodCall::autoderef(expr.id, n as u32); + self.fcx.tables.borrow_mut().method_map.insert(method_call, method); + } + } + } + + for obligation in self.obligations { + self.fcx.register_predicate(obligation); + } + } +} + +impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { + pub fn autoderef(&'a self, + span: Span, + base_ty: Ty<'tcx>) + -> Autoderef<'a, 'gcx, 'tcx> + { + Autoderef { + fcx: self, + steps: vec![], + cur_ty: self.resolve_type_vars_if_possible(&base_ty), + obligations: vec![], + at_start: true, + span: span + } + } + + pub fn try_overloaded_deref(&self, + span: Span, + base_expr: Option<&hir::Expr>, + base_ty: Ty<'tcx>, + lvalue_pref: LvaluePreference) + -> Option> + { + debug!("try_overloaded_deref({:?},{:?},{:?},{:?})", + span, base_expr, base_ty, lvalue_pref); + // Try DerefMut first, if preferred. + let method = match (lvalue_pref, self.tcx.lang_items.deref_mut_trait()) { + (PreferMutLvalue, Some(trait_did)) => { + self.lookup_method_in_trait(span, base_expr, + token::intern("deref_mut"), trait_did, + base_ty, None) + } + _ => None + }; + + // Otherwise, fall back to Deref. + let method = match (method, self.tcx.lang_items.deref_trait()) { + (None, Some(trait_did)) => { + self.lookup_method_in_trait(span, base_expr, + token::intern("deref"), trait_did, + base_ty, None) + } + (method, _) => method + }; + + method + } +} diff --git a/src/librustc_typeck/check/callee.rs b/src/librustc_typeck/check/callee.rs index 7493ca70f55..417b2fafecf 100644 --- a/src/librustc_typeck/check/callee.rs +++ b/src/librustc_typeck/check/callee.rs @@ -9,7 +9,7 @@ // except according to those terms. use super::{DeferredCallResolution, Expectation, FnCtxt, - TupleArgumentsFlag, UnresolvedTypeAction}; + TupleArgumentsFlag}; use CrateCtxt; use middle::cstore::LOCAL_CRATE; @@ -72,15 +72,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { { self.check_expr(callee_expr); let original_callee_ty = self.expr_ty(callee_expr); - let (callee_ty, _, result) = - self.autoderef(callee_expr.span, - original_callee_ty, - || Some(callee_expr), - UnresolvedTypeAction::Error, - LvaluePreference::NoPreference, - |adj_ty, idx| { - self.try_overloaded_call_step(call_expr, callee_expr, adj_ty, idx) - }); + + let mut autoderef = self.autoderef(callee_expr.span, original_callee_ty); + let result = autoderef.by_ref().flat_map(|(adj_ty, idx)| { + self.try_overloaded_call_step(call_expr, callee_expr, adj_ty, idx) + }).next(); + let callee_ty = autoderef.unambiguous_final_ty(); + autoderef.finalize(LvaluePreference::NoPreference, Some(callee_expr)); match result { None => { diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 4861ab15e2c..2225fd588b1 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -60,7 +60,7 @@ //! sort of a minor point so I've opted to leave it for later---after all //! we may want to adjust precisely when coercions occur. -use check::{FnCtxt, UnresolvedTypeAction}; +use check::{FnCtxt}; use rustc::hir; use rustc::infer::{Coercion, InferOk, TypeOrigin, TypeTrace}; @@ -220,7 +220,8 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { -> CoerceResult<'tcx> // FIXME(eddyb) use copyable iterators when that becomes ergonomic. where E: Fn() -> I, - I: IntoIterator { + I: IntoIterator + { debug!("coerce_borrowed_pointer(a={:?}, b={:?})", a, b); @@ -240,18 +241,16 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { let span = self.origin.span(); - let lvalue_pref = LvaluePreference::from_mutbl(mt_b.mutbl); let mut first_error = None; let mut r_borrow_var = None; - let (_, autoderefs, success) = self.autoderef(span, a, exprs, - UnresolvedTypeAction::Ignore, - lvalue_pref, - |referent_ty, autoderef| - { - if autoderef == 0 { + let mut autoderef = self.autoderef(span, a); + let mut success = None; + + for (referent_ty, autoderefs) in autoderef.by_ref() { + if autoderefs == 0 { // Don't let this pass, otherwise it would cause // &T to autoref to &&T. - return None; + continue } // At this point, we have deref'd `a` to `referent_ty`. So @@ -326,7 +325,7 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { // and let regionck figure it out. let r = if !self.use_lub { r_b // [2] above - } else if autoderef == 1 { + } else if autoderefs == 1 { r_a // [3] above } else { if r_borrow_var.is_none() { // create var lazilly, at most once @@ -341,23 +340,22 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { mutbl: mt_b.mutbl // [1] above }); match self.unify(derefd_ty_a, b) { - Ok(ty) => Some(ty), + Ok(ty) => { success = Some((ty, autoderefs)); break }, Err(err) => { if first_error.is_none() { first_error = Some(err); } - None } } - }); + } // Extract type or return an error. We return the first error // we got, which should be from relating the "base" type // (e.g., in example above, the failure from relating `Vec` // to the target type), since that should be the least // confusing. - let ty = match success { - Some(ty) => ty, + let (ty, autoderefs) = match success { + Some(d) => d, None => { let err = first_error.expect("coerce_borrowed_pointer had no error"); debug!("coerce_borrowed_pointer: failed with err = {:?}", err); @@ -365,6 +363,8 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { } }; + autoderef.finalize(LvaluePreference::from_mutbl(mt_b.mutbl), exprs()); + // Now apply the autoref. We have to extract the region out of // the final ref type we got. if ty == a && mt_a.mutbl == hir::MutImmutable && autoderefs == 1 { diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs index 6faf6f415c2..683a67ff07c 100644 --- a/src/librustc_typeck/check/method/confirm.rs +++ b/src/librustc_typeck/check/method/confirm.rs @@ -11,11 +11,10 @@ use super::probe; use check::{FnCtxt, callee}; -use check::UnresolvedTypeAction; use hir::def_id::DefId; use rustc::ty::subst::{self}; use rustc::traits; -use rustc::ty::{self, NoPreference, PreferMutLvalue, Ty}; +use rustc::ty::{self, LvaluePreference, NoPreference, PreferMutLvalue, Ty}; use rustc::ty::adjustment::{AdjustDerefRef, AutoDerefRef, AutoPtr}; use rustc::ty::fold::TypeFoldable; use rustc::infer::{self, InferOk, TypeOrigin}; @@ -133,10 +132,10 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { ty: fty, substs: all_substs }; - // If this is an `&mut self` method, bias the receiver - // expression towards mutability (this will switch - // e.g. `Deref` to `DerefMut` in overloaded derefs and so on). - self.fixup_derefs_on_method_receiver_if_necessary(&callee); + + if let Some(hir::MutMutable) = pick.autoref { + self.convert_lvalue_derefs_to_mutable(); + } callee } @@ -164,22 +163,14 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { (None, None) }; - // Commit the autoderefs by calling `autoderef again, but this + // Commit the autoderefs by calling `autoderef` again, but this // time writing the results into the various tables. - let (autoderefd_ty, n, result) = self.autoderef(self.span, - unadjusted_self_ty, - || Some(self.self_expr), - UnresolvedTypeAction::Error, - NoPreference, - |_, n| { - if n == pick.autoderefs { - Some(()) - } else { - None - } - }); + let mut autoderef = self.autoderef(self.span, unadjusted_self_ty); + let (autoderefd_ty, n) = autoderef.nth(pick.autoderefs).unwrap(); assert_eq!(n, pick.autoderefs); - assert_eq!(result, Some(())); + + autoderef.unambiguous_final_ty(); + autoderef.finalize(LvaluePreference::NoPreference, Some(self.self_expr)); // Write out the final adjustment. self.write_adjustment(self.self_expr.id, AdjustDerefRef(AutoDerefRef { @@ -293,27 +284,21 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { // yield an object-type (e.g., `&Object` or `Box` // etc). - let (_, _, result) = self.fcx.autoderef(self.span, - self_ty, - || None, - UnresolvedTypeAction::Error, - NoPreference, - |ty, _| { - match ty.sty { - ty::TyTrait(ref data) => Some(closure(self, ty, &data)), - _ => None, - } - }); - - match result { - Some(r) => r, - None => { + // FIXME: this feels, like, super dubious + self.fcx.autoderef(self.span, self_ty) + .filter_map(|(ty, _)| { + match ty.sty { + ty::TyTrait(ref data) => Some(closure(self, ty, &data)), + _ => None, + } + }) + .next() + .unwrap_or_else(|| { span_bug!( self.span, "self-type `{}` for ObjectPick never dereferenced to an object", self_ty) - } - } + }) } fn instantiate_method_substs(&mut self, @@ -463,24 +448,10 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { /////////////////////////////////////////////////////////////////////////// // RECONCILIATION - /// When we select a method with an `&mut self` receiver, we have to go convert any + /// When we select a method with a mutable autoref, we have to go convert any /// auto-derefs, indices, etc from `Deref` and `Index` into `DerefMut` and `IndexMut` /// respectively. - fn fixup_derefs_on_method_receiver_if_necessary(&self, - method_callee: &ty::MethodCallee) { - let sig = match method_callee.ty.sty { - ty::TyFnDef(_, _, ref f) => f.sig.clone(), - _ => return, - }; - - match sig.0.inputs[0].sty { - ty::TyRef(_, ty::TypeAndMut { - ty: _, - mutbl: hir::MutMutable, - }) => {} - _ => return, - } - + fn convert_lvalue_derefs_to_mutable(&self) { // Gather up expressions we want to munge. let mut exprs = Vec::new(); exprs.push(self.self_expr); @@ -495,8 +466,7 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { } } - debug!("fixup_derefs_on_method_receiver_if_necessary: exprs={:?}", - exprs); + debug!("convert_lvalue_derefs_to_mutable: exprs={:?}", exprs); // Fix up autoderefs and derefs. for (i, &expr) in exprs.iter().rev().enumerate() { @@ -509,23 +479,17 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> { Some(_) | None => 0, }; - debug!("fixup_derefs_on_method_receiver_if_necessary: i={} expr={:?} \ - autoderef_count={}", + debug!("convert_lvalue_derefs_to_mutable: i={} expr={:?} \ + autoderef_count={}", i, expr, autoderef_count); if autoderef_count > 0 { - self.autoderef(expr.span, - self.expr_ty(expr), - || Some(expr), - UnresolvedTypeAction::Error, - PreferMutLvalue, - |_, autoderefs| { - if autoderefs == autoderef_count + 1 { - Some(()) - } else { - None - } + let mut autoderef = self.autoderef(expr.span, self.expr_ty(expr)); + autoderef.nth(autoderef_count).unwrap_or_else(|| { + span_bug!(expr.span, "expr was deref-able {} times but now isn't?", + autoderef_count); }); + autoderef.finalize(PreferMutLvalue, Some(expr)); } // Don't retry the first one or we might infinite loop! diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 08c04122517..0bb078dfbcb 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -13,13 +13,13 @@ use super::NoMatchData; use super::{CandidateSource, ImplSource, TraitSource}; use super::suggest; -use check::{FnCtxt, UnresolvedTypeAction}; +use check::{FnCtxt}; use hir::def_id::DefId; use hir::def::Def; use rustc::ty::subst; use rustc::ty::subst::Subst; use rustc::traits; -use rustc::ty::{self, NoPreference, Ty, ToPolyTraitRef, TraitRef, TypeFoldable}; +use rustc::ty::{self, Ty, ToPolyTraitRef, TraitRef, TypeFoldable}; use rustc::infer::{InferOk, TypeOrigin}; use syntax::ast; use syntax::codemap::{Span, DUMMY_SP}; @@ -208,25 +208,22 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { fn create_steps(&self, span: Span, self_ty: Ty<'tcx>) - -> Option>> { - let mut steps = Vec::new(); + -> Option>> + { + // FIXME: we don't need to create the entire steps in one pass - let (final_ty, dereferences, _) = self.autoderef(span, - self_ty, - || None, - UnresolvedTypeAction::Error, - NoPreference, - |t, d| { - steps.push(CandidateStep { - self_ty: t, - autoderefs: d, - unsize: false - }); - None::<()> // keep iterating until we can't anymore - }); + let mut autoderef = self.autoderef(span, self_ty); + let mut steps: Vec<_> = autoderef.by_ref().map(|(ty, d)| CandidateStep { + self_ty: ty, + autoderefs: d, + unsize: false + }).collect(); + let final_ty = autoderef.unambiguous_final_ty(); match final_ty.sty { ty::TyArray(elem_ty, _) => { + let dereferences = steps.len() - 1; + steps.push(CandidateStep { self_ty: self.tcx.mk_slice(elem_ty), autoderefs: dereferences, @@ -237,6 +234,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { _ => (), } + debug!("create_steps: steps={:?}", steps); + Some(steps) } } diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index 2cd60d20251..6f0d2bc0ca5 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -13,7 +13,7 @@ use CrateCtxt; -use check::{self, FnCtxt, UnresolvedTypeAction}; +use check::{FnCtxt}; use rustc::hir::map as hir_map; use rustc::ty::{self, Ty, ToPolyTraitRef, ToPredicate, TypeFoldable}; use middle::cstore; @@ -21,7 +21,6 @@ use hir::def::Def; use hir::def_id::DefId; use middle::lang_items::FnOnceTraitLangItem; use rustc::ty::subst::Substs; -use rustc::ty::LvaluePreference; use rustc::traits::{Obligation, SelectionContext}; use util::nodemap::{FnvHashSet}; @@ -48,42 +47,28 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ty::TyClosure(..) | ty::TyFnDef(..) | ty::TyFnPtr(_) => true, // If it's not a simple function, look for things which implement FnOnce _ => { - if let Ok(fn_once_trait_did) = - tcx.lang_items.require(FnOnceTraitLangItem) { - let (_, _, opt_is_fn) = self.autoderef(span, - ty, - || None, - UnresolvedTypeAction::Ignore, - LvaluePreference::NoPreference, - |ty, _| { - self.probe(|_| { - let fn_once_substs = - Substs::new_trait(vec![self.next_ty_var()], vec![], ty); - let trait_ref = - ty::TraitRef::new(fn_once_trait_did, - tcx.mk_substs(fn_once_substs)); - let poly_trait_ref = trait_ref.to_poly_trait_ref(); - let obligation = Obligation::misc(span, - self.body_id, - poly_trait_ref - .to_predicate()); - let mut selcx = SelectionContext::new(self); + let fn_once = match tcx.lang_items.require(FnOnceTraitLangItem) { + Ok(fn_once) => fn_once, + Err(..) => return false + }; - if selcx.evaluate_obligation(&obligation) { - Some(()) - } else { - None - } - }) - }); - - opt_is_fn.is_some() - } else { - false - } + self.autoderef(span, ty).any(|(ty, _)| self.probe(|_| { + let fn_once_substs = + Substs::new_trait(vec![self.next_ty_var()], vec![], ty); + let trait_ref = + ty::TraitRef::new(fn_once, + tcx.mk_substs(fn_once_substs)); + let poly_trait_ref = trait_ref.to_poly_trait_ref(); + let obligation = Obligation::misc(span, + self.body_id, + poly_trait_ref + .to_predicate()); + SelectionContext::new(self).evaluate_obligation(&obligation) + })) } } } + pub fn report_method_error(&self, span: Span, rcvr_ty: Ty<'tcx>, @@ -384,15 +369,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { return is_local(self.resolve_type_vars_with_obligations(rcvr_ty)); } - self.autoderef(span, rcvr_ty, || None, - check::UnresolvedTypeAction::Ignore, ty::NoPreference, - |ty, _| { - if is_local(ty) { - Some(()) - } else { - None - } - }).2.is_some() + self.autoderef(span, rcvr_ty).any(|(ty, _)| is_local(ty)) } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 264003bb62b..5dd00cf3666 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -129,6 +129,7 @@ use rustc_back::slice; use rustc_const_eval::eval_repeat_count; mod assoc; +mod autoderef; pub mod dropck; pub mod _match; pub mod writeback; @@ -1412,17 +1413,6 @@ impl<'a, 'gcx, 'tcx> RegionScope for FnCtxt<'a, 'gcx, 'tcx> { } } -/// Whether `autoderef` requires types to resolve. -#[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub enum UnresolvedTypeAction { - /// Produce an error and return `TyError` whenever a type cannot - /// be resolved (i.e. it is `TyInfer`). - Error, - /// Go on without emitting any errors, and return the unresolved - /// type. Useful for probing, e.g. in coercions. - Ignore -} - /// Controls whether the arguments are tupled. This is used for the call /// operator. /// @@ -2228,120 +2218,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } } - /// Executes an autoderef loop for the type `t`. At each step, invokes `should_stop` - /// to decide whether to terminate the loop. Returns the final type and number of - /// derefs that it performed. - /// - /// Note: this method does not modify the adjustments table. The caller is responsible for - /// inserting an AutoAdjustment record into the `self` using one of the suitable methods. - pub fn autoderef<'b, E, I, T, F>(&self, - sp: Span, - base_ty: Ty<'tcx>, - maybe_exprs: E, - unresolved_type_action: UnresolvedTypeAction, - mut lvalue_pref: LvaluePreference, - mut should_stop: F) - -> (Ty<'tcx>, usize, Option) - // FIXME(eddyb) use copyable iterators when that becomes ergonomic. - where E: Fn() -> I, - I: IntoIterator, - F: FnMut(Ty<'tcx>, usize) -> Option, - { - debug!("autoderef(base_ty={:?}, lvalue_pref={:?})", - base_ty, lvalue_pref); - - let mut t = base_ty; - for autoderefs in 0..self.tcx.sess.recursion_limit.get() { - let resolved_t = match unresolved_type_action { - UnresolvedTypeAction::Error => { - self.structurally_resolved_type(sp, t) - } - UnresolvedTypeAction::Ignore => { - // We can continue even when the type cannot be resolved - // (i.e. it is an inference variable) because `Ty::builtin_deref` - // and `try_overloaded_deref` both simply return `None` - // in such a case without producing spurious errors. - self.resolve_type_vars_if_possible(&t) - } - }; - if resolved_t.references_error() { - return (resolved_t, autoderefs, None); - } - - match should_stop(resolved_t, autoderefs) { - Some(x) => return (resolved_t, autoderefs, Some(x)), - None => {} - } - - // Otherwise, deref if type is derefable: - - // Super subtle: it might seem as though we should - // pass `opt_expr` to `try_overloaded_deref`, so that - // the (implicit) autoref of using an overloaded deref - // would get added to the adjustment table. However we - // do not do that, because it's kind of a - // "meta-adjustment" -- instead, we just leave it - // unrecorded and know that there "will be" an - // autoref. regionck and other bits of the code base, - // when they encounter an overloaded autoderef, have - // to do some reconstructive surgery. This is a pretty - // complex mess that is begging for a proper MIR. - let mt = if let Some(mt) = resolved_t.builtin_deref(false, lvalue_pref) { - mt - } else if let Some(method) = self.try_overloaded_deref(sp, None, - resolved_t, lvalue_pref) { - for expr in maybe_exprs() { - let method_call = MethodCall::autoderef(expr.id, autoderefs as u32); - self.tables.borrow_mut().method_map.insert(method_call, method); - } - self.make_overloaded_lvalue_return_type(method) - } else { - return (resolved_t, autoderefs, None); - }; - - t = mt.ty; - if mt.mutbl == hir::MutImmutable { - lvalue_pref = NoPreference; - } - } - - // We've reached the recursion limit, error gracefully. - span_err!(self.tcx.sess, sp, E0055, - "reached the recursion limit while auto-dereferencing {:?}", - base_ty); - (self.tcx.types.err, 0, None) - } - - fn try_overloaded_deref(&self, - span: Span, - base_expr: Option<&hir::Expr>, - base_ty: Ty<'tcx>, - lvalue_pref: LvaluePreference) - -> Option> - { - // Try DerefMut first, if preferred. - let method = match (lvalue_pref, self.tcx.lang_items.deref_mut_trait()) { - (PreferMutLvalue, Some(trait_did)) => { - self.lookup_method_in_trait(span, base_expr, - token::intern("deref_mut"), trait_did, - base_ty, None) - } - _ => None - }; - - // Otherwise, fall back to Deref. - let method = match (method, self.tcx.lang_items.deref_trait()) { - (None, Some(trait_did)) => { - self.lookup_method_in_trait(span, base_expr, - token::intern("deref"), trait_did, - base_ty, None) - } - (method, _) => method - }; - - method - } - /// For the overloaded lvalue expressions (`*x`, `x[3]`), the trait /// returns a type of `&T`, but the actual type we assign to the /// *expression* is `T`. So this function just peels off the return @@ -2371,29 +2247,28 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // autoderef that normal method probing does. They could likely be // consolidated. - let (ty, autoderefs, final_mt) = self.autoderef(base_expr.span, - base_ty, - || Some(base_expr), - UnresolvedTypeAction::Error, - lvalue_pref, - |adj_ty, idx| { - self.try_index_step(MethodCall::expr(expr.id), expr, base_expr, - adj_ty, idx, false, lvalue_pref, idx_ty) - }); + let mut autoderef = self.autoderef(base_expr.span, base_ty); - if final_mt.is_some() { - return final_mt; - } + while let Some((adj_ty, autoderefs)) = autoderef.next() { + if let Some(final_mt) = self.try_index_step( + MethodCall::expr(expr.id), + expr, base_expr, adj_ty, autoderefs, + false, lvalue_pref, idx_ty) + { + autoderef.finalize(lvalue_pref, Some(base_expr)); + return Some(final_mt); + } - // After we have fully autoderef'd, if the resulting type is [T; n], then - // do a final unsized coercion to yield [T]. - if let ty::TyArray(element_ty, _) = ty.sty { - let adjusted_ty = self.tcx.mk_slice(element_ty); - self.try_index_step(MethodCall::expr(expr.id), expr, base_expr, - adjusted_ty, autoderefs, true, lvalue_pref, idx_ty) - } else { - None + if let ty::TyArray(element_ty, _) = adj_ty.sty { + autoderef.finalize(lvalue_pref, Some(base_expr)); + let adjusted_ty = self.tcx.mk_slice(element_ty); + return self.try_index_step( + MethodCall::expr(expr.id), expr, base_expr, + adjusted_ty, autoderefs, true, lvalue_pref, idx_ty); + } } + autoderef.unambiguous_final_ty(); + None } /// To type-check `base_expr[index_expr]`, we progressively autoderef @@ -3034,32 +2909,23 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let expr_t = self.structurally_resolved_type(expr.span, self.expr_ty(base)); let mut private_candidate = None; - let (_, autoderefs, field_ty) = self.autoderef(expr.span, - expr_t, - || Some(base), - UnresolvedTypeAction::Error, - lvalue_pref, - |base_t, _| { - if let ty::TyStruct(base_def, substs) = base_t.sty { - debug!("struct named {:?}", base_t); - if let Some(field) = base_def.struct_variant().find_field_named(field.node) { - let field_ty = self.field_ty(expr.span, field, substs); - if field.vis.is_accessible_from(self.body_id, &self.tcx().map) { - return Some(field_ty); - } - private_candidate = Some((base_def.did, field_ty)); + let mut autoderef = self.autoderef(expr.span, expr_t); + while let Some((base_t, autoderefs)) = autoderef.next() { + if let ty::TyStruct(base_def, substs) = base_t.sty { + debug!("struct named {:?}", base_t); + if let Some(field) = base_def.struct_variant().find_field_named(field.node) { + let field_ty = self.field_ty(expr.span, field, substs); + if field.vis.is_accessible_from(self.body_id, &self.tcx().map) { + autoderef.finalize(lvalue_pref, Some(base)); + self.write_ty(expr.id, field_ty); + self.write_autoderef_adjustment(base.id, autoderefs); + return; } + private_candidate = Some((base_def.did, field_ty)); } - None - }); - match field_ty { - Some(field_ty) => { - self.write_ty(expr.id, field_ty); - self.write_autoderef_adjustment(base.id, autoderefs); - return; } - None => {} } + autoderef.unambiguous_final_ty(); if let Some((did, field_ty)) = private_candidate { let struct_path = self.tcx().item_path_str(did); @@ -3132,42 +2998,39 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.expr_ty(base)); let mut private_candidate = None; let mut tuple_like = false; - let (_, autoderefs, field_ty) = self.autoderef(expr.span, - expr_t, - || Some(base), - UnresolvedTypeAction::Error, - lvalue_pref, - |base_t, _| { - let (base_def, substs) = match base_t.sty { - ty::TyStruct(base_def, substs) => (base_def, substs), - ty::TyTuple(ref v) => { - tuple_like = true; - return if idx.node < v.len() { Some(v[idx.node]) } else { None } - } - _ => return None, - }; + let mut autoderef = self.autoderef(expr.span, expr_t); + while let Some((base_t, autoderefs)) = autoderef.next() { + let field = match base_t.sty { + ty::TyStruct(base_def, substs) => { + tuple_like = base_def.struct_variant().is_tuple_struct(); + if !tuple_like { continue } - tuple_like = base_def.struct_variant().is_tuple_struct(); - if !tuple_like { return None } - - debug!("tuple struct named {:?}", base_t); - if let Some(field) = base_def.struct_variant().fields.get(idx.node) { - let field_ty = self.field_ty(expr.span, field, substs); - if field.vis.is_accessible_from(self.body_id, &self.tcx().map) { - return Some(field_ty); - } - private_candidate = Some((base_def.did, field_ty)); + debug!("tuple struct named {:?}", base_t); + base_def.struct_variant().fields.get(idx.node).and_then(|field| { + let field_ty = self.field_ty(expr.span, field, substs); + private_candidate = Some((base_def.did, field_ty)); + if field.vis.is_accessible_from(self.body_id, &self.tcx().map) { + Some(field_ty) + } else { + None + } + }) } - None - }); - match field_ty { - Some(field_ty) => { + ty::TyTuple(ref v) => { + tuple_like = true; + v.get(idx.node).cloned() + } + _ => continue + }; + + if let Some(field_ty) = field { + autoderef.finalize(lvalue_pref, Some(base)); self.write_ty(expr.id, field_ty); self.write_autoderef_adjustment(base.id, autoderefs); return; } - None => {} } + autoderef.unambiguous_final_ty(); if let Some((did, field_ty)) = private_candidate { let struct_path = self.tcx().item_path_str(did); diff --git a/src/test/compile-fail/E0055.rs b/src/test/compile-fail/E0055.rs index cb78f4b3bb5..f86d7ec114b 100644 --- a/src/test/compile-fail/E0055.rs +++ b/src/test/compile-fail/E0055.rs @@ -19,5 +19,4 @@ fn main() { let foo = Foo; let ref_foo = &&Foo; ref_foo.foo(); //~ ERROR E0055 - //~^ ERROR E0275 } diff --git a/src/test/compile-fail/borrowck/borrowck-borrow-overloaded-auto-deref-mut.rs b/src/test/compile-fail/borrowck/borrowck-borrow-overloaded-auto-deref-mut.rs index 497b0e63edf..764d05be879 100644 --- a/src/test/compile-fail/borrowck/borrowck-borrow-overloaded-auto-deref-mut.rs +++ b/src/test/compile-fail/borrowck/borrowck-borrow-overloaded-auto-deref-mut.rs @@ -99,7 +99,7 @@ fn assign_field1<'a>(x: Own) { } fn assign_field2<'a>(x: &'a Own) { - x.y = 3; //~ ERROR cannot assign + x.y = 3; //~ ERROR cannot borrow } fn assign_field3<'a>(x: &'a mut Own) { diff --git a/src/test/compile-fail/issue-24819.rs b/src/test/compile-fail/issue-24819.rs new file mode 100644 index 00000000000..52f5f1cd079 --- /dev/null +++ b/src/test/compile-fail/issue-24819.rs @@ -0,0 +1,21 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use std::collections::HashSet; + +fn main() { + let mut v = Vec::new(); + foo(&mut v); + //~^ ERROR mismatched types + //~| expected struct `std::collections::HashSet`, found struct `std::vec::Vec` +} + +fn foo(h: &mut HashSet) { +} From 33dfd0fb16a71cc3c96ca28b57eccbd1e278a3ac Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 23 May 2016 22:29:17 -0700 Subject: [PATCH 10/17] std: Use memalign, not posix_memalign, on Android We've gotten requests to move our Android support as far back as API level 9 where unfortunately the `posix_memalign` API wasn't implemented yet. Thankfully, however, the `memalign` API was and it appears to be usable with `free` on the Android platform (see comments included in commit). This should help fix some of the last few test failures when compiling against API level 9. --- src/liballoc_system/lib.rs | 41 +++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/liballoc_system/lib.rs b/src/liballoc_system/lib.rs index a22299c5e1a..9eade937bfb 100644 --- a/src/liballoc_system/lib.rs +++ b/src/liballoc_system/lib.rs @@ -80,13 +80,40 @@ mod imp { if align <= MIN_ALIGN { libc::malloc(size as libc::size_t) as *mut u8 } else { - let mut out = ptr::null_mut(); - let ret = libc::posix_memalign(&mut out, align as libc::size_t, size as libc::size_t); - if ret != 0 { - ptr::null_mut() - } else { - out as *mut u8 - } + aligned_malloc(size, align) + } + } + + #[cfg(target_os = "android")] + unsafe fn aligned_malloc(size: usize, align: usize) -> *mut u8 { + // On android we currently target API level 9 which unfortunately + // doesn't have the `posix_memalign` API used below. Instead we use + // `memalign`, but this unfortunately has the property on some systems + // where the memory returned cannot be deallocated by `free`! + // + // Upon closer inspection, however, this appears to work just fine with + // Android, so for this platform we should be fine to call `memalign` + // (which is present in API level 9). Some helpful references could + // possibly be chromium using memalign [1], attempts at documenting that + // memalign + free is ok [2] [3], or the current source of chromium + // which still uses memalign on android [4]. + // + // [1]: https://codereview.chromium.org/10796020/ + // [2]: https://code.google.com/p/android/issues/detail?id=35391 + // [3]: https://bugs.chromium.org/p/chromium/issues/detail?id=138579 + // [4]: https://chromium.googlesource.com/chromium/src/base/+/master/ + // /memory/aligned_memory.cc + libc::memalign(align as libc::size_t, size as libc::size_t) as *mut u8 + } + + #[cfg(not(target_os = "android"))] + unsafe fn aligned_malloc(size: usize, align: usize) -> *mut u8 { + let mut out = ptr::null_mut(); + let ret = libc::posix_memalign(&mut out, align as libc::size_t, size as libc::size_t); + if ret != 0 { + ptr::null_mut() + } else { + out as *mut u8 } } From 394c23b084bcdfe8f45de194ebe77d4e06d4d5e9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 25 May 2016 00:49:49 +0200 Subject: [PATCH 11/17] Implement Error trait for fmt::Error type --- src/libstd/error.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libstd/error.rs b/src/libstd/error.rs index d49d9764946..2a2d41112ff 100644 --- a/src/libstd/error.rs +++ b/src/libstd/error.rs @@ -212,6 +212,13 @@ impl Error for Box { } } +#[stable(feature = "fmt_error", since = "1.11.0")] +impl Error for fmt::Error { + fn description(&self) -> &str { + "an error occurred when formatting an argument" + } +} + // copied from any.rs impl Error + 'static { /// Returns true if the boxed type is the same as `T` From 040fc94b4eaeb24b6da297a763a28df66473e34d Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 25 May 2016 21:12:35 +0300 Subject: [PATCH 12/17] catch attempts to leak obligations out of snapshots --- src/librustc/infer/mod.rs | 33 +++++- src/librustc/traits/fulfill.rs | 2 + src/librustc/traits/specialize/mod.rs | 78 ++++++------ src/librustc_typeck/check/coercion.rs | 2 + src/librustc_typeck/check/compare_method.rs | 111 ++++++++---------- .../regions-bound-missing-bound-in-impl.rs | 3 +- src/test/compile-fail/regions-trait-1.rs | 2 +- 7 files changed, 121 insertions(+), 110 deletions(-) diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index 41982ddc78b..7c9c52baa63 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -163,6 +163,11 @@ pub struct InferCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { // If the number of errors increases, that's also a sign (line // `tained_by_errors`) to avoid reporting certain kinds of errors. err_count_on_creation: usize, + + // This flag is used for debugging, and is set to true if there are + // any obligations set during the current snapshot. In that case, the + // snapshot can't be rolled back. + pub obligations_in_snapshot: Cell, } /// A map returned by `skolemize_late_bound_regions()` indicating the skolemized @@ -476,7 +481,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> { normalize: false, projection_mode: ProjectionMode::AnyFinal, tainted_by_errors_flag: Cell::new(false), - err_count_on_creation: self.sess.err_count() + err_count_on_creation: self.sess.err_count(), + obligations_in_snapshot: Cell::new(false), } } } @@ -515,7 +521,8 @@ impl<'a, 'gcx, 'tcx> InferCtxtBuilder<'a, 'gcx, 'tcx> { normalize: normalize, projection_mode: projection_mode, tainted_by_errors_flag: Cell::new(false), - err_count_on_creation: tcx.sess.err_count() + err_count_on_creation: tcx.sess.err_count(), + obligations_in_snapshot: Cell::new(false), })) } } @@ -542,6 +549,7 @@ pub struct CombinedSnapshot { int_snapshot: unify::Snapshot, float_snapshot: unify::Snapshot, region_vars_snapshot: RegionSnapshot, + obligations_in_snapshot: bool, } /// Helper trait for shortening the lifetimes inside a @@ -809,11 +817,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } fn start_snapshot(&self) -> CombinedSnapshot { + let obligations_in_snapshot = self.obligations_in_snapshot.get(); + self.obligations_in_snapshot.set(false); + CombinedSnapshot { type_snapshot: self.type_variables.borrow_mut().snapshot(), int_snapshot: self.int_unification_table.borrow_mut().snapshot(), float_snapshot: self.float_unification_table.borrow_mut().snapshot(), region_vars_snapshot: self.region_vars.start_snapshot(), + obligations_in_snapshot: obligations_in_snapshot, } } @@ -822,7 +834,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let CombinedSnapshot { type_snapshot, int_snapshot, float_snapshot, - region_vars_snapshot } = snapshot; + region_vars_snapshot, + obligations_in_snapshot } = snapshot; + + assert!(!self.obligations_in_snapshot.get()); + self.obligations_in_snapshot.set(obligations_in_snapshot); self.type_variables .borrow_mut() @@ -842,7 +858,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let CombinedSnapshot { type_snapshot, int_snapshot, float_snapshot, - region_vars_snapshot } = snapshot; + region_vars_snapshot, + obligations_in_snapshot } = snapshot; + + self.obligations_in_snapshot.set(obligations_in_snapshot); self.type_variables .borrow_mut() @@ -904,12 +923,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let CombinedSnapshot { type_snapshot, int_snapshot, float_snapshot, - region_vars_snapshot } = self.start_snapshot(); + region_vars_snapshot, + obligations_in_snapshot } = self.start_snapshot(); let r = self.commit_if_ok(|_| f()); debug!("commit_regions_if_ok: rolling back everything but regions"); + assert!(!self.obligations_in_snapshot.get()); + self.obligations_in_snapshot.set(obligations_in_snapshot); + // Roll back any non-region bindings - they should be resolved // inside `f`, with, e.g. `resolve_type_vars_if_possible`. self.type_variables diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index d9d0367bdcb..0d7d7afd120 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -171,6 +171,8 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> { // debug output much nicer to read and so on. let obligation = infcx.resolve_type_vars_if_possible(&obligation); + infcx.obligations_in_snapshot.set(true); + if infcx.tcx.fulfilled_predicates.borrow().check_duplicate(&obligation.predicate) { return diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index b2d14dab9a0..c7a36375576 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -187,51 +187,49 @@ fn fulfill_implication<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>, source_trait_ref: ty::TraitRef<'tcx>, target_impl: DefId) -> Result<&'tcx Substs<'tcx>, ()> { - infcx.commit_if_ok(|_| { - let selcx = &mut SelectionContext::new(&infcx); - let target_substs = fresh_type_vars_for_impl(&infcx, DUMMY_SP, target_impl); - let (target_trait_ref, obligations) = impl_trait_ref_and_oblig(selcx, - target_impl, - &target_substs); + let selcx = &mut SelectionContext::new(&infcx); + let target_substs = fresh_type_vars_for_impl(&infcx, DUMMY_SP, target_impl); + let (target_trait_ref, obligations) = impl_trait_ref_and_oblig(selcx, + target_impl, + &target_substs); - // do the impls unify? If not, no specialization. - if let Err(_) = infcx.eq_trait_refs(true, - TypeOrigin::Misc(DUMMY_SP), - source_trait_ref, - target_trait_ref) { - debug!("fulfill_implication: {:?} does not unify with {:?}", - source_trait_ref, - target_trait_ref); - return Err(()); - } + // do the impls unify? If not, no specialization. + if let Err(_) = infcx.eq_trait_refs(true, + TypeOrigin::Misc(DUMMY_SP), + source_trait_ref, + target_trait_ref) { + debug!("fulfill_implication: {:?} does not unify with {:?}", + source_trait_ref, + target_trait_ref); + return Err(()); + } - // attempt to prove all of the predicates for impl2 given those for impl1 - // (which are packed up in penv) + // attempt to prove all of the predicates for impl2 given those for impl1 + // (which are packed up in penv) - let mut fulfill_cx = FulfillmentContext::new(); - for oblig in obligations.into_iter() { - fulfill_cx.register_predicate_obligation(&infcx, oblig); - } + let mut fulfill_cx = FulfillmentContext::new(); + for oblig in obligations.into_iter() { + fulfill_cx.register_predicate_obligation(&infcx, oblig); + } - if let Err(errors) = infcx.drain_fulfillment_cx(&mut fulfill_cx, &()) { - // no dice! - debug!("fulfill_implication: for impls on {:?} and {:?}, could not fulfill: {:?} given \ - {:?}", - source_trait_ref, - target_trait_ref, - errors, - infcx.parameter_environment.caller_bounds); - Err(()) - } else { - debug!("fulfill_implication: an impl for {:?} specializes {:?}", - source_trait_ref, - target_trait_ref); + if let Err(errors) = infcx.drain_fulfillment_cx(&mut fulfill_cx, &()) { + // no dice! + debug!("fulfill_implication: for impls on {:?} and {:?}, could not fulfill: {:?} given \ + {:?}", + source_trait_ref, + target_trait_ref, + errors, + infcx.parameter_environment.caller_bounds); + Err(()) + } else { + debug!("fulfill_implication: an impl for {:?} specializes {:?}", + source_trait_ref, + target_trait_ref); - // Now resolve the *substitution* we built for the target earlier, replacing - // the inference variables inside with whatever we got from fulfillment. - Ok(infcx.resolve_type_vars_if_possible(&target_substs)) - } - }) + // Now resolve the *substitution* we built for the target earlier, replacing + // the inference variables inside with whatever we got from fulfillment. + Ok(infcx.resolve_type_vars_if_possible(&target_substs)) + } } pub struct SpecializesCache { diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 2225fd588b1..9dd737f3a61 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -363,6 +363,8 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> { } }; + // This commits the obligations to the fulfillcx. After this succeeds, + // this snapshot can't be rolled back. autoderef.finalize(LvaluePreference::from_mutbl(mt_b.mutbl), exprs()); // Now apply the autoref. We have to extract the region out of diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index a1a6a83d34f..20f82271b9c 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -279,78 +279,63 @@ pub fn compare_impl_method<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, // type. // Compute skolemized form of impl and trait method tys. - let impl_fty = tcx.mk_fn_ptr(impl_m.fty); - let impl_fty = impl_fty.subst(tcx, impl_to_skol_substs); - let trait_fty = tcx.mk_fn_ptr(trait_m.fty); - let trait_fty = trait_fty.subst(tcx, &trait_to_skol_substs); + let tcx = infcx.tcx; + let origin = TypeOrigin::MethodCompatCheck(impl_m_span); - let err = infcx.commit_if_ok(|snapshot| { - let tcx = infcx.tcx; - let origin = TypeOrigin::MethodCompatCheck(impl_m_span); + let (impl_sig, _) = + infcx.replace_late_bound_regions_with_fresh_var(impl_m_span, + infer::HigherRankedType, + &impl_m.fty.sig); + let impl_sig = + impl_sig.subst(tcx, impl_to_skol_substs); + let impl_sig = + assoc::normalize_associated_types_in(&infcx, + &mut fulfillment_cx, + impl_m_span, + impl_m_body_id, + &impl_sig); + let impl_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy { + unsafety: impl_m.fty.unsafety, + abi: impl_m.fty.abi, + sig: ty::Binder(impl_sig) + })); + debug!("compare_impl_method: impl_fty={:?}", impl_fty); - let (impl_sig, _) = - infcx.replace_late_bound_regions_with_fresh_var(impl_m_span, - infer::HigherRankedType, - &impl_m.fty.sig); - let impl_sig = - impl_sig.subst(tcx, impl_to_skol_substs); - let impl_sig = - assoc::normalize_associated_types_in(&infcx, - &mut fulfillment_cx, - impl_m_span, - impl_m_body_id, - &impl_sig); - let impl_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy { - unsafety: impl_m.fty.unsafety, - abi: impl_m.fty.abi, - sig: ty::Binder(impl_sig) - })); - debug!("compare_impl_method: impl_fty={:?}", - impl_fty); + let trait_sig = tcx.liberate_late_bound_regions( + infcx.parameter_environment.free_id_outlive, + &trait_m.fty.sig); + let trait_sig = + trait_sig.subst(tcx, &trait_to_skol_substs); + let trait_sig = + assoc::normalize_associated_types_in(&infcx, + &mut fulfillment_cx, + impl_m_span, + impl_m_body_id, + &trait_sig); + let trait_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy { + unsafety: trait_m.fty.unsafety, + abi: trait_m.fty.abi, + sig: ty::Binder(trait_sig) + })); - let (trait_sig, skol_map) = - infcx.skolemize_late_bound_regions(&trait_m.fty.sig, snapshot); - let trait_sig = - trait_sig.subst(tcx, &trait_to_skol_substs); - let trait_sig = - assoc::normalize_associated_types_in(&infcx, - &mut fulfillment_cx, - impl_m_span, - impl_m_body_id, - &trait_sig); - let trait_fty = tcx.mk_fn_ptr(tcx.mk_bare_fn(ty::BareFnTy { - unsafety: trait_m.fty.unsafety, - abi: trait_m.fty.abi, - sig: ty::Binder(trait_sig) - })); + debug!("compare_impl_method: trait_fty={:?}", trait_fty); - debug!("compare_impl_method: trait_fty={:?}", + if let Err(terr) = infcx.sub_types(false, origin, impl_fty, trait_fty) { + debug!("sub_types failed: impl ty {:?}, trait ty {:?}", + impl_fty, trait_fty); - - infcx.sub_types(false, origin, impl_fty, trait_fty)?; - - infcx.leak_check(false, &skol_map, snapshot) - }); - - match err { - Ok(()) => { } - Err(terr) => { - debug!("checking trait method for compatibility: impl ty {:?}, trait ty {:?}", - impl_fty, - trait_fty); - span_err!(tcx.sess, impl_m_span, E0053, - "method `{}` has an incompatible type for trait: {}", - trait_m.name, - terr); - return; - } + span_err!(tcx.sess, impl_m_span, E0053, + "method `{}` has an incompatible type for trait: {}", + trait_m.name, + terr); + return } // Check that all obligations are satisfied by the implementation's // version. - match fulfillment_cx.select_all_or_error(&infcx) { - Err(ref errors) => { infcx.report_fulfillment_errors(errors) } - Ok(_) => {} + if let Err(ref errors) = fulfillment_cx.select_all_or_error(&infcx) { + infcx.report_fulfillment_errors(errors); + return } // Finally, resolve all regions. This catches wily misuses of diff --git a/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs b/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs index abffd33e3f8..6e60a373d9b 100644 --- a/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs +++ b/src/test/compile-fail/regions-bound-missing-bound-in-impl.rs @@ -34,7 +34,8 @@ impl<'a, 't> Foo<'a, 't> for &'a isize { } fn wrong_bound1<'b,'c,'d:'a+'c>(self, b: Inv<'b>, c: Inv<'c>, d: Inv<'d>) { - //~^ ERROR method `wrong_bound1` has an incompatible type for trait + //~^ ERROR method not compatible with trait + //~^^ ERROR method not compatible with trait // // Note: This is a terrible error message. It is caused // because, in the trait, 'b is early bound, and in the impl, diff --git a/src/test/compile-fail/regions-trait-1.rs b/src/test/compile-fail/regions-trait-1.rs index 01439ce5e68..9cd08656b62 100644 --- a/src/test/compile-fail/regions-trait-1.rs +++ b/src/test/compile-fail/regions-trait-1.rs @@ -23,7 +23,7 @@ impl<'a> get_ctxt for has_ctxt<'a> { // Here an error occurs because we used `&self` but // the definition used `&`: - fn get_ctxt(&self) -> &'a ctxt { //~ ERROR method `get_ctxt` has an incompatible type + fn get_ctxt(&self) -> &'a ctxt { //~ ERROR method not compatible with trait self.c } From c30fa92a0aac87ba27df261ece44602f027a1800 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Wed, 30 Dec 2015 10:08:28 +0100 Subject: [PATCH 13/17] `EscapeUnicode` and `EscapeDefault` are `ExactSizeIterator`s In #28662, `size_hint` was made exact for `EscapeUnicode` and `EscapeDefault`, but neither was marked as `ExactSizeIterator`. --- src/libcore/char.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcore/char.rs b/src/libcore/char.rs index 6a2331dddcf..25d90cc6f3a 100644 --- a/src/libcore/char.rs +++ b/src/libcore/char.rs @@ -483,6 +483,9 @@ impl Iterator for EscapeUnicode { } } +#[stable(feature = "exact_size_escape", since = "1.11.0")] +impl ExactSizeIterator for EscapeUnicode { } + /// An iterator that yields the literal escape code of a `char`. /// /// This `struct` is created by the [`escape_default()`] method on [`char`]. See @@ -578,6 +581,9 @@ impl Iterator for EscapeDefault { } } +#[stable(feature = "exact_size_escape", since = "1.11.0")] +impl ExactSizeIterator for EscapeDefault { } + /// An iterator over `u8` entries represending the UTF-8 encoding of a `char` /// value. /// From baa9680a3449a585481bd4b124f3e1f108262877 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Wed, 30 Dec 2015 16:42:52 +0100 Subject: [PATCH 14/17] Implement `count` for `EscapeDefault` and `EscapeUnicode` Trivial implementation, as both are `ExactSizeIterator`s. Part of #24214. --- src/libcore/char.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libcore/char.rs b/src/libcore/char.rs index 25d90cc6f3a..f803b36cede 100644 --- a/src/libcore/char.rs +++ b/src/libcore/char.rs @@ -470,6 +470,11 @@ impl Iterator for EscapeUnicode { (n, Some(n)) } + #[inline] + fn count(self) -> usize { + self.len() + } + fn last(self) -> Option { match self.state { EscapeUnicodeState::Done => None, @@ -535,13 +540,9 @@ impl Iterator for EscapeDefault { } } + #[inline] fn count(self) -> usize { - match self.state { - EscapeDefaultState::Char(_) => 1, - EscapeDefaultState::Unicode(iter) => iter.count(), - EscapeDefaultState::Done => 0, - EscapeDefaultState::Backslash(_) => 2, - } + self.len() } fn nth(&mut self, n: usize) -> Option { From da03950f62a43ff3ca32f931a3edecc71a214f3b Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Mon, 18 Jan 2016 17:36:12 +0100 Subject: [PATCH 15/17] Move length computation to `ExactSizeIterator` impls and reuse it in `size_hint`. --- src/libcore/char.rs | 60 +++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/src/libcore/char.rs b/src/libcore/char.rs index f803b36cede..38337c7493e 100644 --- a/src/libcore/char.rs +++ b/src/libcore/char.rs @@ -413,12 +413,12 @@ pub struct EscapeUnicode { #[derive(Clone, Debug)] enum EscapeUnicodeState { - Backslash, - Type, - LeftBrace, - Value, - RightBrace, Done, + RightBrace, + Value, + LeftBrace, + Type, + Backslash, } #[stable(feature = "rust1", since = "1.0.0")] @@ -457,16 +457,9 @@ impl Iterator for EscapeUnicode { } } + #[inline] fn size_hint(&self) -> (usize, Option) { - let n = match self.state { - EscapeUnicodeState::Backslash => 5, - EscapeUnicodeState::Type => 4, - EscapeUnicodeState::LeftBrace => 3, - EscapeUnicodeState::Value => 2, - EscapeUnicodeState::RightBrace => 1, - EscapeUnicodeState::Done => 0, - }; - let n = n + self.hex_digit_idx; + let n = self.len(); (n, Some(n)) } @@ -489,7 +482,20 @@ impl Iterator for EscapeUnicode { } #[stable(feature = "exact_size_escape", since = "1.11.0")] -impl ExactSizeIterator for EscapeUnicode { } +impl ExactSizeIterator for EscapeUnicode { + #[inline] + fn len(&self) -> usize { + // The match is a single memory access with no branching + self.hex_digit_idx + match self.state { + EscapeUnicodeState::Done => 0, + EscapeUnicodeState::RightBrace => 1, + EscapeUnicodeState::Value => 2, + EscapeUnicodeState::LeftBrace => 3, + EscapeUnicodeState::Type => 4, + EscapeUnicodeState::Backslash => 5, + } + } +} /// An iterator that yields the literal escape code of a `char`. /// @@ -506,9 +512,9 @@ pub struct EscapeDefault { #[derive(Clone, Debug)] enum EscapeDefaultState { - Backslash(char), - Char(char), Done, + Char(char), + Backslash(char), Unicode(EscapeUnicode), } @@ -531,13 +537,10 @@ impl Iterator for EscapeDefault { } } + #[inline] fn size_hint(&self) -> (usize, Option) { - match self.state { - EscapeDefaultState::Char(_) => (1, Some(1)), - EscapeDefaultState::Backslash(_) => (2, Some(2)), - EscapeDefaultState::Unicode(ref iter) => iter.size_hint(), - EscapeDefaultState::Done => (0, Some(0)), - } + let n = self.len(); + (n, Some(n)) } #[inline] @@ -583,7 +586,16 @@ impl Iterator for EscapeDefault { } #[stable(feature = "exact_size_escape", since = "1.11.0")] -impl ExactSizeIterator for EscapeDefault { } +impl ExactSizeIterator for EscapeDefault { + fn len(&self) -> usize { + match self.state { + EscapeDefaultState::Done => 0, + EscapeDefaultState::Char(_) => 1, + EscapeDefaultState::Backslash(_) => 2, + EscapeDefaultState::Unicode(ref iter) => iter.len(), + } + } +} /// An iterator over `u8` entries represending the UTF-8 encoding of a `char` /// value. From 41950c64a1bcb7025b42dde05c3cec4c3993f293 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Thu, 26 May 2016 10:04:05 +0200 Subject: [PATCH 16/17] Explain the order of the enumeration items Simply a micro-optimization to reduce code size and to open up inlining opportunities. --- src/libcore/char.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcore/char.rs b/src/libcore/char.rs index 38337c7493e..d80b456181a 100644 --- a/src/libcore/char.rs +++ b/src/libcore/char.rs @@ -411,6 +411,9 @@ pub struct EscapeUnicode { hex_digit_idx: usize, } +// The enum values are ordered so that their representation is the +// same as the remaining length (besides the hexadecimal digits). This +// likely makes `len()` a single load from memory) and inline-worth. #[derive(Clone, Debug)] enum EscapeUnicodeState { Done, From 6b5e86b0ce543c60e201f95d57d720181281f1da Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Thu, 26 May 2016 10:54:58 +0200 Subject: [PATCH 17/17] Extend the test for `EscapeUnicode` to also check that it is legitimately an `ExactSizeIterator`. --- src/libcoretest/char.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcoretest/char.rs b/src/libcoretest/char.rs index e959e71daf7..7da876b9459 100644 --- a/src/libcoretest/char.rs +++ b/src/libcoretest/char.rs @@ -276,6 +276,12 @@ fn eu_iterator_specializations() { // Check last assert_eq!(iter.clone().last(), Some('}')); + // Check len + assert_eq!(iter.len(), len - offset); + + // Check size_hint (= len in ExactSizeIterator) + assert_eq!(iter.size_hint(), (iter.len(), Some(iter.len()))); + // Check counting assert_eq!(iter.clone().count(), len - offset);