From 877c5e9554c82503a590a2ca68e6300da300fe53 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Fri, 15 Mar 2019 10:26:09 -0700 Subject: [PATCH 01/24] [BETA] Update cargo --- src/tools/cargo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/cargo b/src/tools/cargo index 5c6aa46e6f286..436f2902fd59e 160000 --- a/src/tools/cargo +++ b/src/tools/cargo @@ -1 +1 @@ -Subproject commit 5c6aa46e6f28661270979696e7b4c2f0dff8628f +Subproject commit 436f2902fd59e580c730c76c65b7d74222f8c304 From b72af83e89115c9ff913c30ded6a4ce7f974640b Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 17 Jan 2019 20:03:58 +0000 Subject: [PATCH 02/24] Include bounds from promoted constants in NLL Previously, a promoted that contains a function item wouldn't have the function items bounds propagated to the main function body. --- .../borrow_check/nll/region_infer/values.rs | 4 +- src/librustc_mir/borrow_check/nll/renumber.rs | 8 ++ .../borrow_check/nll/type_check/mod.rs | 114 +++++++++++++++--- src/test/ui/nll/issue-48697.rs | 4 +- src/test/ui/nll/issue-48697.stderr | 11 ++ src/test/ui/nll/promoted-bounds.rs | 27 +++++ src/test/ui/nll/promoted-bounds.stderr | 15 +++ src/test/ui/nll/promoted-closure-pair.rs | 12 ++ src/test/ui/nll/promoted-closure-pair.stderr | 12 ++ 9 files changed, 185 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/nll/issue-48697.stderr create mode 100644 src/test/ui/nll/promoted-bounds.rs create mode 100644 src/test/ui/nll/promoted-bounds.stderr create mode 100644 src/test/ui/nll/promoted-closure-pair.rs create mode 100644 src/test/ui/nll/promoted-closure-pair.stderr diff --git a/src/librustc_mir/borrow_check/nll/region_infer/values.rs b/src/librustc_mir/borrow_check/nll/region_infer/values.rs index ef27fdbde387f..c4491778162f4 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/values.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/values.rs @@ -154,10 +154,10 @@ impl LivenessValues { /// Creates a new set of "region values" that tracks causal information. /// Each of the regions in num_region_variables will be initialized with an /// empty set of points and no causal information. - crate fn new(elements: &Rc) -> Self { + crate fn new(elements: Rc) -> Self { Self { - elements: elements.clone(), points: SparseBitMatrix::new(elements.num_points), + elements: elements, } } diff --git a/src/librustc_mir/borrow_check/nll/renumber.rs b/src/librustc_mir/borrow_check/nll/renumber.rs index e6a974fd8cc94..caf98b31a757c 100644 --- a/src/librustc_mir/borrow_check/nll/renumber.rs +++ b/src/librustc_mir/borrow_check/nll/renumber.rs @@ -47,6 +47,14 @@ impl<'a, 'gcx, 'tcx> NLLVisitor<'a, 'gcx, 'tcx> { } impl<'a, 'gcx, 'tcx> MutVisitor<'tcx> for NLLVisitor<'a, 'gcx, 'tcx> { + fn visit_mir(&mut self, mir: &mut Mir<'tcx>) { + for promoted in mir.promoted.iter_mut() { + self.visit_mir(promoted); + } + + self.super_mir(mir); + } + fn visit_ty(&mut self, ty: &mut Ty<'tcx>, ty_context: TyContext) { debug!("visit_ty(ty={:?}, ty_context={:?})", ty, ty_context); diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 49f90eb90aaf0..08445be138e5e 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -45,7 +45,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::indexed_vec::{IndexVec, Idx}; use rustc::ty::layout::VariantIdx; use std::rc::Rc; -use std::{fmt, iter}; +use std::{fmt, iter, mem}; use syntax_pos::{Span, DUMMY_SP}; macro_rules! span_mirbug { @@ -124,7 +124,7 @@ pub(crate) fn type_check<'gcx, 'tcx>( let mut constraints = MirTypeckRegionConstraints { placeholder_indices: PlaceholderIndices::default(), placeholder_index_to_region: IndexVec::default(), - liveness_constraints: LivenessValues::new(elements), + liveness_constraints: LivenessValues::new(elements.clone()), outlives_constraints: ConstraintSet::default(), closure_bounds_mapping: Default::default(), type_tests: Vec::default(), @@ -253,7 +253,7 @@ enum FieldAccessError { /// is a problem. struct TypeVerifier<'a, 'b: 'a, 'gcx: 'tcx, 'tcx: 'b> { cx: &'a mut TypeChecker<'b, 'gcx, 'tcx>, - mir: &'a Mir<'tcx>, + mir: &'b Mir<'tcx>, last_span: Span, mir_def_id: DefId, errors_reported: bool, @@ -385,7 +385,7 @@ impl<'a, 'b, 'gcx, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'gcx, 'tcx> { } impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { - fn new(cx: &'a mut TypeChecker<'b, 'gcx, 'tcx>, mir: &'a Mir<'tcx>) -> Self { + fn new(cx: &'a mut TypeChecker<'b, 'gcx, 'tcx>, mir: &'b Mir<'tcx>) -> Self { TypeVerifier { mir, mir_def_id: cx.mir_def_id, @@ -454,19 +454,31 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { Place::Local(index) => PlaceTy::Ty { ty: self.mir.local_decls[index].ty, }, - Place::Promoted(box (_index, sty)) => { + Place::Promoted(box (index, sty)) => { let sty = self.sanitize_type(place, sty); - // FIXME -- promoted MIR return types reference - // various "free regions" (e.g., scopes and things) - // that they ought not to do. We have to figure out - // how best to handle that -- probably we want treat - // promoted MIR much like closures, renumbering all - // their free regions and propagating constraints - // upwards. We have the same acyclic guarantees, so - // that should be possible. But for now, ignore them. - // - // let promoted_mir = &self.mir.promoted[index]; - // promoted_mir.return_ty() + + if !self.errors_reported { + let promoted_mir = &self.mir.promoted[index]; + self.sanitize_promoted(promoted_mir, location); + + let promoted_ty = promoted_mir.return_ty(); + + if let Err(terr) = self.cx.eq_types( + sty, + promoted_ty, + location.to_locations(), + ConstraintCategory::Boring, + ) { + span_mirbug!( + self, + place, + "bad promoted type ({:?}: {:?}): {:?}", + promoted_ty, + sty, + terr + ); + }; + } PlaceTy::Ty { ty: sty } } Place::Static(box Static { def_id, ty: sty }) => { @@ -533,6 +545,74 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { place_ty } + fn sanitize_promoted(&mut self, promoted_mir: &'b Mir<'tcx>, location: Location) { + // Determine the constraints from the promoted MIR by running the type + // checker on the promoted MIR, then transfer the constraints back to + // the main MIR, changing the locations to the provided location. + + let main_mir = mem::replace(&mut self.mir, promoted_mir); + self.cx.mir = promoted_mir; + + let all_facts = &mut None; + let mut constraints = Default::default(); + let mut closure_bounds = Default::default(); + if let Some(ref mut bcx) = self.cx.borrowck_context { + // Don't try to add borrow_region facts for the promoted MIR + mem::swap(bcx.all_facts, all_facts); + + // Use a new sets of constraints and closure bounds so that we can + // modify their locations. + mem::swap(&mut bcx.constraints.outlives_constraints, &mut constraints); + mem::swap(&mut bcx.constraints.closure_bounds_mapping, &mut closure_bounds); + }; + + self.visit_mir(promoted_mir); + + if !self.errors_reported { + // if verifier failed, don't do further checks to avoid ICEs + self.cx.typeck_mir(promoted_mir); + } + + self.mir = main_mir; + self.cx.mir = main_mir; + // Merge the outlives constraints back in, at the given location. + if let Some(ref mut base_bcx) = self.cx.borrowck_context { + mem::swap(base_bcx.all_facts, all_facts); + mem::swap(&mut base_bcx.constraints.outlives_constraints, &mut constraints); + mem::swap(&mut base_bcx.constraints.closure_bounds_mapping, &mut closure_bounds); + + let locations = location.to_locations(); + for constraint in constraints.iter() { + let mut constraint = *constraint; + constraint.locations = locations; + if let ConstraintCategory::Return + | ConstraintCategory::UseAsConst + | ConstraintCategory::UseAsStatic = constraint.category + { + // "Returning" from a promoted is an assigment to a + // temporary from the user's point of view. + constraint.category = ConstraintCategory::Boring; + } + base_bcx.constraints.outlives_constraints.push(constraint) + } + + if !closure_bounds.is_empty() { + let combined_bounds_mapping = closure_bounds + .into_iter() + .flat_map(|(_, value)| value) + .collect(); + let existing = base_bcx + .constraints + .closure_bounds_mapping + .insert(location, combined_bounds_mapping); + assert!( + existing.is_none(), + "Multiple promoteds/closures at the same location." + ); + } + } + } + fn sanitize_projection( &mut self, base: PlaceTy<'tcx>, @@ -2266,7 +2346,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { ) -> ty::InstantiatedPredicates<'tcx> { if let Some(closure_region_requirements) = tcx.mir_borrowck(def_id).closure_requirements { let closure_constraints = - closure_region_requirements.apply_requirements(tcx, location, def_id, substs); + closure_region_requirements.apply_requirements(tcx, def_id, substs); if let Some(ref mut borrowck_context) = self.borrowck_context { let bounds_mapping = closure_constraints diff --git a/src/test/ui/nll/issue-48697.rs b/src/test/ui/nll/issue-48697.rs index c60c265b813b8..ececd6fccd84b 100644 --- a/src/test/ui/nll/issue-48697.rs +++ b/src/test/ui/nll/issue-48697.rs @@ -1,14 +1,12 @@ // Regression test for #48697 -// compile-pass - #![feature(nll)] fn foo(x: &i32) -> &i32 { let z = 4; let f = &|y| y; let k = f(&z); - f(x) + f(x) //~ cannot return value referencing local variable } fn main() {} diff --git a/src/test/ui/nll/issue-48697.stderr b/src/test/ui/nll/issue-48697.stderr new file mode 100644 index 0000000000000..16b46c15b90b6 --- /dev/null +++ b/src/test/ui/nll/issue-48697.stderr @@ -0,0 +1,11 @@ +error[E0515]: cannot return value referencing local variable `z` + --> $DIR/issue-48697.rs:9:5 + | +LL | let k = f(&z); + | -- `z` is borrowed here +LL | f(x) //~ cannot return value referencing local variable + | ^^^^ returns a value referencing data owned by the current function + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0515`. diff --git a/src/test/ui/nll/promoted-bounds.rs b/src/test/ui/nll/promoted-bounds.rs new file mode 100644 index 0000000000000..59b21cf9ac2a9 --- /dev/null +++ b/src/test/ui/nll/promoted-bounds.rs @@ -0,0 +1,27 @@ +#![feature(nll)] + +fn shorten_lifetime<'a, 'b, 'min>(a: &'a i32, b: &'b i32) -> &'min i32 +where + 'a: 'min, + 'b: 'min, +{ + if *a < *b { + &a + } else { + &b + } +} + +fn main() { + let promoted_fn_item_ref = &shorten_lifetime; + + let a = &5; + let ptr = { + let l = 3; + let b = &l; //~ ERROR does not live long enough + let c = promoted_fn_item_ref(a, b); + c + }; + + println!("ptr = {:?}", ptr); +} diff --git a/src/test/ui/nll/promoted-bounds.stderr b/src/test/ui/nll/promoted-bounds.stderr new file mode 100644 index 0000000000000..9798f238fc4d0 --- /dev/null +++ b/src/test/ui/nll/promoted-bounds.stderr @@ -0,0 +1,15 @@ +error[E0597]: `l` does not live long enough + --> $DIR/promoted-bounds.rs:21:17 + | +LL | let ptr = { + | --- borrow later stored here +LL | let l = 3; +LL | let b = &l; //~ ERROR does not live long enough + | ^^ borrowed value does not live long enough +... +LL | }; + | - `l` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/promoted-closure-pair.rs b/src/test/ui/nll/promoted-closure-pair.rs new file mode 100644 index 0000000000000..7b3bbad4c1e0c --- /dev/null +++ b/src/test/ui/nll/promoted-closure-pair.rs @@ -0,0 +1,12 @@ +// Check that we handle multiple closures in the same promoted constant. + +#![feature(nll)] + +fn foo() -> &'static i32 { + let z = 0; + let p = &(|y| y, |y| y); + p.0(&z); + p.1(&z) //~ ERROR cannot return +} + +fn main() {} diff --git a/src/test/ui/nll/promoted-closure-pair.stderr b/src/test/ui/nll/promoted-closure-pair.stderr new file mode 100644 index 0000000000000..5f4a6037668b5 --- /dev/null +++ b/src/test/ui/nll/promoted-closure-pair.stderr @@ -0,0 +1,12 @@ +error[E0515]: cannot return value referencing local variable `z` + --> $DIR/promoted-closure-pair.rs:9:5 + | +LL | p.1(&z) //~ ERROR cannot return + | ^^^^--^ + | | | + | | `z` is borrowed here + | returns a value referencing data owned by the current function + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0515`. From 1824067e84b8f9df53470bd46cea81b283707f59 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 17 Jan 2019 20:04:09 +0000 Subject: [PATCH 03/24] Remove unnecessary parameter --- src/librustc_mir/borrow_check/nll/region_infer/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index cbeb5dc206ee6..695af64c4b84c 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -1357,7 +1357,6 @@ pub trait ClosureRegionRequirementsExt<'gcx, 'tcx> { fn apply_requirements( &self, tcx: TyCtxt<'_, 'gcx, 'tcx>, - location: Location, closure_def_id: DefId, closure_substs: &'tcx ty::subst::Substs<'tcx>, ) -> Vec>; @@ -1388,13 +1387,12 @@ impl<'gcx, 'tcx> ClosureRegionRequirementsExt<'gcx, 'tcx> for ClosureRegionRequi fn apply_requirements( &self, tcx: TyCtxt<'_, 'gcx, 'tcx>, - location: Location, closure_def_id: DefId, closure_substs: &'tcx ty::subst::Substs<'tcx>, ) -> Vec> { debug!( - "apply_requirements(location={:?}, closure_def_id={:?}, closure_substs={:?})", - location, closure_def_id, closure_substs + "apply_requirements(closure_def_id={:?}, closure_substs={:?})", + closure_def_id, closure_substs ); // Extract the values of the free regions in `closure_substs` From e2a0a83b14c2fb183bf6d8895ad04d631008c556 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 27 Feb 2019 22:21:49 +0000 Subject: [PATCH 04/24] Handle type annotations in promoted MIR correctly Type annotations are shared between the MIR of a function and the promoted constants for that function, so keep them in the type checker when we check the promoted MIR. --- .../borrow_check/nll/type_check/mod.rs | 32 ++++++++++--------- .../user-annotations/promoted-annotation.rs | 12 +++++++ .../promoted-annotation.stderr | 17 ++++++++++ 3 files changed, 46 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/nll/user-annotations/promoted-annotation.rs create mode 100644 src/test/ui/nll/user-annotations/promoted-annotation.stderr diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 08445be138e5e..7788d95f3bf79 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -39,7 +39,8 @@ use rustc::ty::fold::TypeFoldable; use rustc::ty::subst::{Subst, Substs, UnpackedKind, UserSubsts}; use rustc::ty::{ self, RegionVid, ToPolyTraitRef, Ty, TyCtxt, TyKind, UserType, - CanonicalUserTypeAnnotation, UserTypeAnnotationIndex, + CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, + UserTypeAnnotationIndex, }; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::indexed_vec::{IndexVec, Idx}; @@ -283,7 +284,7 @@ impl<'a, 'b, 'gcx, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'gcx, 'tcx> { location.to_locations(), ConstraintCategory::Boring, ) { - let annotation = &self.mir.user_type_annotations[annotation_index]; + let annotation = &self.cx.user_type_annotations[annotation_index]; span_mirbug!( self, constant, @@ -550,8 +551,7 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { // checker on the promoted MIR, then transfer the constraints back to // the main MIR, changing the locations to the provided location. - let main_mir = mem::replace(&mut self.mir, promoted_mir); - self.cx.mir = promoted_mir; + let parent_mir = mem::replace(&mut self.mir, promoted_mir); let all_facts = &mut None; let mut constraints = Default::default(); @@ -573,8 +573,7 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> { self.cx.typeck_mir(promoted_mir); } - self.mir = main_mir; - self.cx.mir = main_mir; + self.mir = parent_mir; // Merge the outlives constraints back in, at the given location. if let Some(ref mut base_bcx) = self.cx.borrowck_context { mem::swap(base_bcx.all_facts, all_facts); @@ -818,7 +817,9 @@ struct TypeChecker<'a, 'gcx: 'tcx, 'tcx: 'a> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, param_env: ty::ParamEnv<'gcx>, last_span: Span, - mir: &'a Mir<'tcx>, + /// User type annotations are shared between the main MIR and the MIR of + /// all of the promoted items. + user_type_annotations: &'a CanonicalUserTypeAnnotations<'tcx>, mir_def_id: DefId, region_bound_pairs: &'a RegionBoundPairs<'tcx>, implicit_region_bound: Option>, @@ -973,8 +974,8 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { let mut checker = Self { infcx, last_span: DUMMY_SP, - mir, mir_def_id, + user_type_annotations: &mir.user_type_annotations, param_env, region_bound_pairs, implicit_region_bound, @@ -990,9 +991,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { fn check_user_type_annotations(&mut self) { debug!( "check_user_type_annotations: user_type_annotations={:?}", - self.mir.user_type_annotations + self.user_type_annotations ); - for user_annotation in &self.mir.user_type_annotations { + for user_annotation in self.user_type_annotations { let CanonicalUserTypeAnnotation { span, ref user_ty, inferred_ty } = *user_annotation; let (annotation, _) = self.infcx.instantiate_canonical_with_fresh_inference_vars( span, user_ty @@ -1175,7 +1176,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { a, v, user_ty, locations, ); - let annotated_type = self.mir.user_type_annotations[user_ty.base].inferred_ty; + let annotated_type = self.user_type_annotations[user_ty.base].inferred_ty; let mut curr_projected_ty = PlaceTy::from_ty(annotated_type); let tcx = self.infcx.tcx; @@ -1360,7 +1361,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { location.to_locations(), ConstraintCategory::Boring, ) { - let annotation = &mir.user_type_annotations[annotation_index]; + let annotation = &self.user_type_annotations[annotation_index]; span_mirbug!( self, stmt, @@ -1419,7 +1420,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { Locations::All(stmt.source_info.span), ConstraintCategory::TypeAnnotation, ) { - let annotation = &mir.user_type_annotations[projection.base]; + let annotation = &self.user_type_annotations[projection.base]; span_mirbug!( self, stmt, @@ -2069,7 +2070,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } Rvalue::Ref(region, _borrow_kind, borrowed_place) => { - self.add_reborrow_constraint(location, region, borrowed_place); + self.add_reborrow_constraint(mir, location, region, borrowed_place); } // FIXME: These other cases have to be implemented in future PRs @@ -2168,6 +2169,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { /// - `borrowed_place`: the place `P` being borrowed fn add_reborrow_constraint( &mut self, + mir: &Mir<'tcx>, location: Location, borrow_region: ty::Region<'tcx>, borrowed_place: &Place<'tcx>, @@ -2217,7 +2219,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { match *elem { ProjectionElem::Deref => { let tcx = self.infcx.tcx; - let base_ty = base.ty(self.mir, tcx).to_ty(tcx); + let base_ty = base.ty(mir, tcx).to_ty(tcx); debug!("add_reborrow_constraint - base_ty = {:?}", base_ty); match base_ty.sty { diff --git a/src/test/ui/nll/user-annotations/promoted-annotation.rs b/src/test/ui/nll/user-annotations/promoted-annotation.rs new file mode 100644 index 0000000000000..fa2d2fb81183d --- /dev/null +++ b/src/test/ui/nll/user-annotations/promoted-annotation.rs @@ -0,0 +1,12 @@ +// Test that type annotations are checked in promoted constants correctly. + +#![feature(nll)] + +fn foo<'a>() { + let x = 0; + let f = &drop::<&'a i32>; + f(&x); + //~^ ERROR `x` does not live long enough +} + +fn main() {} diff --git a/src/test/ui/nll/user-annotations/promoted-annotation.stderr b/src/test/ui/nll/user-annotations/promoted-annotation.stderr new file mode 100644 index 0000000000000..144af1e0ec120 --- /dev/null +++ b/src/test/ui/nll/user-annotations/promoted-annotation.stderr @@ -0,0 +1,17 @@ +error[E0597]: `x` does not live long enough + --> $DIR/promoted-annotation.rs:8:7 + | +LL | fn foo<'a>() { + | -- lifetime `'a` defined here +LL | let x = 0; +LL | let f = &drop::<&'a i32>; + | ---------------- assignment requires that `x` is borrowed for `'a` +LL | f(&x); + | ^^ borrowed value does not live long enough +LL | //~^ ERROR `x` does not live long enough +LL | } + | - `x` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. From 11bd9980f068995f3110d2b84d1d919ad61042d6 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 20 Feb 2019 22:24:32 +0100 Subject: [PATCH 05/24] Temporarily emulate the (accidentally) omitted recursion during impl Trait check. Note that the two previous visitors were omitting slightly different recursive calls, so I need two flags to properly emulate them. --- src/librustc/lint/builtin.rs | 14 +++++ src/librustc_lint/lib.rs | 5 ++ src/librustc_passes/ast_validation.rs | 80 ++++++++++++++++++++++----- 3 files changed, 84 insertions(+), 15 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 655707ff9bd0d..ad0ed39185c1c 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -386,6 +386,12 @@ declare_lint! { "ambiguous associated items" } +declare_lint! { + pub NESTED_IMPL_TRAIT, + Warn, + "nested occurrence of `impl Trait` type" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// that are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -457,6 +463,7 @@ impl LintPass for HardwiredLints { parser::ILL_FORMED_ATTRIBUTE_INPUT, DEPRECATED_IN_FUTURE, AMBIGUOUS_ASSOCIATED_ITEMS, + NESTED_IMPL_TRAIT, ) } } @@ -474,6 +481,7 @@ pub enum BuiltinLintDiagnostics { ElidedLifetimesInPaths(usize, Span, bool, Span, String), UnknownCrateTypes(Span, String, String), UnusedImports(String, Vec<(Span, String)>), + NestedImplTrait { outer_impl_trait_span: Span, inner_impl_trait_span: Span }, } impl BuiltinLintDiagnostics { @@ -564,6 +572,12 @@ impl BuiltinLintDiagnostics { ); } } + BuiltinLintDiagnostics::NestedImplTrait { + outer_impl_trait_span, inner_impl_trait_span + } => { + db.span_label(outer_impl_trait_span, "outer `impl Trait`"); + db.span_label(inner_impl_trait_span, "nested `impl Trait` here"); + } } } } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 5c243e1389073..5634faff00e6a 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -353,6 +353,11 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { reference: "issue #57593 ", edition: None, }, + FutureIncompatibleInfo { + id: LintId::of(NESTED_IMPL_TRAIT), + reference: "issue #59014 ", + edition: None, + }, ]); // Register renamed and removed lints. diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 606ae27412832..c615d34b0feae 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -9,6 +9,7 @@ use std::mem; use syntax::print::pprust; use rustc::lint; +use rustc::lint::builtin::{BuiltinLintDiagnostics, NESTED_IMPL_TRAIT}; use rustc::session::Session; use rustc_data_structures::fx::FxHashMap; use syntax::ast::*; @@ -36,9 +37,32 @@ struct AstValidator<'a> { // Used to ban `impl Trait` in path projections like `::Item` // or `Foo::Bar` is_impl_trait_banned: bool, + + // rust-lang/rust#57979: the ban of nested `impl Trait` was buggy + // until sometime after PR #57730 landed: it would jump directly + // to walk_ty rather than visit_ty (or skip recurring entirely for + // impl trait in projections), and thus miss some cases. We track + // whether we should downgrade to a warning for short-term via + // these booleans. + warning_period_57979_nested_impl_trait: bool, + warning_period_57979_impl_trait_in_proj: bool, } impl<'a> AstValidator<'a> { + fn with_nested_impl_trait_warning(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T { + let old = mem::replace(&mut self.warning_period_57979_nested_impl_trait, v); + let ret = f(self); + self.warning_period_57979_nested_impl_trait = old; + ret + } + + fn with_impl_trait_in_proj_warning(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T { + let old = mem::replace(&mut self.warning_period_57979_impl_trait_in_proj, v); + let ret = f(self); + self.warning_period_57979_impl_trait_in_proj = old; + ret + } + fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) { let old = mem::replace(&mut self.is_impl_trait_banned, true); f(self); @@ -406,22 +430,41 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } TyKind::ImplTrait(_, ref bounds) => { if self.is_impl_trait_banned { - struct_span_err!(self.session, ty.span, E0667, - "`impl Trait` is not allowed in path parameters").emit(); + if self.warning_period_57979_impl_trait_in_proj { + self.session.buffer_lint( + NESTED_IMPL_TRAIT, ty.id, ty.span, + "`impl Trait` is not allowed in path parameters"); + } else { + struct_span_err!(self.session, ty.span, E0667, + "`impl Trait` is not allowed in path parameters").emit(); + } } if let Some(outer_impl_trait) = self.outer_impl_trait { - struct_span_err!(self.session, ty.span, E0666, - "nested `impl Trait` is not allowed") - .span_label(outer_impl_trait, "outer `impl Trait`") - .span_label(ty.span, "nested `impl Trait` here") - .emit(); - + if self.warning_period_57979_nested_impl_trait { + self.session.buffer_lint_with_diagnostic( + NESTED_IMPL_TRAIT, ty.id, ty.span, + "nested `impl Trait` is not allowed", + BuiltinLintDiagnostics::NestedImplTrait { + outer_impl_trait_span: outer_impl_trait, + inner_impl_trait_span: ty.span, + }); + } else { + struct_span_err!(self.session, ty.span, E0666, + "nested `impl Trait` is not allowed") + .span_label(outer_impl_trait, "outer `impl Trait`") + .span_label(ty.span, "nested `impl Trait` here") + .emit(); + } } + if !bounds.iter() .any(|b| if let GenericBound::Trait(..) = *b { true } else { false }) { self.err_handler().span_err(ty.span, "at least one trait must be specified"); } + + self.with_impl_trait_in_proj_warning(true, |this| this.walk_ty(ty)); + return; } _ => {} } @@ -605,18 +648,23 @@ impl<'a> Visitor<'a> for AstValidator<'a> { GenericArg::Const(..) => ParamKindOrd::Const, }, arg.span(), None) }), GenericPosition::Arg, generic_args.span()); - // Type bindings such as `Item=impl Debug` in `Iterator` - // are allowed to contain nested `impl Trait`. - self.with_impl_trait(None, |this| { - walk_list!(this, visit_assoc_type_binding, &data.bindings); + + self.with_nested_impl_trait_warning(true, |this| { + // Type bindings such as `Item=impl Debug` in `Iterator` + // are allowed to contain nested `impl Trait`. + this.with_impl_trait(None, |this| { + walk_list!(this, visit_assoc_type_binding, &data.bindings); + }); }); } GenericArgs::Parenthesized(ref data) => { walk_list!(self, visit_ty, &data.inputs); if let Some(ref type_) = data.output { - // `-> Foo` syntax is essentially an associated type binding, - // so it is also allowed to contain nested `impl Trait`. - self.with_impl_trait(None, |this| this.visit_ty(type_)); + self.with_nested_impl_trait_warning(true, |this| { + // `-> Foo` syntax is essentially an associated type binding, + // so it is also allowed to contain nested `impl Trait`. + this.with_impl_trait(None, |this| this.visit_ty(type_)); + }); } } } @@ -711,6 +759,8 @@ pub fn check_crate(session: &Session, krate: &Crate) -> (bool, bool) { has_global_allocator: false, outer_impl_trait: None, is_impl_trait_banned: false, + warning_period_57979_nested_impl_trait: false, + warning_period_57979_impl_trait_in_proj: false, }; visit::walk_crate(&mut validator, krate); From a35c8e86685f12b598754de0c69ec63b770e7d50 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 22 Feb 2019 15:10:12 +0100 Subject: [PATCH 06/24] Unit (and regression) tests for warning cycle code. --- .../issue-57979-impl-trait-in-path.rs | 37 ++++++++++++++++ .../issue-57979-impl-trait-in-path.stderr | 30 +++++++++++++ ...e-57979-nested-impl-trait-in-assoc-proj.rs | 37 ++++++++++++++++ ...979-nested-impl-trait-in-assoc-proj.stderr | 36 ++++++++++++++++ src/test/ui/issues/issue-57979.rs | 42 ------------------- src/test/ui/issues/issue-57979.stderr | 17 -------- 6 files changed, 140 insertions(+), 59 deletions(-) create mode 100644 src/test/ui/impl-trait/issue-57979-impl-trait-in-path.rs create mode 100644 src/test/ui/impl-trait/issue-57979-impl-trait-in-path.stderr create mode 100644 src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.rs create mode 100644 src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.stderr delete mode 100644 src/test/ui/issues/issue-57979.rs delete mode 100644 src/test/ui/issues/issue-57979.stderr diff --git a/src/test/ui/impl-trait/issue-57979-impl-trait-in-path.rs b/src/test/ui/impl-trait/issue-57979-impl-trait-in-path.rs new file mode 100644 index 0000000000000..84fcb5e2880a7 --- /dev/null +++ b/src/test/ui/impl-trait/issue-57979-impl-trait-in-path.rs @@ -0,0 +1,37 @@ +// rust-lang/rust#57979 : the initial support for `impl Trait` didn't +// properly check syntax hidden behind an associated type projection. +// Here we test behavior of occurrences of `impl Trait` within a path +// component in that context. + +mod allowed { + #![allow(nested_impl_trait)] + + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux<(), Assoc=<() as Quux>::Assoc>) { } + impl Quux for () { type Assoc = u32; } +} + +mod warned { + #![warn(nested_impl_trait)] + + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux<(), Assoc=<() as Quux>::Assoc>) { } + //~^ WARN `impl Trait` is not allowed in path parameters + //~| WARN will become a hard error in a future release! + impl Quux for () { type Assoc = u32; } +} + +mod denied { + #![deny(nested_impl_trait)] + + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux<(), Assoc=<() as Quux>::Assoc>) { } + //~^ ERROR `impl Trait` is not allowed in path parameters + //~| WARN will become a hard error in a future release! + impl Quux for () { type Assoc = u32; } +} + +fn main() { } diff --git a/src/test/ui/impl-trait/issue-57979-impl-trait-in-path.stderr b/src/test/ui/impl-trait/issue-57979-impl-trait-in-path.stderr new file mode 100644 index 0000000000000..982ecba291f79 --- /dev/null +++ b/src/test/ui/impl-trait/issue-57979-impl-trait-in-path.stderr @@ -0,0 +1,30 @@ +warning: `impl Trait` is not allowed in path parameters + --> $DIR/issue-57979-impl-trait-in-path.rs:20:52 + | +LL | pub fn demo(_: impl Quux<(), Assoc=<() as Quux>::Assoc>) { } + | ^^^^^^^^ + | +note: lint level defined here + --> $DIR/issue-57979-impl-trait-in-path.rs:16:13 + | +LL | #![warn(nested_impl_trait)] + | ^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59014 + +error: `impl Trait` is not allowed in path parameters + --> $DIR/issue-57979-impl-trait-in-path.rs:31:52 + | +LL | pub fn demo(_: impl Quux<(), Assoc=<() as Quux>::Assoc>) { } + | ^^^^^^^^ + | +note: lint level defined here + --> $DIR/issue-57979-impl-trait-in-path.rs:27:13 + | +LL | #![deny(nested_impl_trait)] + | ^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59014 + +error: aborting due to previous error + diff --git a/src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.rs b/src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.rs new file mode 100644 index 0000000000000..5c20ffc7c6724 --- /dev/null +++ b/src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.rs @@ -0,0 +1,37 @@ +// rust-lang/rust#57979 : the initial support for `impl Trait` didn't +// properly check syntax hidden behind an associated type projection. +// Here we test behavior of occurrences of `impl Trait` within an +// `impl Trait` in that context. + +mod allowed { + #![allow(nested_impl_trait)] + + pub trait Foo { } + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux>) { } +} + +mod warned { + #![warn(nested_impl_trait)] + + pub trait Foo { } + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux>) { } + //~^ WARN nested `impl Trait` is not allowed + //~| WARN will become a hard error in a future release! +} + +mod denied { + #![deny(nested_impl_trait)] + + pub trait Foo { } + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux>) { } + //~^ ERROR nested `impl Trait` is not allowed + //~| WARN will become a hard error in a future release! +} + +fn main() { } diff --git a/src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.stderr b/src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.stderr new file mode 100644 index 0000000000000..508aea2432132 --- /dev/null +++ b/src/test/ui/impl-trait/issue-57979-nested-impl-trait-in-assoc-proj.stderr @@ -0,0 +1,36 @@ +warning: nested `impl Trait` is not allowed + --> $DIR/issue-57979-nested-impl-trait-in-assoc-proj.rs:21:45 + | +LL | pub fn demo(_: impl Quux>) { } + | ---------^^^^^^^^- + | | | + | | nested `impl Trait` here + | outer `impl Trait` + | +note: lint level defined here + --> $DIR/issue-57979-nested-impl-trait-in-assoc-proj.rs:16:13 + | +LL | #![warn(nested_impl_trait)] + | ^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59014 + +error: nested `impl Trait` is not allowed + --> $DIR/issue-57979-nested-impl-trait-in-assoc-proj.rs:32:45 + | +LL | pub fn demo(_: impl Quux>) { } + | ---------^^^^^^^^- + | | | + | | nested `impl Trait` here + | outer `impl Trait` + | +note: lint level defined here + --> $DIR/issue-57979-nested-impl-trait-in-assoc-proj.rs:27:13 + | +LL | #![deny(nested_impl_trait)] + | ^^^^^^^^^^^^^^^^^ + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #59014 + +error: aborting due to previous error + diff --git a/src/test/ui/issues/issue-57979.rs b/src/test/ui/issues/issue-57979.rs deleted file mode 100644 index abd46b60ab194..0000000000000 --- a/src/test/ui/issues/issue-57979.rs +++ /dev/null @@ -1,42 +0,0 @@ -// Regression test for #57979. This situation is meant to be an error. -// As noted in the issue thread, we decided to forbid nested impl -// trait of this kind: -// -// ```rust -// fn foo() -> impl Foo { .. } -// ``` -// -// Basically there are two hidden variables here, let's call them `X` -// and `Y`, and we must prove that: -// -// ``` -// X: Foo -// Y: Bar -// ``` -// -// However, the user is only giving us the return type `X`. It's true -// that in some cases, we can infer `Y` from `X`, because `X` only -// implements `Foo` for one type (and indeed the compiler does -// inference of this kind), but I do recall that we intended to forbid -// this -- in part because such inference is fragile, and there is not -// necessarily a way for the user to be more explicit should the -// inference fail (so you could get stuck with no way to port your -// code forward if, for example, more impls are added to an existing -// type). -// -// The same seems to apply in this situation. Here there are three impl traits, so we have -// -// ``` -// X: IntoIterator -// Y: Borrow> -// Z: AsRef<[u8]> -// ``` - -use std::borrow::Borrow; - -pub struct Data(TBody); - -pub fn collect(_: impl IntoIterator>>>) { - //~^ ERROR - unimplemented!() -} diff --git a/src/test/ui/issues/issue-57979.stderr b/src/test/ui/issues/issue-57979.stderr deleted file mode 100644 index 488f30ab7c5a7..0000000000000 --- a/src/test/ui/issues/issue-57979.stderr +++ /dev/null @@ -1,17 +0,0 @@ -error[E0666]: nested `impl Trait` is not allowed - --> $DIR/issue-57979.rs:39:61 - | -LL | pub fn collect(_: impl IntoIterator>>>) { - | -----------------^^^^^^^^^^^^^^^^-- - | | | - | | nested `impl Trait` here - | outer `impl Trait` - -error[E0601]: `main` function not found in crate `issue_57979` - | - = note: consider adding a `main` function to `$DIR/issue-57979.rs` - -error: aborting due to 2 previous errors - -Some errors occurred: E0601, E0666. -For more information about an error, try `rustc --explain E0601`. From 1d6d40b952258efca9361dfad3b6d7dabe3f2568 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 11 Mar 2019 15:14:24 +0100 Subject: [PATCH 07/24] Revised warning-downgrade strategy for nested impl trait. Instead of a sticky-boolean flag that would downgrade errors to warnings during further recursion into the type (which is overly broad because we were not missing errors at arbitrarily deep levels), this instead tracks state closer to what the original bug actually was. In particular, the actual original bug was that we were failing to record the existence of an outer `impl Trait` solely when it occurred as an *immediate child* during the walk of the child types in `visit_generic_args`. Therefore, the correct way to precisely model when that bug would manifest itself (and thus downgrade the error-to-warning accordingly) is to track when those outer `impl Trait` cases were previously unrecorded. That's what this code does, by storing a flag with the recorded outer `impl Trait` indicating at which point in the compiler's control flow it had been stored. I will note that this commit passes the current test suite. A follow-up commit will also include tests illustrating the cases that this commit gets right (and were handled incorrectly by the previous sticky boolean). --- src/librustc_passes/ast_validation.rs | 104 +++++++++++++++++++------- 1 file changed, 77 insertions(+), 27 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index c615d34b0feae..4f4f5aa080873 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -24,6 +24,31 @@ use syntax_pos::Span; use errors::Applicability; use log::debug; +#[derive(Copy, Clone, Debug)] +struct OuterImplTrait { + span: Span, + + /// rust-lang/rust#57979: a bug in original implementation caused + /// us to fail sometimes to record an outer `impl Trait`. + /// Therefore, in order to reliably issue a warning (rather than + /// an error) in the *precise* places where we are newly injecting + /// the diagnostic, we have to distinguish between the places + /// where the outer `impl Trait` has always been recorded, versus + /// the places where it has only recently started being recorded. + only_recorded_since_pull_request_57730: bool, +} + +impl OuterImplTrait { + /// This controls whether we should downgrade the nested impl + /// trait diagnostic to a warning rather than an error, based on + /// whether the outer impl trait had been improperly skipped in + /// earlier implementations of the analysis on the stable + /// compiler. + fn should_warn_instead_of_error(&self) -> bool { + self.only_recorded_since_pull_request_57730 + } +} + struct AstValidator<'a> { session: &'a Session, has_proc_macro_decls: bool, @@ -32,7 +57,7 @@ struct AstValidator<'a> { // Used to ban nested `impl Trait`, e.g., `impl Into`. // Nested `impl Trait` _is_ allowed in associated type position, // e.g `impl Iterator` - outer_impl_trait: Option, + outer_impl_trait: Option, // Used to ban `impl Trait` in path projections like `::Item` // or `Foo::Bar` @@ -44,18 +69,11 @@ struct AstValidator<'a> { // impl trait in projections), and thus miss some cases. We track // whether we should downgrade to a warning for short-term via // these booleans. - warning_period_57979_nested_impl_trait: bool, + warning_period_57979_didnt_record_next_impl_trait: bool, warning_period_57979_impl_trait_in_proj: bool, } impl<'a> AstValidator<'a> { - fn with_nested_impl_trait_warning(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T { - let old = mem::replace(&mut self.warning_period_57979_nested_impl_trait, v); - let ret = f(self); - self.warning_period_57979_nested_impl_trait = old; - ret - } - fn with_impl_trait_in_proj_warning(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T { let old = mem::replace(&mut self.warning_period_57979_impl_trait_in_proj, v); let ret = f(self); @@ -69,17 +87,53 @@ impl<'a> AstValidator<'a> { self.is_impl_trait_banned = old; } - fn with_impl_trait(&mut self, outer_impl_trait: Option, f: impl FnOnce(&mut Self)) { - let old = mem::replace(&mut self.outer_impl_trait, outer_impl_trait); + fn with_impl_trait(&mut self, outer: Option, f: impl FnOnce(&mut Self)) { + let old = mem::replace(&mut self.outer_impl_trait, outer); f(self); self.outer_impl_trait = old; } + fn visit_assoc_type_binding_from_generic_args(&mut self, type_binding: &'a TypeBinding) { + // rust-lang/rust#57979: bug in old visit_generic_args called + // walk_ty rather than visit_ty, skipping outer `impl Trait` + // if it happened to occur at `type_binding.ty` + if let TyKind::ImplTrait(..) = type_binding.ty.node { + self.warning_period_57979_didnt_record_next_impl_trait = true; + } + self.visit_assoc_type_binding(type_binding); + } + + fn visit_ty_from_generic_args(&mut self, ty: &'a Ty) { + // rust-lang/rust#57979: bug in old visit_generic_args called + // walk_ty rather than visit_ty, skippping outer `impl Trait` + // if it happened to occur at `ty` + if let TyKind::ImplTrait(..) = ty.node { + self.warning_period_57979_didnt_record_next_impl_trait = true; + } + self.visit_ty(ty); + } + + fn outer_impl_trait(&mut self, span: Span) -> OuterImplTrait { + let only_recorded_since_pull_request_57730 = + self.warning_period_57979_didnt_record_next_impl_trait; + + // (this flag is designed to be set to true and then only + // reach the construction point for the outer impl trait once, + // so its safe and easiest to unconditionally reset it to + // false) + self.warning_period_57979_didnt_record_next_impl_trait = false; + + OuterImplTrait { + span, only_recorded_since_pull_request_57730, + } + } + // Mirrors visit::walk_ty, but tracks relevant state fn walk_ty(&mut self, t: &'a Ty) { match t.node { TyKind::ImplTrait(..) => { - self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t)) + let outer_impl_trait = self.outer_impl_trait(t.span); + self.with_impl_trait(Some(outer_impl_trait), |this| visit::walk_ty(this, t)) } TyKind::Path(ref qself, ref path) => { // We allow these: @@ -441,18 +495,18 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } if let Some(outer_impl_trait) = self.outer_impl_trait { - if self.warning_period_57979_nested_impl_trait { + if outer_impl_trait.should_warn_instead_of_error() { self.session.buffer_lint_with_diagnostic( NESTED_IMPL_TRAIT, ty.id, ty.span, "nested `impl Trait` is not allowed", BuiltinLintDiagnostics::NestedImplTrait { - outer_impl_trait_span: outer_impl_trait, + outer_impl_trait_span: outer_impl_trait.span, inner_impl_trait_span: ty.span, }); } else { struct_span_err!(self.session, ty.span, E0666, "nested `impl Trait` is not allowed") - .span_label(outer_impl_trait, "outer `impl Trait`") + .span_label(outer_impl_trait.span, "outer `impl Trait`") .span_label(ty.span, "nested `impl Trait` here") .emit(); } @@ -649,22 +703,18 @@ impl<'a> Visitor<'a> for AstValidator<'a> { }, arg.span(), None) }), GenericPosition::Arg, generic_args.span()); - self.with_nested_impl_trait_warning(true, |this| { - // Type bindings such as `Item=impl Debug` in `Iterator` - // are allowed to contain nested `impl Trait`. - this.with_impl_trait(None, |this| { - walk_list!(this, visit_assoc_type_binding, &data.bindings); - }); + // Type bindings such as `Item=impl Debug` in `Iterator` + // are allowed to contain nested `impl Trait`. + self.with_impl_trait(None, |this| { + walk_list!(this, visit_assoc_type_binding_from_generic_args, &data.bindings); }); } GenericArgs::Parenthesized(ref data) => { walk_list!(self, visit_ty, &data.inputs); if let Some(ref type_) = data.output { - self.with_nested_impl_trait_warning(true, |this| { - // `-> Foo` syntax is essentially an associated type binding, - // so it is also allowed to contain nested `impl Trait`. - this.with_impl_trait(None, |this| this.visit_ty(type_)); - }); + // `-> Foo` syntax is essentially an associated type binding, + // so it is also allowed to contain nested `impl Trait`. + self.with_impl_trait(None, |this| this.visit_ty_from_generic_args(type_)); } } } @@ -759,7 +809,7 @@ pub fn check_crate(session: &Session, krate: &Crate) -> (bool, bool) { has_global_allocator: false, outer_impl_trait: None, is_impl_trait_banned: false, - warning_period_57979_nested_impl_trait: false, + warning_period_57979_didnt_record_next_impl_trait: false, warning_period_57979_impl_trait_in_proj: false, }; visit::walk_crate(&mut validator, krate); From 7f31babc148e4383d4ab353d0987fc12466c1a9a Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 11 Mar 2019 16:30:40 +0100 Subject: [PATCH 08/24] Test illustrating that the nested_impl_trait lint should only catch shallow cases. --- ...-deeply-nested-impl-trait-in-assoc-proj.rs | 42 +++++++++++++++++++ ...ply-nested-impl-trait-in-assoc-proj.stderr | 30 +++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs create mode 100644 src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.stderr diff --git a/src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs b/src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs new file mode 100644 index 0000000000000..5eef6a39325fe --- /dev/null +++ b/src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs @@ -0,0 +1,42 @@ +// rust-lang/rust#57979 : the initial support for `impl Trait` didn't +// properly check syntax hidden behind an associated type projection, +// but it did catch *some cases*. This is checking that we continue to +// properly emit errors for those, even with the new +// future-incompatibility warnings. +// +// issue-57979-nested-impl-trait-in-assoc-proj.rs shows the main case +// that we were previously failing to catch. + +struct Deeper(T); + +mod allowed { + #![allow(nested_impl_trait)] + + pub trait Foo { } + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux>>) { } + //~^ ERROR nested `impl Trait` is not allowed +} + +mod warned { + #![warn(nested_impl_trait)] + + pub trait Foo { } + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux>>) { } + //~^ ERROR nested `impl Trait` is not allowed +} + +mod denied { + #![deny(nested_impl_trait)] + + pub trait Foo { } + pub trait Bar { } + pub trait Quux { type Assoc; } + pub fn demo(_: impl Quux>>) { } + //~^ ERROR nested `impl Trait` is not allowed +} + +fn main() { } diff --git a/src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.stderr b/src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.stderr new file mode 100644 index 0000000000000..2b6f15e6d3eb2 --- /dev/null +++ b/src/test/ui/impl-trait/issue-57979-deeply-nested-impl-trait-in-assoc-proj.stderr @@ -0,0 +1,30 @@ +error[E0666]: nested `impl Trait` is not allowed + --> $DIR/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs:18:59 + | +LL | pub fn demo(_: impl Quux>>) { } + | ---------^^^^^^^^- + | | | + | | nested `impl Trait` here + | outer `impl Trait` + +error[E0666]: nested `impl Trait` is not allowed + --> $DIR/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs:28:59 + | +LL | pub fn demo(_: impl Quux>>) { } + | ---------^^^^^^^^- + | | | + | | nested `impl Trait` here + | outer `impl Trait` + +error[E0666]: nested `impl Trait` is not allowed + --> $DIR/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs:38:59 + | +LL | pub fn demo(_: impl Quux>>) { } + | ---------^^^^^^^^- + | | | + | | nested `impl Trait` here + | outer `impl Trait` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0666`. From 8df3fb6f5a5da1536d0ab1cf2417159c15c4582e Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 12 Mar 2019 13:00:50 +0100 Subject: [PATCH 09/24] Addressed review feedback regarding comment phrasing. --- src/librustc_passes/ast_validation.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 4f4f5aa080873..7c6393e30dcda 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -64,8 +64,8 @@ struct AstValidator<'a> { is_impl_trait_banned: bool, // rust-lang/rust#57979: the ban of nested `impl Trait` was buggy - // until sometime after PR #57730 landed: it would jump directly - // to walk_ty rather than visit_ty (or skip recurring entirely for + // until PRs #57730 and #57981 landed: it would jump directly to + // walk_ty rather than visit_ty (or skip recurring entirely for // impl trait in projections), and thus miss some cases. We track // whether we should downgrade to a warning for short-term via // these booleans. From 4e1c03bad0e83ffd24cf98328b40cab565ffac26 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Wed, 27 Feb 2019 17:41:25 +0100 Subject: [PATCH 10/24] Don't promote function calls to nonpromotable things --- src/librustc_mir/transform/qualify_consts.rs | 7 ++----- src/test/ui/consts/invalid_promotion.rs | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/consts/invalid_promotion.rs diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 285c674643f2e..73ac68c4ac3b4 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -507,7 +507,7 @@ impl Qualif for IsNotPromotable { fn in_call( cx: &ConstCx<'_, 'tcx>, callee: &Operand<'tcx>, - _args: &[Operand<'tcx>], + args: &[Operand<'tcx>], _return_ty: Ty<'tcx>, ) -> bool { if cx.mode == Mode::Fn { @@ -520,10 +520,7 @@ impl Qualif for IsNotPromotable { } } - // FIXME(eddyb) do we need "not promotable" in anything - // other than `Mode::Fn` by any chance? - - false + Self::in_operand(cx, callee) || args.iter().any(|arg| Self::in_operand(cx, arg)) } } diff --git a/src/test/ui/consts/invalid_promotion.rs b/src/test/ui/consts/invalid_promotion.rs new file mode 100644 index 0000000000000..f98406e50e9ae --- /dev/null +++ b/src/test/ui/consts/invalid_promotion.rs @@ -0,0 +1,18 @@ +// compile-pass +// note this was only reproducible with lib crates +// compile-flags: --crate-type=lib + +pub struct Hz; + +impl Hz { + pub const fn num(&self) -> u32 { + 42 + } + pub const fn normalized(&self) -> Hz { + Hz + } + + pub const fn as_u32(&self) -> u32 { + self.normalized().num() // this used to promote the `self.normalized()` + } +} From eedebbc3b9671b3a1e443afdee78da8c183ebb96 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 1 Mar 2019 17:10:29 +0100 Subject: [PATCH 11/24] Schedule the demolition of `IsNotPromotable` --- src/librustc_mir/transform/qualify_consts.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 73ac68c4ac3b4..f7b7754cea7bc 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -499,6 +499,8 @@ impl Qualif for IsNotConst { // Refers to temporaries which cannot be promoted as // promote_consts decided they weren't simple enough. +// FIXME(oli-obk,eddyb): Remove this flag entirely and +// solely process this information via `IsNotConst`. struct IsNotPromotable; impl Qualif for IsNotPromotable { From ad293f1a1582319ed4bd06ed860a5528066ccb25 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 27 Feb 2019 21:01:42 +0000 Subject: [PATCH 12/24] Make migrate mode work at item level granularity --- src/librustc/middle/expr_use_visitor.rs | 8 +++-- .../borrowck/gather_loans/mod.rs | 18 +++++++++++ src/librustc_mir/borrow_check/mod.rs | 28 +++------------- ...e-58776-borrowck-scans-children.ast.stderr | 15 +++++++++ ...776-borrowck-scans-children.migrate.stderr | 32 +++++++++++++++++++ ...e-58776-borrowck-scans-children.nll.stderr | 32 +++++++++++++++++++ .../issue-58776-borrowck-scans-children.rs | 21 ++++++++++++ 7 files changed, 129 insertions(+), 25 deletions(-) create mode 100644 src/test/ui/borrowck/issue-58776-borrowck-scans-children.ast.stderr create mode 100644 src/test/ui/borrowck/issue-58776-borrowck-scans-children.migrate.stderr create mode 100644 src/test/ui/borrowck/issue-58776-borrowck-scans-children.nll.stderr create mode 100644 src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 8da20ba426663..2f0940b430b97 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -83,6 +83,9 @@ pub trait Delegate<'tcx> { assignment_span: Span, assignee_cmt: &mc::cmt_<'tcx>, mode: MutateMode); + + // A nested closure or generator - only one layer deep. + fn nested_body(&mut self, _body_id: hir::BodyId) {} } #[derive(Copy, Clone, PartialEq, Debug)] @@ -532,8 +535,9 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { self.consume_expr(&base); } - hir::ExprKind::Closure(.., fn_decl_span, _) => { - self.walk_captures(expr, fn_decl_span) + hir::ExprKind::Closure(_, _, body_id, fn_decl_span, _) => { + self.delegate.nested_body(body_id); + self.walk_captures(expr, fn_decl_span); } hir::ExprKind::Box(ref base) => { diff --git a/src/librustc_borrowck/borrowck/gather_loans/mod.rs b/src/librustc_borrowck/borrowck/gather_loans/mod.rs index cf6053b71b6a8..ab4797913a438 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/mod.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/mod.rs @@ -152,6 +152,24 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for GatherLoanCtxt<'a, 'tcx> { .node_type(self.bccx.tcx.hir().node_to_hir_id(id)); gather_moves::gather_decl(self.bccx, &self.move_data, id, ty); } + + fn nested_body(&mut self, body_id: hir::BodyId) { + debug!("nested_body(body_id={:?})", body_id); + // rust-lang/rust#58776: MIR and AST borrow check disagree on where + // certain closure errors are reported. As such migrate borrowck has to + // operate at the level of items, rather than bodies. Check if the + // contained closure had any errors and set `signalled_any_error` if it + // has. + let bccx = self.bccx; + if bccx.tcx.migrate_borrowck() { + if let SignalledError::NoErrorsSeen = bccx.signalled_any_error.get() { + let closure_def_id = bccx.tcx.hir().body_owner_def_id(body_id); + debug!("checking closure: {:?}", closure_def_id); + + bccx.signalled_any_error.set(bccx.tcx.borrowck(closure_def_id).signalled_any_error); + } + } + } } /// Implements the A-* rules in README.md. diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 1e9dab5016f8d..7c08dbeed6912 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -329,30 +329,12 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( // When borrowck=migrate, check if AST-borrowck would // error on the given code. - // rust-lang/rust#55492: loop over parents to ensure that - // errors that AST-borrowck only detects in some parent of - // a closure still allows NLL to signal an error. - let mut curr_def_id = def_id; - let signalled_any_error = loop { - match tcx.borrowck(curr_def_id).signalled_any_error { - SignalledError::NoErrorsSeen => { - // keep traversing (and borrow-checking) parents - } - SignalledError::SawSomeError => { - // stop search here - break SignalledError::SawSomeError; - } - } - - if tcx.is_closure(curr_def_id) { - curr_def_id = tcx.parent_def_id(curr_def_id) - .expect("a closure must have a parent_def_id"); - } else { - break SignalledError::NoErrorsSeen; - } - }; + // rust-lang/rust#55492, rust-lang/rust#58776 check the base def id + // for errors. AST borrowck is responsible for aggregating + // `signalled_any_error` from all of the nested closures here. + let base_def_id = tcx.closure_base_def_id(def_id); - match signalled_any_error { + match tcx.borrowck(base_def_id).signalled_any_error { SignalledError::NoErrorsSeen => { // if AST-borrowck signalled no errors, then // downgrade all the buffered MIR-borrowck errors diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.ast.stderr b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.ast.stderr new file mode 100644 index 0000000000000..9e0b0aac1e3c6 --- /dev/null +++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.ast.stderr @@ -0,0 +1,15 @@ +error[E0597]: `**greeting` does not live long enough + --> $DIR/issue-58776-borrowck-scans-children.rs:10:24 + | +LL | let res = (|| (|| &greeting)())(); + | -- ^^^^^^^^ - borrowed value only lives until here + | | | + | | borrowed value does not live long enough + | capture occurs here +... +LL | } + | - borrowed value needs to live until here + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.migrate.stderr b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.migrate.stderr new file mode 100644 index 0000000000000..bd8f2286f170c --- /dev/null +++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.migrate.stderr @@ -0,0 +1,32 @@ +error[E0506]: cannot assign to `greeting` because it is borrowed + --> $DIR/issue-58776-borrowck-scans-children.rs:13:5 + | +LL | let res = (|| (|| &greeting)())(); + | -- -------- borrow occurs due to use in closure + | | + | borrow of `greeting` occurs here +... +LL | greeting = "DEALLOCATED".to_string(); + | ^^^^^^^^ assignment to borrowed `greeting` occurs here +... +LL | println!("thread result: {:?}", res); + | --- borrow later used here + +error[E0505]: cannot move out of `greeting` because it is borrowed + --> $DIR/issue-58776-borrowck-scans-children.rs:16:10 + | +LL | let res = (|| (|| &greeting)())(); + | -- -------- borrow occurs due to use in closure + | | + | borrow of `greeting` occurs here +... +LL | drop(greeting); + | ^^^^^^^^ move out of `greeting` occurs here +... +LL | println!("thread result: {:?}", res); + | --- borrow later used here + +error: aborting due to 2 previous errors + +Some errors occurred: E0505, E0506. +For more information about an error, try `rustc --explain E0505`. diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.nll.stderr b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.nll.stderr new file mode 100644 index 0000000000000..bd8f2286f170c --- /dev/null +++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.nll.stderr @@ -0,0 +1,32 @@ +error[E0506]: cannot assign to `greeting` because it is borrowed + --> $DIR/issue-58776-borrowck-scans-children.rs:13:5 + | +LL | let res = (|| (|| &greeting)())(); + | -- -------- borrow occurs due to use in closure + | | + | borrow of `greeting` occurs here +... +LL | greeting = "DEALLOCATED".to_string(); + | ^^^^^^^^ assignment to borrowed `greeting` occurs here +... +LL | println!("thread result: {:?}", res); + | --- borrow later used here + +error[E0505]: cannot move out of `greeting` because it is borrowed + --> $DIR/issue-58776-borrowck-scans-children.rs:16:10 + | +LL | let res = (|| (|| &greeting)())(); + | -- -------- borrow occurs due to use in closure + | | + | borrow of `greeting` occurs here +... +LL | drop(greeting); + | ^^^^^^^^ move out of `greeting` occurs here +... +LL | println!("thread result: {:?}", res); + | --- borrow later used here + +error: aborting due to 2 previous errors + +Some errors occurred: E0505, E0506. +For more information about an error, try `rustc --explain E0505`. diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs new file mode 100644 index 0000000000000..378969f9a1867 --- /dev/null +++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs @@ -0,0 +1,21 @@ +// ignore-compare-mode-nll + +// revisions: ast migrate nll + +//[migrate]compile-flags: -Z borrowck=migrate +#![cfg_attr(nll, feature(nll))] + +fn main() { + let mut greeting = "Hello world!".to_string(); + let res = (|| (|| &greeting)())(); + //[ast]~^ ERROR does not live long enough + + greeting = "DEALLOCATED".to_string(); + //[migrate]~^ ERROR cannot assign + //[nll]~^^ ERROR cannot assign + drop(greeting); + //[migrate]~^ ERROR cannot move + //[nll]~^^ ERROR cannot move + + println!("thread result: {:?}", res); +} From 363024dd27d0d3a7eb8d8dd5dfb6759abf05efbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 1 Mar 2019 14:42:39 -0800 Subject: [PATCH 13/24] Expand where negative supertrait specific error is shown Fix #58857. --- src/libsyntax/parse/parser.rs | 52 ++++++++++++++------------- src/test/ui/issues/issue-58857.rs | 7 ++++ src/test/ui/issues/issue-58857.stderr | 8 +++++ 3 files changed, 43 insertions(+), 24 deletions(-) create mode 100644 src/test/ui/issues/issue-58857.rs create mode 100644 src/test/ui/issues/issue-58857.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 7e900dfeb1eeb..383ea512d02e6 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1794,7 +1794,7 @@ impl<'a> Parser<'a> { let mut bounds = vec![GenericBound::Trait(poly_trait_ref, TraitBoundModifier::None)]; if parse_plus { self.eat_plus(); // `+`, or `+=` gets split and `+` is discarded - bounds.append(&mut self.parse_generic_bounds(None)?); + bounds.append(&mut self.parse_generic_bounds(Some(self.prev_span))?); } Ok(TyKind::TraitObject(bounds, TraitObjectSyntax::None)) } @@ -5502,6 +5502,7 @@ impl<'a> Parser<'a> { let mut bounds = Vec::new(); let mut negative_bounds = Vec::new(); let mut last_plus_span = None; + let mut was_negative = false; loop { // This needs to be synchronized with `Token::can_begin_bound`. let is_bound_start = self.check_path() || self.check_lifetime() || @@ -5546,9 +5547,10 @@ impl<'a> Parser<'a> { } let poly_span = lo.to(self.prev_span); if is_negative { - negative_bounds.push( - last_plus_span.or(colon_span).unwrap() - .to(poly_span)); + was_negative = true; + if let Some(sp) = last_plus_span.or(colon_span) { + negative_bounds.push(sp.to(poly_span)); + } } else { let poly_trait = PolyTraitRef::new(lifetime_defs, path, poly_span); let modifier = if question.is_some() { @@ -5570,26 +5572,28 @@ impl<'a> Parser<'a> { } } - if !negative_bounds.is_empty() { + if !negative_bounds.is_empty() || was_negative { let plural = negative_bounds.len() > 1; let mut err = self.struct_span_err(negative_bounds, "negative trait bounds are not supported"); - let bound_list = colon_span.unwrap().to(self.prev_span); - let mut new_bound_list = String::new(); - if !bounds.is_empty() { - let mut snippets = bounds.iter().map(|bound| bound.span()) - .map(|span| self.sess.source_map().span_to_snippet(span)); - while let Some(Ok(snippet)) = snippets.next() { - new_bound_list.push_str(" + "); - new_bound_list.push_str(&snippet); - } - new_bound_list = new_bound_list.replacen(" +", ":", 1); - } - err.span_suggestion_short(bound_list, - &format!("remove the trait bound{}", - if plural { "s" } else { "" }), - new_bound_list, - Applicability::MachineApplicable); + if let Some(bound_list) = colon_span { + let bound_list = bound_list.to(self.prev_span); + let mut new_bound_list = String::new(); + if !bounds.is_empty() { + let mut snippets = bounds.iter().map(|bound| bound.span()) + .map(|span| self.sess.source_map().span_to_snippet(span)); + while let Some(Ok(snippet)) = snippets.next() { + new_bound_list.push_str(" + "); + new_bound_list.push_str(&snippet); + } + new_bound_list = new_bound_list.replacen(" +", ":", 1); + } + err.span_suggestion_short(bound_list, + &format!("remove the trait bound{}", + if plural { "s" } else { "" }), + new_bound_list, + Applicability::MachineApplicable); + } err.emit(); } @@ -5625,7 +5629,7 @@ impl<'a> Parser<'a> { // Parse optional colon and param bounds. let bounds = if self.eat(&token::Colon) { - self.parse_generic_bounds(None)? + self.parse_generic_bounds(Some(self.prev_span))? } else { Vec::new() }; @@ -6070,7 +6074,7 @@ impl<'a> Parser<'a> { // or with mandatory equality sign and the second type. let ty = self.parse_ty()?; if self.eat(&token::Colon) { - let bounds = self.parse_generic_bounds(None)?; + let bounds = self.parse_generic_bounds(Some(self.prev_span))?; where_clause.predicates.push(ast::WherePredicate::BoundPredicate( ast::WhereBoundPredicate { span: lo.to(self.prev_span), @@ -7626,7 +7630,7 @@ impl<'a> Parser<'a> { tps.where_clause = self.parse_where_clause()?; let alias = if existential { self.expect(&token::Colon)?; - let bounds = self.parse_generic_bounds(None)?; + let bounds = self.parse_generic_bounds(Some(self.prev_span))?; AliasKind::Existential(bounds) } else { self.expect(&token::Eq)?; diff --git a/src/test/ui/issues/issue-58857.rs b/src/test/ui/issues/issue-58857.rs new file mode 100644 index 0000000000000..392e4ea0c2ecc --- /dev/null +++ b/src/test/ui/issues/issue-58857.rs @@ -0,0 +1,7 @@ +struct Conj {a : A} +trait Valid {} + +impl Conj{} +//~^ ERROR negative trait bounds are not supported + +fn main() {} diff --git a/src/test/ui/issues/issue-58857.stderr b/src/test/ui/issues/issue-58857.stderr new file mode 100644 index 0000000000000..040e9eb8a6567 --- /dev/null +++ b/src/test/ui/issues/issue-58857.stderr @@ -0,0 +1,8 @@ +error: negative trait bounds are not supported + --> $DIR/issue-58857.rs:4:7 + | +LL | impl Conj{} + | ^^^^^^^^ help: remove the trait bound + +error: aborting due to previous error + From d954246ea6d910c1e3b43c70ebf7846a15ea5d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 1 Mar 2019 16:28:04 -0800 Subject: [PATCH 14/24] Do not panic on missing close paren Fix #58856. --- src/libsyntax/parse/parser.rs | 4 +- src/test/ui/issues/issue-58856-1.rs | 9 ++++ src/test/ui/issues/issue-58856-1.stderr | 30 ++++++++++++++ src/test/ui/issues/issue-58856-2.rs | 13 ++++++ src/test/ui/issues/issue-58856-2.stderr | 55 +++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/issues/issue-58856-1.rs create mode 100644 src/test/ui/issues/issue-58856-1.stderr create mode 100644 src/test/ui/issues/issue-58856-2.rs create mode 100644 src/test/ui/issues/issue-58856-2.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 383ea512d02e6..2aecfb1e9b247 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -6321,8 +6321,10 @@ impl<'a> Parser<'a> { &token::CloseDelim(token::Paren), sep, parse_arg_fn)?; fn_inputs.append(&mut input); (fn_inputs, recovered) + } else if let Err(err) = self.expect_one_of(&[], &[]) { + return Err(err); } else { - return self.unexpected(); + (vec![self_arg], true) } } else { self.parse_seq_to_before_end(&token::CloseDelim(token::Paren), sep, parse_arg_fn)? diff --git a/src/test/ui/issues/issue-58856-1.rs b/src/test/ui/issues/issue-58856-1.rs new file mode 100644 index 0000000000000..9311bb0802f2a --- /dev/null +++ b/src/test/ui/issues/issue-58856-1.rs @@ -0,0 +1,9 @@ +impl A { +//~^ ERROR cannot find type `A` in this scope + fn b(self> + //~^ ERROR expected one of `)`, `,`, or `:`, found `>` + //~| ERROR expected one of `->`, `where`, or `{`, found `>` + //~| ERROR expected one of `->`, `async`, `const`, `crate`, `default`, `existential`, +} + +fn main() {} diff --git a/src/test/ui/issues/issue-58856-1.stderr b/src/test/ui/issues/issue-58856-1.stderr new file mode 100644 index 0000000000000..3cbfd375e785e --- /dev/null +++ b/src/test/ui/issues/issue-58856-1.stderr @@ -0,0 +1,30 @@ +error: expected one of `)`, `,`, or `:`, found `>` + --> $DIR/issue-58856-1.rs:3:14 + | +LL | fn b(self> + | - ^ + | | | + | | help: `)` may belong here + | unclosed delimiter + +error: expected one of `->`, `where`, or `{`, found `>` + --> $DIR/issue-58856-1.rs:3:14 + | +LL | fn b(self> + | ^ expected one of `->`, `where`, or `{` here + +error: expected one of `->`, `async`, `const`, `crate`, `default`, `existential`, `extern`, `fn`, `pub`, `type`, `unsafe`, `where`, or `}`, found `>` + --> $DIR/issue-58856-1.rs:3:14 + | +LL | fn b(self> + | ^ expected one of 13 possible tokens here + +error[E0412]: cannot find type `A` in this scope + --> $DIR/issue-58856-1.rs:1:6 + | +LL | impl A { + | ^ not found in this scope + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0412`. diff --git a/src/test/ui/issues/issue-58856-2.rs b/src/test/ui/issues/issue-58856-2.rs new file mode 100644 index 0000000000000..4c764761e8ec5 --- /dev/null +++ b/src/test/ui/issues/issue-58856-2.rs @@ -0,0 +1,13 @@ +trait Howness {} +impl Howness for () { + fn how_are_you(&self -> Empty { + //~^ ERROR expected one of `)` or `,`, found `->` + //~| ERROR method `how_are_you` is not a member of trait `Howness` + //~| ERROR cannot find type `Empty` in this scope + Empty + //~^ ERROR cannot find value `Empty` in this scope + } +} +//~^ ERROR expected one of `async`, `const`, `crate`, `default`, `existential`, `extern`, `fn`, + +fn main() {} diff --git a/src/test/ui/issues/issue-58856-2.stderr b/src/test/ui/issues/issue-58856-2.stderr new file mode 100644 index 0000000000000..30027278e23ca --- /dev/null +++ b/src/test/ui/issues/issue-58856-2.stderr @@ -0,0 +1,55 @@ +error: expected one of `)` or `,`, found `->` + --> $DIR/issue-58856-2.rs:3:26 + | +LL | fn how_are_you(&self -> Empty { + | - -^^ + | | | + | | help: `)` may belong here + | unclosed delimiter + +error: expected one of `async`, `const`, `crate`, `default`, `existential`, `extern`, `fn`, `pub`, `type`, `unsafe`, or `}`, found `)` + --> $DIR/issue-58856-2.rs:10:1 + | +LL | } + | - expected one of 11 possible tokens here +LL | } + | ^ unexpected token + +error[E0407]: method `how_are_you` is not a member of trait `Howness` + --> $DIR/issue-58856-2.rs:3:5 + | +LL | / fn how_are_you(&self -> Empty { +LL | | //~^ ERROR expected one of `)` or `,`, found `->` +LL | | //~| ERROR method `how_are_you` is not a member of trait `Howness` +LL | | //~| ERROR cannot find type `Empty` in this scope +LL | | Empty +LL | | //~^ ERROR cannot find value `Empty` in this scope +LL | | } + | |_____^ not a member of trait `Howness` + +error[E0412]: cannot find type `Empty` in this scope + --> $DIR/issue-58856-2.rs:3:29 + | +LL | fn how_are_you(&self -> Empty { + | ^^^^^ not found in this scope +help: possible candidates are found in other modules, you can import them into scope + | +LL | use std::io::Empty; + | +LL | use std::iter::Empty; + | + +error[E0425]: cannot find value `Empty` in this scope + --> $DIR/issue-58856-2.rs:7:9 + | +LL | Empty + | ^^^^^ not found in this scope +help: possible candidate is found in another module, you can import it into scope + | +LL | use std::sync::mpsc::TryRecvError::Empty; + | + +error: aborting due to 5 previous errors + +Some errors occurred: E0407, E0412, E0425. +For more information about an error, try `rustc --explain E0407`. From 8470916723a568a389977a85f3561570a145af3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 1 Mar 2019 21:47:06 -0800 Subject: [PATCH 15/24] Bail when encountering a second unexpected token in the same span --- src/libsyntax/parse/parser.rs | 14 +++++++--- src/test/ui/issues/issue-58856-1.rs | 7 +++-- src/test/ui/issues/issue-58856-1.stderr | 25 +++--------------- src/test/ui/issues/issue-58856-2.rs | 5 ++-- src/test/ui/issues/issue-58856-2.stderr | 35 ++++--------------------- src/test/ui/parser/recover-enum2.rs | 3 --- src/test/ui/parser/recover-enum2.stderr | 14 +--------- 7 files changed, 25 insertions(+), 78 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 2aecfb1e9b247..414c34de5e3d4 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -46,7 +46,7 @@ use crate::ThinVec; use crate::tokenstream::{self, DelimSpan, TokenTree, TokenStream, TreeAndJoint}; use crate::symbol::{Symbol, keywords}; -use errors::{Applicability, DiagnosticBuilder, DiagnosticId}; +use errors::{Applicability, DiagnosticBuilder, DiagnosticId, FatalError}; use rustc_target::spec::abi::{self, Abi}; use syntax_pos::{Span, MultiSpan, BytePos, FileName}; use log::{debug, trace}; @@ -256,6 +256,7 @@ pub struct Parser<'a> { /// it gets removed from here. Every entry left at the end gets emitted as an independent /// error. crate unclosed_delims: Vec, + last_unexpected_token_span: Option, } @@ -582,6 +583,7 @@ impl<'a> Parser<'a> { unmatched_angle_bracket_count: 0, max_angle_bracket_count: 0, unclosed_delims: Vec::new(), + last_unexpected_token_span: None, }; let tok = parser.next_tok(); @@ -775,6 +777,8 @@ impl<'a> Parser<'a> { } else if inedible.contains(&self.token) { // leave it in the input Ok(false) + } else if self.last_unexpected_token_span == Some(self.span) { + FatalError.raise(); } else { let mut expected = edible.iter() .map(|x| TokenType::Token(x.clone())) @@ -802,6 +806,7 @@ impl<'a> Parser<'a> { (self.sess.source_map().next_point(self.prev_span), format!("expected {} here", expect))) }; + self.last_unexpected_token_span = Some(self.span); let mut err = self.fatal(&msg_exp); if self.token.is_ident_named("and") { err.span_suggestion_short( @@ -6321,10 +6326,11 @@ impl<'a> Parser<'a> { &token::CloseDelim(token::Paren), sep, parse_arg_fn)?; fn_inputs.append(&mut input); (fn_inputs, recovered) - } else if let Err(err) = self.expect_one_of(&[], &[]) { - return Err(err); } else { - (vec![self_arg], true) + match self.expect_one_of(&[], &[]) { + Err(err) => return Err(err), + Ok(recovered) => (vec![self_arg], recovered), + } } } else { self.parse_seq_to_before_end(&token::CloseDelim(token::Paren), sep, parse_arg_fn)? diff --git a/src/test/ui/issues/issue-58856-1.rs b/src/test/ui/issues/issue-58856-1.rs index 9311bb0802f2a..f5edac0d2e316 100644 --- a/src/test/ui/issues/issue-58856-1.rs +++ b/src/test/ui/issues/issue-58856-1.rs @@ -1,9 +1,8 @@ +struct A; + impl A { -//~^ ERROR cannot find type `A` in this scope - fn b(self> + fn b(self> {} //~^ ERROR expected one of `)`, `,`, or `:`, found `>` - //~| ERROR expected one of `->`, `where`, or `{`, found `>` - //~| ERROR expected one of `->`, `async`, `const`, `crate`, `default`, `existential`, } fn main() {} diff --git a/src/test/ui/issues/issue-58856-1.stderr b/src/test/ui/issues/issue-58856-1.stderr index 3cbfd375e785e..85101e467b137 100644 --- a/src/test/ui/issues/issue-58856-1.stderr +++ b/src/test/ui/issues/issue-58856-1.stderr @@ -1,30 +1,11 @@ error: expected one of `)`, `,`, or `:`, found `>` - --> $DIR/issue-58856-1.rs:3:14 + --> $DIR/issue-58856-1.rs:4:14 | -LL | fn b(self> +LL | fn b(self> {} | - ^ | | | | | help: `)` may belong here | unclosed delimiter -error: expected one of `->`, `where`, or `{`, found `>` - --> $DIR/issue-58856-1.rs:3:14 - | -LL | fn b(self> - | ^ expected one of `->`, `where`, or `{` here - -error: expected one of `->`, `async`, `const`, `crate`, `default`, `existential`, `extern`, `fn`, `pub`, `type`, `unsafe`, `where`, or `}`, found `>` - --> $DIR/issue-58856-1.rs:3:14 - | -LL | fn b(self> - | ^ expected one of 13 possible tokens here - -error[E0412]: cannot find type `A` in this scope - --> $DIR/issue-58856-1.rs:1:6 - | -LL | impl A { - | ^ not found in this scope - -error: aborting due to 4 previous errors +error: aborting due to previous error -For more information about this error, try `rustc --explain E0412`. diff --git a/src/test/ui/issues/issue-58856-2.rs b/src/test/ui/issues/issue-58856-2.rs index 4c764761e8ec5..acc38e4c20163 100644 --- a/src/test/ui/issues/issue-58856-2.rs +++ b/src/test/ui/issues/issue-58856-2.rs @@ -1,11 +1,12 @@ +struct Empty; + trait Howness {} + impl Howness for () { fn how_are_you(&self -> Empty { //~^ ERROR expected one of `)` or `,`, found `->` //~| ERROR method `how_are_you` is not a member of trait `Howness` - //~| ERROR cannot find type `Empty` in this scope Empty - //~^ ERROR cannot find value `Empty` in this scope } } //~^ ERROR expected one of `async`, `const`, `crate`, `default`, `existential`, `extern`, `fn`, diff --git a/src/test/ui/issues/issue-58856-2.stderr b/src/test/ui/issues/issue-58856-2.stderr index 30027278e23ca..55a9e9d5cb863 100644 --- a/src/test/ui/issues/issue-58856-2.stderr +++ b/src/test/ui/issues/issue-58856-2.stderr @@ -1,5 +1,5 @@ error: expected one of `)` or `,`, found `->` - --> $DIR/issue-58856-2.rs:3:26 + --> $DIR/issue-58856-2.rs:6:26 | LL | fn how_are_you(&self -> Empty { | - -^^ @@ -8,7 +8,7 @@ LL | fn how_are_you(&self -> Empty { | unclosed delimiter error: expected one of `async`, `const`, `crate`, `default`, `existential`, `extern`, `fn`, `pub`, `type`, `unsafe`, or `}`, found `)` - --> $DIR/issue-58856-2.rs:10:1 + --> $DIR/issue-58856-2.rs:11:1 | LL | } | - expected one of 11 possible tokens here @@ -16,40 +16,15 @@ LL | } | ^ unexpected token error[E0407]: method `how_are_you` is not a member of trait `Howness` - --> $DIR/issue-58856-2.rs:3:5 + --> $DIR/issue-58856-2.rs:6:5 | LL | / fn how_are_you(&self -> Empty { LL | | //~^ ERROR expected one of `)` or `,`, found `->` LL | | //~| ERROR method `how_are_you` is not a member of trait `Howness` -LL | | //~| ERROR cannot find type `Empty` in this scope LL | | Empty -LL | | //~^ ERROR cannot find value `Empty` in this scope LL | | } | |_____^ not a member of trait `Howness` -error[E0412]: cannot find type `Empty` in this scope - --> $DIR/issue-58856-2.rs:3:29 - | -LL | fn how_are_you(&self -> Empty { - | ^^^^^ not found in this scope -help: possible candidates are found in other modules, you can import them into scope - | -LL | use std::io::Empty; - | -LL | use std::iter::Empty; - | - -error[E0425]: cannot find value `Empty` in this scope - --> $DIR/issue-58856-2.rs:7:9 - | -LL | Empty - | ^^^^^ not found in this scope -help: possible candidate is found in another module, you can import it into scope - | -LL | use std::sync::mpsc::TryRecvError::Empty; - | - -error: aborting due to 5 previous errors +error: aborting due to 3 previous errors -Some errors occurred: E0407, E0412, E0425. -For more information about an error, try `rustc --explain E0407`. +For more information about this error, try `rustc --explain E0407`. diff --git a/src/test/ui/parser/recover-enum2.rs b/src/test/ui/parser/recover-enum2.rs index 65a187737879d..7f2f2cc7ab039 100644 --- a/src/test/ui/parser/recover-enum2.rs +++ b/src/test/ui/parser/recover-enum2.rs @@ -25,9 +25,6 @@ fn main() { // fail again enum Test4 { Nope(i32 {}) //~ ERROR: found `{` - //~^ ERROR: found `{` } } - // still recover later - let bad_syntax = _; //~ ERROR: expected expression, found reserved identifier `_` } diff --git a/src/test/ui/parser/recover-enum2.stderr b/src/test/ui/parser/recover-enum2.stderr index 2473420a77930..315bfde77c73c 100644 --- a/src/test/ui/parser/recover-enum2.stderr +++ b/src/test/ui/parser/recover-enum2.stderr @@ -10,17 +10,5 @@ error: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `{` LL | Nope(i32 {}) //~ ERROR: found `{` | ^ expected one of 7 possible tokens here -error: expected one of `!`, `&&`, `&`, `(`, `)`, `*`, `+`, `,`, `::`, `<`, `?`, `[`, `_`, `crate`, `dyn`, `extern`, `fn`, `for`, `impl`, `pub`, `unsafe`, `}`, or lifetime, found `{` - --> $DIR/recover-enum2.rs:27:22 - | -LL | Nope(i32 {}) //~ ERROR: found `{` - | ^ expected one of 23 possible tokens here - -error: expected expression, found reserved identifier `_` - --> $DIR/recover-enum2.rs:32:22 - | -LL | let bad_syntax = _; //~ ERROR: expected expression, found reserved identifier `_` - | ^ expected expression - -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors From d60f22c4722da624fe7e1bd688ce22a62d1ab113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 1 Mar 2019 22:14:22 -0800 Subject: [PATCH 16/24] Emit unclosed delimiters during recovery --- src/libsyntax/parse/parser.rs | 1 + src/test/ui/issues/issue-58856-1.rs | 4 ++++ src/test/ui/issues/issue-58856-1.stderr | 10 +++++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 414c34de5e3d4..d2811c23e05db 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -778,6 +778,7 @@ impl<'a> Parser<'a> { // leave it in the input Ok(false) } else if self.last_unexpected_token_span == Some(self.span) { + emit_unclosed_delims(&self.unclosed_delims, self.diagnostic()); FatalError.raise(); } else { let mut expected = edible.iter() diff --git a/src/test/ui/issues/issue-58856-1.rs b/src/test/ui/issues/issue-58856-1.rs index f5edac0d2e316..7dc3658776e60 100644 --- a/src/test/ui/issues/issue-58856-1.rs +++ b/src/test/ui/issues/issue-58856-1.rs @@ -5,4 +5,8 @@ impl A { //~^ ERROR expected one of `)`, `,`, or `:`, found `>` } +// verify that mismatched delimiters get emitted +fn foo(] {} +//~^ ERROR incorrect close delimiter + fn main() {} diff --git a/src/test/ui/issues/issue-58856-1.stderr b/src/test/ui/issues/issue-58856-1.stderr index 85101e467b137..f26ebbe15c5b9 100644 --- a/src/test/ui/issues/issue-58856-1.stderr +++ b/src/test/ui/issues/issue-58856-1.stderr @@ -7,5 +7,13 @@ LL | fn b(self> {} | | help: `)` may belong here | unclosed delimiter -error: aborting due to previous error +error: incorrect close delimiter: `]` + --> $DIR/issue-58856-1.rs:9:8 + | +LL | fn foo(] {} + | -^ incorrect close delimiter + | | + | un-closed delimiter + +error: aborting due to 2 previous errors From a5c464aea4d044c9e0f362bc4f3afd4fedcee822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 1 Mar 2019 22:35:21 -0800 Subject: [PATCH 17/24] Reduce test case --- src/test/ui/issues/issue-58856-1.rs | 8 +------- src/test/ui/issues/issue-58856-1.stderr | 14 +++----------- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/test/ui/issues/issue-58856-1.rs b/src/test/ui/issues/issue-58856-1.rs index 7dc3658776e60..db3984cd18987 100644 --- a/src/test/ui/issues/issue-58856-1.rs +++ b/src/test/ui/issues/issue-58856-1.rs @@ -1,12 +1,6 @@ -struct A; - impl A { - fn b(self> {} + fn b(self> //~^ ERROR expected one of `)`, `,`, or `:`, found `>` } -// verify that mismatched delimiters get emitted -fn foo(] {} -//~^ ERROR incorrect close delimiter - fn main() {} diff --git a/src/test/ui/issues/issue-58856-1.stderr b/src/test/ui/issues/issue-58856-1.stderr index f26ebbe15c5b9..20cdf55365fc7 100644 --- a/src/test/ui/issues/issue-58856-1.stderr +++ b/src/test/ui/issues/issue-58856-1.stderr @@ -1,19 +1,11 @@ error: expected one of `)`, `,`, or `:`, found `>` - --> $DIR/issue-58856-1.rs:4:14 + --> $DIR/issue-58856-1.rs:2:14 | -LL | fn b(self> {} +LL | fn b(self> | - ^ | | | | | help: `)` may belong here | unclosed delimiter -error: incorrect close delimiter: `]` - --> $DIR/issue-58856-1.rs:9:8 - | -LL | fn foo(] {} - | -^ incorrect close delimiter - | | - | un-closed delimiter - -error: aborting due to 2 previous errors +error: aborting due to previous error From d5b360695e36b056273bdcb4a8bb45c0e0db8ed0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 3 Mar 2019 11:13:19 -0800 Subject: [PATCH 18/24] Panic when unmatched delimiters aren't emitted --- src/libsyntax/ext/tt/macro_parser.rs | 2 +- src/libsyntax/parse/parser.rs | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/ext/tt/macro_parser.rs b/src/libsyntax/ext/tt/macro_parser.rs index fe1cffb092b1c..1a419e7fadaa0 100644 --- a/src/libsyntax/ext/tt/macro_parser.rs +++ b/src/libsyntax/ext/tt/macro_parser.rs @@ -761,7 +761,7 @@ pub fn parse( else if bb_items.is_empty() && next_items.is_empty() { return Failure( parser.span, - parser.token, + parser.token.clone(), "no rules expected this token in macro call", ); } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d2811c23e05db..72c38c675b3f7 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -259,6 +259,13 @@ pub struct Parser<'a> { last_unexpected_token_span: Option, } +impl<'a> Drop for Parser<'a> { + fn drop(&mut self) { + if !self.unclosed_delims.is_empty() { + panic!("unclosed delimiter errors not emitted"); + } + } +} #[derive(Clone)] struct TokenCursor { From d21b8522ea06699d6037432adef4df5f1d66bbbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 3 Mar 2019 12:14:25 -0800 Subject: [PATCH 19/24] Emit missing unclosed delimiter errors --- src/librustc_metadata/cstore_impl.rs | 4 +- src/libsyntax/parse/parser.rs | 14 ++++-- src/libsyntax/parse/token.rs | 12 ++--- src/libsyntax_ext/proc_macro_server.rs | 4 +- src/test/ui/parser-recovery-2.stderr | 12 ++--- src/test/ui/resolve/token-error-correct-3.rs | 16 +++--- .../ui/resolve/token-error-correct-3.stderr | 49 +++++++++---------- 7 files changed, 56 insertions(+), 55 deletions(-) diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index f49b88f14e60e..67a249e605ecc 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -439,8 +439,8 @@ impl cstore::CStore { let source_file = sess.parse_sess.source_map().new_source_file(source_name, def.body); let local_span = Span::new(source_file.start_pos, source_file.end_pos, NO_EXPANSION); - let (body, errors) = source_file_to_stream(&sess.parse_sess, source_file, None); - emit_unclosed_delims(&errors, &sess.diagnostic()); + let (body, mut errors) = source_file_to_stream(&sess.parse_sess, source_file, None); + emit_unclosed_delims(&mut errors, &sess.diagnostic()); // Mark the attrs as used let attrs = data.get_item_attrs(id.index, sess); diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 72c38c675b3f7..317e65c627eaf 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -7785,7 +7785,10 @@ impl<'a> Parser<'a> { attributes_allowed: bool, ) -> PResult<'a, Option>> { let (ret, tokens) = self.collect_tokens(|this| { - this.parse_item_implementation(attrs, macros_allowed, attributes_allowed) + let item = this.parse_item_implementation(attrs, macros_allowed, attributes_allowed); + let diag = this.diagnostic(); + emit_unclosed_delims(&mut this.unclosed_delims, diag); + item })?; // Once we've parsed an item and recorded the tokens we got while @@ -8532,8 +8535,8 @@ impl<'a> Parser<'a> { module: self.parse_mod_items(&token::Eof, lo)?, span: lo.to(self.span), }); - emit_unclosed_delims(&self.unclosed_delims, self.diagnostic()); - self.unclosed_delims.clear(); + let diag = self.diagnostic(); + emit_unclosed_delims(&mut self.unclosed_delims, diag); krate } @@ -8564,8 +8567,8 @@ impl<'a> Parser<'a> { } } -pub fn emit_unclosed_delims(unclosed_delims: &[UnmatchedBrace], handler: &errors::Handler) { - for unmatched in unclosed_delims { +pub fn emit_unclosed_delims(unclosed_delims: &mut Vec, handler: &errors::Handler) { + for unmatched in unclosed_delims.iter() { let mut err = handler.struct_span_err(unmatched.found_span, &format!( "incorrect close delimiter: `{}`", pprust::token_to_string(&token::Token::CloseDelim(unmatched.found_delim)), @@ -8579,4 +8582,5 @@ pub fn emit_unclosed_delims(unclosed_delims: &[UnmatchedBrace], handler: &errors } err.emit(); } + unclosed_delims.clear(); } diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index eec422d6266c3..bb4da12bae893 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -675,9 +675,9 @@ impl Nonterminal { // FIXME(#43081): Avoid this pretty-print + reparse hack let source = pprust::nonterminal_to_string(self); let filename = FileName::macro_expansion_source_code(&source); - let (tokens_for_real, errors) = + let (tokens_for_real, mut errors) = parse_stream_from_source_str(filename, source, sess, Some(span)); - emit_unclosed_delims(&errors, &sess.span_diagnostic); + emit_unclosed_delims(&mut errors, &sess.span_diagnostic); // During early phases of the compiler the AST could get modified // directly (e.g., attributes added or removed) and the internal cache @@ -740,13 +740,13 @@ fn prepend_attrs(sess: &ParseSess, let source = pprust::attr_to_string(attr); let macro_filename = FileName::macro_expansion_source_code(&source); if attr.is_sugared_doc { - let (stream, errors) = parse_stream_from_source_str( + let (stream, mut errors) = parse_stream_from_source_str( macro_filename, source, sess, Some(span), ); - emit_unclosed_delims(&errors, &sess.span_diagnostic); + emit_unclosed_delims(&mut errors, &sess.span_diagnostic); builder.push(stream); continue } @@ -763,13 +763,13 @@ fn prepend_attrs(sess: &ParseSess, // ... and for more complicated paths, fall back to a reparse hack that // should eventually be removed. } else { - let (stream, errors) = parse_stream_from_source_str( + let (stream, mut errors) = parse_stream_from_source_str( macro_filename, source, sess, Some(span), ); - emit_unclosed_delims(&errors, &sess.span_diagnostic); + emit_unclosed_delims(&mut errors, &sess.span_diagnostic); brackets.push(stream); } diff --git a/src/libsyntax_ext/proc_macro_server.rs b/src/libsyntax_ext/proc_macro_server.rs index 4c4b33c04422b..5822b5607f7df 100644 --- a/src/libsyntax_ext/proc_macro_server.rs +++ b/src/libsyntax_ext/proc_macro_server.rs @@ -410,13 +410,13 @@ impl server::TokenStream for Rustc<'_> { stream.is_empty() } fn from_str(&mut self, src: &str) -> Self::TokenStream { - let (tokens, errors) = parse::parse_stream_from_source_str( + let (tokens, mut errors) = parse::parse_stream_from_source_str( FileName::proc_macro_source_code(src.clone()), src.to_string(), self.sess, Some(self.call_site), ); - emit_unclosed_delims(&errors, &self.sess.span_diagnostic); + emit_unclosed_delims(&mut errors, &self.sess.span_diagnostic); tokens } fn to_string(&mut self, stream: &Self::TokenStream) -> String { diff --git a/src/test/ui/parser-recovery-2.stderr b/src/test/ui/parser-recovery-2.stderr index 76f7af38e776d..92d8cbc100a03 100644 --- a/src/test/ui/parser-recovery-2.stderr +++ b/src/test/ui/parser-recovery-2.stderr @@ -1,9 +1,3 @@ -error: unexpected token: `;` - --> $DIR/parser-recovery-2.rs:12:15 - | -LL | let x = y.; //~ ERROR unexpected token - | ^ - error: incorrect close delimiter: `)` --> $DIR/parser-recovery-2.rs:8:5 | @@ -13,6 +7,12 @@ LL | let x = foo(); //~ ERROR cannot find function `foo` in this scope LL | ) //~ ERROR incorrect close delimiter: `)` | ^ incorrect close delimiter +error: unexpected token: `;` + --> $DIR/parser-recovery-2.rs:12:15 + | +LL | let x = y.; //~ ERROR unexpected token + | ^ + error[E0425]: cannot find function `foo` in this scope --> $DIR/parser-recovery-2.rs:7:17 | diff --git a/src/test/ui/resolve/token-error-correct-3.rs b/src/test/ui/resolve/token-error-correct-3.rs index b1ca0bbfc57c1..05bdbeacf72fe 100644 --- a/src/test/ui/resolve/token-error-correct-3.rs +++ b/src/test/ui/resolve/token-error-correct-3.rs @@ -10,16 +10,14 @@ pub mod raw { pub fn ensure_dir_exists, F: FnOnce(&Path)>(path: P, callback: F) -> io::Result { - if !is_directory(path.as_ref()) { //~ ERROR: cannot find function `is_directory` - callback(path.as_ref(); //~ ERROR expected one of - fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: mismatched types - //~^ expected (), found enum `std::result::Result` - //~| expected type `()` - //~| found type `std::result::Result` - //~| expected one of + if !is_directory(path.as_ref()) { + //~^ ERROR cannot find function `is_directory` + callback(path.as_ref(); + //~^ ERROR expected one of + //~| ERROR this function takes 1 parameter but 2 parameters were supplied + fs::create_dir_all(path.as_ref()).map(|()| true) } else { - //~^ ERROR: expected one of - //~| unexpected token + //~^ ERROR incorrect close delimiter: `}` Ok(false); } diff --git a/src/test/ui/resolve/token-error-correct-3.stderr b/src/test/ui/resolve/token-error-correct-3.stderr index a6bb83c71f313..0f1cbd6c2f772 100644 --- a/src/test/ui/resolve/token-error-correct-3.stderr +++ b/src/test/ui/resolve/token-error-correct-3.stderr @@ -1,39 +1,38 @@ -error: expected one of `)`, `,`, `.`, `?`, or an operator, found `;` - --> $DIR/token-error-correct-3.rs:14:35 - | -LL | callback(path.as_ref(); //~ ERROR expected one of - | - ^ - | | | - | | help: `)` may belong here - | unclosed delimiter - -error: expected one of `.`, `;`, `?`, `}`, or an operator, found `)` - --> $DIR/token-error-correct-3.rs:20:9 +error: incorrect close delimiter: `}` + --> $DIR/token-error-correct-3.rs:19:9 | -LL | fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: mismatched types - | - expected one of `.`, `;`, `?`, `}`, or an operator here +LL | if !is_directory(path.as_ref()) { + | - close delimiter possibly meant for this +LL | //~^ ERROR cannot find function `is_directory` +LL | callback(path.as_ref(); + | - un-closed delimiter ... LL | } else { - | ^ unexpected token + | ^ incorrect close delimiter + +error: expected one of `)`, `,`, `.`, `?`, or an operator, found `;` + --> $DIR/token-error-correct-3.rs:15:35 + | +LL | callback(path.as_ref(); + | ^ expected one of `)`, `,`, `.`, `?`, or an operator here error[E0425]: cannot find function `is_directory` in this scope --> $DIR/token-error-correct-3.rs:13:13 | -LL | if !is_directory(path.as_ref()) { //~ ERROR: cannot find function `is_directory` +LL | if !is_directory(path.as_ref()) { | ^^^^^^^^^^^^ not found in this scope -error[E0308]: mismatched types +error[E0057]: this function takes 1 parameter but 2 parameters were supplied --> $DIR/token-error-correct-3.rs:15:13 | -LL | fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: mismatched types - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try adding a semicolon: `;` - | | - | expected (), found enum `std::result::Result` - | - = note: expected type `()` - found type `std::result::Result` +LL | / callback(path.as_ref(); +LL | | //~^ ERROR expected one of +LL | | //~| ERROR this function takes 1 parameter but 2 parameters were supplied +LL | | fs::create_dir_all(path.as_ref()).map(|()| true) +LL | | } else { + | |_________^ expected 1 parameter error: aborting due to 4 previous errors -Some errors occurred: E0308, E0425. -For more information about an error, try `rustc --explain E0308`. +Some errors occurred: E0057, E0425. +For more information about an error, try `rustc --explain E0057`. From a460b4ba6aad7664bd016f8d247238194ada09e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 3 Mar 2019 12:45:49 -0800 Subject: [PATCH 20/24] Collect unclosed delimiters in parent parser --- src/libsyntax/parse/parser.rs | 17 +++++-- src/test/ui/parser-recovery-2.stderr | 12 ++--- src/test/ui/resolve/token-error-correct-3.rs | 4 +- .../ui/resolve/token-error-correct-3.stderr | 47 ++++++++++--------- 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 317e65c627eaf..428d88bdaf85c 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1510,9 +1510,13 @@ impl<'a> Parser<'a> { pub fn parse_trait_item(&mut self, at_end: &mut bool) -> PResult<'a, TraitItem> { maybe_whole!(self, NtTraitItem, |x| x); let attrs = self.parse_outer_attributes()?; + let mut unclosed_delims = vec![]; let (mut item, tokens) = self.collect_tokens(|this| { - this.parse_trait_item_(at_end, attrs) + let item = this.parse_trait_item_(at_end, attrs); + unclosed_delims.append(&mut this.unclosed_delims); + item })?; + self.unclosed_delims.append(&mut unclosed_delims); // See `parse_item` for why this clause is here. if !item.attrs.iter().any(|attr| attr.style == AttrStyle::Inner) { item.tokens = Some(tokens); @@ -6462,9 +6466,13 @@ impl<'a> Parser<'a> { pub fn parse_impl_item(&mut self, at_end: &mut bool) -> PResult<'a, ImplItem> { maybe_whole!(self, NtImplItem, |x| x); let attrs = self.parse_outer_attributes()?; + let mut unclosed_delims = vec![]; let (mut item, tokens) = self.collect_tokens(|this| { - this.parse_impl_item_(at_end, attrs) + let item = this.parse_impl_item_(at_end, attrs); + unclosed_delims.append(&mut this.unclosed_delims); + item })?; + self.unclosed_delims.append(&mut unclosed_delims); // See `parse_item` for why this clause is here. if !item.attrs.iter().any(|attr| attr.style == AttrStyle::Inner) { @@ -7784,12 +7792,13 @@ impl<'a> Parser<'a> { macros_allowed: bool, attributes_allowed: bool, ) -> PResult<'a, Option>> { + let mut unclosed_delims = vec![]; let (ret, tokens) = self.collect_tokens(|this| { let item = this.parse_item_implementation(attrs, macros_allowed, attributes_allowed); - let diag = this.diagnostic(); - emit_unclosed_delims(&mut this.unclosed_delims, diag); + unclosed_delims.append(&mut this.unclosed_delims); item })?; + self.unclosed_delims.append(&mut unclosed_delims); // Once we've parsed an item and recorded the tokens we got while // parsing we may want to store `tokens` into the item we're about to diff --git a/src/test/ui/parser-recovery-2.stderr b/src/test/ui/parser-recovery-2.stderr index 92d8cbc100a03..76f7af38e776d 100644 --- a/src/test/ui/parser-recovery-2.stderr +++ b/src/test/ui/parser-recovery-2.stderr @@ -1,3 +1,9 @@ +error: unexpected token: `;` + --> $DIR/parser-recovery-2.rs:12:15 + | +LL | let x = y.; //~ ERROR unexpected token + | ^ + error: incorrect close delimiter: `)` --> $DIR/parser-recovery-2.rs:8:5 | @@ -7,12 +13,6 @@ LL | let x = foo(); //~ ERROR cannot find function `foo` in this scope LL | ) //~ ERROR incorrect close delimiter: `)` | ^ incorrect close delimiter -error: unexpected token: `;` - --> $DIR/parser-recovery-2.rs:12:15 - | -LL | let x = y.; //~ ERROR unexpected token - | ^ - error[E0425]: cannot find function `foo` in this scope --> $DIR/parser-recovery-2.rs:7:17 | diff --git a/src/test/ui/resolve/token-error-correct-3.rs b/src/test/ui/resolve/token-error-correct-3.rs index 05bdbeacf72fe..212b88ac8b05f 100644 --- a/src/test/ui/resolve/token-error-correct-3.rs +++ b/src/test/ui/resolve/token-error-correct-3.rs @@ -14,10 +14,10 @@ pub mod raw { //~^ ERROR cannot find function `is_directory` callback(path.as_ref(); //~^ ERROR expected one of - //~| ERROR this function takes 1 parameter but 2 parameters were supplied fs::create_dir_all(path.as_ref()).map(|()| true) + //~^ ERROR mismatched types } else { - //~^ ERROR incorrect close delimiter: `}` + //~^ ERROR expected one of `.`, `;`, `?`, `}`, or an operator, found `)` Ok(false); } diff --git a/src/test/ui/resolve/token-error-correct-3.stderr b/src/test/ui/resolve/token-error-correct-3.stderr index 0f1cbd6c2f772..035a5ede45384 100644 --- a/src/test/ui/resolve/token-error-correct-3.stderr +++ b/src/test/ui/resolve/token-error-correct-3.stderr @@ -1,20 +1,20 @@ -error: incorrect close delimiter: `}` - --> $DIR/token-error-correct-3.rs:19:9 - | -LL | if !is_directory(path.as_ref()) { - | - close delimiter possibly meant for this -LL | //~^ ERROR cannot find function `is_directory` -LL | callback(path.as_ref(); - | - un-closed delimiter -... -LL | } else { - | ^ incorrect close delimiter - error: expected one of `)`, `,`, `.`, `?`, or an operator, found `;` --> $DIR/token-error-correct-3.rs:15:35 | LL | callback(path.as_ref(); - | ^ expected one of `)`, `,`, `.`, `?`, or an operator here + | - ^ + | | | + | | help: `)` may belong here + | unclosed delimiter + +error: expected one of `.`, `;`, `?`, `}`, or an operator, found `)` + --> $DIR/token-error-correct-3.rs:19:9 + | +LL | fs::create_dir_all(path.as_ref()).map(|()| true) + | - expected one of `.`, `;`, `?`, `}`, or an operator here +LL | //~^ ERROR mismatched types +LL | } else { + | ^ unexpected token error[E0425]: cannot find function `is_directory` in this scope --> $DIR/token-error-correct-3.rs:13:13 @@ -22,17 +22,18 @@ error[E0425]: cannot find function `is_directory` in this scope LL | if !is_directory(path.as_ref()) { | ^^^^^^^^^^^^ not found in this scope -error[E0057]: this function takes 1 parameter but 2 parameters were supplied - --> $DIR/token-error-correct-3.rs:15:13 +error[E0308]: mismatched types + --> $DIR/token-error-correct-3.rs:17:13 + | +LL | fs::create_dir_all(path.as_ref()).map(|()| true) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- help: try adding a semicolon: `;` + | | + | expected (), found enum `std::result::Result` | -LL | / callback(path.as_ref(); -LL | | //~^ ERROR expected one of -LL | | //~| ERROR this function takes 1 parameter but 2 parameters were supplied -LL | | fs::create_dir_all(path.as_ref()).map(|()| true) -LL | | } else { - | |_________^ expected 1 parameter + = note: expected type `()` + found type `std::result::Result` error: aborting due to 4 previous errors -Some errors occurred: E0057, E0425. -For more information about an error, try `rustc --explain E0057`. +Some errors occurred: E0308, E0425. +For more information about an error, try `rustc --explain E0308`. From 51c47f32e4101069b81bf91a42b9ae52a2f7741f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 3 Mar 2019 14:11:41 -0800 Subject: [PATCH 21/24] Always emit mismatched delim errors, never panic --- src/libsyntax/parse/parser.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 428d88bdaf85c..2f1bee9b61173 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -262,7 +262,8 @@ pub struct Parser<'a> { impl<'a> Drop for Parser<'a> { fn drop(&mut self) { if !self.unclosed_delims.is_empty() { - panic!("unclosed delimiter errors not emitted"); + let diag = self.diagnostic(); + emit_unclosed_delims(&mut self.unclosed_delims, diag); } } } @@ -8544,8 +8545,6 @@ impl<'a> Parser<'a> { module: self.parse_mod_items(&token::Eof, lo)?, span: lo.to(self.span), }); - let diag = self.diagnostic(); - emit_unclosed_delims(&mut self.unclosed_delims, diag); krate } From e808e1e0f0a7454c4cf2261c2a9c4d67af1a5e47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 3 Mar 2019 16:59:24 -0800 Subject: [PATCH 22/24] Add regression test for #58886 --- .../ui/parser/unclosed-delimiter-in-dep.rs | 6 +++++ .../parser/unclosed-delimiter-in-dep.stderr | 23 +++++++++++++++++++ src/test/ui/parser/unclosed_delim_mod.rs | 6 +++++ src/test/ui/parser/unclosed_delim_mod.stderr | 18 +++++++++++++++ 4 files changed, 53 insertions(+) create mode 100644 src/test/ui/parser/unclosed-delimiter-in-dep.rs create mode 100644 src/test/ui/parser/unclosed-delimiter-in-dep.stderr create mode 100644 src/test/ui/parser/unclosed_delim_mod.rs create mode 100644 src/test/ui/parser/unclosed_delim_mod.stderr diff --git a/src/test/ui/parser/unclosed-delimiter-in-dep.rs b/src/test/ui/parser/unclosed-delimiter-in-dep.rs new file mode 100644 index 0000000000000..6db1b66e9f785 --- /dev/null +++ b/src/test/ui/parser/unclosed-delimiter-in-dep.rs @@ -0,0 +1,6 @@ +mod unclosed_delim_mod; + +fn main() { + let _: usize = unclosed_delim_mod::new(); + //~^ ERROR mismatched types +} diff --git a/src/test/ui/parser/unclosed-delimiter-in-dep.stderr b/src/test/ui/parser/unclosed-delimiter-in-dep.stderr new file mode 100644 index 0000000000000..633c63bea9105 --- /dev/null +++ b/src/test/ui/parser/unclosed-delimiter-in-dep.stderr @@ -0,0 +1,23 @@ +error: incorrect close delimiter: `}` + --> $DIR/unclosed_delim_mod.rs:5:1 + | +LL | pub fn new() -> Result { + | - close delimiter possibly meant for this +LL | Ok(Value { + | - un-closed delimiter +LL | } +LL | } + | ^ incorrect close delimiter + +error[E0308]: mismatched types + --> $DIR/unclosed-delimiter-in-dep.rs:4:20 + | +LL | let _: usize = unclosed_delim_mod::new(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected usize, found enum `std::result::Result` + | + = note: expected type `usize` + found type `std::result::Result` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/parser/unclosed_delim_mod.rs b/src/test/ui/parser/unclosed_delim_mod.rs new file mode 100644 index 0000000000000..b1664f49dc591 --- /dev/null +++ b/src/test/ui/parser/unclosed_delim_mod.rs @@ -0,0 +1,6 @@ +pub struct Value {} +pub fn new() -> Result { + Ok(Value { + } +} +//~^ ERROR incorrect close delimiter diff --git a/src/test/ui/parser/unclosed_delim_mod.stderr b/src/test/ui/parser/unclosed_delim_mod.stderr new file mode 100644 index 0000000000000..cc04eb531cbea --- /dev/null +++ b/src/test/ui/parser/unclosed_delim_mod.stderr @@ -0,0 +1,18 @@ +error: incorrect close delimiter: `}` + --> $DIR/unclosed_delim_mod.rs:5:1 + | +LL | pub fn new() -> Result { + | - close delimiter possibly meant for this +LL | Ok(Value { + | - un-closed delimiter +LL | } +LL | } + | ^ incorrect close delimiter + +error[E0601]: `main` function not found in crate `unclosed_delim_mod` + | + = note: consider adding a `main` function to `$DIR/unclosed_delim_mod.rs` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0601`. From 78554edf574f96b0d3cccf502d028b3a69f36832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 4 Mar 2019 12:59:43 -0800 Subject: [PATCH 23/24] Simplify code --- src/libsyntax/parse/mod.rs | 11 +++++++++-- src/libsyntax/parse/parser.rs | 6 ++---- src/libsyntax/parse/token.rs | 21 +++------------------ src/libsyntax_ext/proc_macro_server.rs | 7 ++----- 4 files changed, 16 insertions(+), 29 deletions(-) diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index b2d4d97d57d89..6583458b44694 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -6,6 +6,7 @@ use crate::source_map::{SourceMap, FilePathMapping}; use crate::feature_gate::UnstableFeatures; use crate::parse::parser::Parser; use crate::symbol::Symbol; +use crate::syntax::parse::parser::emit_unclosed_delims; use crate::tokenstream::{TokenStream, TokenTree}; use crate::diagnostics::plugin::ErrorMap; use crate::print::pprust::token_to_string; @@ -141,8 +142,14 @@ pub fn parse_stream_from_source_str( source: String, sess: &ParseSess, override_span: Option, -) -> (TokenStream, Vec) { - source_file_to_stream(sess, sess.source_map().new_source_file(name, source), override_span) +) -> TokenStream { + let (stream, mut errors) = source_file_to_stream( + sess, + sess.source_map().new_source_file(name, source), + override_span, + ); + emit_unclosed_delims(&mut errors, &sess.span_diagnostic); + stream } /// Creates a new parser from a source string. diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 2f1bee9b61173..db55c014b9d73 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -261,10 +261,8 @@ pub struct Parser<'a> { impl<'a> Drop for Parser<'a> { fn drop(&mut self) { - if !self.unclosed_delims.is_empty() { - let diag = self.diagnostic(); - emit_unclosed_delims(&mut self.unclosed_delims, diag); - } + let diag = self.diagnostic(); + emit_unclosed_delims(&mut self.unclosed_delims, diag); } } diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index bb4da12bae893..2fa4f5263fbc5 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -10,7 +10,6 @@ use crate::print::pprust; use crate::ptr::P; use crate::symbol::keywords; use crate::syntax::parse::parse_stream_from_source_str; -use crate::syntax::parse::parser::emit_unclosed_delims; use crate::tokenstream::{self, DelimSpan, TokenStream, TokenTree}; use syntax_pos::symbol::{self, Symbol}; @@ -675,9 +674,7 @@ impl Nonterminal { // FIXME(#43081): Avoid this pretty-print + reparse hack let source = pprust::nonterminal_to_string(self); let filename = FileName::macro_expansion_source_code(&source); - let (tokens_for_real, mut errors) = - parse_stream_from_source_str(filename, source, sess, Some(span)); - emit_unclosed_delims(&mut errors, &sess.span_diagnostic); + let tokens_for_real = parse_stream_from_source_str(filename, source, sess, Some(span)); // During early phases of the compiler the AST could get modified // directly (e.g., attributes added or removed) and the internal cache @@ -740,13 +737,7 @@ fn prepend_attrs(sess: &ParseSess, let source = pprust::attr_to_string(attr); let macro_filename = FileName::macro_expansion_source_code(&source); if attr.is_sugared_doc { - let (stream, mut errors) = parse_stream_from_source_str( - macro_filename, - source, - sess, - Some(span), - ); - emit_unclosed_delims(&mut errors, &sess.span_diagnostic); + let stream = parse_stream_from_source_str(macro_filename, source, sess, Some(span)); builder.push(stream); continue } @@ -763,13 +754,7 @@ fn prepend_attrs(sess: &ParseSess, // ... and for more complicated paths, fall back to a reparse hack that // should eventually be removed. } else { - let (stream, mut errors) = parse_stream_from_source_str( - macro_filename, - source, - sess, - Some(span), - ); - emit_unclosed_delims(&mut errors, &sess.span_diagnostic); + let stream = parse_stream_from_source_str(macro_filename, source, sess, Some(span)); brackets.push(stream); } diff --git a/src/libsyntax_ext/proc_macro_server.rs b/src/libsyntax_ext/proc_macro_server.rs index 5822b5607f7df..a7ac95ba9ef50 100644 --- a/src/libsyntax_ext/proc_macro_server.rs +++ b/src/libsyntax_ext/proc_macro_server.rs @@ -12,7 +12,6 @@ use syntax::ast; use syntax::ext::base::ExtCtxt; use syntax::parse::lexer::comments; use syntax::parse::{self, token, ParseSess}; -use syntax::parse::parser::emit_unclosed_delims; use syntax::tokenstream::{self, DelimSpan, IsJoint::*, TokenStream, TreeAndJoint}; use syntax_pos::hygiene::{SyntaxContext, Transparency}; use syntax_pos::symbol::{keywords, Symbol}; @@ -410,14 +409,12 @@ impl server::TokenStream for Rustc<'_> { stream.is_empty() } fn from_str(&mut self, src: &str) -> Self::TokenStream { - let (tokens, mut errors) = parse::parse_stream_from_source_str( + parse::parse_stream_from_source_str( FileName::proc_macro_source_code(src.clone()), src.to_string(), self.sess, Some(self.call_site), - ); - emit_unclosed_delims(&mut errors, &self.sess.span_diagnostic); - tokens + ) } fn to_string(&mut self, stream: &Self::TokenStream) -> String { stream.to_string() From 159cc95c176236585abcf46ed2be88cd1b08f1bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 6 Mar 2019 19:09:24 -0800 Subject: [PATCH 24/24] Rely on drop to emit unclosed delims --- src/libsyntax/parse/parser.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index db55c014b9d73..42aebfce383ed 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -784,7 +784,6 @@ impl<'a> Parser<'a> { // leave it in the input Ok(false) } else if self.last_unexpected_token_span == Some(self.span) { - emit_unclosed_delims(&self.unclosed_delims, self.diagnostic()); FatalError.raise(); } else { let mut expected = edible.iter()