From f13b8fb686ca5ae178de211cf8ae2dcc1f134526 Mon Sep 17 00:00:00 2001 From: Janusz Marcinkiewicz Date: Sat, 30 Nov 2019 18:59:51 +0100 Subject: [PATCH 01/27] Suggest calling method when first argument is `self` --- src/doc/rustc-guide | 2 +- src/librustc_resolve/build_reduced_graph.rs | 10 ++--- src/librustc_resolve/late.rs | 2 +- src/librustc_resolve/late/diagnostics.rs | 45 +++++++++++++++++---- src/test/ui/self/suggest-self-2.rs | 22 ++++++++++ src/test/ui/self/suggest-self-2.stderr | 28 +++++++++++++ 6 files changed, 95 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/self/suggest-self-2.rs create mode 100644 src/test/ui/self/suggest-self-2.stderr diff --git a/src/doc/rustc-guide b/src/doc/rustc-guide index 7c56708aab798..934380b7cfcea 160000 --- a/src/doc/rustc-guide +++ b/src/doc/rustc-guide @@ -1 +1 @@ -Subproject commit 7c56708aab7986ca390221e8e8902f7de7f9b076 +Subproject commit 934380b7cfceaaa4e1b9bb0de4a372f32725520b diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 566ba12907437..55b20b31ad8d4 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -719,8 +719,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // These items live in both the type and value namespaces. ItemKind::Struct(ref vdata, _) => { // Define a name in the type namespace. - let def_id = self.r.definitions.local_def_id(item.id); - let res = Res::Def(DefKind::Struct, def_id); + let item_def_id = self.r.definitions.local_def_id(item.id); + let res = Res::Def(DefKind::Struct, item_def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); // Record field names for error reporting. @@ -757,12 +757,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } ItemKind::Union(ref vdata, _) => { - let def_id = self.r.definitions.local_def_id(item.id); - let res = Res::Def(DefKind::Union, def_id); + let item_def_id = self.r.definitions.local_def_id(item.id); + let res = Res::Def(DefKind::Union, item_def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); // Record field names for error reporting. - self.insert_field_names_local(def_id, vdata); + self.insert_field_names_local(item_def_id, vdata); } ItemKind::Impl(.., ref impl_items) => { diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs index 4f95d6fe70f17..064000438607d 100644 --- a/src/librustc_resolve/late.rs +++ b/src/librustc_resolve/late.rs @@ -345,7 +345,7 @@ struct DiagnosticMetadata { /// The current self item if inside an ADT (used for better errors). current_self_item: Option, - /// The current enclosing funciton (used for better errors). + /// The current enclosing function (used for better errors). current_function: Option, /// A list of labels as of yet unused. Labels will be removed from this map when diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 3be033697d286..fd0f6b01d4ce7 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -253,6 +253,37 @@ impl<'a> LateResolutionVisitor<'a, '_> { } return (err, candidates); } + + // Check if the first argument is `self` and suggest calling a method. + let mut has_self_arg = false; + if let PathSource::Expr(parent) = source { + match &parent.map(|p| &p.kind) { + Some(ExprKind::Call(_, args)) if args.len() > 0 => { + let mut expr_kind = &args.first().unwrap().kind; + loop { + match expr_kind { + ExprKind::Path(_, arg_name) if arg_name.segments.len() == 1 => { + has_self_arg = arg_name.segments[0].ident.name == kw::SelfLower; + break; + }, + ExprKind::AddrOf(_, _, expr) => { expr_kind = &expr.kind; } + _ => break, + } + } + } + _ => (), + } + }; + + if has_self_arg { + err.span_suggestion( + span, + &"try calling method instead of passing `self` as parameter", + format!("self.{}", path_str), + Applicability::MachineApplicable, + ); + return (err, candidates); + } } // Try Levenshtein algorithm. @@ -553,13 +584,13 @@ impl<'a> LateResolutionVisitor<'a, '_> { // Look for associated items in the current trait. if let Some((module, _)) = self.current_trait_ref { if let Ok(binding) = self.r.resolve_ident_in_module( - ModuleOrUniformRoot::Module(module), - ident, - ns, - &self.parent_scope, - false, - module.span, - ) { + ModuleOrUniformRoot::Module(module), + ident, + ns, + &self.parent_scope, + false, + module.span, + ) { let res = binding.res(); if filter_fn(res) { return Some(if self.r.has_self.contains(&res.def_id()) { diff --git a/src/test/ui/self/suggest-self-2.rs b/src/test/ui/self/suggest-self-2.rs new file mode 100644 index 0000000000000..1926ebe4b8317 --- /dev/null +++ b/src/test/ui/self/suggest-self-2.rs @@ -0,0 +1,22 @@ +struct Foo {} + +impl Foo { + fn foo(&self) { + bar(self); + //~^ ERROR cannot find function `bar` in this scope + //~| HELP try calling method instead of passing `self` as parameter + + + bar(&self); + //~^ ERROR cannot find function `bar` in this scope + //~| HELP try calling method instead of passing `self` as parameter + + bar(); + //~^ ERROR cannot find function `bar` in this scope + + self.bar(); + //~^ ERROR no method named `bar` found for type + } +} + +fn main() {} diff --git a/src/test/ui/self/suggest-self-2.stderr b/src/test/ui/self/suggest-self-2.stderr new file mode 100644 index 0000000000000..84dbaa9637898 --- /dev/null +++ b/src/test/ui/self/suggest-self-2.stderr @@ -0,0 +1,28 @@ +error[E0425]: cannot find function `bar` in this scope + --> $DIR/suggest-self-2.rs:5:9 + | +LL | bar(self); + | ^^^ help: try calling method instead of passing `self` as parameter: `self.bar` + +error[E0425]: cannot find function `bar` in this scope + --> $DIR/suggest-self-2.rs:10:9 + | +LL | bar(&self); + | ^^^ help: try calling method instead of passing `self` as parameter: `self.bar` + +error[E0425]: cannot find function `bar` in this scope + --> $DIR/suggest-self-2.rs:14:9 + | +LL | bar(); + | ^^^ not found in this scope + +error[E0599]: no method named `bar` found for type `&Foo` in the current scope + --> $DIR/suggest-self-2.rs:17:14 + | +LL | self.bar(); + | ^^^ method not found in `&Foo` + +error: aborting due to 4 previous errors + +Some errors have detailed explanations: E0425, E0599. +For more information about an error, try `rustc --explain E0425`. From 4b6305cfa49a7d9e764baacc640d519383cac8c2 Mon Sep 17 00:00:00 2001 From: Janusz Marcinkiewicz Date: Mon, 9 Dec 2019 16:05:45 +0100 Subject: [PATCH 02/27] Add more detailed suggestion --- src/doc/rustc-guide | 2 +- src/librustc_resolve/build_reduced_graph.rs | 10 +++++----- src/librustc_resolve/late/diagnostics.rs | 6 +++--- src/test/ui/self/suggest-self-2.rs | 9 ++++++--- src/test/ui/self/suggest-self-2.stderr | 20 +++++++++++++------- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/src/doc/rustc-guide b/src/doc/rustc-guide index 934380b7cfcea..7c56708aab798 160000 --- a/src/doc/rustc-guide +++ b/src/doc/rustc-guide @@ -1 +1 @@ -Subproject commit 934380b7cfceaaa4e1b9bb0de4a372f32725520b +Subproject commit 7c56708aab7986ca390221e8e8902f7de7f9b076 diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 55b20b31ad8d4..566ba12907437 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -719,8 +719,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // These items live in both the type and value namespaces. ItemKind::Struct(ref vdata, _) => { // Define a name in the type namespace. - let item_def_id = self.r.definitions.local_def_id(item.id); - let res = Res::Def(DefKind::Struct, item_def_id); + let def_id = self.r.definitions.local_def_id(item.id); + let res = Res::Def(DefKind::Struct, def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); // Record field names for error reporting. @@ -757,12 +757,12 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { } ItemKind::Union(ref vdata, _) => { - let item_def_id = self.r.definitions.local_def_id(item.id); - let res = Res::Def(DefKind::Union, item_def_id); + let def_id = self.r.definitions.local_def_id(item.id); + let res = Res::Def(DefKind::Union, def_id); self.r.define(parent, ident, TypeNS, (res, vis, sp, expansion)); // Record field names for error reporting. - self.insert_field_names_local(item_def_id, vdata); + self.insert_field_names_local(def_id, vdata); } ItemKind::Impl(.., ref impl_items) => { diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index fd0f6b01d4ce7..1df8ff02fcebe 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -259,14 +259,14 @@ impl<'a> LateResolutionVisitor<'a, '_> { if let PathSource::Expr(parent) = source { match &parent.map(|p| &p.kind) { Some(ExprKind::Call(_, args)) if args.len() > 0 => { - let mut expr_kind = &args.first().unwrap().kind; + let mut expr_kind = &args[0].kind; loop { match expr_kind { ExprKind::Path(_, arg_name) if arg_name.segments.len() == 1 => { has_self_arg = arg_name.segments[0].ident.name == kw::SelfLower; break; }, - ExprKind::AddrOf(_, _, expr) => { expr_kind = &expr.kind; } + ExprKind::AddrOf(_, _, expr) => expr_kind = &expr.kind, _ => break, } } @@ -278,7 +278,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { if has_self_arg { err.span_suggestion( span, - &"try calling method instead of passing `self` as parameter", + &format!("try calling `{}` as a method", ident), format!("self.{}", path_str), Applicability::MachineApplicable, ); diff --git a/src/test/ui/self/suggest-self-2.rs b/src/test/ui/self/suggest-self-2.rs index 1926ebe4b8317..d6bf543352701 100644 --- a/src/test/ui/self/suggest-self-2.rs +++ b/src/test/ui/self/suggest-self-2.rs @@ -4,12 +4,15 @@ impl Foo { fn foo(&self) { bar(self); //~^ ERROR cannot find function `bar` in this scope - //~| HELP try calling method instead of passing `self` as parameter + //~| HELP try calling `bar` as a method + bar(&&self, 102); + //~^ ERROR cannot find function `bar` in this scope + //~| HELP try calling `bar` as a method - bar(&self); + bar(&mut self, 102, &"str"); //~^ ERROR cannot find function `bar` in this scope - //~| HELP try calling method instead of passing `self` as parameter + //~| HELP try calling `bar` as a method bar(); //~^ ERROR cannot find function `bar` in this scope diff --git a/src/test/ui/self/suggest-self-2.stderr b/src/test/ui/self/suggest-self-2.stderr index 84dbaa9637898..ba71498fae656 100644 --- a/src/test/ui/self/suggest-self-2.stderr +++ b/src/test/ui/self/suggest-self-2.stderr @@ -2,27 +2,33 @@ error[E0425]: cannot find function `bar` in this scope --> $DIR/suggest-self-2.rs:5:9 | LL | bar(self); - | ^^^ help: try calling method instead of passing `self` as parameter: `self.bar` + | ^^^ help: try calling `bar` as a method: `self.bar` error[E0425]: cannot find function `bar` in this scope - --> $DIR/suggest-self-2.rs:10:9 + --> $DIR/suggest-self-2.rs:9:9 | -LL | bar(&self); - | ^^^ help: try calling method instead of passing `self` as parameter: `self.bar` +LL | bar(&&self, 102); + | ^^^ help: try calling `bar` as a method: `self.bar` error[E0425]: cannot find function `bar` in this scope - --> $DIR/suggest-self-2.rs:14:9 + --> $DIR/suggest-self-2.rs:13:9 + | +LL | bar(&mut self, 102, &"str"); + | ^^^ help: try calling `bar` as a method: `self.bar` + +error[E0425]: cannot find function `bar` in this scope + --> $DIR/suggest-self-2.rs:17:9 | LL | bar(); | ^^^ not found in this scope error[E0599]: no method named `bar` found for type `&Foo` in the current scope - --> $DIR/suggest-self-2.rs:17:14 + --> $DIR/suggest-self-2.rs:20:14 | LL | self.bar(); | ^^^ method not found in `&Foo` -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors Some errors have detailed explanations: E0425, E0599. For more information about an error, try `rustc --explain E0425`. From 6bac6939e804c55464eaed7cfa047adb4674c267 Mon Sep 17 00:00:00 2001 From: Christopher Durham Date: Sat, 9 Nov 2019 13:43:11 -0500 Subject: [PATCH 03/27] Make Layout::new const --- src/libcore/alloc.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libcore/alloc.rs b/src/libcore/alloc.rs index 71517ffb0065e..b049a446cdcd0 100644 --- a/src/libcore/alloc.rs +++ b/src/libcore/alloc.rs @@ -17,7 +17,7 @@ use crate::num::NonZeroUsize; #[derive(Debug)] pub struct Excess(pub NonNull, pub usize); -fn size_align() -> (usize, usize) { +const fn size_align() -> (usize, usize) { (mem::size_of::(), mem::align_of::()) } @@ -119,13 +119,12 @@ impl Layout { /// Constructs a `Layout` suitable for holding a value of type `T`. #[stable(feature = "alloc_layout", since = "1.28.0")] #[inline] - pub fn new() -> Self { + pub const fn new() -> Self { let (size, align) = size_align::(); // Note that the align is guaranteed by rustc to be a power of two and // the size+align combo is guaranteed to fit in our address space. As a // result use the unchecked constructor here to avoid inserting code // that panics if it isn't optimized well enough. - debug_assert!(Layout::from_size_align(size, align).is_ok()); unsafe { Layout::from_size_align_unchecked(size, align) } From 878d3b3171d98f5c68c826d99d30986d60f4f7bb Mon Sep 17 00:00:00 2001 From: CAD97 Date: Tue, 17 Dec 2019 14:55:46 -0500 Subject: [PATCH 04/27] Mark Layout::new as const stable --- src/libcore/alloc.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libcore/alloc.rs b/src/libcore/alloc.rs index b049a446cdcd0..ceb4a73db61cd 100644 --- a/src/libcore/alloc.rs +++ b/src/libcore/alloc.rs @@ -118,6 +118,7 @@ impl Layout { /// Constructs a `Layout` suitable for holding a value of type `T`. #[stable(feature = "alloc_layout", since = "1.28.0")] + #[rustc_const_stable(feature = "alloc_layout_const_new", since = "1.42.0")] #[inline] pub const fn new() -> Self { let (size, align) = size_align::(); From 43888e8c7c39588bfeb4ae1e0a45774932a9f934 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 11 Dec 2019 16:40:49 -0600 Subject: [PATCH 05/27] Refactor region error handling to be done by mirborrowckctx --- .../borrow_check/diagnostics/mod.rs | 2 +- .../borrow_check/diagnostics/region_errors.rs | 69 +++++- src/librustc_mir/borrow_check/mod.rs | 121 ++++++++++- src/librustc_mir/borrow_check/nll.rs | 65 +++--- .../borrow_check/region_infer/mod.rs | 197 ++++-------------- 5 files changed, 253 insertions(+), 201 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/mod.rs b/src/librustc_mir/borrow_check/diagnostics/mod.rs index 1a76265fbdcf0..ede0c2084097c 100644 --- a/src/librustc_mir/borrow_check/diagnostics/mod.rs +++ b/src/librustc_mir/borrow_check/diagnostics/mod.rs @@ -32,7 +32,7 @@ mod explain_borrow; crate use mutability_errors::AccessKind; crate use region_name::{RegionName, RegionNameSource, RegionErrorNamingCtx}; -crate use region_errors::{ErrorReportingCtx, ErrorConstraintInfo}; +crate use region_errors::{ErrorReportingCtx, ErrorConstraintInfo, RegionErrors, RegionErrorKind}; crate use outlives_suggestion::OutlivesSuggestionBuilder; pub(super) struct IncludingDowncast(pub(super) bool); diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index b78cd6bccf8ca..1cb6643e7d091 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -3,12 +3,13 @@ use rustc::hir::def_id::DefId; use rustc::infer::{ error_reporting::nice_region_error::NiceRegionError, + region_constraints::GenericKind, InferCtxt, NLLRegionVariableOrigin, }; use rustc::mir::{ ConstraintCategory, Local, Location, Body, }; -use rustc::ty::{self, RegionVid}; +use rustc::ty::{self, RegionVid, Ty}; use rustc_index::vec::IndexVec; use rustc_errors::DiagnosticBuilder; use std::collections::VecDeque; @@ -53,13 +54,61 @@ impl ConstraintDescription for ConstraintCategory { } } -#[derive(Copy, Clone, PartialEq, Eq)] +#[derive(Copy, Clone, PartialEq, Eq, Debug)] enum Trace { StartRegion, FromOutlivesConstraint(OutlivesConstraint), NotVisited, } +/// A collection of errors encountered during region inference. This is needed to efficiently +/// report errors after borrow checking. +#[derive(Clone, Debug)] +crate struct RegionErrors<'tcx> { + errors: smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>, +} + +#[derive(Clone, Debug)] +crate enum RegionErrorKind<'tcx> { + /// An error for a type test: `T: 'a` does not live long enough + TypeTestDoesNotLiveLongEnough { + /// The span of the type test + span: Span, + /// The generic type of the type test + generic: GenericKind<'tcx>, + }, + + /// A generic bound failure for a type test + TypeTestGenericBoundError { + /// The span of the type test + span: Span, + /// The generic type of the type test + generic: GenericKind<'tcx>, + /// The lower bound region + lower_bound_region: ty::Region<'tcx>, + }, + + /// An unexpected hidden region for an opaque type + UnexpectedHiddenRegion { + /// The def id of the opaque type + opaque_type_def_id: DefId, + /// The hidden type + hidden_ty: Ty<'tcx>, + /// The unexpected region + member_region: ty::Region<'tcx>, + }, + + /// Any other lifetime error + RegionError { + /// The origin of the region + fr_origin: NLLRegionVariableOrigin, + /// The region that should outlive `shorter_fr` + longer_fr: RegionVid, + /// The region that should be shorter, but we can't prove it + shorter_fr: RegionVid, + }, +} + /// Various pieces of state used when reporting borrow checker errors. pub struct ErrorReportingCtx<'a, 'b, 'tcx> { /// The region inference context used for borrow chekcing this MIR body. @@ -95,6 +144,22 @@ pub struct ErrorConstraintInfo { pub(super) span: Span, } +impl<'tcx> RegionErrors<'tcx> { + pub fn new() -> Self { + RegionErrors { + errors: smallvec::SmallVec::new(), + } + } + + pub fn push(&mut self, error: RegionErrorKind<'tcx>) { + self.errors.push(error) + } + + pub fn into_iter(self) -> impl Iterator> { + self.errors.into_iter() + } +} + impl<'tcx> RegionInferenceContext<'tcx> { /// Converts a region inference variable into a `ty::Region` that /// we can use for error reporting. If `r` is universally bound, diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 2554d5e729da9..1f45da083bce2 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -3,7 +3,7 @@ use rustc::hir::{self, HirId}; use rustc::hir::Node; use rustc::hir::def_id::DefId; -use rustc::infer::InferCtxt; +use rustc::infer::{opaque_types, InferCtxt}; use rustc::lint::builtin::UNUSED_MUT; use rustc::lint::builtin::{MUTABLE_BORROW_RESERVATION_CONFLICT}; use rustc::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind}; @@ -39,12 +39,15 @@ use crate::dataflow::MoveDataParamEnv; use crate::dataflow::{do_dataflow, DebugFormatted}; use crate::dataflow::EverInitializedPlaces; use crate::dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces}; +use crate::transform::MirSource; use self::flows::Flows; use self::location::LocationTable; use self::prefixes::PrefixSet; use self::MutateMode::{JustWrite, WriteAndRead}; -use self::diagnostics::AccessKind; +use self::diagnostics::{ + AccessKind, RegionErrors, RegionErrorKind, OutlivesSuggestionBuilder, RegionErrorNamingCtx, +}; use self::path_utils::*; @@ -211,20 +214,40 @@ fn do_mir_borrowck<'a, 'tcx>( let borrow_set = Rc::new(BorrowSet::build( tcx, body, locals_are_invalidated_at_exit, &mdpe.move_data)); - // If we are in non-lexical mode, compute the non-lexical lifetimes. - let (regioncx, polonius_output, opt_closure_req) = nll::compute_regions( + // Compute non-lexical lifetimes. + let nll::NllOutput { + regioncx, polonius_output, opt_closure_req, nll_errors + } = nll::compute_regions( infcx, def_id, free_regions, body, &promoted, - &local_names, - &upvars, location_table, param_env, &mut flow_inits, &mdpe.move_data, &borrow_set, + ); + + // Dump MIR results into a file, if that is enabled. This let us + // write unit-tests, as well as helping with debugging. + nll::dump_mir_results( + infcx, + MirSource::item(def_id), + &body, + ®ioncx, + &opt_closure_req, + ); + + // We also have a `#[rustc_nll]` annotation that causes us to dump + // information. + nll::dump_annotation( + infcx, + &body, + def_id, + ®ioncx, + &opt_closure_req, &mut errors_buffer, ); @@ -297,6 +320,9 @@ fn do_mir_borrowck<'a, 'tcx>( local_names, }; + // Compute and report region errors, if any. + mbcx.report_region_errors(nll_errors); + let mut state = Flows::new( flow_borrows, flow_uninits, @@ -1560,6 +1586,89 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // initial reservation. } } + + /// Produces nice borrowck error diagnostics for all the errors collected in `nll_errors`. + fn report_region_errors(&mut self, nll_errors: RegionErrors<'tcx>) { + // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are + // buffered in the `MirBorrowckCtxt`. + + // TODO(mark-i-m): Would be great to get rid of the naming context. + let mut region_naming = RegionErrorNamingCtx::new(); + let mut outlives_suggestion = OutlivesSuggestionBuilder::new(self.mir_def_id, &self.local_names); + + for nll_error in nll_errors.into_iter() { + match nll_error { + RegionErrorKind::TypeTestDoesNotLiveLongEnough { span, generic } => { + // FIXME. We should handle this case better. It + // indicates that we have e.g., some region variable + // whose value is like `'a+'b` where `'a` and `'b` are + // distinct unrelated univesal regions that are not + // known to outlive one another. It'd be nice to have + // some examples where this arises to decide how best + // to report it; we could probably handle it by + // iterating over the universal regions and reporting + // an error that multiple bounds are required. + self.infcx.tcx.sess + .struct_span_err( + span, + &format!("`{}` does not live long enough", generic), + ) + .buffer(&mut self.errors_buffer); + }, + + RegionErrorKind::TypeTestGenericBoundError { + span, generic, lower_bound_region + } => { + let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); + self.infcx + .construct_generic_bound_failure( + region_scope_tree, + span, + None, + generic, + lower_bound_region, + ) + .buffer(&mut self.errors_buffer); + }, + + RegionErrorKind::UnexpectedHiddenRegion { + opaque_type_def_id, hidden_ty, member_region, + } => { + let region_scope_tree = &self.infcx.tcx.region_scope_tree(self.mir_def_id); + opaque_types::unexpected_hidden_region_diagnostic( + self.infcx.tcx, + Some(region_scope_tree), + opaque_type_def_id, + hidden_ty, + member_region, + ) + .buffer(&mut self.errors_buffer); + } + + RegionErrorKind::RegionError { + fr_origin, longer_fr, shorter_fr, + } => { + let db = self.nonlexical_regioncx.report_error( + &self.body, + &self.local_names, + &self.upvars, + self.infcx, + self.mir_def_id, + longer_fr, + fr_origin, + shorter_fr, + &mut outlives_suggestion, + &mut region_naming, + ); + + db.buffer(&mut self.errors_buffer); + + // Emit outlives suggestions + //outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); + } + } + } + } } impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { diff --git a/src/librustc_mir/borrow_check/nll.rs b/src/librustc_mir/borrow_check/nll.rs index 6d28a8caa92bd..2bbf6c9c84799 100644 --- a/src/librustc_mir/borrow_check/nll.rs +++ b/src/librustc_mir/borrow_check/nll.rs @@ -3,12 +3,11 @@ use rustc::hir::def_id::DefId; use rustc::infer::InferCtxt; use rustc::mir::{ClosureOutlivesSubject, ClosureRegionRequirements, - Local, Location, Body, BodyAndCache, LocalKind, BasicBlock, + Location, Body, BodyAndCache, LocalKind, BasicBlock, Promoted, ReadOnlyBodyAndCache}; use rustc::ty::{self, RegionKind, RegionVid}; use rustc_index::vec::IndexVec; use rustc_errors::Diagnostic; -use syntax_pos::symbol::Symbol; use std::fmt::Debug; use std::env; use std::io; @@ -29,19 +28,28 @@ use crate::transform::MirSource; use crate::borrow_check::{ borrow_set::BorrowSet, + diagnostics::RegionErrors, location::LocationTable, facts::{AllFacts, AllFactsExt, RustcFacts}, region_infer::{RegionInferenceContext, values::RegionValueElements}, universal_regions::UniversalRegions, type_check::{self, MirTypeckResults, MirTypeckRegionConstraints}, - Upvar, renumber, constraint_generation, invalidation, + renumber, constraint_generation, invalidation, }; crate type PoloniusOutput = Output; -/// Rewrites the regions in the MIR to use NLL variables, also -/// scraping out the set of universal regions (e.g., region parameters) -/// declared on the function. That set will need to be given to +/// The output of `nll::compute_regions`. This includes the computed `RegionInferenceContext`, any +/// closure requirements to propagate, and any generated errors. +crate struct NllOutput<'tcx> { + pub regioncx: RegionInferenceContext<'tcx>, + pub polonius_output: Option>, + pub opt_closure_req: Option>, + pub nll_errors: RegionErrors<'tcx>, +} + +/// Rewrites the regions in the MIR to use NLL variables, also scraping out the set of universal +/// regions (e.g., region parameters) declared on the function. That set will need to be given to /// `compute_regions`. pub(in crate::borrow_check) fn replace_regions_in_mir<'cx, 'tcx>( infcx: &InferCtxt<'cx, 'tcx>, @@ -152,19 +160,12 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( universal_regions: UniversalRegions<'tcx>, body: ReadOnlyBodyAndCache<'_, 'tcx>, promoted: &IndexVec>, - local_names: &IndexVec>, - upvars: &[Upvar], location_table: &LocationTable, param_env: ty::ParamEnv<'tcx>, flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'cx, 'tcx>>, move_data: &MoveData<'tcx>, borrow_set: &BorrowSet<'tcx>, - errors_buffer: &mut Vec, -) -> ( - RegionInferenceContext<'tcx>, - Option>, - Option>, -) { +) -> NllOutput<'tcx> { let mut all_facts = AllFacts::enabled(infcx.tcx).then_some(AllFacts::default()); let universal_regions = Rc::new(universal_regions); @@ -306,40 +307,22 @@ pub(in crate::borrow_check) fn compute_regions<'cx, 'tcx>( }); // Solve the region constraints. - let closure_region_requirements = regioncx.solve( + let (closure_region_requirements, nll_errors) = regioncx.solve( infcx, &body, - local_names, - upvars, def_id, - errors_buffer, polonius_output.clone(), ); - // Dump MIR results into a file, if that is enabled. This let us - // write unit-tests, as well as helping with debugging. - dump_mir_results( - infcx, - MirSource::item(def_id), - &body, - ®ioncx, - &closure_region_requirements, - ); - - // We also have a `#[rustc_nll]` annotation that causes us to dump - // information - dump_annotation( - infcx, - &body, - def_id, - ®ioncx, - &closure_region_requirements, - errors_buffer); - - (regioncx, polonius_output, closure_region_requirements) + NllOutput { + regioncx, + polonius_output, + opt_closure_req: closure_region_requirements, + nll_errors, + } } -fn dump_mir_results<'a, 'tcx>( +pub(super) fn dump_mir_results<'a, 'tcx>( infcx: &InferCtxt<'a, 'tcx>, source: MirSource<'tcx>, body: &Body<'tcx>, @@ -400,7 +383,7 @@ fn dump_mir_results<'a, 'tcx>( }; } -fn dump_annotation<'a, 'tcx>( +pub(super) fn dump_annotation<'a, 'tcx>( infcx: &InferCtxt<'a, 'tcx>, body: &Body<'tcx>, mir_def_id: DefId, diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index dedc6b9b09af2..210106c8e6415 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -2,7 +2,6 @@ use std::rc::Rc; use rustc::hir::def_id::DefId; use rustc::infer::canonical::QueryOutlivesConstraint; -use rustc::infer::opaque_types; use rustc::infer::region_constraints::{GenericKind, VarInfos, VerifyBound}; use rustc::infer::{InferCtxt, NLLRegionVariableOrigin, RegionVariableOrigin}; use rustc::mir::{ @@ -11,6 +10,7 @@ use rustc::mir::{ }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; use rustc::util::common::ErrorReported; +use rustc_errors::DiagnosticBuilder; use rustc_data_structures::binary_search_util; use rustc_index::bit_set::BitSet; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -18,9 +18,7 @@ use rustc_data_structures::graph::WithSuccessors; use rustc_data_structures::graph::scc::Sccs; use rustc_data_structures::graph::vec_graph::VecGraph; use rustc_index::vec::IndexVec; -use rustc_errors::{Diagnostic, DiagnosticBuilder}; use syntax_pos::Span; -use syntax_pos::symbol::Symbol; use crate::borrow_check::{ constraints::{ @@ -35,12 +33,9 @@ use crate::borrow_check::{ RegionValues, }, type_check::{free_region_relations::UniversalRegionRelations, Locations}, - diagnostics::{ - OutlivesSuggestionBuilder, RegionErrorNamingCtx, - }, + diagnostics::{RegionErrors, RegionErrorKind}, nll::{ToRegionVid, PoloniusOutput}, universal_regions::UniversalRegions, - Upvar, }; mod dump_mir; @@ -477,14 +472,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { &mut self, infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], mir_def_id: DefId, - errors_buffer: &mut Vec, polonius_output: Option>, - ) -> Option> { + ) -> (Option>, RegionErrors<'tcx>) { self.propagate_constraints(body); + let mut errors_buffer = RegionErrors::new(); + // If this is a closure, we can propagate unsatisfied // `outlives_requirements` to our creator, so create a vector // to store those. Otherwise, we'll pass in `None` to the @@ -496,17 +490,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.check_type_tests( infcx, body, - mir_def_id, outlives_requirements.as_mut(), - errors_buffer, + &mut errors_buffer, ); - // If we produce any errors, we keep track of the names of all regions, so that we can use - // the same error names in any suggestions we produce. Note that we need names to be unique - // across different errors for the same MIR def so that we can make suggestions that fix - // multiple problems. - let mut region_naming = RegionErrorNamingCtx::new(); - // In Polonius mode, the errors about missing universal region relations are in the output // and need to be emitted or propagated. Otherwise, we need to check whether the // constraints were too strong, and if so, emit or propagate those errors. @@ -514,36 +501,33 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.check_polonius_subset_errors( infcx, body, - local_names, - upvars, mir_def_id, outlives_requirements.as_mut(), - errors_buffer, - &mut region_naming, + &mut errors_buffer, polonius_output.expect("Polonius output is unavailable despite `-Z polonius`"), ); } else { self.check_universal_regions( infcx, body, - local_names, - upvars, mir_def_id, outlives_requirements.as_mut(), - errors_buffer, - &mut region_naming, + &mut errors_buffer, ); } - self.check_member_constraints(infcx, mir_def_id, errors_buffer); + self.check_member_constraints(infcx, &mut errors_buffer); let outlives_requirements = outlives_requirements.unwrap_or(vec![]); if outlives_requirements.is_empty() { - None + (None, errors_buffer) } else { let num_external_vids = self.universal_regions.num_global_and_external_regions(); - Some(ClosureRegionRequirements { num_external_vids, outlives_requirements }) + ( + Some(ClosureRegionRequirements { num_external_vids, outlives_requirements }), + errors_buffer, + ) } } @@ -838,9 +822,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, - errors_buffer: &mut Vec, + errors_buffer: &mut RegionErrors<'tcx>, ) { let tcx = infcx.tcx; @@ -898,32 +881,16 @@ impl<'tcx> RegionInferenceContext<'tcx> { } if let Some(lower_bound_region) = lower_bound_region { - let region_scope_tree = &tcx.region_scope_tree(mir_def_id); - infcx - .construct_generic_bound_failure( - region_scope_tree, - type_test_span, - None, - type_test.generic_kind, - lower_bound_region, - ) - .buffer(errors_buffer); + errors_buffer.push(RegionErrorKind::TypeTestGenericBoundError { + span: type_test_span, + generic: type_test.generic_kind, + lower_bound_region, + }); } else { - // FIXME. We should handle this case better. It - // indicates that we have e.g., some region variable - // whose value is like `'a+'b` where `'a` and `'b` are - // distinct unrelated univesal regions that are not - // known to outlive one another. It'd be nice to have - // some examples where this arises to decide how best - // to report it; we could probably handle it by - // iterating over the universal regions and reporting - // an error that multiple bounds are required. - tcx.sess - .struct_span_err( - type_test_span, - &format!("`{}` does not live long enough", type_test.generic_kind,), - ) - .buffer(errors_buffer); + errors_buffer.push(RegionErrorKind::TypeTestDoesNotLiveLongEnough { + span: type_test_span, + generic: type_test.generic_kind, + }); } } } @@ -1138,7 +1105,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// include the CFG anyhow. /// - For each `end('x)` element in `'r`, compute the mutual LUB, yielding /// a result `'y`. - pub (in crate::borrow_check) fn universal_upper_bound(&self, r: RegionVid) -> RegionVid { + pub(in crate::borrow_check) fn universal_upper_bound(&self, r: RegionVid) -> RegionVid { debug!("universal_upper_bound(r={:?}={})", r, self.region_value_str(r)); // Find the smallest universal region that contains all other @@ -1318,15 +1285,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, + errors_buffer: &mut RegionErrors<'tcx>, ) { - let mut outlives_suggestion = OutlivesSuggestionBuilder::new(mir_def_id, local_names); - for (fr, fr_definition) in self.definitions.iter_enumerated() { match fr_definition.origin { NLLRegionVariableOrigin::FreeRegion => { @@ -1334,16 +1296,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { // they did not grow too large, accumulating any requirements // for our caller into the `outlives_requirements` vector. self.check_universal_region( - infcx, body, - local_names, - upvars, - mir_def_id, fr, &mut propagated_outlives_requirements, - &mut outlives_suggestion, errors_buffer, - region_naming, ); } @@ -1356,9 +1312,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } } - - // Emit outlives suggestions - outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); } /// Checks if Polonius has found any unexpected free region relations. @@ -1386,12 +1339,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, + errors_buffer: &mut RegionErrors<'tcx>, polonius_output: Rc, ) { debug!( @@ -1399,8 +1349,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { polonius_output.subset_errors.len() ); - let mut outlives_suggestion = OutlivesSuggestionBuilder::new(mir_def_id, local_names); - // Similarly to `check_universal_regions`: a free region relation, which was not explicitly // declared ("known") was found by Polonius, so emit an error, or propagate the // requirements for our caller into the `propagated_outlives_requirements` vector. @@ -1439,26 +1387,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { &mut propagated_outlives_requirements, ); if !propagated { - // If we are not in a context where we can't propagate errors, or we - // could not shrink `fr` to something smaller, then just report an - // error. - // - // Note: in this case, we use the unapproximated regions to report the - // error. This gives better error messages in some cases. - let db = self.report_error( - body, - local_names, - upvars, - infcx, - mir_def_id, - *longer_fr, - NLLRegionVariableOrigin::FreeRegion, - *shorter_fr, - &mut outlives_suggestion, - region_naming, - ); - - db.buffer(errors_buffer); + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr: *longer_fr, + shorter_fr: *shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); } } @@ -1480,8 +1413,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } - // Emit outlives suggestions - outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); } /// Checks the final value for the free region `fr` to see if it @@ -1494,16 +1425,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// `outlives_requirements` vector. fn check_universal_region( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], - mir_def_id: DefId, longer_fr: RegionVid, propagated_outlives_requirements: &mut Option<&mut Vec>>, - outlives_suggestion: &mut OutlivesSuggestionBuilder<'_>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, + errors_buffer: &mut RegionErrors<'tcx>, ) { debug!("check_universal_region(fr={:?})", longer_fr); @@ -1525,15 +1450,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.check_universal_region_relation( longer_fr, representative, - infcx, body, - local_names, - upvars, - mir_def_id, propagated_outlives_requirements, - outlives_suggestion, errors_buffer, - region_naming, ); return; } @@ -1544,21 +1463,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { if let Some(ErrorReported) = self.check_universal_region_relation( longer_fr, shorter_fr, - infcx, body, - local_names, - upvars, - mir_def_id, propagated_outlives_requirements, - outlives_suggestion, errors_buffer, - region_naming, ) { // continuing to iterate just reports more errors than necessary // // FIXME It would also allow us to report more Outlives Suggestions, though, so // it's not clear that that's a bad thing. Somebody should try commenting out this // line and see it is actually a regression. + // + // TODO(mark-i-m): perhaps we could add them to the `RegionErrors` with a flag that + // says "don't report me"? return; } } @@ -1568,15 +1484,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, longer_fr: RegionVid, shorter_fr: RegionVid, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - local_names: &IndexVec>, - upvars: &[Upvar], - mir_def_id: DefId, propagated_outlives_requirements: &mut Option<&mut Vec>>, - outlives_suggestion: &mut OutlivesSuggestionBuilder<'_>, - errors_buffer: &mut Vec, - region_naming: &mut RegionErrorNamingCtx, + errors_buffer: &mut RegionErrors<'tcx>, ) -> Option { // If it is known that `fr: o`, carry on. if self.universal_region_relations.outlives(longer_fr, shorter_fr) { @@ -1599,20 +1509,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { // // Note: in this case, we use the unapproximated regions to report the // error. This gives better error messages in some cases. - let db = self.report_error( - body, - local_names, - upvars, - infcx, - mir_def_id, - longer_fr, - NLLRegionVariableOrigin::FreeRegion, - shorter_fr, - outlives_suggestion, - region_naming, - ); - - db.buffer(errors_buffer); + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); Some(ErrorReported) } @@ -1727,8 +1627,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fn check_member_constraints( &self, infcx: &InferCtxt<'_, 'tcx>, - mir_def_id: DefId, - errors_buffer: &mut Vec, + errors_buffer: &mut RegionErrors<'tcx>, ) { let member_constraints = self.member_constraints.clone(); for m_c_i in member_constraints.all_indices() { @@ -1752,16 +1651,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { } // If not, report an error. - let region_scope_tree = &infcx.tcx.region_scope_tree(mir_def_id); let member_region = infcx.tcx.mk_region(ty::ReVar(member_region_vid)); - opaque_types::unexpected_hidden_region_diagnostic( - infcx.tcx, - Some(region_scope_tree), - m_c.opaque_type_def_id, - m_c.hidden_ty, + errors_buffer.push(RegionErrorKind::UnexpectedHiddenRegion { + opaque_type_def_id: m_c.opaque_type_def_id, + hidden_ty: m_c.hidden_ty, member_region, - ) - .buffer(errors_buffer); + }); } } } From 94a9c6262bba181ca97e5df75ee7a305348cf036 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Tue, 17 Dec 2019 10:43:00 -0600 Subject: [PATCH 06/27] add unreported error variant --- .../borrow_check/diagnostics/region_errors.rs | 11 +++++++++++ src/librustc_mir/borrow_check/mod.rs | 19 ++++++++++++++++++- .../borrow_check/region_infer/mod.rs | 16 +++++++--------- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 1cb6643e7d091..58f2991088355 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -107,6 +107,17 @@ crate enum RegionErrorKind<'tcx> { /// The region that should be shorter, but we can't prove it shorter_fr: RegionVid, }, + + /// The same as `RegionError`, but this is an unreported error. We currently only report the + /// first error encountered and leave the rest unreported so as not to overwhelm the user. + UnreportedError { + /// The origin of the region + fr_origin: NLLRegionVariableOrigin, + /// The region that should outlive `shorter_fr` + longer_fr: RegionVid, + /// The region that should be shorter, but we can't prove it + shorter_fr: RegionVid, + } } /// Various pieces of state used when reporting borrow checker errors. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 1f45da083bce2..0d40b571cf3fa 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1664,7 +1664,24 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { db.buffer(&mut self.errors_buffer); // Emit outlives suggestions - //outlives_suggestion.add_suggestion(body, self, infcx, errors_buffer, region_naming); + outlives_suggestion.add_suggestion( + &self.body, + &self.nonlexical_regioncx, + self.infcx, + &mut self.errors_buffer, + &mut region_naming + ); + } + + RegionErrorKind::UnreportedError { + longer_fr, shorter_fr, + fr_origin: _, + } => { + // FIXME: currently we do nothing with these, but perhaps we can do better? + debug!( + "Unreported region error: can't prove that {:?}: {:?}", + longer_fr, shorter_fr + ); } } } diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 210106c8e6415..c3e469e3b9f7b 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -1467,15 +1467,13 @@ impl<'tcx> RegionInferenceContext<'tcx> { propagated_outlives_requirements, errors_buffer, ) { - // continuing to iterate just reports more errors than necessary - // - // FIXME It would also allow us to report more Outlives Suggestions, though, so - // it's not clear that that's a bad thing. Somebody should try commenting out this - // line and see it is actually a regression. - // - // TODO(mark-i-m): perhaps we could add them to the `RegionErrors` with a flag that - // says "don't report me"? - return; + // We only report the first region error. Subsequent errors are hidden so as not to + // overwhelm the user, but we do record them so as to potentially print better + // diagnostics elsewhere... + errors_buffer.push(RegionErrorKind::UnreportedError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); } } } From 3902e563fb1054aa0fb4e142fdef8dc59d5845d5 Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 13:27:01 -0600 Subject: [PATCH 07/27] tidy --- src/librustc_mir/borrow_check/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 0d40b571cf3fa..d669a75071c59 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1592,9 +1592,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { // Iterate through all the errors, producing a diagnostic for each one. The diagnostics are // buffered in the `MirBorrowckCtxt`. - // TODO(mark-i-m): Would be great to get rid of the naming context. + // FIXME(mark-i-m): Would be great to get rid of the naming context. let mut region_naming = RegionErrorNamingCtx::new(); - let mut outlives_suggestion = OutlivesSuggestionBuilder::new(self.mir_def_id, &self.local_names); + let mut outlives_suggestion = OutlivesSuggestionBuilder::new( + self.mir_def_id, &self.local_names + ); for nll_error in nll_errors.into_iter() { match nll_error { From e5a1f48fa7e6ada9c1bd6ab5a00afd07eac94fac Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 17:00:00 -0600 Subject: [PATCH 08/27] fix outlives suggestions --- .../diagnostics/outlives_suggestion.rs | 4 +++- src/librustc_mir/borrow_check/mod.rs | 20 ++++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs index b61c37b061396..dc5fed4822984 100644 --- a/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs +++ b/src/librustc_mir/borrow_check/diagnostics/outlives_suggestion.rs @@ -250,7 +250,9 @@ impl OutlivesSuggestionBuilder<'a> { // If there is only one constraint to suggest, then we already suggested it in the // intermediate suggestion above. - if self.constraints_to_add.len() == 1 { + if self.constraints_to_add.len() == 1 + && self.constraints_to_add.values().next().unwrap().len() == 1 + { debug!("Only 1 suggestion. Skipping."); return; } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index d669a75071c59..4171190805665 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1664,15 +1664,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ); db.buffer(&mut self.errors_buffer); - - // Emit outlives suggestions - outlives_suggestion.add_suggestion( - &self.body, - &self.nonlexical_regioncx, - self.infcx, - &mut self.errors_buffer, - &mut region_naming - ); } RegionErrorKind::UnreportedError { @@ -1680,6 +1671,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { fr_origin: _, } => { // FIXME: currently we do nothing with these, but perhaps we can do better? + // FIXME: try collecting these constraints on the outlives suggestion builder. + // Does it make the suggestions any better? debug!( "Unreported region error: can't prove that {:?}: {:?}", longer_fr, shorter_fr @@ -1687,6 +1680,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } } + + // Emit one outlives suggestions for each MIR def we borrowck + outlives_suggestion.add_suggestion( + &self.body, + &self.nonlexical_regioncx, + self.infcx, + &mut self.errors_buffer, + &mut region_naming + ); } } From 5d9e74ebd1d5cc02be354a0791df1f6ed295f09f Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 17:02:49 -0600 Subject: [PATCH 09/27] some more refactoring + maintain diagnostic status quo --- .../borrow_check/region_infer/mod.rs | 90 ++++++++++--------- 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index c3e469e3b9f7b..05a21e6958e3e 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -9,7 +9,6 @@ use rustc::mir::{ ConstraintCategory, Local, Location, }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; -use rustc::util::common::ErrorReported; use rustc_errors::DiagnosticBuilder; use rustc_data_structures::binary_search_util; use rustc_index::bit_set::BitSet; @@ -225,6 +224,15 @@ pub struct TypeTest<'tcx> { pub verify_bound: VerifyBound<'tcx>, } +/// When we have an unmet lifetime constraint, we try to propagate it outward (e.g. to a closure +/// environment). If we can't, it is an error. +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum RegionRelationCheckResult { + Ok, + Propagated, + Error, +} + impl<'tcx> RegionInferenceContext<'tcx> { /// Creates a new region inference context with a total of /// `num_region_variables` valid inference variables; the first N @@ -1386,7 +1394,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { body, &mut propagated_outlives_requirements, ); - if !propagated { + if propagated == RegionRelationCheckResult::Error { errors_buffer.push(RegionErrorKind::RegionError { longer_fr: *longer_fr, shorter_fr: *shorter_fr, @@ -1412,7 +1420,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } } - } /// Checks the final value for the free region `fr` to see if it @@ -1447,59 +1454,64 @@ impl<'tcx> RegionInferenceContext<'tcx> { // one in this SCC, so we will always check the representative here. let representative = self.scc_representatives[longer_fr_scc]; if representative != longer_fr { - self.check_universal_region_relation( + if let RegionRelationCheckResult::Error = self.check_universal_region_relation( longer_fr, representative, body, propagated_outlives_requirements, - errors_buffer, - ); + ) { + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, + shorter_fr: representative, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); + } return; } // Find every region `o` such that `fr: o` // (because `fr` includes `end(o)`). + let mut error_reported = false; for shorter_fr in self.scc_values.universal_regions_outlived_by(longer_fr_scc) { - if let Some(ErrorReported) = self.check_universal_region_relation( + if let RegionRelationCheckResult::Error = self.check_universal_region_relation( longer_fr, shorter_fr, body, propagated_outlives_requirements, - errors_buffer, ) { - // We only report the first region error. Subsequent errors are hidden so as not to - // overwhelm the user, but we do record them so as to potentially print better - // diagnostics elsewhere... - errors_buffer.push(RegionErrorKind::UnreportedError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); + // We only report the first region error. Subsequent errors are hidden so as + // not to overwhelm the user, but we do record them so as to potentially print + // better diagnostics elsewhere... + if error_reported { + errors_buffer.push(RegionErrorKind::UnreportedError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); + } else { + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + }); + } + + error_reported = true; } } } + /// Checks that we can prove that `longer_fr: shorter_fr`. If we can't we attempt to propagate + /// the constraint outward (e.g. to a closure environment), but if that fails, there is an + /// error. fn check_universal_region_relation( &self, longer_fr: RegionVid, shorter_fr: RegionVid, body: &Body<'tcx>, propagated_outlives_requirements: &mut Option<&mut Vec>>, - errors_buffer: &mut RegionErrors<'tcx>, - ) -> Option { + ) -> RegionRelationCheckResult { // If it is known that `fr: o`, carry on. if self.universal_region_relations.outlives(longer_fr, shorter_fr) { - return None; - } - - let propagated = self.try_propagate_universal_region_error( - longer_fr, - shorter_fr, - body, - propagated_outlives_requirements, - ); - - if propagated { - None + RegionRelationCheckResult::Ok } else { // If we are not in a context where we can't propagate errors, or we // could not shrink `fr` to something smaller, then just report an @@ -1507,26 +1519,24 @@ impl<'tcx> RegionInferenceContext<'tcx> { // // Note: in this case, we use the unapproximated regions to report the // error. This gives better error messages in some cases. - errors_buffer.push(RegionErrorKind::RegionError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); - - Some(ErrorReported) + self.try_propagate_universal_region_error( + longer_fr, + shorter_fr, + body, + propagated_outlives_requirements, + ) } } /// Attempt to propagate a region error (e.g. `'a: 'b`) that is not met to a closure's /// creator. If we cannot, then the caller should report an error to the user. - /// - /// Returns `true` if the error was propagated, and `false` otherwise. fn try_propagate_universal_region_error( &self, longer_fr: RegionVid, shorter_fr: RegionVid, body: &Body<'tcx>, propagated_outlives_requirements: &mut Option<&mut Vec>>, - ) -> bool { + ) -> RegionRelationCheckResult { if let Some(propagated_outlives_requirements) = propagated_outlives_requirements { // Shrink `longer_fr` until we find a non-local region (if we do). // We'll call it `fr-` -- it's ever so slightly smaller than @@ -1558,11 +1568,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { category: blame_span_category.0, }); } - return true; + return RegionRelationCheckResult::Propagated; } } - false + RegionRelationCheckResult::Error } fn check_bound_universal_region( From e4379e66ff2c1af052b7714f54433ef2e2afe0ff Mon Sep 17 00:00:00 2001 From: Mark Mansi Date: Wed, 18 Dec 2019 17:45:05 -0600 Subject: [PATCH 10/27] make regionerrors a typedef --- .../borrow_check/diagnostics/region_errors.rs | 21 +------------------ 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 58f2991088355..59bd81c52ddc8 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -63,10 +63,7 @@ enum Trace { /// A collection of errors encountered during region inference. This is needed to efficiently /// report errors after borrow checking. -#[derive(Clone, Debug)] -crate struct RegionErrors<'tcx> { - errors: smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>, -} +crate type RegionErrors<'tcx> = smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>; #[derive(Clone, Debug)] crate enum RegionErrorKind<'tcx> { @@ -155,22 +152,6 @@ pub struct ErrorConstraintInfo { pub(super) span: Span, } -impl<'tcx> RegionErrors<'tcx> { - pub fn new() -> Self { - RegionErrors { - errors: smallvec::SmallVec::new(), - } - } - - pub fn push(&mut self, error: RegionErrorKind<'tcx>) { - self.errors.push(error) - } - - pub fn into_iter(self) -> impl Iterator> { - self.errors.into_iter() - } -} - impl<'tcx> RegionInferenceContext<'tcx> { /// Converts a region inference variable into a `ty::Region` that /// we can use for error reporting. If `r` is universally bound, From e047368d80519c35e42fea283baba85f9c3a919e Mon Sep 17 00:00:00 2001 From: Janusz Marcinkiewicz Date: Sat, 21 Dec 2019 19:13:12 +0100 Subject: [PATCH 11/27] Add arguments to suggestion method call --- src/librustc_resolve/late/diagnostics.rs | 17 ++++++++++++++++- src/test/ui/self/suggest-self-2.stderr | 6 +++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index 1df8ff02fcebe..859307c51ed9f 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -256,6 +256,7 @@ impl<'a> LateResolutionVisitor<'a, '_> { // Check if the first argument is `self` and suggest calling a method. let mut has_self_arg = false; + let mut args_span = None; if let PathSource::Expr(parent) = source { match &parent.map(|p| &p.kind) { Some(ExprKind::Call(_, args)) if args.len() > 0 => { @@ -264,6 +265,13 @@ impl<'a> LateResolutionVisitor<'a, '_> { match expr_kind { ExprKind::Path(_, arg_name) if arg_name.segments.len() == 1 => { has_self_arg = arg_name.segments[0].ident.name == kw::SelfLower; + if args.len() > 1 { + args_span = Some(Span::new( + args[1].span.lo(), + args.last().unwrap().span.hi(), + parent.unwrap().span.ctxt(), + )); + } break; }, ExprKind::AddrOf(_, _, expr) => expr_kind = &expr.kind, @@ -276,10 +284,17 @@ impl<'a> LateResolutionVisitor<'a, '_> { }; if has_self_arg { + let mut args_snippet: String = String::from(""); + if let Some(args_span) = args_span { + if let Ok(snippet) = self.r.session.source_map().span_to_snippet(args_span) { + args_snippet = snippet; + } + } + err.span_suggestion( span, &format!("try calling `{}` as a method", ident), - format!("self.{}", path_str), + format!("self.{}({})", path_str, args_snippet), Applicability::MachineApplicable, ); return (err, candidates); diff --git a/src/test/ui/self/suggest-self-2.stderr b/src/test/ui/self/suggest-self-2.stderr index ba71498fae656..6148012ac0d92 100644 --- a/src/test/ui/self/suggest-self-2.stderr +++ b/src/test/ui/self/suggest-self-2.stderr @@ -2,19 +2,19 @@ error[E0425]: cannot find function `bar` in this scope --> $DIR/suggest-self-2.rs:5:9 | LL | bar(self); - | ^^^ help: try calling `bar` as a method: `self.bar` + | ^^^ help: try calling `bar` as a method: `self.bar()` error[E0425]: cannot find function `bar` in this scope --> $DIR/suggest-self-2.rs:9:9 | LL | bar(&&self, 102); - | ^^^ help: try calling `bar` as a method: `self.bar` + | ^^^ help: try calling `bar` as a method: `self.bar(102)` error[E0425]: cannot find function `bar` in this scope --> $DIR/suggest-self-2.rs:13:9 | LL | bar(&mut self, 102, &"str"); - | ^^^ help: try calling `bar` as a method: `self.bar` + | ^^^ help: try calling `bar` as a method: `self.bar(102, &"str")` error[E0425]: cannot find function `bar` in this scope --> $DIR/suggest-self-2.rs:17:9 From 5b06025f99a68b39a49b7de5932a5b652edf4ac7 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 12:54:43 -0600 Subject: [PATCH 12/27] is_reported flag --- .../borrow_check/diagnostics/region_errors.rs | 14 ++--- src/librustc_mir/borrow_check/mod.rs | 51 +++++++++---------- .../borrow_check/region_infer/mod.rs | 18 +++---- 3 files changed, 34 insertions(+), 49 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 59bd81c52ddc8..4e85b110ddaf4 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -103,18 +103,10 @@ crate enum RegionErrorKind<'tcx> { longer_fr: RegionVid, /// The region that should be shorter, but we can't prove it shorter_fr: RegionVid, + /// Indicates whether this is a reported error. We currently only report the first error + /// encountered and leave the rest unreported so as not to overwhelm the user. + is_reported: bool, }, - - /// The same as `RegionError`, but this is an unreported error. We currently only report the - /// first error encountered and leave the rest unreported so as not to overwhelm the user. - UnreportedError { - /// The origin of the region - fr_origin: NLLRegionVariableOrigin, - /// The region that should outlive `shorter_fr` - longer_fr: RegionVid, - /// The region that should be shorter, but we can't prove it - shorter_fr: RegionVid, - } } /// Various pieces of state used when reporting borrow checker errors. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 4171190805665..751ebc155f5ce 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1648,35 +1648,32 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } RegionErrorKind::RegionError { - fr_origin, longer_fr, shorter_fr, + fr_origin, longer_fr, shorter_fr, is_reported, } => { - let db = self.nonlexical_regioncx.report_error( - &self.body, - &self.local_names, - &self.upvars, - self.infcx, - self.mir_def_id, - longer_fr, - fr_origin, - shorter_fr, - &mut outlives_suggestion, - &mut region_naming, - ); - - db.buffer(&mut self.errors_buffer); - } + if is_reported { + let db = self.nonlexical_regioncx.report_error( + &self.body, + &self.local_names, + &self.upvars, + self.infcx, + self.mir_def_id, + longer_fr, + fr_origin, + shorter_fr, + &mut outlives_suggestion, + &mut region_naming, + ); - RegionErrorKind::UnreportedError { - longer_fr, shorter_fr, - fr_origin: _, - } => { - // FIXME: currently we do nothing with these, but perhaps we can do better? - // FIXME: try collecting these constraints on the outlives suggestion builder. - // Does it make the suggestions any better? - debug!( - "Unreported region error: can't prove that {:?}: {:?}", - longer_fr, shorter_fr - ); + db.buffer(&mut self.errors_buffer); + } else { + // FIXME: currently we do nothing with these, but perhaps we can do better? + // FIXME: try collecting these constraints on the outlives suggestion + // builder. Does it make the suggestions any better? + debug!( + "Unreported region error: can't prove that {:?}: {:?}", + longer_fr, shorter_fr + ); + } } } } diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 05a21e6958e3e..3afe2056a926f 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -1399,6 +1399,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { longer_fr: *longer_fr, shorter_fr: *shorter_fr, fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: true, }); } } @@ -1464,6 +1465,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { longer_fr, shorter_fr: representative, fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: true, }); } return; @@ -1482,17 +1484,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { // We only report the first region error. Subsequent errors are hidden so as // not to overwhelm the user, but we do record them so as to potentially print // better diagnostics elsewhere... - if error_reported { - errors_buffer.push(RegionErrorKind::UnreportedError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); - } else { - errors_buffer.push(RegionErrorKind::RegionError { - longer_fr, shorter_fr, - fr_origin: NLLRegionVariableOrigin::FreeRegion, - }); - } + errors_buffer.push(RegionErrorKind::RegionError { + longer_fr, shorter_fr, + fr_origin: NLLRegionVariableOrigin::FreeRegion, + is_reported: !error_reported, + }); error_reported = true; } From bd0ea19eaa96dc607c285d0aeba2fffa44fc1019 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 13:27:00 -0600 Subject: [PATCH 13/27] add comments --- src/librustc_mir/borrow_check/diagnostics/region_errors.rs | 3 +++ src/librustc_mir/borrow_check/mod.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 4e85b110ddaf4..4ce0311f3bb4c 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -63,6 +63,9 @@ enum Trace { /// A collection of errors encountered during region inference. This is needed to efficiently /// report errors after borrow checking. +/// +/// Usually we expect this to either be empty or contain a small number of items, so we can avoid +/// allocation most of the time. crate type RegionErrors<'tcx> = smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>; #[derive(Clone, Debug)] diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 751ebc155f5ce..153ee23811e92 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1666,6 +1666,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { db.buffer(&mut self.errors_buffer); } else { + // We only report the first error, so as not to overwhelm the user. See + // `RegRegionErrorKind` docs. + // // FIXME: currently we do nothing with these, but perhaps we can do better? // FIXME: try collecting these constraints on the outlives suggestion // builder. Does it make the suggestions any better? From a155a9b67d9688a3d5b90361e5ea8a4a9bf3b0d6 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 13:35:21 -0600 Subject: [PATCH 14/27] use vec instead of smallvec --- src/librustc_mir/borrow_check/diagnostics/region_errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 4ce0311f3bb4c..481494f6a3abe 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -66,7 +66,7 @@ enum Trace { /// /// Usually we expect this to either be empty or contain a small number of items, so we can avoid /// allocation most of the time. -crate type RegionErrors<'tcx> = smallvec::SmallVec<[RegionErrorKind<'tcx>; 4]>; +crate type RegionErrors<'tcx> = Vec>; #[derive(Clone, Debug)] crate enum RegionErrorKind<'tcx> { From d85578287c40031b61c252daba0572181b6117f4 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 15:31:52 -0600 Subject: [PATCH 15/27] convert hrtb error --- .../borrow_check/diagnostics/region_errors.rs | 10 ++++++ src/librustc_mir/borrow_check/mod.rs | 19 ++++++++++ .../borrow_check/region_infer/mod.rs | 36 +++++-------------- 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 481494f6a3abe..98a1d7dad94bd 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -98,6 +98,16 @@ crate enum RegionErrorKind<'tcx> { member_region: ty::Region<'tcx>, }, + /// Higher-ranked subtyping error + BoundUniversalRegionError { + /// The placeholder free region. + longer_fr: RegionVid, + /// The region that erroneously must be outlived by `longer_fr`. + error_region: RegionVid, + /// The origin of the placeholder region + fr_origin: NLLRegionVariableOrigin, + }, + /// Any other lifetime error RegionError { /// The origin of the region diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 153ee23811e92..9fc6b0ffb2d59 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1647,6 +1647,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { .buffer(&mut self.errors_buffer); } + RegionErrorKind::BoundUniversalRegionError { + longer_fr, fr_origin, error_region, + } => { + // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. + let (_, span) = self.nonlexical_regioncx.find_outlives_blame_span( + &self.body, + longer_fr, + fr_origin, + error_region, + ); + + // FIXME: improve this error message + self.infcx + .tcx + .sess + .struct_span_err(span, "higher-ranked subtype error") + .buffer(&mut self.errors_buffer); + } + RegionErrorKind::RegionError { fr_origin, longer_fr, shorter_fr, is_reported, } => { diff --git a/src/librustc_mir/borrow_check/region_infer/mod.rs b/src/librustc_mir/borrow_check/region_infer/mod.rs index 3afe2056a926f..5310ac6c48faa 100644 --- a/src/librustc_mir/borrow_check/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/region_infer/mod.rs @@ -9,7 +9,6 @@ use rustc::mir::{ ConstraintCategory, Local, Location, }; use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable}; -use rustc_errors::DiagnosticBuilder; use rustc_data_structures::binary_search_util; use rustc_index::bit_set::BitSet; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -435,7 +434,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } /// Adds annotations for `#[rustc_regions]`; see `UniversalRegions::annotate`. - crate fn annotate(&self, tcx: TyCtxt<'tcx>, err: &mut DiagnosticBuilder<'_>) { + crate fn annotate(&self, tcx: TyCtxt<'tcx>, err: &mut rustc_errors::DiagnosticBuilder<'_>) { self.universal_regions.annotate(tcx, err) } @@ -507,18 +506,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { // constraints were too strong, and if so, emit or propagate those errors. if infcx.tcx.sess.opts.debugging_opts.polonius { self.check_polonius_subset_errors( - infcx, body, - mir_def_id, outlives_requirements.as_mut(), &mut errors_buffer, polonius_output.expect("Polonius output is unavailable despite `-Z polonius`"), ); } else { self.check_universal_regions( - infcx, body, - mir_def_id, outlives_requirements.as_mut(), &mut errors_buffer, ); @@ -1291,9 +1286,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// report them as errors. fn check_universal_regions( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, errors_buffer: &mut RegionErrors<'tcx>, ) { @@ -1312,7 +1305,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } NLLRegionVariableOrigin::Placeholder(placeholder) => { - self.check_bound_universal_region(infcx, body, mir_def_id, fr, placeholder); + self.check_bound_universal_region(fr, placeholder, errors_buffer); } NLLRegionVariableOrigin::Existential { .. } => { @@ -1345,9 +1338,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// report them as errors. fn check_polonius_subset_errors( &self, - infcx: &InferCtxt<'_, 'tcx>, body: &Body<'tcx>, - mir_def_id: DefId, mut propagated_outlives_requirements: Option<&mut Vec>>, errors_buffer: &mut RegionErrors<'tcx>, polonius_output: Rc, @@ -1413,7 +1404,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } NLLRegionVariableOrigin::Placeholder(placeholder) => { - self.check_bound_universal_region(infcx, body, mir_def_id, fr, placeholder); + self.check_bound_universal_region(fr, placeholder, errors_buffer); } NLLRegionVariableOrigin::Existential { .. } => { @@ -1573,11 +1564,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { fn check_bound_universal_region( &self, - infcx: &InferCtxt<'_, 'tcx>, - body: &Body<'tcx>, - _mir_def_id: DefId, longer_fr: RegionVid, placeholder: ty::PlaceholderRegion, + errors_buffer: &mut RegionErrors<'tcx>, ) { debug!("check_bound_universal_region(fr={:?}, placeholder={:?})", longer_fr, placeholder,); @@ -1614,18 +1603,11 @@ impl<'tcx> RegionInferenceContext<'tcx> { .unwrap(), }; - // Find the code to blame for the fact that `longer_fr` outlives `error_fr`. - let (_, span) = self.find_outlives_blame_span( - body, longer_fr, NLLRegionVariableOrigin::Placeholder(placeholder), error_region - ); - - // Obviously, this error message is far from satisfactory. - // At present, though, it only appears in unit tests -- - // the AST-based checker uses a more conservative check, - // so to even see this error, one must pass in a special - // flag. - let mut diag = infcx.tcx.sess.struct_span_err(span, "higher-ranked subtype error"); - diag.emit(); + errors_buffer.push(RegionErrorKind::BoundUniversalRegionError { + longer_fr, + error_region, + fr_origin: NLLRegionVariableOrigin::Placeholder(placeholder), + }); } fn check_member_constraints( From 1d9c56189f4b6f09ffd269e63bff98ede0b93b11 Mon Sep 17 00:00:00 2001 From: mark Date: Sat, 21 Dec 2019 15:33:34 -0600 Subject: [PATCH 16/27] minor updates to comments --- .../borrow_check/diagnostics/region_errors.rs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs index 98a1d7dad94bd..801f801fc9097 100644 --- a/src/librustc_mir/borrow_check/diagnostics/region_errors.rs +++ b/src/librustc_mir/borrow_check/diagnostics/region_errors.rs @@ -70,51 +70,51 @@ crate type RegionErrors<'tcx> = Vec>; #[derive(Clone, Debug)] crate enum RegionErrorKind<'tcx> { - /// An error for a type test: `T: 'a` does not live long enough + /// An error for a type test: `T: 'a` does not live long enough. TypeTestDoesNotLiveLongEnough { - /// The span of the type test + /// The span of the type test. span: Span, - /// The generic type of the type test + /// The generic type of the type test. generic: GenericKind<'tcx>, }, - /// A generic bound failure for a type test + /// A generic bound failure for a type test. TypeTestGenericBoundError { - /// The span of the type test + /// The span of the type test. span: Span, - /// The generic type of the type test + /// The generic type of the type test. generic: GenericKind<'tcx>, - /// The lower bound region + /// The lower bound region. lower_bound_region: ty::Region<'tcx>, }, - /// An unexpected hidden region for an opaque type + /// An unexpected hidden region for an opaque type. UnexpectedHiddenRegion { - /// The def id of the opaque type + /// The def id of the opaque type. opaque_type_def_id: DefId, - /// The hidden type + /// The hidden type. hidden_ty: Ty<'tcx>, - /// The unexpected region + /// The unexpected region. member_region: ty::Region<'tcx>, }, - /// Higher-ranked subtyping error + /// Higher-ranked subtyping error. BoundUniversalRegionError { /// The placeholder free region. longer_fr: RegionVid, /// The region that erroneously must be outlived by `longer_fr`. error_region: RegionVid, - /// The origin of the placeholder region + /// The origin of the placeholder region. fr_origin: NLLRegionVariableOrigin, }, - /// Any other lifetime error + /// Any other lifetime error. RegionError { - /// The origin of the region + /// The origin of the region. fr_origin: NLLRegionVariableOrigin, - /// The region that should outlive `shorter_fr` + /// The region that should outlive `shorter_fr`. longer_fr: RegionVid, - /// The region that should be shorter, but we can't prove it + /// The region that should be shorter, but we can't prove it. shorter_fr: RegionVid, /// Indicates whether this is a reported error. We currently only report the first error /// encountered and leave the rest unreported so as not to overwhelm the user. From bbcf9b17dc050c6dddc253913f9143bb3542368e Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 22 Dec 2019 18:18:49 +0000 Subject: [PATCH 17/27] Add the full issue reference to equality constraints in `where` clauses --- src/librustc_passes/ast_validation.rs | 10 ++++++++-- .../ui/where-clauses/where-equality-constraints.rs | 4 ++-- .../ui/where-clauses/where-equality-constraints.stderr | 8 ++++++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index ee6a67802ade3..1d5e65c6d27cd 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -737,8 +737,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> { for predicate in &generics.where_clause.predicates { if let WherePredicate::EqPredicate(ref predicate) = *predicate { self.err_handler() - .span_err(predicate.span, "equality constraints are not yet \ - supported in where clauses (see #20041)"); + .struct_span_err( + predicate.span, + "equality constraints are not yet supported in `where` clauses", + ) + .note( + "for more information, see https://github.com/rust-lang/rust/issues/20041", + ) + .emit(); } } diff --git a/src/test/ui/where-clauses/where-equality-constraints.rs b/src/test/ui/where-clauses/where-equality-constraints.rs index f2349144b88ae..8828f09d92d33 100644 --- a/src/test/ui/where-clauses/where-equality-constraints.rs +++ b/src/test/ui/where-clauses/where-equality-constraints.rs @@ -1,6 +1,6 @@ fn f() where u8 = u16 {} -//~^ ERROR equality constraints are not yet supported in where clauses +//~^ ERROR equality constraints are not yet supported in `where` clauses fn g() where for<'a> &'static (u8,) == u16, {} -//~^ ERROR equality constraints are not yet supported in where clauses +//~^ ERROR equality constraints are not yet supported in `where` clauses fn main() {} diff --git a/src/test/ui/where-clauses/where-equality-constraints.stderr b/src/test/ui/where-clauses/where-equality-constraints.stderr index 220447079c629..c0241fe708f64 100644 --- a/src/test/ui/where-clauses/where-equality-constraints.stderr +++ b/src/test/ui/where-clauses/where-equality-constraints.stderr @@ -1,14 +1,18 @@ -error: equality constraints are not yet supported in where clauses (see #20041) +error: equality constraints are not yet supported in `where` clauses --> $DIR/where-equality-constraints.rs:1:14 | LL | fn f() where u8 = u16 {} | ^^^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/20041 -error: equality constraints are not yet supported in where clauses (see #20041) +error: equality constraints are not yet supported in `where` clauses --> $DIR/where-equality-constraints.rs:3:14 | LL | fn g() where for<'a> &'static (u8,) == u16, {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: for more information, see https://github.com/rust-lang/rust/issues/20041 error: aborting due to 2 previous errors From 5c92da4fb61181a26d0834493da87ec543791af7 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 22 Dec 2019 18:42:15 +0000 Subject: [PATCH 18/27] Improve invalid assignment error --- src/librustc_typeck/check/expr.rs | 10 +++--- src/librustc_typeck/check/op.rs | 11 +++---- src/test/ui/bad/bad-expr-lhs.rs | 10 +++--- src/test/ui/bad/bad-expr-lhs.stderr | 32 ++++++++++++------- src/test/ui/error-codes/E0067.stderr | 8 +++-- src/test/ui/error-codes/E0070.stderr | 18 +++++++---- src/test/ui/issues/issue-13407.rs | 2 +- src/test/ui/issues/issue-13407.stderr | 6 ++-- src/test/ui/issues/issue-26093.rs | 4 ++- src/test/ui/issues/issue-26093.stderr | 26 ++++++++++++--- src/test/ui/issues/issue-34334.rs | 2 +- src/test/ui/issues/issue-34334.stderr | 6 ++-- .../type-check/assignment-expected-bool.rs | 2 +- .../assignment-expected-bool.stderr | 6 ++-- .../ui/type/type-check/assignment-in-if.rs | 2 +- 15 files changed, 94 insertions(+), 51 deletions(-) diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 17b168cfab05a..bc2d19b413cec 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -791,10 +791,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } err.emit(); } else if !lhs.is_syntactic_place_expr() { - struct_span_err!(self.tcx.sess, expr.span, E0070, - "invalid left-hand side expression") - .span_label(expr.span, "left-hand of expression not valid") - .emit(); + struct_span_err!( + self.tcx.sess, + expr.span, + E0070, + "invalid left-hand side of assignment", + ).span_label(lhs.span, "cannot assign to this expression").emit(); } self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized); diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index 041a6c6f44c51..f049728c3f235 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -35,12 +35,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if !lhs_expr.is_syntactic_place_expr() { struct_span_err!( - self.tcx.sess, lhs_expr.span, - E0067, "invalid left-hand side expression") - .span_label( - lhs_expr.span, - "invalid expression for left-hand side") - .emit(); + self.tcx.sess, + op.span, + E0067, + "invalid left-hand side of assignment", + ).span_label(lhs_expr.span, "cannot assign to this expression").emit(); } ty } diff --git a/src/test/ui/bad/bad-expr-lhs.rs b/src/test/ui/bad/bad-expr-lhs.rs index 2cd8bc9d47333..d7cf1b7700514 100644 --- a/src/test/ui/bad/bad-expr-lhs.rs +++ b/src/test/ui/bad/bad-expr-lhs.rs @@ -1,10 +1,10 @@ fn main() { - 1 = 2; //~ ERROR invalid left-hand side expression - 1 += 2; //~ ERROR invalid left-hand side expression - (1, 2) = (3, 4); //~ ERROR invalid left-hand side expression + 1 = 2; //~ ERROR invalid left-hand side of assignment + 1 += 2; //~ ERROR invalid left-hand side of assignment + (1, 2) = (3, 4); //~ ERROR invalid left-hand side of assignment let (a, b) = (1, 2); - (a, b) = (3, 4); //~ ERROR invalid left-hand side expression + (a, b) = (3, 4); //~ ERROR invalid left-hand side of assignment - None = Some(3); //~ ERROR invalid left-hand side expression + None = Some(3); //~ ERROR invalid left-hand side of assignment } diff --git a/src/test/ui/bad/bad-expr-lhs.stderr b/src/test/ui/bad/bad-expr-lhs.stderr index a0de6a73797e2..07cffbe97a8d9 100644 --- a/src/test/ui/bad/bad-expr-lhs.stderr +++ b/src/test/ui/bad/bad-expr-lhs.stderr @@ -1,32 +1,42 @@ -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/bad-expr-lhs.rs:2:5 | LL | 1 = 2; - | ^^^^^ left-hand of expression not valid + | -^^^^ + | | + | cannot assign to this expression -error[E0067]: invalid left-hand side expression - --> $DIR/bad-expr-lhs.rs:3:5 +error[E0067]: invalid left-hand side of assignment + --> $DIR/bad-expr-lhs.rs:3:7 | LL | 1 += 2; - | ^ invalid expression for left-hand side + | - ^^ + | | + | cannot assign to this expression -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/bad-expr-lhs.rs:4:5 | LL | (1, 2) = (3, 4); - | ^^^^^^^^^^^^^^^ left-hand of expression not valid + | ------^^^^^^^^^ + | | + | cannot assign to this expression -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/bad-expr-lhs.rs:7:5 | LL | (a, b) = (3, 4); - | ^^^^^^^^^^^^^^^ left-hand of expression not valid + | ------^^^^^^^^^ + | | + | cannot assign to this expression -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/bad-expr-lhs.rs:9:5 | LL | None = Some(3); - | ^^^^^^^^^^^^^^ left-hand of expression not valid + | ----^^^^^^^^^^ + | | + | cannot assign to this expression error: aborting due to 5 previous errors diff --git a/src/test/ui/error-codes/E0067.stderr b/src/test/ui/error-codes/E0067.stderr index 0334565840f83..526503798b3d4 100644 --- a/src/test/ui/error-codes/E0067.stderr +++ b/src/test/ui/error-codes/E0067.stderr @@ -8,11 +8,13 @@ LL | LinkedList::new() += 1; | = note: an implementation of `std::ops::AddAssign` might be missing for `std::collections::LinkedList<_>` -error[E0067]: invalid left-hand side expression - --> $DIR/E0067.rs:4:5 +error[E0067]: invalid left-hand side of assignment + --> $DIR/E0067.rs:4:23 | LL | LinkedList::new() += 1; - | ^^^^^^^^^^^^^^^^^ invalid expression for left-hand side + | ----------------- ^^ + | | + | cannot assign to this expression error: aborting due to 2 previous errors diff --git a/src/test/ui/error-codes/E0070.stderr b/src/test/ui/error-codes/E0070.stderr index 845833bc82f70..1fb812d94672f 100644 --- a/src/test/ui/error-codes/E0070.stderr +++ b/src/test/ui/error-codes/E0070.stderr @@ -1,14 +1,18 @@ -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/E0070.rs:6:5 | LL | SOME_CONST = 14; - | ^^^^^^^^^^^^^^^ left-hand of expression not valid + | ----------^^^^^ + | | + | cannot assign to this expression -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/E0070.rs:7:5 | LL | 1 = 3; - | ^^^^^ left-hand of expression not valid + | -^^^^ + | | + | cannot assign to this expression error[E0308]: mismatched types --> $DIR/E0070.rs:8:25 @@ -16,11 +20,13 @@ error[E0308]: mismatched types LL | some_other_func() = 4; | ^ expected `()`, found integer -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/E0070.rs:8:5 | LL | some_other_func() = 4; - | ^^^^^^^^^^^^^^^^^^^^^ left-hand of expression not valid + | -----------------^^^^ + | | + | cannot assign to this expression error: aborting due to 4 previous errors diff --git a/src/test/ui/issues/issue-13407.rs b/src/test/ui/issues/issue-13407.rs index 322e67cc13180..fa53d55f5b3d7 100644 --- a/src/test/ui/issues/issue-13407.rs +++ b/src/test/ui/issues/issue-13407.rs @@ -4,7 +4,7 @@ mod A { fn main() { A::C = 1; - //~^ ERROR: invalid left-hand side expression + //~^ ERROR: invalid left-hand side of assignment //~| ERROR: mismatched types //~| ERROR: struct `C` is private } diff --git a/src/test/ui/issues/issue-13407.stderr b/src/test/ui/issues/issue-13407.stderr index 5a465cc533bb7..05fd97b025f60 100644 --- a/src/test/ui/issues/issue-13407.stderr +++ b/src/test/ui/issues/issue-13407.stderr @@ -10,11 +10,13 @@ error[E0308]: mismatched types LL | A::C = 1; | ^ expected struct `A::C`, found integer -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/issue-13407.rs:6:5 | LL | A::C = 1; - | ^^^^^^^^ left-hand of expression not valid + | ----^^^^ + | | + | cannot assign to this expression error: aborting due to 3 previous errors diff --git a/src/test/ui/issues/issue-26093.rs b/src/test/ui/issues/issue-26093.rs index 7895c90068fe2..c838515caf997 100644 --- a/src/test/ui/issues/issue-26093.rs +++ b/src/test/ui/issues/issue-26093.rs @@ -1,7 +1,9 @@ macro_rules! not_a_place { ($thing:expr) => { $thing = 42; - //~^ ERROR invalid left-hand side expression + //~^ ERROR invalid left-hand side of assignment + $thing += 42; + //~^ ERROR invalid left-hand side of assignment } } diff --git a/src/test/ui/issues/issue-26093.stderr b/src/test/ui/issues/issue-26093.stderr index 947c52f08d2e6..48f72cef0a85a 100644 --- a/src/test/ui/issues/issue-26093.stderr +++ b/src/test/ui/issues/issue-26093.stderr @@ -1,12 +1,28 @@ -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/issue-26093.rs:3:9 | LL | $thing = 42; - | ^^^^^^^^^^^ left-hand of expression not valid + | ^^^^^^^^^^^ ... LL | not_a_place!(99); - | ----------------- in this macro invocation + | ----------------- + | | | + | | cannot assign to this expression + | in this macro invocation -error: aborting due to previous error +error[E0067]: invalid left-hand side of assignment + --> $DIR/issue-26093.rs:5:16 + | +LL | $thing += 42; + | ^^ +... +LL | not_a_place!(99); + | ----------------- + | | | + | | cannot assign to this expression + | in this macro invocation + +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0070`. +Some errors have detailed explanations: E0067, E0070. +For more information about an error, try `rustc --explain E0067`. diff --git a/src/test/ui/issues/issue-34334.rs b/src/test/ui/issues/issue-34334.rs index 4457d71cbb4a7..e34b5c9a0f47e 100644 --- a/src/test/ui/issues/issue-34334.rs +++ b/src/test/ui/issues/issue-34334.rs @@ -3,7 +3,7 @@ fn main () { //~^ ERROR expected one of `,` or `>`, found `=` //~| ERROR expected value, found struct `Vec` //~| ERROR mismatched types - //~| ERROR invalid left-hand side expression + //~| ERROR invalid left-hand side of assignment //~| ERROR expected expression, found reserved identifier `_` //~| ERROR expected expression, found reserved identifier `_` let sr2: Vec<(u32, _, _)> = sr.iter().map(|(faction, th_sender, th_receiver)| {}).collect(); diff --git a/src/test/ui/issues/issue-34334.stderr b/src/test/ui/issues/issue-34334.stderr index fc90e0674cf55..e54f0c77cd973 100644 --- a/src/test/ui/issues/issue-34334.stderr +++ b/src/test/ui/issues/issue-34334.stderr @@ -35,11 +35,13 @@ LL | let sr: Vec<(u32, _, _) = vec![]; found struct `std::vec::Vec<_>` = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/issue-34334.rs:2:13 | LL | let sr: Vec<(u32, _, _) = vec![]; - | ^^^^^^^^^^^^^^^^^^^^^^^^ left-hand of expression not valid + | ---------------^^^^^^^^^ + | | + | cannot assign to this expression error[E0599]: no method named `iter` found for type `()` in the current scope --> $DIR/issue-34334.rs:9:36 diff --git a/src/test/ui/type/type-check/assignment-expected-bool.rs b/src/test/ui/type/type-check/assignment-expected-bool.rs index 03830fea062cf..191939bdb705b 100644 --- a/src/test/ui/type/type-check/assignment-expected-bool.rs +++ b/src/test/ui/type/type-check/assignment-expected-bool.rs @@ -30,5 +30,5 @@ fn main() { // A test to check that not expecting `bool` behaves well: let _: usize = 0 = 0; //~^ ERROR mismatched types [E0308] - //~| ERROR invalid left-hand side expression [E0070] + //~| ERROR invalid left-hand side of assignment [E0070] } diff --git a/src/test/ui/type/type-check/assignment-expected-bool.stderr b/src/test/ui/type/type-check/assignment-expected-bool.stderr index 9a1cf5b25625c..bbd961f845016 100644 --- a/src/test/ui/type/type-check/assignment-expected-bool.stderr +++ b/src/test/ui/type/type-check/assignment-expected-bool.stderr @@ -97,11 +97,13 @@ LL | || (0 = 0); | expected `bool`, found `()` | help: try comparing for equality: `0 == 0` -error[E0070]: invalid left-hand side expression +error[E0070]: invalid left-hand side of assignment --> $DIR/assignment-expected-bool.rs:31:20 | LL | let _: usize = 0 = 0; - | ^^^^^ left-hand of expression not valid + | -^^^^ + | | + | cannot assign to this expression error[E0308]: mismatched types --> $DIR/assignment-expected-bool.rs:31:20 diff --git a/src/test/ui/type/type-check/assignment-in-if.rs b/src/test/ui/type/type-check/assignment-in-if.rs index 77b122b0a794a..8da7b32b47b14 100644 --- a/src/test/ui/type/type-check/assignment-in-if.rs +++ b/src/test/ui/type/type-check/assignment-in-if.rs @@ -26,7 +26,7 @@ fn main() { //~^ ERROR mismatched types println!("{}", x); } - // "invalid left-hand side expression" error is suppresed + // "invalid left-hand side of assignment" error is suppresed if 3 = x { //~^ ERROR mismatched types println!("{}", x); From a2cc3d6ab8677b63d7c98bb2ba299af2894f3119 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 22 Dec 2019 20:46:14 +0100 Subject: [PATCH 19/27] Move `{hir::lowering -> hir}::is_range_literal`. The function is never used inside lowering, but only ever in external crates. By moving it, we faciliate lowering as its own crate. --- src/librustc/hir/lowering.rs | 62 ------------------------------------ src/librustc/hir/mod.rs | 62 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 83869951ea2a1..a702eb839845e 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3437,65 +3437,3 @@ fn body_ids(bodies: &BTreeMap>) -> Vec body_ids.sort_by_key(|b| bodies[b].value.span); body_ids } - -/// Checks if the specified expression is a built-in range literal. -/// (See: `LoweringContext::lower_expr()`). -pub fn is_range_literal(sess: &Session, expr: &hir::Expr) -> bool { - use hir::{Path, QPath, ExprKind, TyKind}; - - // Returns whether the given path represents a (desugared) range, - // either in std or core, i.e. has either a `::std::ops::Range` or - // `::core::ops::Range` prefix. - fn is_range_path(path: &Path) -> bool { - let segs: Vec<_> = path.segments.iter().map(|seg| seg.ident.to_string()).collect(); - let segs: Vec<_> = segs.iter().map(|seg| &**seg).collect(); - - // "{{root}}" is the equivalent of `::` prefix in `Path`. - if let ["{{root}}", std_core, "ops", range] = segs.as_slice() { - (*std_core == "std" || *std_core == "core") && range.starts_with("Range") - } else { - false - } - }; - - // Check whether a span corresponding to a range expression is a - // range literal, rather than an explicit struct or `new()` call. - fn is_lit(sess: &Session, span: &Span) -> bool { - let source_map = sess.source_map(); - let end_point = source_map.end_point(*span); - - if let Ok(end_string) = source_map.span_to_snippet(end_point) { - !(end_string.ends_with("}") || end_string.ends_with(")")) - } else { - false - } - }; - - match expr.kind { - // All built-in range literals but `..=` and `..` desugar to `Struct`s. - ExprKind::Struct(ref qpath, _, _) => { - if let QPath::Resolved(None, ref path) = **qpath { - return is_range_path(&path) && is_lit(sess, &expr.span); - } - } - - // `..` desugars to its struct path. - ExprKind::Path(QPath::Resolved(None, ref path)) => { - return is_range_path(&path) && is_lit(sess, &expr.span); - } - - // `..=` desugars into `::std::ops::RangeInclusive::new(...)`. - ExprKind::Call(ref func, _) => { - if let ExprKind::Path(QPath::TypeRelative(ref ty, ref segment)) = func.kind { - if let TyKind::Path(QPath::Resolved(None, ref path)) = ty.kind { - let new_call = segment.ident.name == sym::new; - return is_range_path(&path) && is_lit(sess, &expr.span) && new_call; - } - } - } - - _ => {} - } - - false -} diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index ff6801a85c7e1..71060ed04a385 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1595,6 +1595,68 @@ impl fmt::Debug for Expr { } } +/// Checks if the specified expression is a built-in range literal. +/// (See: `LoweringContext::lower_expr()`). +pub fn is_range_literal(sess: &Session, expr: &hir::Expr) -> bool { + use hir::{Path, QPath, ExprKind, TyKind}; + + // Returns whether the given path represents a (desugared) range, + // either in std or core, i.e. has either a `::std::ops::Range` or + // `::core::ops::Range` prefix. + fn is_range_path(path: &Path) -> bool { + let segs: Vec<_> = path.segments.iter().map(|seg| seg.ident.to_string()).collect(); + let segs: Vec<_> = segs.iter().map(|seg| &**seg).collect(); + + // "{{root}}" is the equivalent of `::` prefix in `Path`. + if let ["{{root}}", std_core, "ops", range] = segs.as_slice() { + (*std_core == "std" || *std_core == "core") && range.starts_with("Range") + } else { + false + } + }; + + // Check whether a span corresponding to a range expression is a + // range literal, rather than an explicit struct or `new()` call. + fn is_lit(sess: &Session, span: &Span) -> bool { + let source_map = sess.source_map(); + let end_point = source_map.end_point(*span); + + if let Ok(end_string) = source_map.span_to_snippet(end_point) { + !(end_string.ends_with("}") || end_string.ends_with(")")) + } else { + false + } + }; + + match expr.kind { + // All built-in range literals but `..=` and `..` desugar to `Struct`s. + ExprKind::Struct(ref qpath, _, _) => { + if let QPath::Resolved(None, ref path) = **qpath { + return is_range_path(&path) && is_lit(sess, &expr.span); + } + } + + // `..` desugars to its struct path. + ExprKind::Path(QPath::Resolved(None, ref path)) => { + return is_range_path(&path) && is_lit(sess, &expr.span); + } + + // `..=` desugars into `::std::ops::RangeInclusive::new(...)`. + ExprKind::Call(ref func, _) => { + if let ExprKind::Path(QPath::TypeRelative(ref ty, ref segment)) = func.kind { + if let TyKind::Path(QPath::Resolved(None, ref path)) = ty.kind { + let new_call = segment.ident.name == sym::new; + return is_range_path(&path) && is_lit(sess, &expr.span) && new_call; + } + } + } + + _ => {} + } + + false +} + #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum ExprKind { /// A `box x` expression. From 80a83eadbcede560a161e37bcdac8b5d2ec846cc Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 22 Dec 2019 20:53:30 +0100 Subject: [PATCH 20/27] is_rnge_literal: fix fallout --- src/librustc/hir/mod.rs | 20 +++++++++----------- src/librustc_lint/types.rs | 8 +++----- src/librustc_typeck/check/demand.rs | 8 +++----- 3 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 71060ed04a385..d3c15db902f98 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -24,9 +24,10 @@ use syntax::ast::{AttrVec, Attribute, Label, LitKind, StrStyle, FloatTy, IntTy, pub use syntax::ast::{Mutability, Constness, Unsafety, Movability, CaptureBy}; pub use syntax::ast::{IsAuto, ImplPolarity, BorrowKind}; use syntax::attr::{InlineAttr, OptimizeAttr}; -use syntax::symbol::{Symbol, kw}; use syntax::tokenstream::TokenStream; use syntax::util::parser::ExprPrecedence; +use syntax_pos::symbol::{Symbol, kw, sym}; +use syntax_pos::source_map::SourceMap; use rustc_target::spec::abi::Abi; use rustc_data_structures::sync::{par_for_each_in, Send, Sync}; use rustc_macros::HashStable; @@ -1597,9 +1598,7 @@ impl fmt::Debug for Expr { /// Checks if the specified expression is a built-in range literal. /// (See: `LoweringContext::lower_expr()`). -pub fn is_range_literal(sess: &Session, expr: &hir::Expr) -> bool { - use hir::{Path, QPath, ExprKind, TyKind}; - +pub fn is_range_literal(sm: &SourceMap, expr: &Expr) -> bool { // Returns whether the given path represents a (desugared) range, // either in std or core, i.e. has either a `::std::ops::Range` or // `::core::ops::Range` prefix. @@ -1617,11 +1616,10 @@ pub fn is_range_literal(sess: &Session, expr: &hir::Expr) -> bool { // Check whether a span corresponding to a range expression is a // range literal, rather than an explicit struct or `new()` call. - fn is_lit(sess: &Session, span: &Span) -> bool { - let source_map = sess.source_map(); - let end_point = source_map.end_point(*span); + fn is_lit(sm: &SourceMap, span: &Span) -> bool { + let end_point = sm.end_point(*span); - if let Ok(end_string) = source_map.span_to_snippet(end_point) { + if let Ok(end_string) = sm.span_to_snippet(end_point) { !(end_string.ends_with("}") || end_string.ends_with(")")) } else { false @@ -1632,13 +1630,13 @@ pub fn is_range_literal(sess: &Session, expr: &hir::Expr) -> bool { // All built-in range literals but `..=` and `..` desugar to `Struct`s. ExprKind::Struct(ref qpath, _, _) => { if let QPath::Resolved(None, ref path) = **qpath { - return is_range_path(&path) && is_lit(sess, &expr.span); + return is_range_path(&path) && is_lit(sm, &expr.span); } } // `..` desugars to its struct path. ExprKind::Path(QPath::Resolved(None, ref path)) => { - return is_range_path(&path) && is_lit(sess, &expr.span); + return is_range_path(&path) && is_lit(sm, &expr.span); } // `..=` desugars into `::std::ops::RangeInclusive::new(...)`. @@ -1646,7 +1644,7 @@ pub fn is_range_literal(sess: &Session, expr: &hir::Expr) -> bool { if let ExprKind::Path(QPath::TypeRelative(ref ty, ref segment)) = func.kind { if let TyKind::Path(QPath::Resolved(None, ref path)) = ty.kind { let new_call = segment.ident.name == sym::new; - return is_range_path(&path) && is_lit(sess, &expr.span) && new_call; + return is_range_path(&path) && is_lit(sm, &expr.span) && new_call; } } } diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index f1cd2037edd7a..20882d642151f 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -1,8 +1,6 @@ #![allow(non_snake_case)] -use rustc::hir::{ExprKind, Node}; -use crate::hir::def_id::DefId; -use rustc::hir::lowering::is_range_literal; +use rustc::hir::{ExprKind, Node, is_range_literal, def_id::DefId}; use rustc::ty::subst::SubstsRef; use rustc::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt}; use rustc::ty::layout::{self, IntegerExt, LayoutOf, VariantIdx, SizeSkeleton}; @@ -275,7 +273,7 @@ fn lint_int_literal<'a, 'tcx>( let par_id = cx.tcx.hir().get_parent_node(e.hir_id); if let Node::Expr(par_e) = cx.tcx.hir().get(par_id) { if let hir::ExprKind::Struct(..) = par_e.kind { - if is_range_literal(cx.sess(), par_e) + if is_range_literal(cx.sess().source_map(), par_e) && lint_overflowing_range_endpoint(cx, lit, v, max, e, par_e, t.name_str()) { // The overflowing literal lint was overridden. @@ -328,7 +326,7 @@ fn lint_uint_literal<'a, 'tcx>( } } hir::ExprKind::Struct(..) - if is_range_literal(cx.sess(), par_e) => { + if is_range_literal(cx.sess().source_map(), par_e) => { let t = t.name_str(); if lint_overflowing_range_endpoint(cx, lit, lit_val, max, e, par_e, t) { // The overflowing literal lint was overridden. diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 1e3ede4c18219..81c2bef74fe5b 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -2,12 +2,10 @@ use crate::check::FnCtxt; use rustc::infer::InferOk; use rustc::traits::{self, ObligationCause, ObligationCauseCode}; -use syntax::symbol::sym; use syntax::util::parser::PREC_POSTFIX; +use syntax_pos::symbol::sym; use syntax_pos::Span; -use rustc::hir; -use rustc::hir::Node; -use rustc::hir::{print, lowering::is_range_literal}; +use rustc::hir::{self, Node, print, is_range_literal}; use rustc::ty::{self, Ty, AssocItem}; use rustc::ty::adjustment::AllowTwoPhase; use errors::{Applicability, DiagnosticBuilder}; @@ -466,7 +464,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { hir::ExprKind::Cast(_, _) | hir::ExprKind::Binary(_, _, _) => true, // parenthesize borrows of range literals (Issue #54505) - _ if is_range_literal(self.tcx.sess, expr) => true, + _ if is_range_literal(self.tcx.sess.source_map(), expr) => true, _ => false, }; let sugg_expr = if needs_parens { From db4cc3bd24779ffcc1d0a0fe2961c947eb58085c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 22 Dec 2019 20:55:27 +0100 Subject: [PATCH 21/27] is_range_literal: leave FIXME --- src/librustc/hir/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index d3c15db902f98..ae7a90d4f5f90 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1598,6 +1598,9 @@ impl fmt::Debug for Expr { /// Checks if the specified expression is a built-in range literal. /// (See: `LoweringContext::lower_expr()`). +/// +/// FIXME(#60607): This function is a hack. If and when we have `QPath::Lang(...)`, +/// we can use that instead as simpler, more reliable mechanism, as opposed to using `SourceMap`. pub fn is_range_literal(sm: &SourceMap, expr: &Expr) -> bool { // Returns whether the given path represents a (desugared) range, // either in std or core, i.e. has either a `::std::ops::Range` or From a2a0bb287725dc9de8c82a303d643baf9ba8e08b Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 22 Dec 2019 20:14:08 +0000 Subject: [PATCH 22/27] Add note about destructuring assignments --- src/librustc_typeck/check/expr.rs | 44 +++++-- src/librustc_typeck/check/op.rs | 18 +-- src/test/ui/bad/bad-expr-lhs.stderr | 3 + src/test/ui/bad/destructuring-assignment.rs | 21 ++++ .../ui/bad/destructuring-assignment.stderr | 111 ++++++++++++++++++ 5 files changed, 177 insertions(+), 20 deletions(-) create mode 100644 src/test/ui/bad/destructuring-assignment.rs create mode 100644 src/test/ui/bad/destructuring-assignment.stderr diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index bc2d19b413cec..3268f8b9a85f1 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -17,7 +17,7 @@ use crate::util::common::ErrorReported; use crate::util::nodemap::FxHashMap; use crate::astconv::AstConv as _; -use errors::{Applicability, DiagnosticBuilder, pluralize}; +use errors::{Applicability, DiagnosticBuilder, DiagnosticId, pluralize}; use syntax_pos::hygiene::DesugaringKind; use syntax::ast; use syntax::symbol::{Symbol, kw, sym}; @@ -761,6 +761,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } + pub(crate) fn check_lhs_assignable( + &self, + lhs: &'tcx hir::Expr, + err_code: &'static str, + expr_span: &Span, + ) { + if !lhs.is_syntactic_place_expr() { + let mut err = self.tcx.sess.struct_span_err_with_code( + *expr_span, + "invalid left-hand side of assignment", + DiagnosticId::Error(err_code.into()), + ); + err.span_label(lhs.span, "cannot assign to this expression"); + let destructuring_assignment = match &lhs.kind { + ExprKind::Array(comps) | ExprKind::Tup(comps) => { + comps.iter().all(|e| e.is_syntactic_place_expr()) + } + ExprKind::Struct(_path, fields, rest) => { + rest.as_ref().map(|e| e.is_syntactic_place_expr()).unwrap_or(true) && + fields.iter().all(|f| f.expr.is_syntactic_place_expr()) + } + _ => false, + }; + if destructuring_assignment { + err.note("destructuring assignments are not yet supported"); + err.note( + "for more information, see https://github.com/rust-lang/rfcs/issues/372", + ); + } + err.emit(); + } + } + /// Type check assignment expression `expr` of form `lhs = rhs`. /// The expected type is `()` and is passsed to the function for the purposes of diagnostics. fn check_expr_assign( @@ -790,13 +823,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.help(msg); } err.emit(); - } else if !lhs.is_syntactic_place_expr() { - struct_span_err!( - self.tcx.sess, - expr.span, - E0070, - "invalid left-hand side of assignment", - ).span_label(lhs.span, "cannot assign to this expression").emit(); + } else { + self.check_lhs_assignable(lhs, "E0070", &expr.span); } self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized); diff --git a/src/librustc_typeck/check/op.rs b/src/librustc_typeck/check/op.rs index f049728c3f235..6601102edec6e 100644 --- a/src/librustc_typeck/check/op.rs +++ b/src/librustc_typeck/check/op.rs @@ -19,28 +19,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, expr: &'tcx hir::Expr, op: hir::BinOp, - lhs_expr: &'tcx hir::Expr, - rhs_expr: &'tcx hir::Expr, + lhs: &'tcx hir::Expr, + rhs: &'tcx hir::Expr, ) -> Ty<'tcx> { let (lhs_ty, rhs_ty, return_ty) = - self.check_overloaded_binop(expr, lhs_expr, rhs_expr, op, IsAssign::Yes); + self.check_overloaded_binop(expr, lhs, rhs, op, IsAssign::Yes); let ty = if !lhs_ty.is_ty_var() && !rhs_ty.is_ty_var() && is_builtin_binop(lhs_ty, rhs_ty, op) { - self.enforce_builtin_binop_types(lhs_expr, lhs_ty, rhs_expr, rhs_ty, op); + self.enforce_builtin_binop_types(lhs, lhs_ty, rhs, rhs_ty, op); self.tcx.mk_unit() } else { return_ty }; - if !lhs_expr.is_syntactic_place_expr() { - struct_span_err!( - self.tcx.sess, - op.span, - E0067, - "invalid left-hand side of assignment", - ).span_label(lhs_expr.span, "cannot assign to this expression").emit(); - } + self.check_lhs_assignable(lhs, "E0067", &op.span); + ty } diff --git a/src/test/ui/bad/bad-expr-lhs.stderr b/src/test/ui/bad/bad-expr-lhs.stderr index 07cffbe97a8d9..61c25bb471c37 100644 --- a/src/test/ui/bad/bad-expr-lhs.stderr +++ b/src/test/ui/bad/bad-expr-lhs.stderr @@ -29,6 +29,9 @@ LL | (a, b) = (3, 4); | ------^^^^^^^^^ | | | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 error[E0070]: invalid left-hand side of assignment --> $DIR/bad-expr-lhs.rs:9:5 diff --git a/src/test/ui/bad/destructuring-assignment.rs b/src/test/ui/bad/destructuring-assignment.rs new file mode 100644 index 0000000000000..7112cedfd0009 --- /dev/null +++ b/src/test/ui/bad/destructuring-assignment.rs @@ -0,0 +1,21 @@ +struct S { x: u8, y: u8 } + +fn main() { + let (a, b) = (1, 2); + + (a, b) = (3, 4); //~ ERROR invalid left-hand side of assignment + (a, b) += (3, 4); //~ ERROR invalid left-hand side of assignment + //~^ ERROR binary assignment operation `+=` cannot be applied + + [a, b] = [3, 4]; //~ ERROR invalid left-hand side of assignment + [a, b] += [3, 4]; //~ ERROR invalid left-hand side of assignment + //~^ ERROR binary assignment operation `+=` cannot be applied + + let s = S { x: 3, y: 4 }; + + S { x: a, y: b } = s; //~ ERROR invalid left-hand side of assignment + S { x: a, y: b } += s; //~ ERROR invalid left-hand side of assignment + //~^ ERROR binary assignment operation `+=` cannot be applied + + S { x: a, ..s } = S { x: 3, y: 4 }; //~ ERROR invalid left-hand side of assignment +} diff --git a/src/test/ui/bad/destructuring-assignment.stderr b/src/test/ui/bad/destructuring-assignment.stderr new file mode 100644 index 0000000000000..676576b7bc526 --- /dev/null +++ b/src/test/ui/bad/destructuring-assignment.stderr @@ -0,0 +1,111 @@ +error[E0070]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:6:5 + | +LL | (a, b) = (3, 4); + | ------^^^^^^^^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error[E0368]: binary assignment operation `+=` cannot be applied to type `({integer}, {integer})` + --> $DIR/destructuring-assignment.rs:7:5 + | +LL | (a, b) += (3, 4); + | ------^^^^^^^^^^ + | | + | cannot use `+=` on type `({integer}, {integer})` + | + = note: an implementation of `std::ops::AddAssign` might be missing for `({integer}, {integer})` + +error[E0067]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:7:12 + | +LL | (a, b) += (3, 4); + | ------ ^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error[E0070]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:10:5 + | +LL | [a, b] = [3, 4]; + | ------^^^^^^^^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error[E0368]: binary assignment operation `+=` cannot be applied to type `[{integer}; 2]` + --> $DIR/destructuring-assignment.rs:11:5 + | +LL | [a, b] += [3, 4]; + | ------^^^^^^^^^^ + | | + | cannot use `+=` on type `[{integer}; 2]` + | + = note: an implementation of `std::ops::AddAssign` might be missing for `[{integer}; 2]` + +error[E0067]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:11:12 + | +LL | [a, b] += [3, 4]; + | ------ ^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error[E0070]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:16:5 + | +LL | S { x: a, y: b } = s; + | ----------------^^^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error[E0368]: binary assignment operation `+=` cannot be applied to type `S` + --> $DIR/destructuring-assignment.rs:17:5 + | +LL | S { x: a, y: b } += s; + | ----------------^^^^^ + | | + | cannot use `+=` on type `S` + | + = note: an implementation of `std::ops::AddAssign` might be missing for `S` + +error[E0067]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:17:22 + | +LL | S { x: a, y: b } += s; + | ---------------- ^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error[E0070]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:20:5 + | +LL | S { x: a, ..s } = S { x: 3, y: 4 }; + | ---------------^^^^^^^^^^^^^^^^^^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error: aborting due to 10 previous errors + +Some errors have detailed explanations: E0067, E0070, E0368. +For more information about an error, try `rustc --explain E0067`. From c74083910ecaadb88eb3324d338e20f469b7beec Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 22 Dec 2019 20:27:42 +0000 Subject: [PATCH 23/27] Recognise nested tuples/arrays/structs --- src/librustc_typeck/check/expr.rs | 25 +++++++++++-------- src/test/ui/bad/destructuring-assignment.rs | 4 +++ .../ui/bad/destructuring-assignment.stderr | 13 +++++++++- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index 3268f8b9a85f1..f6262a0ae9788 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -761,6 +761,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); } + fn is_destructuring_place_expr(&self, expr: &'tcx hir::Expr) -> bool { + match &expr.kind { + ExprKind::Array(comps) | ExprKind::Tup(comps) => { + comps.iter().all(|e| self.is_destructuring_place_expr(e)) + } + ExprKind::Struct(_path, fields, rest) => { + rest.as_ref().map(|e| self.is_destructuring_place_expr(e)).unwrap_or(true) && + fields.iter().all(|f| self.is_destructuring_place_expr(&f.expr)) + } + _ => expr.is_syntactic_place_expr(), + } + } + pub(crate) fn check_lhs_assignable( &self, lhs: &'tcx hir::Expr, @@ -774,17 +787,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { DiagnosticId::Error(err_code.into()), ); err.span_label(lhs.span, "cannot assign to this expression"); - let destructuring_assignment = match &lhs.kind { - ExprKind::Array(comps) | ExprKind::Tup(comps) => { - comps.iter().all(|e| e.is_syntactic_place_expr()) - } - ExprKind::Struct(_path, fields, rest) => { - rest.as_ref().map(|e| e.is_syntactic_place_expr()).unwrap_or(true) && - fields.iter().all(|f| f.expr.is_syntactic_place_expr()) - } - _ => false, - }; - if destructuring_assignment { + if self.is_destructuring_place_expr(lhs) { err.note("destructuring assignments are not yet supported"); err.note( "for more information, see https://github.com/rust-lang/rfcs/issues/372", diff --git a/src/test/ui/bad/destructuring-assignment.rs b/src/test/ui/bad/destructuring-assignment.rs index 7112cedfd0009..876c9efea2647 100644 --- a/src/test/ui/bad/destructuring-assignment.rs +++ b/src/test/ui/bad/destructuring-assignment.rs @@ -18,4 +18,8 @@ fn main() { //~^ ERROR binary assignment operation `+=` cannot be applied S { x: a, ..s } = S { x: 3, y: 4 }; //~ ERROR invalid left-hand side of assignment + + let c = 3; + + ((a, b), c) = ((3, 4), 5); //~ ERROR invalid left-hand side of assignment } diff --git a/src/test/ui/bad/destructuring-assignment.stderr b/src/test/ui/bad/destructuring-assignment.stderr index 676576b7bc526..845008b06937c 100644 --- a/src/test/ui/bad/destructuring-assignment.stderr +++ b/src/test/ui/bad/destructuring-assignment.stderr @@ -105,7 +105,18 @@ LL | S { x: a, ..s } = S { x: 3, y: 4 }; = note: destructuring assignments are not yet supported = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 -error: aborting due to 10 previous errors +error[E0070]: invalid left-hand side of assignment + --> $DIR/destructuring-assignment.rs:24:5 + | +LL | ((a, b), c) = ((3, 4), 5); + | -----------^^^^^^^^^^^^^^ + | | + | cannot assign to this expression + | + = note: destructuring assignments are not yet supported + = note: for more information, see https://github.com/rust-lang/rfcs/issues/372 + +error: aborting due to 11 previous errors Some errors have detailed explanations: E0067, E0070, E0368. For more information about an error, try `rustc --explain E0067`. From ddd45009150cc09d0ecfc160efffe92885c70463 Mon Sep 17 00:00:00 2001 From: varkor Date: Sun, 22 Dec 2019 21:08:53 +0000 Subject: [PATCH 24/27] Add span information to `ExprKind::Assign` --- src/librustc/hir/intravisit.rs | 6 +++--- src/librustc/hir/lowering/expr.rs | 6 +++--- src/librustc/hir/mod.rs | 2 +- src/librustc/hir/print.rs | 4 ++-- src/librustc_lint/unused.rs | 2 +- src/librustc_mir/hair/cx/expr.rs | 2 +- src/librustc_parse/parser/expr.rs | 4 +++- src/librustc_passes/liveness.rs | 4 ++-- src/librustc_privacy/lib.rs | 2 +- src/librustc_typeck/check/demand.rs | 2 +- src/librustc_typeck/check/expr.rs | 9 +++++---- src/librustc_typeck/expr_use_visitor.rs | 2 +- src/libsyntax/ast.rs | 2 +- src/libsyntax/mut_visit.rs | 2 +- src/libsyntax/print/pprust.rs | 2 +- src/libsyntax/util/parser.rs | 2 +- src/libsyntax/visit.rs | 6 +++--- src/test/ui-fulldeps/pprust-expr-roundtrip.rs | 4 ++-- src/test/ui/bad/bad-expr-lhs.stderr | 16 +++++++-------- .../ui/bad/destructuring-assignment.stderr | 20 +++++++++---------- src/test/ui/error-codes/E0070.stderr | 12 +++++------ src/test/ui/issues/issue-13407.stderr | 4 ++-- src/test/ui/issues/issue-26093.stderr | 4 ++-- src/test/ui/issues/issue-34334.stderr | 4 ++-- .../assignment-expected-bool.stderr | 4 ++-- 25 files changed, 65 insertions(+), 62 deletions(-) diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 26e287037f72c..cf2971cd8f1cf 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -1056,9 +1056,9 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) { walk_list!(visitor, visit_label, opt_label); visitor.visit_block(block); } - ExprKind::Assign(ref left_hand_expression, ref right_hand_expression) => { - visitor.visit_expr(right_hand_expression); - visitor.visit_expr(left_hand_expression) + ExprKind::Assign(ref lhs, ref rhs, _) => { + visitor.visit_expr(rhs); + visitor.visit_expr(lhs) } ExprKind::AssignOp(_, ref left_expression, ref right_expression) => { visitor.visit_expr(right_expression); diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index 08b00ce69addb..cfcc1f3d3bcff 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -112,8 +112,8 @@ impl LoweringContext<'_, '_> { opt_label.is_some()), self.lower_label(opt_label)) } - ExprKind::Assign(ref el, ref er) => { - hir::ExprKind::Assign(P(self.lower_expr(el)), P(self.lower_expr(er))) + ExprKind::Assign(ref el, ref er, span) => { + hir::ExprKind::Assign(P(self.lower_expr(el)), P(self.lower_expr(er)), span) } ExprKind::AssignOp(op, ref el, ref er) => hir::ExprKind::AssignOp( self.lower_binop(op), @@ -1084,7 +1084,7 @@ impl LoweringContext<'_, '_> { let next_expr = P(self.expr_ident(pat.span, next_ident, next_pat_hid)); let assign = P(self.expr( pat.span, - hir::ExprKind::Assign(next_expr, val_expr), + hir::ExprKind::Assign(next_expr, val_expr, pat.span), ThinVec::new(), )); let some_pat = self.pat_some(pat.span, val_pat); diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index ff6801a85c7e1..3d490fcc9605d 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1659,7 +1659,7 @@ pub enum ExprKind { Block(P, Option