From 4890ae254f39b1721724e8f663531e53245a347f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 May 2019 13:13:51 -0700 Subject: [PATCH 1/8] Do not ICE on missing access place description during mutability error reporting --- .../borrow_check/mutability_errors.rs | 16 +++++++--------- src/test/ui/issues/issue-61187.rs | 9 +++++++++ src/test/ui/issues/issue-61187.stderr | 11 +++++++++++ 3 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/issues/issue-61187.rs create mode 100644 src/test/ui/issues/issue-61187.stderr diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 16fbc8d6bb299..aa78545989955 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -41,14 +41,17 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { ); let mut err; - let item_msg; + let mut item_msg; let reason; let access_place_desc = self.describe_place(access_place); debug!("report_mutability_error: access_place_desc={:?}", access_place_desc); + item_msg = match &access_place_desc { + Some(desc) => format!("`{}`", desc), + None => "temporary place".to_string(), + }; match the_place_err { Place::Base(PlaceBase::Local(local)) => { - item_msg = format!("`{}`", access_place_desc.unwrap()); if let Place::Base(PlaceBase::Local(_)) = access_place { reason = ", as it is not declared as mutable".to_string(); } else { @@ -67,7 +70,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { base.ty(self.mir, self.infcx.tcx).ty )); - item_msg = format!("`{}`", access_place_desc.unwrap()); if self.is_upvar_field_projection(access_place).is_some() { reason = ", as it is not declared as mutable".to_string(); } else { @@ -82,7 +84,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { }) => { if *base == Place::Base(PlaceBase::Local(Local::new(1))) && !self.upvars.is_empty() { - item_msg = format!("`{}`", access_place_desc.unwrap()); debug_assert!(self.mir.local_decls[Local::new(1)].ty.is_region_ptr()); debug_assert!(is_closure_or_generator( the_place_err.ty(self.mir, self.infcx.tcx).ty @@ -105,7 +106,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { false } } { - item_msg = format!("`{}`", access_place_desc.unwrap()); reason = ", as it is immutable for the pattern guard".to_string(); } else { let pointer_type = @@ -114,8 +114,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { } else { "`*const` pointer" }; - if let Some(desc) = access_place_desc { - item_msg = format!("`{}`", desc); + if access_place_desc.is_some() { reason = match error_access { AccessKind::Move | AccessKind::Mutate => format!(" which is behind a {}", pointer_type), @@ -135,10 +134,9 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), .. })) => { if let Place::Base(PlaceBase::Static(_)) = access_place { - item_msg = format!("immutable static item `{}`", access_place_desc.unwrap()); + item_msg = format!("immutable static item {}", item_msg); reason = String::new(); } else { - item_msg = format!("`{}`", access_place_desc.unwrap()); let static_name = &self.infcx.tcx.item_name(*def_id); reason = format!(", as `{}` is an immutable static item", static_name); } diff --git a/src/test/ui/issues/issue-61187.rs b/src/test/ui/issues/issue-61187.rs new file mode 100644 index 0000000000000..8b939b43b8bd4 --- /dev/null +++ b/src/test/ui/issues/issue-61187.rs @@ -0,0 +1,9 @@ +// edition:2018 +#![feature(async_await)] + +fn main() { +} + +async fn response(data: Vec) { + data.reverse(); //~ ERROR E0596 +} diff --git a/src/test/ui/issues/issue-61187.stderr b/src/test/ui/issues/issue-61187.stderr new file mode 100644 index 0000000000000..1019999a4bf30 --- /dev/null +++ b/src/test/ui/issues/issue-61187.stderr @@ -0,0 +1,11 @@ +error[E0596]: cannot borrow temporary place as mutable, as it is not declared as mutable + --> $DIR/issue-61187.rs:8:5 + | +LL | async fn response(data: Vec) { + | ---- help: consider changing this to be mutable: `mut data` +LL | data.reverse(); + | ^^^^ cannot borrow as mutable + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0596`. From 285064ca21b8a96687a96dfc5ece713b38170d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 May 2019 20:42:00 -0700 Subject: [PATCH 2/8] Probe for CompilerDesugaringKind::Async argument --- src/librustc_mir/borrow_check/mutability_errors.rs | 12 +++++++----- src/test/ui/issues/issue-61187.stderr | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index aa78545989955..e90795c964691 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -7,7 +7,7 @@ use rustc::mir::{ use rustc::mir::{Terminator, TerminatorKind}; use rustc::ty::{self, Const, DefIdTree, Ty, TyS, TyCtxt}; use rustc_data_structures::indexed_vec::Idx; -use syntax_pos::Span; +use syntax_pos::{Span, CompilerDesugaringKind}; use syntax_pos::symbol::kw; use crate::dataflow::move_paths::InitLocation; @@ -41,14 +41,16 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { ); let mut err; - let mut item_msg; let reason; let access_place_desc = self.describe_place(access_place); debug!("report_mutability_error: access_place_desc={:?}", access_place_desc); - item_msg = match &access_place_desc { - Some(desc) => format!("`{}`", desc), - None => "temporary place".to_string(), + let mut item_msg = match (&access_place_desc, &the_place_err) { + (Some(desc), _) => format!("`{}`", desc), + (None, Place::Base(PlaceBase::Local(local))) if self.mir.local_decls[*local] + .source_info.span.is_compiler_desugaring(CompilerDesugaringKind::Async) + => "async `fn` parameter".to_string(), + (None, _) => "temporary place".to_string(), }; match the_place_err { Place::Base(PlaceBase::Local(local)) => { diff --git a/src/test/ui/issues/issue-61187.stderr b/src/test/ui/issues/issue-61187.stderr index 1019999a4bf30..6f6f32a9ca781 100644 --- a/src/test/ui/issues/issue-61187.stderr +++ b/src/test/ui/issues/issue-61187.stderr @@ -1,4 +1,4 @@ -error[E0596]: cannot borrow temporary place as mutable, as it is not declared as mutable +error[E0596]: cannot borrow async `fn` parameter as mutable, as it is not declared as mutable --> $DIR/issue-61187.rs:8:5 | LL | async fn response(data: Vec) { From 025a5595e6230c50fe84c1a35215afdd96370655 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 25 May 2019 20:45:00 -0700 Subject: [PATCH 3/8] Move tests to async-await directory --- src/test/ui/{ => async-await}/issues/issue-61187.rs | 0 src/test/ui/{ => async-await}/issues/issue-61187.stderr | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename src/test/ui/{ => async-await}/issues/issue-61187.rs (100%) rename src/test/ui/{ => async-await}/issues/issue-61187.stderr (100%) diff --git a/src/test/ui/issues/issue-61187.rs b/src/test/ui/async-await/issues/issue-61187.rs similarity index 100% rename from src/test/ui/issues/issue-61187.rs rename to src/test/ui/async-await/issues/issue-61187.rs diff --git a/src/test/ui/issues/issue-61187.stderr b/src/test/ui/async-await/issues/issue-61187.stderr similarity index 100% rename from src/test/ui/issues/issue-61187.stderr rename to src/test/ui/async-await/issues/issue-61187.stderr From 24b2e20b3127b5a8da20c4910080bf9756dd2a45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 26 May 2019 11:39:48 -0700 Subject: [PATCH 4/8] Account for short-hand init structs when suggesting conversion --- src/librustc_typeck/check/demand.rs | 6 ++++- src/librustc_typeck/check/mod.rs | 20 +++++++++++----- src/test/ui/suggestions/issue-52820.rs | 12 ++++++++++ src/test/ui/suggestions/issue-52820.stderr | 27 ++++++++++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/suggestions/issue-52820.rs create mode 100644 src/test/ui/suggestions/issue-52820.stderr diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index a4e687b8f9080..bc872f8c03c0e 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -270,7 +270,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { None } - fn is_hir_id_from_struct_pattern_shorthand_field(&self, hir_id: hir::HirId, sp: Span) -> bool { + crate fn is_hir_id_from_struct_pattern_shorthand_field( + &self, + hir_id: hir::HirId, + sp: Span, + ) -> bool { let cm = self.sess().source_map(); let parent_id = self.tcx.hir().get_parent_node_by_hir_id(hir_id); if let Some(parent) = self.tcx.hir().find_by_hir_id(parent_id) { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 655bf5722ae5a..e6c9448a6057c 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5014,6 +5014,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { Applicability::MachineApplicable, ); } else if !self.check_for_cast(err, expr, found, expected) { + let is_struct_pat_shorthand_field = self.is_hir_id_from_struct_pattern_shorthand_field( + expr.hir_id, + expr.span, + ); let methods = self.get_conversion_methods(expr.span, expected, found); if let Ok(expr_text) = self.sess().source_map().span_to_snippet(expr.span) { let mut suggestions = iter::repeat(&expr_text).zip(methods.iter()) @@ -5023,14 +5027,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { None // do not suggest code that is already there (#53348) } else { let method_call_list = [".to_vec()", ".to_string()"]; - if receiver.ends_with(".clone()") + let sugg = if receiver.ends_with(".clone()") && method_call_list.contains(&method_call.as_str()) { let max_len = receiver.rfind(".").unwrap(); - Some(format!("{}{}", &receiver[..max_len], method_call)) - } - else { - Some(format!("{}{}", receiver, method_call)) - } + format!("{}{}", &receiver[..max_len], method_call) + } else { + format!("{}{}", receiver, method_call) + }; + Some(if is_struct_pat_shorthand_field { + format!("{}: {}", receiver, sugg) + } else { + sugg + }) } }).peekable(); if suggestions.peek().is_some() { diff --git a/src/test/ui/suggestions/issue-52820.rs b/src/test/ui/suggestions/issue-52820.rs new file mode 100644 index 0000000000000..075b07f565203 --- /dev/null +++ b/src/test/ui/suggestions/issue-52820.rs @@ -0,0 +1,12 @@ +struct Bravery { + guts: String, + brains: String, +} + +fn main() { + let guts = "mettle"; + let _ = Bravery { + guts, //~ ERROR mismatched types + brains: guts.clone(), //~ ERROR mismatched types + }; +} diff --git a/src/test/ui/suggestions/issue-52820.stderr b/src/test/ui/suggestions/issue-52820.stderr new file mode 100644 index 0000000000000..fb568aca250e7 --- /dev/null +++ b/src/test/ui/suggestions/issue-52820.stderr @@ -0,0 +1,27 @@ +error[E0308]: mismatched types + --> $DIR/issue-52820.rs:9:9 + | +LL | guts, + | ^^^^ + | | + | expected struct `std::string::String`, found &str + | help: try using a conversion method: `guts: guts.to_string()` + | + = note: expected type `std::string::String` + found type `&str` + +error[E0308]: mismatched types + --> $DIR/issue-52820.rs:10:17 + | +LL | brains: guts.clone(), + | ^^^^^^^^^^^^ + | | + | expected struct `std::string::String`, found &str + | help: try using a conversion method: `guts.to_string()` + | + = note: expected type `std::string::String` + found type `&str` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 06eb412741894f3efe4298f9c7993d0cd49bf577 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Mon, 27 May 2019 13:42:21 +0000 Subject: [PATCH 5/8] Stabilize bufreader_buffer feature --- src/libstd/io/buffered.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libstd/io/buffered.rs b/src/libstd/io/buffered.rs index e309f81192cf3..88186c50097ec 100644 --- a/src/libstd/io/buffered.rs +++ b/src/libstd/io/buffered.rs @@ -158,7 +158,6 @@ impl BufReader { /// # Examples /// /// ```no_run - /// # #![feature(bufreader_buffer)] /// use std::io::{BufReader, BufRead}; /// use std::fs::File; /// @@ -173,7 +172,7 @@ impl BufReader { /// Ok(()) /// } /// ``` - #[unstable(feature = "bufreader_buffer", issue = "45323")] + #[stable(feature = "bufreader_buffer", since = "1.37.0")] pub fn buffer(&self) -> &[u8] { &self.buf[self.pos..self.cap] } @@ -552,7 +551,6 @@ impl BufWriter { /// # Examples /// /// ```no_run - /// # #![feature(bufreader_buffer)] /// use std::io::BufWriter; /// use std::net::TcpStream; /// @@ -561,7 +559,7 @@ impl BufWriter { /// // See how many bytes are currently buffered /// let bytes_buffered = buf_writer.buffer().len(); /// ``` - #[unstable(feature = "bufreader_buffer", issue = "45323")] + #[stable(feature = "bufreader_buffer", since = "1.37.0")] pub fn buffer(&self) -> &[u8] { &self.buf } From 1b86bd73cd5e8e463f50e5c53968125d0ab4e1f0 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 28 May 2019 15:18:37 +0200 Subject: [PATCH 6/8] is_union returns ty to avoid computing it twice --- .../borrow_check/conflict_errors.rs | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index 8022d1f0c7315..58adc24b80987 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -595,11 +595,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ) -> (String, String, String, String) { // Define a small closure that we can use to check if the type of a place // is a union. - let is_union = |place: &Place<'tcx>| -> bool { - place.ty(self.mir, self.infcx.tcx).ty - .ty_adt_def() - .map(|adt| adt.is_union()) - .unwrap_or(false) + let union_ty = |place: &Place<'tcx>| -> Option> { + let ty = place.ty(self.mir, self.infcx.tcx).ty; + ty.ty_adt_def().filter(|adt| adt.is_union()).map(|_| ty) }; // Start with an empty tuple, so we can use the functions on `Option` to reduce some @@ -619,7 +617,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let mut current = first_borrowed_place; while let Place::Projection(box Projection { base, elem }) = current { match elem { - ProjectionElem::Field(field, _) if is_union(base) => { + ProjectionElem::Field(field, _) if union_ty(base).is_some() => { return Some((base, field)); }, _ => current = base, @@ -632,25 +630,29 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // borrowed place and look for a access to a different field of the same union. let mut current = second_borrowed_place; while let Place::Projection(box Projection { base, elem }) = current { - match elem { - ProjectionElem::Field(field, _) if { - is_union(base) && field != target_field && base == target_base - } => { - let desc_base = self.describe_place(base) - .unwrap_or_else(|| "_".to_owned()); - let desc_first = self.describe_place(first_borrowed_place) - .unwrap_or_else(|| "_".to_owned()); - let desc_second = self.describe_place(second_borrowed_place) - .unwrap_or_else(|| "_".to_owned()); - - // Also compute the name of the union type, eg. `Foo` so we - // can add a helpful note with it. - let ty = base.ty(self.mir, self.infcx.tcx).ty; - - return Some((desc_base, desc_first, desc_second, ty.to_string())); - }, - _ => current = base, + if let ProjectionElem::Field(field, _) = elem { + if let Some(union_ty) = union_ty(base) { + if field != target_field && base == target_base { + let desc_base = + self.describe_place(base).unwrap_or_else(|| "_".to_owned()); + let desc_first = self + .describe_place(first_borrowed_place) + .unwrap_or_else(|| "_".to_owned()); + let desc_second = self + .describe_place(second_borrowed_place) + .unwrap_or_else(|| "_".to_owned()); + + return Some(( + desc_base, + desc_first, + desc_second, + union_ty.to_string(), + )); + } + } } + + current = base; } None }) From bb94fc00695be648f62751e8f393e11644d938ae Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 28 May 2019 16:47:59 +0200 Subject: [PATCH 7/8] Use closure to avoid self.describe_place(...).unwrap_or_else(...) repetition --- .../borrow_check/conflict_errors.rs | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index 58adc24b80987..5c5d495466cda 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -599,6 +599,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { let ty = place.ty(self.mir, self.infcx.tcx).ty; ty.ty_adt_def().filter(|adt| adt.is_union()).map(|_| ty) }; + let describe_place = |place| self.describe_place(place).unwrap_or_else(|| "_".to_owned()); // Start with an empty tuple, so we can use the functions on `Option` to reduce some // code duplication (particularly around returning an empty description in the failure @@ -633,19 +634,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { if let ProjectionElem::Field(field, _) = elem { if let Some(union_ty) = union_ty(base) { if field != target_field && base == target_base { - let desc_base = - self.describe_place(base).unwrap_or_else(|| "_".to_owned()); - let desc_first = self - .describe_place(first_borrowed_place) - .unwrap_or_else(|| "_".to_owned()); - let desc_second = self - .describe_place(second_borrowed_place) - .unwrap_or_else(|| "_".to_owned()); - return Some(( - desc_base, - desc_first, - desc_second, + describe_place(base), + describe_place(first_borrowed_place), + describe_place(second_borrowed_place), union_ty.to_string(), )); } @@ -659,9 +651,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { .unwrap_or_else(|| { // If we didn't find a field access into a union, or both places match, then // only return the description of the first place. - let desc_place = self.describe_place(first_borrowed_place) - .unwrap_or_else(|| "_".to_owned()); - (desc_place, "".to_string(), "".to_string(), "".to_string()) + ( + describe_place(first_borrowed_place), + "".to_string(), + "".to_string(), + "".to_string(), + ) }) } From d72f97dfa90db5757d58689180939d574c5ab259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 28 May 2019 13:33:57 -0700 Subject: [PATCH 8/8] review comment: tweak error wording --- src/librustc_mir/borrow_check/mutability_errors.rs | 2 +- src/test/ui/async-await/issues/issue-61187.stderr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index e90795c964691..9bb359a8911e4 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -49,7 +49,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { (Some(desc), _) => format!("`{}`", desc), (None, Place::Base(PlaceBase::Local(local))) if self.mir.local_decls[*local] .source_info.span.is_compiler_desugaring(CompilerDesugaringKind::Async) - => "async `fn` parameter".to_string(), + => "`async fn` parameter".to_string(), (None, _) => "temporary place".to_string(), }; match the_place_err { diff --git a/src/test/ui/async-await/issues/issue-61187.stderr b/src/test/ui/async-await/issues/issue-61187.stderr index 6f6f32a9ca781..52fe15e8cf7c7 100644 --- a/src/test/ui/async-await/issues/issue-61187.stderr +++ b/src/test/ui/async-await/issues/issue-61187.stderr @@ -1,4 +1,4 @@ -error[E0596]: cannot borrow async `fn` parameter as mutable, as it is not declared as mutable +error[E0596]: cannot borrow `async fn` parameter as mutable, as it is not declared as mutable --> $DIR/issue-61187.rs:8:5 | LL | async fn response(data: Vec) {