From 1b86bd73cd5e8e463f50e5c53968125d0ab4e1f0 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 28 May 2019 15:18:37 +0200 Subject: [PATCH 1/2] 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 2/2] 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(), + ) }) }