From 3b229f144160067409dd9d321748ab7ae77bd99c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 27 Jul 2019 11:02:52 +0200 Subject: [PATCH 01/37] check that ptr is valid already when doing Deref, not only when doing the access --- src/librustc_mir/interpret/place.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 8fe882934dfb5..e90fc28a521b6 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -304,7 +304,16 @@ where ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { let val = self.read_immediate(src)?; trace!("deref to {} on {:?}", val.layout.ty, *val); - self.ref_to_mplace(val) + let mut place = self.ref_to_mplace(val)?; + let (size, align) = self.size_and_align_of_mplace(place)? + .unwrap_or((place.layout.size, place.layout.align.abi)); + assert!(place.mplace.align <= align, "dynamic alignment less strict than static one?"); + place.mplace.align = align; // maximally strict checking + // When dereferencing a pointer, it must be non-NULL, aligned, and live. + if let Some(ptr) = self.check_mplace_access(place, Some(size))? { + place.mplace.ptr = ptr.into(); + } + Ok(place) } /// Check if the given place is good for memory access with the given From 5800bec22363615fe7a3b55b8e15a28dc51f687b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Jul 2019 12:10:29 +0200 Subject: [PATCH 02/37] discourage use of ref_to_mplace --- src/librustc_mir/interpret/place.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index e90fc28a521b6..d0670f64ed8da 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -277,6 +277,10 @@ where { /// Take a value, which represents a (thin or fat) reference, and make it a place. /// Alignment is just based on the type. This is the inverse of `MemPlace::to_ref()`. + /// + /// Only call this if you are sure the place is "valid" (aligned and inbounds), or do not + /// want to ever use the place for memory access! + /// Generally prefer `deref_operand`. pub fn ref_to_mplace( &self, val: ImmTy<'tcx, M::PointerTag>, From 3677c5be56168508fea082e1651c774e34600ca8 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Jul 2019 12:29:20 +0200 Subject: [PATCH 03/37] the alignment checks on access can no longer fail now --- src/librustc_mir/interpret/operand.rs | 4 +++- src/librustc_mir/interpret/place.rs | 10 +++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 1816171d7b127..974481a993c67 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -238,7 +238,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return Ok(None); } - let ptr = match self.check_mplace_access(mplace, None)? { + let ptr = match self.check_mplace_access(mplace, None) + .expect("places should be checked on creation") + { Some(ptr) => ptr, None => return Ok(Some(ImmTy { // zero-sized type imm: Immediate::Scalar(Scalar::zst().into()), diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index d0670f64ed8da..1588ed35b13f2 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -763,7 +763,9 @@ where // to handle padding properly, which is only correct if we never look at this data with the // wrong type. - let ptr = match self.check_mplace_access(dest, None)? { + let ptr = match self.check_mplace_access(dest, None) + .expect("places should be checked on creation") + { Some(ptr) => ptr, None => return Ok(()), // zero-sized access }; @@ -866,8 +868,10 @@ where }); assert_eq!(src.meta, dest.meta, "Can only copy between equally-sized instances"); - let src = self.check_mplace_access(src, Some(size))?; - let dest = self.check_mplace_access(dest, Some(size))?; + let src = self.check_mplace_access(src, Some(size)) + .expect("places should be checked on creation"); + let dest = self.check_mplace_access(dest, Some(size)) + .expect("places should be checked on creation"); let (src_ptr, dest_ptr) = match (src, dest) { (Some(src_ptr), Some(dest_ptr)) => (src_ptr, dest_ptr), (None, None) => return Ok(()), // zero-sized copy From e4c39e1bc28185cc85511baa4bd4fd8b2fe29aa1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Jul 2019 14:19:13 +0200 Subject: [PATCH 04/37] better name for check_in_alloc --- src/librustc/mir/interpret/pointer.rs | 5 ++++- src/librustc_mir/interpret/memory.rs | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/librustc/mir/interpret/pointer.rs b/src/librustc/mir/interpret/pointer.rs index 0e3b8459115e3..fceae75d72421 100644 --- a/src/librustc/mir/interpret/pointer.rs +++ b/src/librustc/mir/interpret/pointer.rs @@ -191,8 +191,11 @@ impl<'tcx, Tag> Pointer { Pointer { alloc_id: self.alloc_id, offset: self.offset, tag: () } } + /// Test if the pointer is "inbounds" of an allocation of the given size. + /// A pointer is "inbounds" even if its offset is equal to the size; this is + /// a "one-past-the-end" pointer. #[inline(always)] - pub fn check_in_alloc( + pub fn check_inbounds_alloc( self, allocation_size: Size, msg: CheckInAllocMsg, diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 4575784ac3703..ad1ec5a11ed6c 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -357,7 +357,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // It is sufficient to check this for the end pointer. The addition // checks for overflow. let end_ptr = ptr.offset(size, self)?; - end_ptr.check_in_alloc(allocation_size, CheckInAllocMsg::MemoryAccessTest)?; + end_ptr.check_inbounds_alloc(allocation_size, CheckInAllocMsg::MemoryAccessTest)?; // Test align. Check this last; if both bounds and alignment are violated // we want the error to be about the bounds. if alloc_align.bytes() < align.bytes() { @@ -387,7 +387,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> bool { let (size, _align) = self.get_size_and_align(ptr.alloc_id, AllocCheck::MaybeDead) .expect("alloc info with MaybeDead cannot fail"); - ptr.check_in_alloc(size, CheckInAllocMsg::NullPointerTest).is_err() + ptr.check_inbounds_alloc(size, CheckInAllocMsg::NullPointerTest).is_err() } } From 388d99d3a555e8c47c9465b896966298d11dc43f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Jul 2019 14:25:04 +0200 Subject: [PATCH 05/37] fix tests --- src/test/ui/consts/const-eval/ub-nonnull.rs | 7 +++-- .../ui/consts/const-eval/ub-nonnull.stderr | 29 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/test/ui/consts/const-eval/ub-nonnull.rs b/src/test/ui/consts/const-eval/ub-nonnull.rs index bcbb4358aec03..431ff356ade19 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.rs +++ b/src/test/ui/consts/const-eval/ub-nonnull.rs @@ -11,10 +11,11 @@ const NON_NULL_PTR: NonNull = unsafe { mem::transmute(&1) }; const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; //~^ ERROR it is undefined behavior to use this value +#[deny(const_err)] // this triggers a `const_err` so validation does not even happen const OUT_OF_BOUNDS_PTR: NonNull = { unsafe { -//~^ ERROR it is undefined behavior to use this value - let ptr: &(u8, u8, u8) = mem::transmute(&0u8); // &0 gets promoted so it does not dangle - let out_of_bounds_ptr = &ptr.2; // use address-of-field for pointer arithmetic + let ptr: &[u8; 256] = mem::transmute(&0u8); // &0 gets promoted so it does not dangle + // Use address-of-element for pointer arithmetic. This could wrap around to NULL! + let out_of_bounds_ptr = &ptr[255]; //~ ERROR any use of this value will cause an error mem::transmute(out_of_bounds_ptr) } }; diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr index 2f9423fed3530..7b3c97e5fbf96 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.stderr @@ -6,21 +6,26 @@ LL | const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior -error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:14:1 +error: any use of this value will cause an error + --> $DIR/ub-nonnull.rs:18:29 | LL | / const OUT_OF_BOUNDS_PTR: NonNull = { unsafe { -LL | | -LL | | let ptr: &(u8, u8, u8) = mem::transmute(&0u8); // &0 gets promoted so it does not dangle -LL | | let out_of_bounds_ptr = &ptr.2; // use address-of-field for pointer arithmetic +LL | | let ptr: &[u8; 256] = mem::transmute(&0u8); // &0 gets promoted so it does not dangle +LL | | // Use address-of-element for pointer arithmetic. This could wrap around to NULL! +LL | | let out_of_bounds_ptr = &ptr[255]; + | | ^^^^^^^^^ Memory access failed: pointer must be in-bounds at offset 256, but is outside bounds of allocation 6 which has size 1 LL | | mem::transmute(out_of_bounds_ptr) LL | | } }; - | |____^ type validation failed: encountered a potentially NULL pointer, but expected something that cannot possibly fail to be greater or equal to 1 + | |____- | - = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior +note: lint level defined here + --> $DIR/ub-nonnull.rs:14:8 + | +LL | #[deny(const_err)] // this triggers a `const_err` so validation does not even happen + | ^^^^^^^^^ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:21:1 + --> $DIR/ub-nonnull.rs:22:1 | LL | const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -28,7 +33,7 @@ LL | const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:23:1 + --> $DIR/ub-nonnull.rs:24:1 | LL | const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -36,7 +41,7 @@ LL | const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:30:1 + --> $DIR/ub-nonnull.rs:31:1 | LL | const UNINIT: NonZeroU8 = unsafe { Transmute { uninit: () }.out }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected something greater or equal to 1 @@ -44,7 +49,7 @@ LL | const UNINIT: NonZeroU8 = unsafe { Transmute { uninit: () }.out }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:38:1 + --> $DIR/ub-nonnull.rs:39:1 | LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 10..=30 @@ -52,7 +57,7 @@ LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:44:1 + --> $DIR/ub-nonnull.rs:45:1 | LL | const BAD_RANGE2: RestrictedRange2 = unsafe { RestrictedRange2(20) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 20, but expected something less or equal to 10, or greater or equal to 30 From 74fbdb6eb8826c2a6627b666091a16691637832d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Jul 2019 22:41:52 +0200 Subject: [PATCH 06/37] move 'get me the access-checked version of an mplace' into separate function --- src/librustc_mir/interpret/place.rs | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index 1588ed35b13f2..d35f9127da40c 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -308,16 +308,8 @@ where ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { let val = self.read_immediate(src)?; trace!("deref to {} on {:?}", val.layout.ty, *val); - let mut place = self.ref_to_mplace(val)?; - let (size, align) = self.size_and_align_of_mplace(place)? - .unwrap_or((place.layout.size, place.layout.align.abi)); - assert!(place.mplace.align <= align, "dynamic alignment less strict than static one?"); - place.mplace.align = align; // maximally strict checking - // When dereferencing a pointer, it must be non-NULL, aligned, and live. - if let Some(ptr) = self.check_mplace_access(place, Some(size))? { - place.mplace.ptr = ptr.into(); - } - Ok(place) + let place = self.ref_to_mplace(val)?; + self.mplace_access_checked(place) } /// Check if the given place is good for memory access with the given @@ -340,6 +332,23 @@ where self.memory.check_ptr_access(place.ptr, size, place.align) } + /// Return the "access-checked" version of this `MPlace`, where for non-ZST + /// this is definitely a `Pointer`. + pub fn mplace_access_checked( + &self, + mut place: MPlaceTy<'tcx, M::PointerTag>, + ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::PointerTag>> { + let (size, align) = self.size_and_align_of_mplace(place)? + .unwrap_or((place.layout.size, place.layout.align.abi)); + assert!(place.mplace.align <= align, "dynamic alignment less strict than static one?"); + place.mplace.align = align; // maximally strict checking + // When dereferencing a pointer, it must be non-NULL, aligned, and live. + if let Some(ptr) = self.check_mplace_access(place, Some(size))? { + place.mplace.ptr = ptr.into(); + } + Ok(place) + } + /// Force `place.ptr` to a `Pointer`. /// Can be helpful to avoid lots of `force_ptr` calls later, if this place is used a lot. pub fn force_mplace_ptr( From 647c0e06365aa4570870e78d3b29c2a8fffc0089 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 28 Jul 2019 22:57:50 +0200 Subject: [PATCH 07/37] 'Ref' can now be sure it gets a 'Pointer' --- src/librustc_mir/interpret/step.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 246c90ba48e3a..e7876c9dee904 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -240,8 +240,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ref(_, _, ref place) => { let src = self.eval_place(place)?; - let val = self.force_allocation(src)?; - self.write_immediate(val.to_ref(), dest)?; + let place = self.force_allocation(src)?; + if place.layout.size.bytes() > 0 { + // definitely not a ZST + assert!(place.ptr.is_ptr(), "non-ZST places should be normalized to `Pointer`"); + } + self.write_immediate(place.to_ref(), dest)?; } NullaryOp(mir::NullOp::Box, _) => { From a4af9d1ac25113362898ca598556db5eaa3d8f31 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 07:34:08 +0200 Subject: [PATCH 08/37] parse_pat_with_range_pat: remove unnecessary assignments. --- src/libsyntax/parse/parser/pat.rs | 54 ++++++++++++++----------------- 1 file changed, 25 insertions(+), 29 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 5cc428a4df1de..21b38751831b9 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -108,8 +108,7 @@ impl<'a> Parser<'a> { maybe_whole!(self, NtPat, |x| x); let lo = self.token.span; - let pat; - match self.token.kind { + let pat = match self.token.kind { token::BinOp(token::And) | token::AndAnd => { // Parse &pat / &mut pat self.expect_and()?; @@ -120,7 +119,7 @@ impl<'a> Parser<'a> { return Err(err); } let subpat = self.parse_pat_with_range_pat(false, expected)?; - pat = PatKind::Ref(subpat, mutbl); + PatKind::Ref(subpat, mutbl) } token::OpenDelim(token::Paren) => { // Parse a tuple or parenthesis pattern. @@ -128,41 +127,40 @@ impl<'a> Parser<'a> { // Here, `(pat,)` is a tuple pattern. // For backward compatibility, `(..)` is a tuple pattern as well. - pat = if fields.len() == 1 && !(trailing_comma || fields[0].is_rest()) { + if fields.len() == 1 && !(trailing_comma || fields[0].is_rest()) { PatKind::Paren(fields.into_iter().nth(0).unwrap()) } else { PatKind::Tuple(fields) - }; + } } token::OpenDelim(token::Bracket) => { // Parse `[pat, pat,...]` as a slice pattern. - let (slice, _) = self.parse_delim_comma_seq(token::Bracket, |p| p.parse_pat(None))?; - pat = PatKind::Slice(slice); + PatKind::Slice(self.parse_delim_comma_seq(token::Bracket, |p| p.parse_pat(None))?.0) } token::DotDot => { self.bump(); - pat = if self.is_pat_range_end_start() { + if self.is_pat_range_end_start() { // Parse `..42` for recovery. self.parse_pat_range_to(RangeEnd::Excluded, "..")? } else { // A rest pattern `..`. PatKind::Rest - }; + } } token::DotDotEq => { // Parse `..=42` for recovery. self.bump(); - pat = self.parse_pat_range_to(RangeEnd::Included(RangeSyntax::DotDotEq), "..=")?; + self.parse_pat_range_to(RangeEnd::Included(RangeSyntax::DotDotEq), "..=")? } token::DotDotDot => { // Parse `...42` for recovery. self.bump(); - pat = self.parse_pat_range_to(RangeEnd::Included(RangeSyntax::DotDotDot), "...")?; + self.parse_pat_range_to(RangeEnd::Included(RangeSyntax::DotDotDot), "...")? } // At this point, token != &, &&, (, [ _ => if self.eat_keyword(kw::Underscore) { // Parse _ - pat = PatKind::Wild; + PatKind::Wild } else if self.eat_keyword(kw::Mut) { // Parse mut ident @ pat / mut ref ident @ pat let mutref_span = self.prev_span.to(self.token.span); @@ -179,22 +177,20 @@ impl<'a> Parser<'a> { } else { BindingMode::ByValue(Mutability::Mutable) }; - pat = self.parse_pat_ident(binding_mode)?; + self.parse_pat_ident(binding_mode)? } else if self.eat_keyword(kw::Ref) { // Parse ref ident @ pat / ref mut ident @ pat let mutbl = self.parse_mutability(); - pat = self.parse_pat_ident(BindingMode::ByRef(mutbl))?; + self.parse_pat_ident(BindingMode::ByRef(mutbl))? } else if self.eat_keyword(kw::Box) { - // Parse box pat - let subpat = self.parse_pat_with_range_pat(false, None)?; - pat = PatKind::Box(subpat); + // Parse `box pat` + PatKind::Box(self.parse_pat_with_range_pat(false, None)?) } else if self.token.is_ident() && !self.token.is_reserved_ident() && self.parse_as_ident() { - // Parse ident @ pat + // Parse `ident @ pat` // This can give false positives and parse nullary enums, - // they are dealt with later in resolve - let binding_mode = BindingMode::ByValue(Mutability::Immutable); - pat = self.parse_pat_ident(binding_mode)?; + // they are dealt with later in resolve. + self.parse_pat_ident(BindingMode::ByValue(Mutability::Immutable))? } else if self.token.is_path_start() { // Parse pattern starting with a path let (qself, path) = if self.eat_lt() { @@ -216,7 +212,7 @@ impl<'a> Parser<'a> { delim, prior_type_ascription: self.last_type_ascription, }); - pat = PatKind::Mac(mac); + PatKind::Mac(mac) } token::DotDotDot | token::DotDotEq | token::DotDot => { let (end_kind, form) = match self.token.kind { @@ -232,7 +228,7 @@ impl<'a> Parser<'a> { let begin = self.mk_expr(span, ExprKind::Path(qself, path), ThinVec::new()); self.bump(); let end = self.parse_pat_range_end_opt(&begin, form)?; - pat = PatKind::Range(begin, end, respan(op_span, end_kind)); + PatKind::Range(begin, end, respan(op_span, end_kind)) } token::OpenDelim(token::Brace) => { if qself.is_some() { @@ -249,7 +245,7 @@ impl<'a> Parser<'a> { (vec![], true) }); self.bump(); - pat = PatKind::Struct(path, fields, etc); + PatKind::Struct(path, fields, etc) } token::OpenDelim(token::Paren) => { if qself.is_some() { @@ -260,9 +256,9 @@ impl<'a> Parser<'a> { } // Parse tuple struct or enum pattern let (fields, _) = self.parse_paren_comma_seq(|p| p.parse_pat(None))?; - pat = PatKind::TupleStruct(path, fields) + PatKind::TupleStruct(path, fields) } - _ => pat = PatKind::Path(qself, path), + _ => PatKind::Path(qself, path), } } else { // Try to parse everything else as literal with optional minus @@ -282,9 +278,9 @@ impl<'a> Parser<'a> { on a range-operator token") }; let end = self.parse_pat_range_end_opt(&begin, form)?; - pat = PatKind::Range(begin, end, respan(op_span, end_kind)) + PatKind::Range(begin, end, respan(op_span, end_kind)) } else { - pat = PatKind::Lit(begin); + PatKind::Lit(begin) } } Err(mut err) => { @@ -305,7 +301,7 @@ impl<'a> Parser<'a> { } } } - } + }; let pat = self.mk_pat(lo.to(self.prev_span), pat); let pat = self.maybe_recover_from_bad_qpath(pat, true)?; From 90793c0f126a9d5a0ffab297e9fef8bbbed6ae70 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 07:37:08 +0200 Subject: [PATCH 09/37] extract parse_pat_deref --- src/libsyntax/parse/parser/pat.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 21b38751831b9..95678f9f7a147 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -109,18 +109,7 @@ impl<'a> Parser<'a> { let lo = self.token.span; let pat = match self.token.kind { - token::BinOp(token::And) | token::AndAnd => { - // Parse &pat / &mut pat - self.expect_and()?; - let mutbl = self.parse_mutability(); - if let token::Lifetime(name) = self.token.kind { - let mut err = self.fatal(&format!("unexpected lifetime `{}` in pattern", name)); - err.span_label(self.token.span, "unexpected lifetime"); - return Err(err); - } - let subpat = self.parse_pat_with_range_pat(false, expected)?; - PatKind::Ref(subpat, mutbl) - } + token::BinOp(token::And) | token::AndAnd => self.parse_pat_deref(expected)?, token::OpenDelim(token::Paren) => { // Parse a tuple or parenthesis pattern. let (fields, trailing_comma) = self.parse_paren_comma_seq(|p| p.parse_pat(None))?; @@ -332,6 +321,21 @@ impl<'a> Parser<'a> { Ok(pat) } + /// Parse `&pat` / `&mut pat`. + fn parse_pat_deref(&mut self, expected: Option<&'static str>) -> PResult<'a, PatKind> { + self.expect_and()?; + let mutbl = self.parse_mutability(); + + if let token::Lifetime(name) = self.token.kind { + let mut err = self.fatal(&format!("unexpected lifetime `{}` in pattern", name)); + err.span_label(self.token.span, "unexpected lifetime"); + return Err(err); + } + + let subpat = self.parse_pat_with_range_pat(false, expected)?; + Ok(PatKind::Ref(subpat, mutbl)) + } + // Helper function to decide whether to parse as ident binding // or to try to do something more complex like range patterns. fn parse_as_ident(&mut self) -> bool { From c69b3ede8a98b45633736f7a84757fe7f3b5a392 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 07:41:50 +0200 Subject: [PATCH 10/37] extract parse_pat_tuple_or_parens --- src/libsyntax/parse/parser/pat.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 95678f9f7a147..b7e40969d3eb5 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -110,18 +110,7 @@ impl<'a> Parser<'a> { let lo = self.token.span; let pat = match self.token.kind { token::BinOp(token::And) | token::AndAnd => self.parse_pat_deref(expected)?, - token::OpenDelim(token::Paren) => { - // Parse a tuple or parenthesis pattern. - let (fields, trailing_comma) = self.parse_paren_comma_seq(|p| p.parse_pat(None))?; - - // Here, `(pat,)` is a tuple pattern. - // For backward compatibility, `(..)` is a tuple pattern as well. - if fields.len() == 1 && !(trailing_comma || fields[0].is_rest()) { - PatKind::Paren(fields.into_iter().nth(0).unwrap()) - } else { - PatKind::Tuple(fields) - } - } + token::OpenDelim(token::Paren) => self.parse_pat_tuple_or_parens()?, token::OpenDelim(token::Bracket) => { // Parse `[pat, pat,...]` as a slice pattern. PatKind::Slice(self.parse_delim_comma_seq(token::Bracket, |p| p.parse_pat(None))?.0) @@ -336,6 +325,19 @@ impl<'a> Parser<'a> { Ok(PatKind::Ref(subpat, mutbl)) } + /// Parse a tuple or parenthesis pattern. + fn parse_pat_tuple_or_parens(&mut self) -> PResult<'a, PatKind> { + let (fields, trailing_comma) = self.parse_paren_comma_seq(|p| p.parse_pat(None))?; + + // Here, `(pat,)` is a tuple pattern. + // For backward compatibility, `(..)` is a tuple pattern as well. + Ok(if fields.len() == 1 && !(trailing_comma || fields[0].is_rest()) { + PatKind::Paren(fields.into_iter().nth(0).unwrap()) + } else { + PatKind::Tuple(fields) + }) + } + // Helper function to decide whether to parse as ident binding // or to try to do something more complex like range patterns. fn parse_as_ident(&mut self) -> bool { From 3b651330e0ff4090f18fc834486a8f0a9aa62748 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 07:45:55 +0200 Subject: [PATCH 11/37] extract recover_pat_ident_mut_first --- src/libsyntax/parse/parser/pat.rs | 36 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index b7e40969d3eb5..1b6baf09d816b 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -140,22 +140,7 @@ impl<'a> Parser<'a> { // Parse _ PatKind::Wild } else if self.eat_keyword(kw::Mut) { - // Parse mut ident @ pat / mut ref ident @ pat - let mutref_span = self.prev_span.to(self.token.span); - let binding_mode = if self.eat_keyword(kw::Ref) { - self.diagnostic() - .struct_span_err(mutref_span, "the order of `mut` and `ref` is incorrect") - .span_suggestion( - mutref_span, - "try switching the order", - "ref mut".into(), - Applicability::MachineApplicable - ).emit(); - BindingMode::ByRef(Mutability::Mutable) - } else { - BindingMode::ByValue(Mutability::Mutable) - }; - self.parse_pat_ident(binding_mode)? + self.recover_pat_ident_mut_first()? } else if self.eat_keyword(kw::Ref) { // Parse ref ident @ pat / ref mut ident @ pat let mutbl = self.parse_mutability(); @@ -338,6 +323,25 @@ impl<'a> Parser<'a> { }) } + // Recover on `mut ref? ident @ pat` and suggest that the order of `mut` and `ref` is incorrect. + fn recover_pat_ident_mut_first(&mut self) -> PResult<'a, PatKind> { + let mutref_span = self.prev_span.to(self.token.span); + let binding_mode = if self.eat_keyword(kw::Ref) { + self.struct_span_err(mutref_span, "the order of `mut` and `ref` is incorrect") + .span_suggestion( + mutref_span, + "try switching the order", + "ref mut".into(), + Applicability::MachineApplicable + ) + .emit(); + BindingMode::ByRef(Mutability::Mutable) + } else { + BindingMode::ByValue(Mutability::Mutable) + }; + self.parse_pat_ident(binding_mode) + } + // Helper function to decide whether to parse as ident binding // or to try to do something more complex like range patterns. fn parse_as_ident(&mut self) -> bool { From 231da7e044255286ba92675e89ca168a4932452c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 08:16:04 +0200 Subject: [PATCH 12/37] extract ban_pat_range_if_ambiguous --- src/libsyntax/parse/parser/pat.rs | 45 +++++++++++++++++-------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 1b6baf09d816b..7c7dad1fd94b8 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -270,31 +270,36 @@ impl<'a> Parser<'a> { let pat = self.maybe_recover_from_bad_qpath(pat, true)?; if !allow_range_pat { - match pat.node { - PatKind::Range( - _, _, Spanned { node: RangeEnd::Included(RangeSyntax::DotDotDot), .. } - ) => {}, - PatKind::Range(..) => { - let mut err = self.struct_span_err( - pat.span, - "the range pattern here has ambiguous interpretation", - ); - err.span_suggestion( - pat.span, - "add parentheses to clarify the precedence", - format!("({})", pprust::pat_to_string(&pat)), - // "ambiguous interpretation" implies that we have to be guessing - Applicability::MaybeIncorrect - ); - return Err(err); - } - _ => {} - } + self.ban_pat_range_if_ambiguous(&pat)? } Ok(pat) } + /// Ban a range pattern if it has an ambiguous interpretation. + fn ban_pat_range_if_ambiguous(&self, pat: &Pat) -> PResult<'a, ()> { + match pat.node { + PatKind::Range( + .., Spanned { node: RangeEnd::Included(RangeSyntax::DotDotDot), .. } + ) => return Ok(()), + PatKind::Range(..) => {} + _ => return Ok(()), + } + + let mut err = self.struct_span_err( + pat.span, + "the range pattern here has ambiguous interpretation", + ); + err.span_suggestion( + pat.span, + "add parentheses to clarify the precedence", + format!("({})", pprust::pat_to_string(&pat)), + // "ambiguous interpretation" implies that we have to be guessing + Applicability::MaybeIncorrect + ); + Err(err) + } + /// Parse `&pat` / `&mut pat`. fn parse_pat_deref(&mut self, expected: Option<&'static str>) -> PResult<'a, PatKind> { self.expect_and()?; From e32bd69d0f7443bf76af4a8129fc43b381e5afaa Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 08:27:01 +0200 Subject: [PATCH 13/37] extract parse_pat_mac_invoc --- src/libsyntax/parse/parser/pat.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 7c7dad1fd94b8..40dfa86834f1c 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -3,7 +3,7 @@ use super::{Parser, PResult, PathStyle}; use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole}; use crate::ptr::P; use crate::ast::{self, Attribute, Pat, PatKind, FieldPat, RangeEnd, RangeSyntax, Mac_}; -use crate::ast::{BindingMode, Ident, Mutability, Expr, ExprKind}; +use crate::ast::{BindingMode, Ident, Mutability, Path, Expr, ExprKind}; use crate::parse::token::{self}; use crate::print::pprust; use crate::source_map::{respan, Span, Spanned}; @@ -165,18 +165,7 @@ impl<'a> Parser<'a> { (None, self.parse_path(PathStyle::Expr)?) }; match self.token.kind { - token::Not if qself.is_none() => { - // Parse macro invocation - self.bump(); - let (delim, tts) = self.expect_delimited_token_tree()?; - let mac = respan(lo.to(self.prev_span), Mac_ { - path, - tts, - delim, - prior_type_ascription: self.last_type_ascription, - }); - PatKind::Mac(mac) - } + token::Not if qself.is_none() => self.parse_pat_mac_invoc(lo, path)?, token::DotDotDot | token::DotDotEq | token::DotDot => { let (end_kind, form) = match self.token.kind { token::DotDot => (RangeEnd::Excluded, ".."), @@ -328,7 +317,8 @@ impl<'a> Parser<'a> { }) } - // Recover on `mut ref? ident @ pat` and suggest that the order of `mut` and `ref` is incorrect. + /// Recover on `mut ref? ident @ pat` and suggest + /// that the order of `mut` and `ref` is incorrect. fn recover_pat_ident_mut_first(&mut self) -> PResult<'a, PatKind> { let mutref_span = self.prev_span.to(self.token.span); let binding_mode = if self.eat_keyword(kw::Ref) { @@ -347,6 +337,19 @@ impl<'a> Parser<'a> { self.parse_pat_ident(binding_mode) } + /// Parse macro invocation + fn parse_pat_mac_invoc(&mut self, lo: Span, path: Path) -> PResult<'a, PatKind> { + self.bump(); + let (delim, tts) = self.expect_delimited_token_tree()?; + let mac = respan(lo.to(self.prev_span), Mac_ { + path, + tts, + delim, + prior_type_ascription: self.last_type_ascription, + }); + Ok(PatKind::Mac(mac)) + } + // Helper function to decide whether to parse as ident binding // or to try to do something more complex like range patterns. fn parse_as_ident(&mut self) -> bool { From e6f980f9b804acb42e72ba4b071320ca9e7f22e0 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 08:35:24 +0200 Subject: [PATCH 14/37] extract parse_pat_range_starting_with_path --- src/libsyntax/parse/parser/pat.rs | 40 +++++++++++++++++++------------ 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 40dfa86834f1c..5c53a497ff475 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -3,7 +3,7 @@ use super::{Parser, PResult, PathStyle}; use crate::{maybe_recover_from_interpolated_ty_qpath, maybe_whole}; use crate::ptr::P; use crate::ast::{self, Attribute, Pat, PatKind, FieldPat, RangeEnd, RangeSyntax, Mac_}; -use crate::ast::{BindingMode, Ident, Mutability, Path, Expr, ExprKind}; +use crate::ast::{BindingMode, Ident, Mutability, Path, QSelf, Expr, ExprKind}; use crate::parse::token::{self}; use crate::print::pprust; use crate::source_map::{respan, Span, Spanned}; @@ -167,20 +167,7 @@ impl<'a> Parser<'a> { match self.token.kind { token::Not if qself.is_none() => self.parse_pat_mac_invoc(lo, path)?, token::DotDotDot | token::DotDotEq | token::DotDot => { - let (end_kind, form) = match self.token.kind { - token::DotDot => (RangeEnd::Excluded, ".."), - token::DotDotDot => (RangeEnd::Included(RangeSyntax::DotDotDot), "..."), - token::DotDotEq => (RangeEnd::Included(RangeSyntax::DotDotEq), "..="), - _ => panic!("can only parse `..`/`...`/`..=` for ranges \ - (checked above)"), - }; - let op_span = self.token.span; - // Parse range - let span = lo.to(self.prev_span); - let begin = self.mk_expr(span, ExprKind::Path(qself, path), ThinVec::new()); - self.bump(); - let end = self.parse_pat_range_end_opt(&begin, form)?; - PatKind::Range(begin, end, respan(op_span, end_kind)) + self.parse_pat_range_starting_with_path(lo, qself, path)? } token::OpenDelim(token::Brace) => { if qself.is_some() { @@ -350,6 +337,29 @@ impl<'a> Parser<'a> { Ok(PatKind::Mac(mac)) } + /// Parse a range pattern `$path $form $end?` where `$form = ".." | "..." | "..=" ;`. + /// The `$path` has already been parsed and the next token is the `$form`. + fn parse_pat_range_starting_with_path( + &mut self, + lo: Span, + qself: Option, + path: Path + ) -> PResult<'a, PatKind> { + let (end_kind, form) = match self.token.kind { + token::DotDot => (RangeEnd::Excluded, ".."), + token::DotDotDot => (RangeEnd::Included(RangeSyntax::DotDotDot), "..."), + token::DotDotEq => (RangeEnd::Included(RangeSyntax::DotDotEq), "..="), + _ => panic!("can only parse `..`/`...`/`..=` for ranges (checked above)"), + }; + let op_span = self.token.span; + // Parse range + let span = lo.to(self.prev_span); + let begin = self.mk_expr(span, ExprKind::Path(qself, path), ThinVec::new()); + self.bump(); + let end = self.parse_pat_range_end_opt(&begin, form)?; + Ok(PatKind::Range(begin, end, respan(op_span, end_kind))) + } + // Helper function to decide whether to parse as ident binding // or to try to do something more complex like range patterns. fn parse_as_ident(&mut self) -> bool { From 49740b792ddf1bc6d98445b8955b2ebfb742772b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 09:01:08 +0200 Subject: [PATCH 15/37] extract parse_pat_range_starting_with_lit --- src/libsyntax/parse/parser/pat.rs | 41 ++++++++++++++++--------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 5c53a497ff475..b821d9da3548f 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -202,26 +202,10 @@ impl<'a> Parser<'a> { } else { // Try to parse everything else as literal with optional minus match self.parse_literal_maybe_minus() { - Ok(begin) => { - let op_span = self.token.span; - if self.check(&token::DotDot) || self.check(&token::DotDotEq) || - self.check(&token::DotDotDot) { - let (end_kind, form) = if self.eat(&token::DotDotDot) { - (RangeEnd::Included(RangeSyntax::DotDotDot), "...") - } else if self.eat(&token::DotDotEq) { - (RangeEnd::Included(RangeSyntax::DotDotEq), "..=") - } else if self.eat(&token::DotDot) { - (RangeEnd::Excluded, "..") - } else { - panic!("impossible case: we already matched \ - on a range-operator token") - }; - let end = self.parse_pat_range_end_opt(&begin, form)?; - PatKind::Range(begin, end, respan(op_span, end_kind)) - } else { - PatKind::Lit(begin) - } - } + Ok(begin) if self.check(&token::DotDot) || self.check(&token::DotDotEq) + || self.check(&token::DotDotDot) + => self.parse_pat_range_starting_with_lit(begin)?, + Ok(begin) => PatKind::Lit(begin), Err(mut err) => { self.cancel(&mut err); let expected = expected.unwrap_or("pattern"); @@ -360,6 +344,23 @@ impl<'a> Parser<'a> { Ok(PatKind::Range(begin, end, respan(op_span, end_kind))) } + /// Parse a range pattern `$literal $form $end?` where `$form = ".." | "..." | "..=" ;`. + /// The `$path` has already been parsed and the next token is the `$form`. + fn parse_pat_range_starting_with_lit(&mut self, begin: P) -> PResult<'a, PatKind> { + let op_span = self.token.span; + let (end_kind, form) = if self.eat(&token::DotDotDot) { + (RangeEnd::Included(RangeSyntax::DotDotDot), "...") + } else if self.eat(&token::DotDotEq) { + (RangeEnd::Included(RangeSyntax::DotDotEq), "..=") + } else if self.eat(&token::DotDot) { + (RangeEnd::Excluded, "..") + } else { + panic!("impossible case: we already matched on a range-operator token") + }; + let end = self.parse_pat_range_end_opt(&begin, form)?; + Ok(PatKind::Range(begin, end, respan(op_span, end_kind))) + } + // Helper function to decide whether to parse as ident binding // or to try to do something more complex like range patterns. fn parse_as_ident(&mut self) -> bool { From 37f37a5df1b4873ab2a4562fca04dc6454817429 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 09:28:30 +0200 Subject: [PATCH 16/37] parser/pat: minor misc cleanup --- src/libsyntax/parse/parser/pat.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index b821d9da3548f..b7a60a2a4fe80 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -202,9 +202,13 @@ impl<'a> Parser<'a> { } else { // Try to parse everything else as literal with optional minus match self.parse_literal_maybe_minus() { - Ok(begin) if self.check(&token::DotDot) || self.check(&token::DotDotEq) - || self.check(&token::DotDotDot) - => self.parse_pat_range_starting_with_lit(begin)?, + Ok(begin) + if self.check(&token::DotDot) + || self.check(&token::DotDotEq) + || self.check(&token::DotDotDot) => + { + self.parse_pat_range_starting_with_lit(begin)? + } Ok(begin) => PatKind::Lit(begin), Err(mut err) => { self.cancel(&mut err); @@ -446,11 +450,9 @@ impl<'a> Parser<'a> { } /// Parses `ident` or `ident @ pat`. - /// used by the copy foo and ref foo patterns to give a good + /// Used by the copy foo and ref foo patterns to give a good /// error message when parsing mistakes like `ref foo(a, b)`. - fn parse_pat_ident(&mut self, - binding_mode: ast::BindingMode) - -> PResult<'a, PatKind> { + fn parse_pat_ident(&mut self, binding_mode: BindingMode) -> PResult<'a, PatKind> { let ident = self.parse_ident()?; let sub = if self.eat(&token::At) { Some(self.parse_pat(Some("binding pattern"))?) @@ -458,16 +460,16 @@ impl<'a> Parser<'a> { None }; - // just to be friendly, if they write something like - // ref Some(i) - // we end up here with ( as the current token. This shortly - // leads to a parse error. Note that if there is no explicit + // Just to be friendly, if they write something like `ref Some(i)`, + // we end up here with `(` as the current token. + // This shortly leads to a parse error. Note that if there is no explicit // binding mode then we do not end up here, because the lookahead - // will direct us over to parse_enum_variant() + // will direct us over to `parse_enum_variant()`. if self.token == token::OpenDelim(token::Paren) { return Err(self.span_fatal( self.prev_span, - "expected identifier, found enum pattern")) + "expected identifier, found enum pattern", + )) } Ok(PatKind::Ident(binding_mode, ident, sub)) From ddf734deb2c48247e06603262145aec3eedbb315 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 11:39:44 +0200 Subject: [PATCH 17/37] extract fatal_unexpected_non_pat --- src/libsyntax/parse/parser/pat.rs | 38 ++++++++++++++++++------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index b7a60a2a4fe80..49090a57f62a1 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -210,22 +210,7 @@ impl<'a> Parser<'a> { self.parse_pat_range_starting_with_lit(begin)? } Ok(begin) => PatKind::Lit(begin), - Err(mut err) => { - self.cancel(&mut err); - let expected = expected.unwrap_or("pattern"); - let msg = format!( - "expected {}, found {}", - expected, - self.this_token_descr(), - ); - let mut err = self.fatal(&msg); - err.span_label(self.token.span, format!("expected {}", expected)); - let sp = self.sess.source_map().start_point(self.token.span); - if let Some(sp) = self.sess.ambiguous_block_expr_parse.borrow().get(&sp) { - self.sess.expr_parentheses_needed(&mut err, *sp, None); - } - return Err(err); - } + Err(err) => return self.fatal_unexpected_non_pat(err, expected), } } }; @@ -365,6 +350,27 @@ impl<'a> Parser<'a> { Ok(PatKind::Range(begin, end, respan(op_span, end_kind))) } + fn fatal_unexpected_non_pat( + &mut self, + mut err: DiagnosticBuilder<'a>, + expected: Option<&'static str>, + ) -> PResult<'a, P> { + self.cancel(&mut err); + + let expected = expected.unwrap_or("pattern"); + let msg = format!("expected {}, found {}", expected, self.this_token_descr()); + + let mut err = self.fatal(&msg); + err.span_label(self.token.span, format!("expected {}", expected)); + + let sp = self.sess.source_map().start_point(self.token.span); + if let Some(sp) = self.sess.ambiguous_block_expr_parse.borrow().get(&sp) { + self.sess.expr_parentheses_needed(&mut err, *sp, None); + } + + Err(err) + } + // Helper function to decide whether to parse as ident binding // or to try to do something more complex like range patterns. fn parse_as_ident(&mut self) -> bool { From c8fc4c106cfb7594dedf3372e33959e9b859c228 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 12 Aug 2019 12:27:43 +0200 Subject: [PATCH 18/37] extract parse_pat_{tuple_}struct + recover_one_fewer_dotdot --- src/libsyntax/parse/parser/pat.rs | 90 +++++++++++++++++-------------- 1 file changed, 51 insertions(+), 39 deletions(-) diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 49090a57f62a1..53f4d0998c3e1 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -169,34 +169,8 @@ impl<'a> Parser<'a> { token::DotDotDot | token::DotDotEq | token::DotDot => { self.parse_pat_range_starting_with_path(lo, qself, path)? } - token::OpenDelim(token::Brace) => { - if qself.is_some() { - let msg = "unexpected `{` after qualified path"; - let mut err = self.fatal(msg); - err.span_label(self.token.span, msg); - return Err(err); - } - // Parse struct pattern - self.bump(); - let (fields, etc) = self.parse_pat_fields().unwrap_or_else(|mut e| { - e.emit(); - self.recover_stmt(); - (vec![], true) - }); - self.bump(); - PatKind::Struct(path, fields, etc) - } - token::OpenDelim(token::Paren) => { - if qself.is_some() { - let msg = "unexpected `(` after qualified path"; - let mut err = self.fatal(msg); - err.span_label(self.token.span, msg); - return Err(err); - } - // Parse tuple struct or enum pattern - let (fields, _) = self.parse_paren_comma_seq(|p| p.parse_pat(None))?; - PatKind::TupleStruct(path, fields) - } + token::OpenDelim(token::Brace) => self.parse_pat_struct(qself, path)?, + token::OpenDelim(token::Paren) => self.parse_pat_tuple_struct(qself, path)?, _ => PatKind::Path(qself, path), } } else { @@ -481,6 +455,37 @@ impl<'a> Parser<'a> { Ok(PatKind::Ident(binding_mode, ident, sub)) } + /// Parse a struct ("record") pattern (e.g. `Foo { ... }` or `Foo::Bar { ... }`). + fn parse_pat_struct(&mut self, qself: Option, path: Path) -> PResult<'a, PatKind> { + if qself.is_some() { + let msg = "unexpected `{` after qualified path"; + let mut err = self.fatal(msg); + err.span_label(self.token.span, msg); + return Err(err); + } + + self.bump(); + let (fields, etc) = self.parse_pat_fields().unwrap_or_else(|mut e| { + e.emit(); + self.recover_stmt(); + (vec![], true) + }); + self.bump(); + Ok(PatKind::Struct(path, fields, etc)) + } + + /// Parse tuple struct or tuple variant pattern (e.g. `Foo(...)` or `Foo::Bar(...)`). + fn parse_pat_tuple_struct(&mut self, qself: Option, path: Path) -> PResult<'a, PatKind> { + if qself.is_some() { + let msg = "unexpected `(` after qualified path"; + let mut err = self.fatal(msg); + err.span_label(self.token.span, msg); + return Err(err); + } + let (fields, _) = self.parse_paren_comma_seq(|p| p.parse_pat(None))?; + Ok(PatKind::TupleStruct(path, fields)) + } + /// Parses the fields of a struct-like pattern. fn parse_pat_fields(&mut self) -> PResult<'a, (Vec>, bool)> { let mut fields = Vec::new(); @@ -515,17 +520,7 @@ impl<'a> Parser<'a> { etc = true; let mut etc_sp = self.token.span; - if self.token == token::DotDotDot { // Issue #46718 - // Accept `...` as if it were `..` to avoid further errors - self.struct_span_err(self.token.span, "expected field pattern, found `...`") - .span_suggestion( - self.token.span, - "to omit remaining fields, use one fewer `.`", - "..".to_owned(), - Applicability::MachineApplicable - ) - .emit(); - } + self.recover_one_fewer_dotdot(); self.bump(); // `..` || `...` if self.token == token::CloseDelim(token::Brace) { @@ -607,6 +602,23 @@ impl<'a> Parser<'a> { return Ok((fields, etc)); } + /// Recover on `...` as if it were `..` to avoid further errors. + /// See issue #46718. + fn recover_one_fewer_dotdot(&self) { + if self.token != token::DotDotDot { + return; + } + + self.struct_span_err(self.token.span, "expected field pattern, found `...`") + .span_suggestion( + self.token.span, + "to omit remaining fields, use one fewer `.`", + "..".to_owned(), + Applicability::MachineApplicable + ) + .emit(); + } + fn parse_pat_field( &mut self, lo: Span, From 71415ef9bd697a49db34742172aacb792ce8d116 Mon Sep 17 00:00:00 2001 From: nathanwhit Date: Thu, 25 Jul 2019 11:51:05 -0400 Subject: [PATCH 19/37] Parse excess semicolons as empty stmts for linting --- src/libsyntax/parse/parser/stmt.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/parse/parser/stmt.rs b/src/libsyntax/parse/parser/stmt.rs index f182edcbff437..750d8fbbddc00 100644 --- a/src/libsyntax/parse/parser/stmt.rs +++ b/src/libsyntax/parse/parser/stmt.rs @@ -167,7 +167,22 @@ impl<'a> Parser<'a> { if self.token == token::Semi { unused_attrs(&attrs, self); self.bump(); - return Ok(None); + let mut last_semi = lo; + while self.token == token::Semi { + last_semi = self.token.span; + self.bump(); + } + // We are encoding a string of semicolons as an + // an empty tuple that spans the excess semicolons + // to preserve this info until the lint stage + return Ok(Some(Stmt { + id: ast::DUMMY_NODE_ID, + span: lo.to(last_semi), + node: StmtKind::Semi(self.mk_expr(lo.to(last_semi), + ExprKind::Tup(Vec::new()), + ThinVec::new() + )), + })); } if self.token == token::CloseDelim(token::Brace) { From 2f6cb5f75e080d7886129bd9fa2dbc9ee52b20ac Mon Sep 17 00:00:00 2001 From: Nathan Date: Tue, 30 Jul 2019 13:48:39 -0400 Subject: [PATCH 20/37] Add lint for excess trailing semicolons --- src/librustc_lint/lib.rs | 3 ++ src/librustc_lint/redundant_semicolon.rs | 52 ++++++++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 src/librustc_lint/redundant_semicolon.rs diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 3a540fdf4b91f..fc416be8eeb50 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -24,6 +24,7 @@ extern crate rustc; mod error_codes; mod nonstandard_style; +mod redundant_semicolon; pub mod builtin; mod types; mod unused; @@ -55,6 +56,7 @@ use session::Session; use lint::LintId; use lint::FutureIncompatibleInfo; +use redundant_semicolon::*; use nonstandard_style::*; use builtin::*; use types::*; @@ -98,6 +100,7 @@ macro_rules! early_lint_passes { WhileTrue: WhileTrue, NonAsciiIdents: NonAsciiIdents, IncompleteFeatures: IncompleteFeatures, + RedundantSemicolon: RedundantSemicolon, ]); ) } diff --git a/src/librustc_lint/redundant_semicolon.rs b/src/librustc_lint/redundant_semicolon.rs new file mode 100644 index 0000000000000..7c9df3578b59c --- /dev/null +++ b/src/librustc_lint/redundant_semicolon.rs @@ -0,0 +1,52 @@ +use crate::lint::{EarlyLintPass, LintPass, EarlyContext, LintArray, LintContext}; +use syntax::ast::{Stmt, StmtKind, ExprKind}; +use syntax::errors::Applicability; + +declare_lint! { + pub REDUNDANT_SEMICOLON, + Warn, + "detects unnecessary trailing semicolons" +} + +declare_lint_pass!(RedundantSemicolon => [REDUNDANT_SEMICOLON]); + +impl EarlyLintPass for RedundantSemicolon { + fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &Stmt) { + if let StmtKind::Semi(expr) = &stmt.node { + if let ExprKind::Tup(ref v) = &expr.node { + if v.is_empty() { + // Strings of excess semicolons are encoded as empty tuple expressions + // during the parsing stage, so we check for empty tuple expressions + // which span only semicolons + if let Ok(source_str) = cx.sess().source_map().span_to_snippet(stmt.span) { + if source_str.chars().all(|c| c == ';') { + let multiple = (stmt.span.hi() - stmt.span.lo()).0 > 1; + let msg = if multiple { + "unnecessary trailing semicolons" + } else { + "unnecessary trailing semicolon" + }; + let mut err = cx.struct_span_lint( + REDUNDANT_SEMICOLON, + stmt.span, + &msg + ); + let suggest_msg = if multiple { + "remove these semicolons" + } else { + "remove this semicolon" + }; + err.span_suggestion( + stmt.span, + &suggest_msg, + String::new(), + Applicability::MaybeIncorrect + ); + err.emit(); + } + } + } + } + } + } +} From 76a134524295a04ed5f336265239ef5f39d089de Mon Sep 17 00:00:00 2001 From: Nathan Date: Tue, 30 Jul 2019 13:52:32 -0400 Subject: [PATCH 21/37] Update tests for excess semicolon lint --- src/test/ui/block-expr-precedence.stderr | 8 ++++++++ .../issue-46836-identifier-not-instead-of-negation.stderr | 5 ++++- src/test/ui/parser/doc-before-semi.rs | 2 ++ src/test/ui/parser/doc-before-semi.stderr | 8 ++++++++ src/test/ui/proc-macro/span-preservation.rs | 2 +- src/test/ui/proc-macro/span-preservation.stderr | 2 +- 6 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/block-expr-precedence.stderr diff --git a/src/test/ui/block-expr-precedence.stderr b/src/test/ui/block-expr-precedence.stderr new file mode 100644 index 0000000000000..1307b5d6ddd82 --- /dev/null +++ b/src/test/ui/block-expr-precedence.stderr @@ -0,0 +1,8 @@ +warning: unnecessary trailing semicolons + --> $DIR/block-expr-precedence.rs:60:21 + | +LL | if (true) { 12; };;; -num; + | ^^ help: remove these semicolons + | + = note: `#[warn(redundant_semicolon)]` on by default + diff --git a/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr b/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr index f1c93d5463767..f5edbe2a3af53 100644 --- a/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr +++ b/src/test/ui/did_you_mean/issue-46836-identifier-not-instead-of-negation.stderr @@ -28,7 +28,10 @@ error: expected `{`, found `;` LL | if not // lack of braces is [sic] | -- this `if` statement has a condition, but no block LL | println!("Then when?"); - | ^ expected `{` + | ^ + | | + | expected `{` + | help: try placing this code inside a block: `{ ; }` error: unexpected `2` after identifier --> $DIR/issue-46836-identifier-not-instead-of-negation.rs:26:24 diff --git a/src/test/ui/parser/doc-before-semi.rs b/src/test/ui/parser/doc-before-semi.rs index 405a7e1e2a33b..c3f478fe42077 100644 --- a/src/test/ui/parser/doc-before-semi.rs +++ b/src/test/ui/parser/doc-before-semi.rs @@ -3,4 +3,6 @@ fn main() { //~^ ERROR found a documentation comment that doesn't document anything //~| HELP maybe a comment was intended ; + //~^ WARNING unnecessary trailing semicolon + //~| HELP remove this semicolon } diff --git a/src/test/ui/parser/doc-before-semi.stderr b/src/test/ui/parser/doc-before-semi.stderr index e6bade18d0a2d..b9ac30b09b2f8 100644 --- a/src/test/ui/parser/doc-before-semi.stderr +++ b/src/test/ui/parser/doc-before-semi.stderr @@ -6,6 +6,14 @@ LL | /// hi | = help: doc comments must come before what they document, maybe a comment was intended with `//`? +warning: unnecessary trailing semicolon + --> $DIR/doc-before-semi.rs:5:5 + | +LL | ; + | ^ help: remove this semicolon + | + = note: `#[warn(redundant_semicolon)]` on by default + error: aborting due to previous error For more information about this error, try `rustc --explain E0585`. diff --git a/src/test/ui/proc-macro/span-preservation.rs b/src/test/ui/proc-macro/span-preservation.rs index 0a82d28e9e544..55835cb88f4e3 100644 --- a/src/test/ui/proc-macro/span-preservation.rs +++ b/src/test/ui/proc-macro/span-preservation.rs @@ -9,7 +9,7 @@ extern crate test_macros; #[recollect_attr] fn a() { - let x: usize = "hello";;;;; //~ ERROR mismatched types + let x: usize = "hello"; //~ ERROR mismatched types } #[recollect_attr] diff --git a/src/test/ui/proc-macro/span-preservation.stderr b/src/test/ui/proc-macro/span-preservation.stderr index cf03deee7e445..0290f4b2cc982 100644 --- a/src/test/ui/proc-macro/span-preservation.stderr +++ b/src/test/ui/proc-macro/span-preservation.stderr @@ -6,7 +6,7 @@ error[E0308]: mismatched types error[E0308]: mismatched types --> $DIR/span-preservation.rs:12:20 | -LL | let x: usize = "hello";;;;; +LL | let x: usize = "hello"; | ^^^^^^^ expected usize, found reference | = note: expected type `usize` From b8708e2c9a264f1db74700c7471b8038e2d7f8da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 12 Aug 2019 16:50:46 -0700 Subject: [PATCH 22/37] When needing type annotations in local bindings, account for impl Trait and closures Do not suggest nonsensical types when the type inference is failing on `impl Trait` or anonymous closures. --- .../infer/error_reporting/need_type_info.rs | 18 ++++++++++++++++-- ...infer-async-enabled-impl-trait-bindings.rs | 18 ++++++++++++++++++ ...r-async-enabled-impl-trait-bindings.stderr | 19 +++++++++++++++++++ src/test/ui/inference/cannot-infer-async.rs | 16 ++++++++++++++++ .../ui/inference/cannot-infer-async.stderr | 11 +++++++++++ src/test/ui/inference/cannot-infer-closure.rs | 6 ++++++ .../ui/inference/cannot-infer-closure.stderr | 11 +++++++++++ 7 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs create mode 100644 src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr create mode 100644 src/test/ui/inference/cannot-infer-async.rs create mode 100644 src/test/ui/inference/cannot-infer-async.stderr create mode 100644 src/test/ui/inference/cannot-infer-closure.rs create mode 100644 src/test/ui/inference/cannot-infer-closure.stderr diff --git a/src/librustc/infer/error_reporting/need_type_info.rs b/src/librustc/infer/error_reporting/need_type_info.rs index 770d5155777bb..263b052e71f0f 100644 --- a/src/librustc/infer/error_reporting/need_type_info.rs +++ b/src/librustc/infer/error_reporting/need_type_info.rs @@ -151,12 +151,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // | the type parameter `E` is specified // ``` let (ty_msg, suffix) = match &local_visitor.found_ty { - Some(ty) if &ty.to_string() != "_" && name == "_" => { + Some(ty) if &ty.to_string() != "_" && + name == "_" && + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. + (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && + !ty.is_closure() => // The suggestion doesn't make sense for closures. + { let ty = ty_to_string(ty); (format!(" for `{}`", ty), format!("the explicit type `{}`, with the type parameters specified", ty)) } - Some(ty) if &ty.to_string() != "_" && ty.to_string() != name => { + Some(ty) if &ty.to_string() != "_" && + ty.to_string() != name && + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. + (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && + !ty.is_closure() => // The suggestion doesn't make sense for closures. + { let ty = ty_to_string(ty); (format!(" for `{}`", ty), format!( @@ -165,6 +175,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { name, )) } + Some(ty) if ty.is_closure() => ( + " for the closure".to_string(), + "a boxed closure type like `Box _>`".to_string(), + ), _ => (String::new(), "a type".to_owned()), }; let mut labels = vec![(span, InferCtxt::missing_type_msg(&name))]; diff --git a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs new file mode 100644 index 0000000000000..fb6fd60c22f04 --- /dev/null +++ b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs @@ -0,0 +1,18 @@ +// edition:2018 +#![feature(async_await)] +#![feature(impl_trait_in_bindings)] +//~^ WARN the feature `impl_trait_in_bindings` is incomplete and may cause the compiler to crash + +use std::io::Error; + +fn make_unit() -> Result<(), Error> { + Ok(()) +} + +fn main() { + let fut = async { + make_unit()?; //~ ERROR type annotations needed + + Ok(()) + }; +} diff --git a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr new file mode 100644 index 0000000000000..67a834a2e957a --- /dev/null +++ b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.stderr @@ -0,0 +1,19 @@ +warning: the feature `impl_trait_in_bindings` is incomplete and may cause the compiler to crash + --> $DIR/cannot-infer-async-enabled-impl-trait-bindings.rs:3:12 + | +LL | #![feature(impl_trait_in_bindings)] + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + +error[E0282]: type annotations needed for `impl std::future::Future` + --> $DIR/cannot-infer-async-enabled-impl-trait-bindings.rs:14:9 + | +LL | let fut = async { + | --- consider giving `fut` the explicit type `impl std::future::Future`, with the type parameters specified +LL | make_unit()?; + | ^^^^^^^^^^^^ cannot infer type + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/src/test/ui/inference/cannot-infer-async.rs b/src/test/ui/inference/cannot-infer-async.rs new file mode 100644 index 0000000000000..354942121b0a5 --- /dev/null +++ b/src/test/ui/inference/cannot-infer-async.rs @@ -0,0 +1,16 @@ +// edition:2018 +#![feature(async_await)] + +use std::io::Error; + +fn make_unit() -> Result<(), Error> { + Ok(()) +} + +fn main() { + let fut = async { + make_unit()?; //~ ERROR type annotations needed + + Ok(()) + }; +} diff --git a/src/test/ui/inference/cannot-infer-async.stderr b/src/test/ui/inference/cannot-infer-async.stderr new file mode 100644 index 0000000000000..36608a11bb730 --- /dev/null +++ b/src/test/ui/inference/cannot-infer-async.stderr @@ -0,0 +1,11 @@ +error[E0282]: type annotations needed + --> $DIR/cannot-infer-async.rs:12:9 + | +LL | let fut = async { + | --- consider giving `fut` a type +LL | make_unit()?; + | ^^^^^^^^^^^^ cannot infer type + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/src/test/ui/inference/cannot-infer-closure.rs b/src/test/ui/inference/cannot-infer-closure.rs new file mode 100644 index 0000000000000..9c1e609ce6431 --- /dev/null +++ b/src/test/ui/inference/cannot-infer-closure.rs @@ -0,0 +1,6 @@ +fn main() { + let x = || { + Err(())?; //~ ERROR type annotations needed for the closure + Ok(()) + }; +} diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr new file mode 100644 index 0000000000000..b8ca099aec13d --- /dev/null +++ b/src/test/ui/inference/cannot-infer-closure.stderr @@ -0,0 +1,11 @@ +error[E0282]: type annotations needed for the closure + --> $DIR/cannot-infer-closure.rs:3:9 + | +LL | let x = || { + | - consider giving `x` a boxed closure type like `Box _>` +LL | Err(())?; + | ^^^^^^^^ cannot infer type + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. From 3b6d46c6404cbb076ac07c568c6c4eb6d370994e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 12 Aug 2019 18:12:19 -0700 Subject: [PATCH 23/37] Resolve types when suggesting boxed closure --- .../infer/error_reporting/need_type_info.rs | 24 +++++++++++++++---- src/librustc/ty/sty.rs | 3 +++ src/test/ui/inference/cannot-infer-closure.rs | 6 ++--- .../ui/inference/cannot-infer-closure.stderr | 8 +++---- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/librustc/infer/error_reporting/need_type_info.rs b/src/librustc/infer/error_reporting/need_type_info.rs index 263b052e71f0f..cff2b41e9eca1 100644 --- a/src/librustc/infer/error_reporting/need_type_info.rs +++ b/src/librustc/infer/error_reporting/need_type_info.rs @@ -175,10 +175,26 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { name, )) } - Some(ty) if ty.is_closure() => ( - " for the closure".to_string(), - "a boxed closure type like `Box _>`".to_string(), - ), + Some(ty::TyS { sty: ty::TyKind::Closure(def_id, substs), .. }) => { + let msg = " for the closure".to_string(); + let fn_sig = substs.closure_sig(*def_id, self.tcx); + let args = fn_sig.inputs() + .skip_binder() + .iter() + .next() + .map(|args| args.tuple_fields() + .map(|arg| arg.to_string()) + .collect::>().join(", ")) + .unwrap_or_else(String::new); + // This suggestion is incomplete, as the user will get further type inference + // errors due to the `_` placeholders and the introduction of `Box`, but it does + // nudge them in the right direction. + (msg, format!( + "a boxed closure type like `Box {}>`", + args, + fn_sig.output().skip_binder().to_string(), + )) + } _ => (String::new(), "a type".to_owned()), }; let mut labels = vec![(span, InferCtxt::missing_type_msg(&name))]; diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 129ea9b5b674a..02593d4d609fd 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -2068,6 +2068,9 @@ impl<'tcx> TyS<'tcx> { Error => { // ignore errors (#54954) ty::Binder::dummy(FnSig::fake()) } + Closure(..) => bug!( + "to get the signature of a closure, use `closure_sig()` not `fn_sig()`", + ), _ => bug!("Ty::fn_sig() called on non-fn type: {:?}", self) } } diff --git a/src/test/ui/inference/cannot-infer-closure.rs b/src/test/ui/inference/cannot-infer-closure.rs index 9c1e609ce6431..8f48483c25421 100644 --- a/src/test/ui/inference/cannot-infer-closure.rs +++ b/src/test/ui/inference/cannot-infer-closure.rs @@ -1,6 +1,6 @@ fn main() { - let x = || { - Err(())?; //~ ERROR type annotations needed for the closure - Ok(()) + let x = |a: (), b: ()| { + Err(a)?; //~ ERROR type annotations needed for the closure + Ok(b) }; } diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr index b8ca099aec13d..20b2784a45d84 100644 --- a/src/test/ui/inference/cannot-infer-closure.stderr +++ b/src/test/ui/inference/cannot-infer-closure.stderr @@ -1,10 +1,10 @@ error[E0282]: type annotations needed for the closure --> $DIR/cannot-infer-closure.rs:3:9 | -LL | let x = || { - | - consider giving `x` a boxed closure type like `Box _>` -LL | Err(())?; - | ^^^^^^^^ cannot infer type +LL | let x = |a: (), b: ()| { + | - consider giving `x` a boxed closure type like `Box std::result::Result<(), _>>` +LL | Err(a)?; + | ^^^^^^^ cannot infer type error: aborting due to previous error From 1e6f7537629b668eca7a5c2e94205552b7f8f4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 12 Aug 2019 18:42:19 -0700 Subject: [PATCH 24/37] Fix tidy --- src/librustc/infer/error_reporting/need_type_info.rs | 2 +- .../cannot-infer-async-enabled-impl-trait-bindings.rs | 4 ++-- src/test/ui/inference/cannot-infer-async.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc/infer/error_reporting/need_type_info.rs b/src/librustc/infer/error_reporting/need_type_info.rs index cff2b41e9eca1..6d16580384ed7 100644 --- a/src/librustc/infer/error_reporting/need_type_info.rs +++ b/src/librustc/infer/error_reporting/need_type_info.rs @@ -175,7 +175,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { name, )) } - Some(ty::TyS { sty: ty::TyKind::Closure(def_id, substs), .. }) => { + Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => { let msg = " for the closure".to_string(); let fn_sig = substs.closure_sig(*def_id, self.tcx); let args = fn_sig.inputs() diff --git a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs index fb6fd60c22f04..30ed5050433a6 100644 --- a/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs +++ b/src/test/ui/inference/cannot-infer-async-enabled-impl-trait-bindings.rs @@ -5,14 +5,14 @@ use std::io::Error; -fn make_unit() -> Result<(), Error> { +fn make_unit() -> Result<(), Error> { Ok(()) } fn main() { let fut = async { make_unit()?; //~ ERROR type annotations needed - + Ok(()) }; } diff --git a/src/test/ui/inference/cannot-infer-async.rs b/src/test/ui/inference/cannot-infer-async.rs index 354942121b0a5..edc64276e7ce4 100644 --- a/src/test/ui/inference/cannot-infer-async.rs +++ b/src/test/ui/inference/cannot-infer-async.rs @@ -3,14 +3,14 @@ use std::io::Error; -fn make_unit() -> Result<(), Error> { +fn make_unit() -> Result<(), Error> { Ok(()) } fn main() { let fut = async { make_unit()?; //~ ERROR type annotations needed - + Ok(()) }; } From fb2511c3c581d811d61e6022f863a5c0e7006ae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 12 Aug 2019 20:22:58 -0700 Subject: [PATCH 25/37] Suggest `Box::new` when appropriate When encountering a boxed value as expected and a stack allocated value that could be boxed to fulfill the expectation, like in the following snippet, suggest `Box::new` wrapping. --- src/librustc/hir/map/mod.rs | 28 ++++++++++++++--- src/librustc_typeck/check/demand.rs | 1 + src/librustc_typeck/check/mod.rs | 35 ++++++++++++++++++++++ src/test/ui/suggestions/suggest-box.fixed | 8 +++++ src/test/ui/suggestions/suggest-box.rs | 8 +++++ src/test/ui/suggestions/suggest-box.stderr | 24 +++++++++++++++ 6 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/suggestions/suggest-box.fixed create mode 100644 src/test/ui/suggestions/suggest-box.rs create mode 100644 src/test/ui/suggestions/suggest-box.stderr diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index b85738dd29a6d..b68cef2b6ea92 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -650,11 +650,31 @@ impl<'hir> Map<'hir> { } pub fn is_const_scope(&self, hir_id: HirId) -> bool { - self.walk_parent_nodes(hir_id, |node| match *node { - Node::Item(Item { node: ItemKind::Const(_, _), .. }) => true, - Node::Item(Item { node: ItemKind::Fn(_, header, _, _), .. }) => header.is_const(), + let parent_id = self.get_parent_item(hir_id); + match self.get(parent_id) { + Node::Item(&Item { + node: ItemKind::Const(..), + .. + }) + | Node::TraitItem(&TraitItem { + node: TraitItemKind::Const(..), + .. + }) + | Node::ImplItem(&ImplItem { + node: ImplItemKind::Const(..), + .. + }) + | Node::AnonConst(_) + | Node::Item(&Item { + node: ItemKind::Static(..), + .. + }) => true, + Node::Item(&Item { + node: ItemKind::Fn(_, header, ..), + .. + }) => header.constness == Constness::Const, _ => false, - }, |_| false).map(|id| id != CRATE_HIR_ID).unwrap_or(false) + } } /// If there is some error when walking the parents (e.g., a node does not diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 3229d49841e79..bb499a97a2d2d 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -127,6 +127,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.suggest_compatible_variants(&mut err, expr, expected, expr_ty); self.suggest_ref_or_into(&mut err, expr, expected, expr_ty); + self.suggest_boxing_when_appropriate(&mut err, expr, expected, expr_ty); self.suggest_missing_await(&mut err, expr, expected, expr_ty); (expected, Some(err)) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 4fb28db6e94fa..5b708b69e2d7e 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3820,6 +3820,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err, &fn_decl, expected, found, can_suggest); } self.suggest_ref_or_into(err, expression, expected, found); + self.suggest_boxing_when_appropriate(err, expression, expected, found); pointing_at_return_type } @@ -3980,6 +3981,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// When encountering the expected boxed value allocated in the stack, suggest allocating it + /// in the heap by calling `Box::new()`. + fn suggest_boxing_when_appropriate( + &self, + err: &mut DiagnosticBuilder<'tcx>, + expr: &hir::Expr, + expected: Ty<'tcx>, + found: Ty<'tcx>, + ) { + if self.tcx.hir().is_const_scope(expr.hir_id) { + // Do not suggest `Box::new` in const context. + return; + } + if expected.is_box() && !found.is_box() { + let boxed_found = self.tcx.mk_box(found); + if let (true, Ok(snippet)) = ( + self.can_coerce(boxed_found, expected), + self.sess().source_map().span_to_snippet(expr.span), + ) { + err.span_suggestion( + expr.span, + "you can store this in the heap calling `Box::new`", + format!("Box::new({})", snippet), + Applicability::MachineApplicable, + ); + err.note("for more information about the distinction between the stack and the \ + heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \ + https://doc.rust-lang.org/rust-by-example/std/box.html and \ + https://doc.rust-lang.org/std/boxed/index.html"); + } + } + } + + /// A common error is to forget to add a semicolon at the end of a block, e.g., /// /// ``` diff --git a/src/test/ui/suggestions/suggest-box.fixed b/src/test/ui/suggestions/suggest-box.fixed new file mode 100644 index 0000000000000..3de02cd0bd481 --- /dev/null +++ b/src/test/ui/suggestions/suggest-box.fixed @@ -0,0 +1,8 @@ +// run-rustfix + +fn main() { + let _x: Box Result<(), ()>> = Box::new(|| { //~ ERROR mismatched types + Err(())?; + Ok(()) + }); +} diff --git a/src/test/ui/suggestions/suggest-box.rs b/src/test/ui/suggestions/suggest-box.rs new file mode 100644 index 0000000000000..e680a61db3b17 --- /dev/null +++ b/src/test/ui/suggestions/suggest-box.rs @@ -0,0 +1,8 @@ +// run-rustfix + +fn main() { + let _x: Box Result<(), ()>> = || { //~ ERROR mismatched types + Err(())?; + Ok(()) + }; +} diff --git a/src/test/ui/suggestions/suggest-box.stderr b/src/test/ui/suggestions/suggest-box.stderr new file mode 100644 index 0000000000000..aacee2cda1341 --- /dev/null +++ b/src/test/ui/suggestions/suggest-box.stderr @@ -0,0 +1,24 @@ +error[E0308]: mismatched types + --> $DIR/suggest-box.rs:4:47 + | +LL | let _x: Box Result<(), ()>> = || { + | _______________________________________________^ +LL | | Err(())?; +LL | | Ok(()) +LL | | }; + | |_____^ expected struct `std::boxed::Box`, found closure + | + = note: expected type `std::boxed::Box std::result::Result<(), ()>>` + found type `[closure@$DIR/suggest-box.rs:4:47: 7:6]` + = note: for more information about the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html and https://doc.rust-lang.org/std/boxed/index.html +help: you can store this in the heap calling `Box::new` + | +LL | let _x: Box Result<(), ()>> = Box::new(|| { +LL | Err(())?; +LL | Ok(()) +LL | }); + | + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 13fd4666b076245e143102e0a43249af0001f7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 12 Aug 2019 23:56:13 -0700 Subject: [PATCH 26/37] Point at the right enclosing scope when using `await` in non-async fn --- src/librustc/hir/lowering/expr.rs | 5 ++++- src/test/ui/issues/issue-63398.rs | 11 +++++++++++ src/test/ui/issues/issue-63398.stderr | 11 +++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/issues/issue-63398.rs create mode 100644 src/test/ui/issues/issue-63398.stderr diff --git a/src/librustc/hir/lowering/expr.rs b/src/librustc/hir/lowering/expr.rs index d273006fbe07e..e3a5400942d1a 100644 --- a/src/librustc/hir/lowering/expr.rs +++ b/src/librustc/hir/lowering/expr.rs @@ -677,6 +677,7 @@ impl LoweringContext<'_> { let fn_decl = self.lower_fn_decl(decl, None, false, None); self.with_new_scopes(|this| { + let prev = this.current_item; this.current_item = Some(fn_decl_span); let mut generator_kind = None; let body_id = this.lower_fn_body(decl, |this| { @@ -690,8 +691,10 @@ impl LoweringContext<'_> { generator_kind, movability, ); + let capture_clause = this.lower_capture_clause(capture_clause); + this.current_item = prev; hir::ExprKind::Closure( - this.lower_capture_clause(capture_clause), + capture_clause, fn_decl, body_id, fn_decl_span, diff --git a/src/test/ui/issues/issue-63398.rs b/src/test/ui/issues/issue-63398.rs new file mode 100644 index 0000000000000..a031687845652 --- /dev/null +++ b/src/test/ui/issues/issue-63398.rs @@ -0,0 +1,11 @@ +// edition:2018 +#![feature(async_await)] + +async fn do_the_thing() -> u8 { + 8 +} + +fn main() { + let x = move || {}; + let y = do_the_thing().await; //~ ERROR `await` is only allowed inside `async` functions +} diff --git a/src/test/ui/issues/issue-63398.stderr b/src/test/ui/issues/issue-63398.stderr new file mode 100644 index 0000000000000..b2ba46ce317cd --- /dev/null +++ b/src/test/ui/issues/issue-63398.stderr @@ -0,0 +1,11 @@ +error[E0728]: `await` is only allowed inside `async` functions and blocks + --> $DIR/issue-63398.rs:10:13 + | +LL | fn main() { + | ---- this is not `async` +LL | let x = move || {}; +LL | let y = do_the_thing().await; + | ^^^^^^^^^^^^^^^^^^^^ only allowed inside `async` functions and blocks + +error: aborting due to previous error + From 376636e51719588edba82fc284328e14ce1f2d74 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 13 Aug 2019 20:51:54 +0300 Subject: [PATCH 27/37] syntax: Remove `DummyResult::expn_only` --- src/libsyntax/ext/base.rs | 39 +++++--------------------------- src/libsyntax_ext/asm.rs | 4 ++-- src/libsyntax_ext/assert.rs | 2 +- src/libsyntax_ext/cfg.rs | 2 +- src/libsyntax_ext/concat.rs | 8 +++---- src/libsyntax_ext/env.rs | 14 ++++++------ src/libsyntax_ext/format.rs | 2 +- src/libsyntax_ext/source_util.rs | 10 ++++---- 8 files changed, 27 insertions(+), 54 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 7f4feff6be670..11544d43ac3e3 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -405,7 +405,6 @@ impl MacResult for MacEager { /// after hitting errors. #[derive(Copy, Clone)] pub struct DummyResult { - expr_only: bool, is_error: bool, span: Span, } @@ -416,21 +415,12 @@ impl DummyResult { /// Use this as a return value after hitting any errors and /// calling `span_err`. pub fn any(span: Span) -> Box { - Box::new(DummyResult { expr_only: false, is_error: true, span }) + Box::new(DummyResult { is_error: true, span }) } /// Same as `any`, but must be a valid fragment, not error. pub fn any_valid(span: Span) -> Box { - Box::new(DummyResult { expr_only: false, is_error: false, span }) - } - - /// Creates a default MacResult that can only be an expression. - /// - /// Use this for macros that must expand to an expression, so even - /// if an error is encountered internally, the user will receive - /// an error that they also used it in the wrong place. - pub fn expr(span: Span) -> Box { - Box::new(DummyResult { expr_only: true, is_error: true, span }) + Box::new(DummyResult { is_error: false, span }) } /// A plain dummy expression. @@ -472,36 +462,19 @@ impl MacResult for DummyResult { } fn make_items(self: Box) -> Option; 1]>> { - // this code needs a comment... why not always just return the Some() ? - if self.expr_only { - None - } else { - Some(SmallVec::new()) - } + Some(SmallVec::new()) } fn make_impl_items(self: Box) -> Option> { - if self.expr_only { - None - } else { - Some(SmallVec::new()) - } + Some(SmallVec::new()) } fn make_trait_items(self: Box) -> Option> { - if self.expr_only { - None - } else { - Some(SmallVec::new()) - } + Some(SmallVec::new()) } fn make_foreign_items(self: Box) -> Option> { - if self.expr_only { - None - } else { - Some(SmallVec::new()) - } + Some(SmallVec::new()) } fn make_stmts(self: Box) -> Option> { diff --git a/src/libsyntax_ext/asm.rs b/src/libsyntax_ext/asm.rs index c1c2732605c46..950166f9260e2 100644 --- a/src/libsyntax_ext/asm.rs +++ b/src/libsyntax_ext/asm.rs @@ -47,10 +47,10 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt<'_>, -> Box { let mut inline_asm = match parse_inline_asm(cx, sp, tts) { Ok(Some(inline_asm)) => inline_asm, - Ok(None) => return DummyResult::expr(sp), + Ok(None) => return DummyResult::any(sp), Err(mut err) => { err.emit(); - return DummyResult::expr(sp); + return DummyResult::any(sp); } }; diff --git a/src/libsyntax_ext/assert.rs b/src/libsyntax_ext/assert.rs index d7571f43eddd0..e3ef39075e236 100644 --- a/src/libsyntax_ext/assert.rs +++ b/src/libsyntax_ext/assert.rs @@ -20,7 +20,7 @@ pub fn expand_assert<'cx>( Ok(assert) => assert, Err(mut err) => { err.emit(); - return DummyResult::expr(sp); + return DummyResult::any(sp); } }; diff --git a/src/libsyntax_ext/cfg.rs b/src/libsyntax_ext/cfg.rs index 84830e6ddda1a..0e52c1af9086f 100644 --- a/src/libsyntax_ext/cfg.rs +++ b/src/libsyntax_ext/cfg.rs @@ -25,7 +25,7 @@ pub fn expand_cfg( } Err(mut err) => { err.emit(); - DummyResult::expr(sp) + DummyResult::any(sp) } } } diff --git a/src/libsyntax_ext/concat.rs b/src/libsyntax_ext/concat.rs index f1d079eb05379..4cd17531a4500 100644 --- a/src/libsyntax_ext/concat.rs +++ b/src/libsyntax_ext/concat.rs @@ -1,5 +1,5 @@ use syntax::ast; -use syntax::ext::base; +use syntax::ext::base::{self, DummyResult}; use syntax::symbol::Symbol; use syntax::tokenstream; @@ -12,7 +12,7 @@ pub fn expand_syntax_ext( ) -> Box { let es = match base::get_exprs_from_tts(cx, sp, tts) { Some(e) => e, - None => return base::DummyResult::expr(sp), + None => return DummyResult::any(sp), }; let mut accumulator = String::new(); let mut missing_literal = vec![]; @@ -55,9 +55,9 @@ pub fn expand_syntax_ext( let mut err = cx.struct_span_err(missing_literal, "expected a literal"); err.note("only literals (like `\"foo\"`, `42` and `3.14`) can be passed to `concat!()`"); err.emit(); - return base::DummyResult::expr(sp); + return DummyResult::any(sp); } else if has_errors { - return base::DummyResult::expr(sp); + return DummyResult::any(sp); } let sp = sp.apply_mark(cx.current_expansion.id); base::MacEager::expr(cx.expr_str(sp, Symbol::intern(&accumulator))) diff --git a/src/libsyntax_ext/env.rs b/src/libsyntax_ext/env.rs index 39fc90decc92a..442f27c782185 100644 --- a/src/libsyntax_ext/env.rs +++ b/src/libsyntax_ext/env.rs @@ -16,7 +16,7 @@ pub fn expand_option_env<'cx>(cx: &'cx mut ExtCtxt<'_>, tts: &[tokenstream::TokenTree]) -> Box { let var = match get_single_str_from_tts(cx, sp, tts, "option_env!") { - None => return DummyResult::expr(sp), + None => return DummyResult::any(sp), Some(v) => v, }; @@ -50,21 +50,21 @@ pub fn expand_env<'cx>(cx: &'cx mut ExtCtxt<'_>, let mut exprs = match get_exprs_from_tts(cx, sp, tts) { Some(ref exprs) if exprs.is_empty() => { cx.span_err(sp, "env! takes 1 or 2 arguments"); - return DummyResult::expr(sp); + return DummyResult::any(sp); } - None => return DummyResult::expr(sp), + None => return DummyResult::any(sp), Some(exprs) => exprs.into_iter(), }; let var = match expr_to_string(cx, exprs.next().unwrap(), "expected string literal") { - None => return DummyResult::expr(sp), + None => return DummyResult::any(sp), Some((v, _style)) => v, }; let msg = match exprs.next() { None => Symbol::intern(&format!("environment variable `{}` not defined", var)), Some(second) => { match expr_to_string(cx, second, "expected string literal") { - None => return DummyResult::expr(sp), + None => return DummyResult::any(sp), Some((s, _style)) => s, } } @@ -72,13 +72,13 @@ pub fn expand_env<'cx>(cx: &'cx mut ExtCtxt<'_>, if exprs.next().is_some() { cx.span_err(sp, "env! takes 1 or 2 arguments"); - return DummyResult::expr(sp); + return DummyResult::any(sp); } let e = match env::var(&*var.as_str()) { Err(_) => { cx.span_err(sp, &msg.as_str()); - return DummyResult::expr(sp); + return DummyResult::any(sp); } Ok(s) => cx.expr_str(sp, Symbol::intern(&s)), }; diff --git a/src/libsyntax_ext/format.rs b/src/libsyntax_ext/format.rs index 2ae13b66e2853..d699b3b1a90c2 100644 --- a/src/libsyntax_ext/format.rs +++ b/src/libsyntax_ext/format.rs @@ -805,7 +805,7 @@ fn expand_format_args_impl<'cx>( } Err(mut err) => { err.emit(); - DummyResult::expr(sp) + DummyResult::any(sp) } } } diff --git a/src/libsyntax_ext/source_util.rs b/src/libsyntax_ext/source_util.rs index 2c8d53a231550..cbc01b48afd03 100644 --- a/src/libsyntax_ext/source_util.rs +++ b/src/libsyntax_ext/source_util.rs @@ -111,7 +111,7 @@ pub fn expand_include_str(cx: &mut ExtCtxt<'_>, sp: Span, tts: &[tokenstream::To -> Box { let file = match get_single_str_from_tts(cx, sp, tts, "include_str!") { Some(f) => f, - None => return DummyResult::expr(sp) + None => return DummyResult::any(sp) }; let file = cx.resolve_path(file, sp); match fs::read_to_string(&file) { @@ -126,11 +126,11 @@ pub fn expand_include_str(cx: &mut ExtCtxt<'_>, sp: Span, tts: &[tokenstream::To }, Err(ref e) if e.kind() == ErrorKind::InvalidData => { cx.span_err(sp, &format!("{} wasn't a utf-8 file", file.display())); - DummyResult::expr(sp) + DummyResult::any(sp) } Err(e) => { cx.span_err(sp, &format!("couldn't read {}: {}", file.display(), e)); - DummyResult::expr(sp) + DummyResult::any(sp) } } } @@ -139,7 +139,7 @@ pub fn expand_include_bytes(cx: &mut ExtCtxt<'_>, sp: Span, tts: &[tokenstream:: -> Box { let file = match get_single_str_from_tts(cx, sp, tts, "include_bytes!") { Some(f) => f, - None => return DummyResult::expr(sp) + None => return DummyResult::any(sp) }; let file = cx.resolve_path(file, sp); match fs::read(&file) { @@ -158,7 +158,7 @@ pub fn expand_include_bytes(cx: &mut ExtCtxt<'_>, sp: Span, tts: &[tokenstream:: }, Err(e) => { cx.span_err(sp, &format!("couldn't read {}: {}", file.display(), e)); - DummyResult::expr(sp) + DummyResult::any(sp) } } } From 939c1cb349f81a3ce488f5c17f195a5fcd84691c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 13 Aug 2019 11:24:08 -0700 Subject: [PATCH 28/37] review comments --- src/librustc/hir/map/mod.rs | 4 +- .../infer/error_reporting/need_type_info.rs | 8 ++-- src/librustc_typeck/check/demand.rs | 2 +- src/librustc_typeck/check/mod.rs | 37 ++++++++++--------- .../ui/inference/cannot-infer-closure.stderr | 2 +- src/test/ui/suggestions/suggest-box.stderr | 4 +- 6 files changed, 30 insertions(+), 27 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index b68cef2b6ea92..571ee393782f9 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -649,7 +649,9 @@ impl<'hir> Map<'hir> { } } - pub fn is_const_scope(&self, hir_id: HirId) -> bool { + /// Whether the expression pointed at by `hir_id` belongs to a `const` evaluation context. + /// Used exclusively for diagnostics, to avoid suggestion function calls. + pub fn is_const_context(&self, hir_id: HirId) -> bool { let parent_id = self.get_parent_item(hir_id); match self.get(parent_id) { Node::Item(&Item { diff --git a/src/librustc/infer/error_reporting/need_type_info.rs b/src/librustc/infer/error_reporting/need_type_info.rs index 6d16580384ed7..b5d78a80bf674 100644 --- a/src/librustc/infer/error_reporting/need_type_info.rs +++ b/src/librustc/infer/error_reporting/need_type_info.rs @@ -153,7 +153,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let (ty_msg, suffix) = match &local_visitor.found_ty { Some(ty) if &ty.to_string() != "_" && name == "_" && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && !ty.is_closure() => // The suggestion doesn't make sense for closures. { @@ -163,7 +163,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } Some(ty) if &ty.to_string() != "_" && ty.to_string() != name && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && !ty.is_closure() => // The suggestion doesn't make sense for closures. { @@ -185,12 +185,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { .map(|args| args.tuple_fields() .map(|arg| arg.to_string()) .collect::>().join(", ")) - .unwrap_or_else(String::new); + .unwrap_or_default(); // This suggestion is incomplete, as the user will get further type inference // errors due to the `_` placeholders and the introduction of `Box`, but it does // nudge them in the right direction. (msg, format!( - "a boxed closure type like `Box {}>`", + "a boxed closure type like `Box {}>`", args, fn_sig.output().skip_binder().to_string(), )) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index bb499a97a2d2d..ed25601208ad1 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -549,7 +549,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { checked_ty: Ty<'tcx>, expected_ty: Ty<'tcx>, ) -> bool { - if self.tcx.hir().is_const_scope(expr.hir_id) { + if self.tcx.hir().is_const_context(expr.hir_id) { // Shouldn't suggest `.into()` on `const`s. // FIXME(estebank): modify once we decide to suggest `as` casts return false; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 5b708b69e2d7e..8565cbd3708c9 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3990,27 +3990,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, found: Ty<'tcx>, ) { - if self.tcx.hir().is_const_scope(expr.hir_id) { + if self.tcx.hir().is_const_context(expr.hir_id) { // Do not suggest `Box::new` in const context. return; } - if expected.is_box() && !found.is_box() { - let boxed_found = self.tcx.mk_box(found); - if let (true, Ok(snippet)) = ( - self.can_coerce(boxed_found, expected), - self.sess().source_map().span_to_snippet(expr.span), - ) { - err.span_suggestion( - expr.span, - "you can store this in the heap calling `Box::new`", - format!("Box::new({})", snippet), - Applicability::MachineApplicable, - ); - err.note("for more information about the distinction between the stack and the \ - heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \ - https://doc.rust-lang.org/rust-by-example/std/box.html and \ - https://doc.rust-lang.org/std/boxed/index.html"); - } + if !expected.is_box() || found.is_box() { + return; + } + let boxed_found = self.tcx.mk_box(found); + if let (true, Ok(snippet)) = ( + self.can_coerce(boxed_found, expected), + self.sess().source_map().span_to_snippet(expr.span), + ) { + err.span_suggestion( + expr.span, + "store this in the heap by calling `Box::new`", + format!("Box::new({})", snippet), + Applicability::MachineApplicable, + ); + err.note("for more on the distinction between the stack and the \ + heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \ + https://doc.rust-lang.org/rust-by-example/std/box.html, and \ + https://doc.rust-lang.org/std/boxed/index.html"); } } diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr index 20b2784a45d84..fb78dedc887ff 100644 --- a/src/test/ui/inference/cannot-infer-closure.stderr +++ b/src/test/ui/inference/cannot-infer-closure.stderr @@ -2,7 +2,7 @@ error[E0282]: type annotations needed for the closure --> $DIR/cannot-infer-closure.rs:3:9 | LL | let x = |a: (), b: ()| { - | - consider giving `x` a boxed closure type like `Box std::result::Result<(), _>>` + | - consider giving `x` a boxed closure type like `Box std::result::Result<(), _>>` LL | Err(a)?; | ^^^^^^^ cannot infer type diff --git a/src/test/ui/suggestions/suggest-box.stderr b/src/test/ui/suggestions/suggest-box.stderr index aacee2cda1341..50c106d63a02b 100644 --- a/src/test/ui/suggestions/suggest-box.stderr +++ b/src/test/ui/suggestions/suggest-box.stderr @@ -10,8 +10,8 @@ LL | | }; | = note: expected type `std::boxed::Box std::result::Result<(), ()>>` found type `[closure@$DIR/suggest-box.rs:4:47: 7:6]` - = note: for more information about the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html and https://doc.rust-lang.org/std/boxed/index.html -help: you can store this in the heap calling `Box::new` + = note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html +help: store this in the heap by calling `Box::new` | LL | let _x: Box Result<(), ()>> = Box::new(|| { LL | Err(())?; From 25d507f497885cd26f1dba8c8e8158c12d02bd6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 13 Aug 2019 11:27:00 -0700 Subject: [PATCH 29/37] review comment: move test --- .../issues/non-async-enclosing-span.rs} | 5 +++-- .../issues/non-async-enclosing-span.stderr} | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) rename src/test/ui/{issues/issue-63398.rs => async-await/issues/non-async-enclosing-span.rs} (52%) rename src/test/ui/{issues/issue-63398.stderr => async-await/issues/non-async-enclosing-span.stderr} (87%) diff --git a/src/test/ui/issues/issue-63398.rs b/src/test/ui/async-await/issues/non-async-enclosing-span.rs similarity index 52% rename from src/test/ui/issues/issue-63398.rs rename to src/test/ui/async-await/issues/non-async-enclosing-span.rs index a031687845652..838911d9b6e8f 100644 --- a/src/test/ui/issues/issue-63398.rs +++ b/src/test/ui/async-await/issues/non-async-enclosing-span.rs @@ -4,8 +4,9 @@ async fn do_the_thing() -> u8 { 8 } - -fn main() { +// #63398: point at the enclosing scope and not the previously seen closure +fn main() { //~ NOTE this is not `async` let x = move || {}; let y = do_the_thing().await; //~ ERROR `await` is only allowed inside `async` functions + //~^ NOTE only allowed inside `async` functions and blocks } diff --git a/src/test/ui/issues/issue-63398.stderr b/src/test/ui/async-await/issues/non-async-enclosing-span.stderr similarity index 87% rename from src/test/ui/issues/issue-63398.stderr rename to src/test/ui/async-await/issues/non-async-enclosing-span.stderr index b2ba46ce317cd..f492c1a8045b5 100644 --- a/src/test/ui/issues/issue-63398.stderr +++ b/src/test/ui/async-await/issues/non-async-enclosing-span.stderr @@ -1,5 +1,5 @@ error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/issue-63398.rs:10:13 + --> $DIR/non-async-enclosing-span.rs:10:13 | LL | fn main() { | ---- this is not `async` From 0d29142aad9554a23f0881be95110ad96365bfcf Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 14 Aug 2019 01:59:14 +0300 Subject: [PATCH 30/37] expand: `expand_fragment` -> `fully_expand_fragment` --- src/libsyntax/ext/expand.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 9a3195b1165b1..21cf232ecc34a 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -118,13 +118,13 @@ macro_rules! ast_fragments { impl<'a, 'b> MutVisitor for MacroExpander<'a, 'b> { fn filter_map_expr(&mut self, expr: P) -> Option> { - self.expand_fragment(AstFragment::OptExpr(Some(expr))).make_opt_expr() + self.fully_expand_fragment(AstFragment::OptExpr(Some(expr))).make_opt_expr() } $($(fn $mut_visit_ast(&mut self, ast: &mut $AstTy) { - visit_clobber(ast, |ast| self.expand_fragment(AstFragment::$Kind(ast)).$make_ast()); + visit_clobber(ast, |ast| self.fully_expand_fragment(AstFragment::$Kind(ast)).$make_ast()); })?)* $($(fn $flat_map_ast_elt(&mut self, ast_elt: <$AstTy as IntoIterator>::Item) -> $AstTy { - self.expand_fragment(AstFragment::$Kind(smallvec![ast_elt])).$make_ast() + self.fully_expand_fragment(AstFragment::$Kind(smallvec![ast_elt])).$make_ast() })?)* } @@ -265,7 +265,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { tokens: None, })]); - match self.expand_fragment(krate_item).make_items().pop().map(P::into_inner) { + match self.fully_expand_fragment(krate_item).make_items().pop().map(P::into_inner) { Some(ast::Item { attrs, node: ast::ItemKind::Mod(module), .. }) => { krate.attrs = attrs; krate.module = module; @@ -285,8 +285,8 @@ impl<'a, 'b> MacroExpander<'a, 'b> { krate } - // Fully expand all macro invocations in this AST fragment. - fn expand_fragment(&mut self, input_fragment: AstFragment) -> AstFragment { + // Recursively expand all macro invocations in this AST fragment. + fn fully_expand_fragment(&mut self, input_fragment: AstFragment) -> AstFragment { let orig_expansion_data = self.cx.current_expansion.clone(); self.cx.current_expansion.depth = 0; From d416ebeb6ee265c980778df9bc4d84dc4a7b8580 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Wed, 14 Aug 2019 02:30:09 +0300 Subject: [PATCH 31/37] expand: Unimplement `MutVisitor` on `MacroExpander` Each call to `fully_expand_fragment` is something unique, interesting, and requiring attention. It represents a "root" of expansion and its use means that something unusual is happening, like eager expansion or expansion performed outside of the primary expansion pass. So, it shouldn't be hide under a generic visitor call. Also, from all the implemented visitor methods only two were actually used. --- src/libsyntax/ext/base.rs | 14 +++++++++---- src/libsyntax/ext/expand.rs | 14 +------------ src/libsyntax_ext/proc_macro_harness.rs | 9 +++++---- src/libsyntax_ext/test_harness.rs | 27 ++++++++++++++----------- 4 files changed, 31 insertions(+), 33 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 7f4feff6be670..532de05eea214 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -947,8 +947,10 @@ pub fn expr_to_spanned_string<'a>( // Update `expr.span`'s ctxt now in case expr is an `include!` macro invocation. expr.span = expr.span.apply_mark(cx.current_expansion.id); - // we want to be able to handle e.g., `concat!("foo", "bar")` - cx.expander().visit_expr(&mut expr); + // Perform eager expansion on the expression. + // We want to be able to handle e.g., `concat!("foo", "bar")`. + let expr = cx.expander().fully_expand_fragment(AstFragment::Expr(expr)).make_expr(); + Err(match expr.node { ast::ExprKind::Lit(ref l) => match l.node { ast::LitKind::Str(s, style) => return Ok(respan(expr.span, (s, style))), @@ -1013,8 +1015,12 @@ pub fn get_exprs_from_tts(cx: &mut ExtCtxt<'_>, let mut p = cx.new_parser_from_tts(tts); let mut es = Vec::new(); while p.token != token::Eof { - let mut expr = panictry!(p.parse_expr()); - cx.expander().visit_expr(&mut expr); + let expr = panictry!(p.parse_expr()); + + // Perform eager expansion on the expression. + // We want to be able to handle e.g., `concat!("foo", "bar")`. + let expr = cx.expander().fully_expand_fragment(AstFragment::Expr(expr)).make_expr(); + es.push(expr); if p.eat(&token::Comma) { continue; diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 21cf232ecc34a..402b42dfbc80d 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -116,18 +116,6 @@ macro_rules! ast_fragments { } } - impl<'a, 'b> MutVisitor for MacroExpander<'a, 'b> { - fn filter_map_expr(&mut self, expr: P) -> Option> { - self.fully_expand_fragment(AstFragment::OptExpr(Some(expr))).make_opt_expr() - } - $($(fn $mut_visit_ast(&mut self, ast: &mut $AstTy) { - visit_clobber(ast, |ast| self.fully_expand_fragment(AstFragment::$Kind(ast)).$make_ast()); - })?)* - $($(fn $flat_map_ast_elt(&mut self, ast_elt: <$AstTy as IntoIterator>::Item) -> $AstTy { - self.fully_expand_fragment(AstFragment::$Kind(smallvec![ast_elt])).$make_ast() - })?)* - } - impl<'a> MacResult for crate::ext::tt::macro_rules::ParserAnyMacro<'a> { $(fn $make_ast(self: Box>) -> Option<$AstTy> { @@ -286,7 +274,7 @@ impl<'a, 'b> MacroExpander<'a, 'b> { } // Recursively expand all macro invocations in this AST fragment. - fn fully_expand_fragment(&mut self, input_fragment: AstFragment) -> AstFragment { + pub fn fully_expand_fragment(&mut self, input_fragment: AstFragment) -> AstFragment { let orig_expansion_data = self.cx.current_expansion.clone(); self.cx.current_expansion.depth = 0; diff --git a/src/libsyntax_ext/proc_macro_harness.rs b/src/libsyntax_ext/proc_macro_harness.rs index 7913a7442edc7..70325539f301f 100644 --- a/src/libsyntax_ext/proc_macro_harness.rs +++ b/src/libsyntax_ext/proc_macro_harness.rs @@ -1,18 +1,17 @@ use std::mem; +use smallvec::smallvec; use syntax::ast::{self, Ident}; use syntax::attr; use syntax::source_map::{ExpnInfo, ExpnKind, respan}; use syntax::ext::base::{ExtCtxt, MacroKind}; -use syntax::ext::expand::ExpansionConfig; +use syntax::ext::expand::{AstFragment, ExpansionConfig}; use syntax::ext::hygiene::ExpnId; use syntax::ext::proc_macro::is_proc_macro_attr; -use syntax::mut_visit::MutVisitor; use syntax::parse::ParseSess; use syntax::ptr::P; use syntax::symbol::{kw, sym}; use syntax::visit::{self, Visitor}; - use syntax_pos::{Span, DUMMY_SP}; struct ProcMacroDerive { @@ -409,5 +408,7 @@ fn mk_decls( i }); - cx.monotonic_expander().flat_map_item(module).pop().unwrap() + // Integrate the new module into existing module structures. + let module = AstFragment::Items(smallvec![module]); + cx.monotonic_expander().fully_expand_fragment(module).make_items().pop().unwrap() } diff --git a/src/libsyntax_ext/test_harness.rs b/src/libsyntax_ext/test_harness.rs index eec8a3f802343..0267637e54062 100644 --- a/src/libsyntax_ext/test_harness.rs +++ b/src/libsyntax_ext/test_harness.rs @@ -6,7 +6,7 @@ use syntax::ast::{self, Ident}; use syntax::attr; use syntax::entry::{self, EntryPointType}; use syntax::ext::base::{ExtCtxt, Resolver}; -use syntax::ext::expand::ExpansionConfig; +use syntax::ext::expand::{AstFragment, ExpansionConfig}; use syntax::ext::hygiene::{ExpnId, MacroKind}; use syntax::feature_gate::Features; use syntax::mut_visit::{*, ExpectOne}; @@ -74,12 +74,7 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> { noop_visit_crate(c, self); // Create a main function to run our tests - let test_main = { - let unresolved = mk_main(&mut self.cx); - self.cx.ext_cx.monotonic_expander().flat_map_item(unresolved).pop().unwrap() - }; - - c.module.items.push(test_main); + c.module.items.push(mk_main(&mut self.cx)); } fn flat_map_item(&mut self, i: P) -> SmallVec<[P; 1]> { @@ -216,7 +211,7 @@ fn mk_reexport_mod(cx: &mut TestCtxt<'_>, let name = Ident::from_str("__test_reexports").gensym(); let parent = if parent == ast::DUMMY_NODE_ID { ast::CRATE_NODE_ID } else { parent }; cx.ext_cx.current_expansion.id = cx.ext_cx.resolver.get_module_scope(parent); - let it = cx.ext_cx.monotonic_expander().flat_map_item(P(ast::Item { + let module = P(ast::Item { ident: name, attrs: Vec::new(), id: ast::DUMMY_NODE_ID, @@ -224,9 +219,14 @@ fn mk_reexport_mod(cx: &mut TestCtxt<'_>, vis: dummy_spanned(ast::VisibilityKind::Public), span: DUMMY_SP, tokens: None, - })).pop().unwrap(); + }); - (it, name) + // Integrate the new module into existing module structures. + let module = AstFragment::Items(smallvec![module]); + let module = + cx.ext_cx.monotonic_expander().fully_expand_fragment(module).make_items().pop().unwrap(); + + (module, name) } /// Crawl over the crate, inserting test reexports and the test main function @@ -321,7 +321,7 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P { None => Ident::from_str_and_span("main", sp).gensym(), }; - P(ast::Item { + let main = P(ast::Item { ident: main_id, attrs: vec![main_attr], id: ast::DUMMY_NODE_ID, @@ -329,8 +329,11 @@ fn mk_main(cx: &mut TestCtxt<'_>) -> P { vis: dummy_spanned(ast::VisibilityKind::Public), span: sp, tokens: None, - }) + }); + // Integrate the new item into existing module structures. + let main = AstFragment::Items(smallvec![main]); + cx.ext_cx.monotonic_expander().fully_expand_fragment(main).make_items().pop().unwrap() } fn path_name_i(idents: &[Ident]) -> String { From 806476c840a19ee2da4113ec3e68f9681018325e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 13 Aug 2019 16:41:43 -0700 Subject: [PATCH 32/37] Suggest giving return type to closures on E0282 --- src/librustc/hir/mod.rs | 2 +- .../infer/error_reporting/need_type_info.rs | 211 ++++++++++++------ src/test/ui/error-codes/E0282.stderr | 5 +- ...for-loop-unconstrained-element-type.stderr | 5 +- .../ui/inference/cannot-infer-closure.stderr | 8 +- src/test/ui/issues/issue-18159.stderr | 5 +- .../ui/match/match-unresolved-one-arm.stderr | 5 +- .../suggest-closure-return-type-1.rs | 3 + .../suggest-closure-return-type-1.stderr | 13 ++ .../suggest-closure-return-type-2.rs | 3 + .../suggest-closure-return-type-2.stderr | 13 ++ .../suggest-closure-return-type-3.rs | 3 + .../suggest-closure-return-type-3.stderr | 13 ++ 13 files changed, 199 insertions(+), 90 deletions(-) create mode 100644 src/test/ui/suggestions/suggest-closure-return-type-1.rs create mode 100644 src/test/ui/suggestions/suggest-closure-return-type-1.stderr create mode 100644 src/test/ui/suggestions/suggest-closure-return-type-2.rs create mode 100644 src/test/ui/suggestions/suggest-closure-return-type-2.stderr create mode 100644 src/test/ui/suggestions/suggest-closure-return-type-3.rs create mode 100644 src/test/ui/suggestions/suggest-closure-return-type-3.stderr diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 3d049fe4ccdad..e8d44ee1310e7 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1541,7 +1541,7 @@ pub enum ExprKind { Match(P, HirVec, MatchSource), /// A closure (e.g., `move |a, b, c| {a + b + c}`). /// - /// The final span is the span of the argument block `|...|`. + /// The `Span` is the argument block `|...|`. /// /// This may also be a generator literal or an `async block` as indicated by the /// `Option`. diff --git a/src/librustc/infer/error_reporting/need_type_info.rs b/src/librustc/infer/error_reporting/need_type_info.rs index b5d78a80bf674..16fed3d42d679 100644 --- a/src/librustc/infer/error_reporting/need_type_info.rs +++ b/src/librustc/infer/error_reporting/need_type_info.rs @@ -1,5 +1,5 @@ use crate::hir::def::Namespace; -use crate::hir::{self, Local, Pat, Body, HirId}; +use crate::hir::{self, Body, FunctionRetTy, Expr, ExprKind, HirId, Local, Pat}; use crate::hir::intravisit::{self, Visitor, NestedVisitorMap}; use crate::infer::InferCtxt; use crate::infer::type_variable::TypeVariableOriginKind; @@ -7,7 +7,7 @@ use crate::ty::{self, Ty, Infer, TyVar}; use crate::ty::print::Print; use syntax::source_map::DesugaringKind; use syntax_pos::Span; -use errors::DiagnosticBuilder; +use errors::{Applicability, DiagnosticBuilder}; struct FindLocalByTypeVisitor<'a, 'tcx> { infcx: &'a InferCtxt<'a, 'tcx>, @@ -16,9 +16,26 @@ struct FindLocalByTypeVisitor<'a, 'tcx> { found_local_pattern: Option<&'tcx Pat>, found_arg_pattern: Option<&'tcx Pat>, found_ty: Option>, + found_closure: Option<&'tcx ExprKind>, } impl<'a, 'tcx> FindLocalByTypeVisitor<'a, 'tcx> { + fn new( + infcx: &'a InferCtxt<'a, 'tcx>, + target_ty: Ty<'tcx>, + hir_map: &'a hir::map::Map<'tcx>, + ) -> FindLocalByTypeVisitor<'a, 'tcx> { + FindLocalByTypeVisitor { + infcx, + target_ty, + hir_map, + found_local_pattern: None, + found_arg_pattern: None, + found_ty: None, + found_closure: None, + } + } + fn node_matches_type(&mut self, hir_id: HirId) -> Option> { let ty_opt = self.infcx.in_progress_tables.and_then(|tables| { tables.borrow().node_type_opt(hir_id) @@ -72,6 +89,16 @@ impl<'a, 'tcx> Visitor<'tcx> for FindLocalByTypeVisitor<'a, 'tcx> { } intravisit::walk_body(self, body); } + + fn visit_expr(&mut self, expr: &'tcx Expr) { + if let (ExprKind::Closure(_, _fn_decl, _id, _sp, _), Some(_)) = ( + &expr.node, + self.node_matches_type(expr.hir_id), + ) { + self.found_closure = Some(&expr.node); + } + intravisit::walk_expr(self, expr); + } } impl<'a, 'tcx> InferCtxt<'a, 'tcx> { @@ -106,16 +133,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let ty = self.resolve_vars_if_possible(&ty); let name = self.extract_type_name(&ty, None); - let mut err_span = span; - - let mut local_visitor = FindLocalByTypeVisitor { - infcx: &self, - target_ty: ty, - hir_map: &self.tcx.hir(), - found_local_pattern: None, - found_arg_pattern: None, - found_ty: None, - }; + let mut local_visitor = FindLocalByTypeVisitor::new(&self, ty, &self.tcx.hir()); let ty_to_string = |ty: Ty<'tcx>| -> String { let mut s = String::new(); let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS); @@ -136,6 +154,35 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let expr = self.tcx.hir().expect_expr(body_id.hir_id); local_visitor.visit_expr(expr); } + let err_span = if let Some(pattern) = local_visitor.found_arg_pattern { + pattern.span + } else { + span + }; + + let ty_msg = match local_visitor.found_ty { + Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => { + let fn_sig = substs.closure_sig(*def_id, self.tcx); + let args = fn_sig.inputs() + .skip_binder() + .iter() + .next() + .map(|args| args.tuple_fields() + .map(|arg| arg.to_string()) + .collect::>().join(", ")) + .unwrap_or_default(); + let ret = fn_sig.output().skip_binder().to_string(); + format!(" for the closure `fn({}) -> {}`", args, ret) + } + Some(ty) if &ty.to_string() != "_" && + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 + (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) => + { + let ty = ty_to_string(ty); + format!(" for `{}`", ty) + } + _ => String::new(), + }; // When `name` corresponds to a type argument, show the path of the full type we're // trying to infer. In the following example, `ty_msg` contains @@ -150,34 +197,47 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // | consider giving `b` the explicit type `std::result::Result`, where // | the type parameter `E` is specified // ``` - let (ty_msg, suffix) = match &local_visitor.found_ty { - Some(ty) if &ty.to_string() != "_" && - name == "_" && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 - (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && - !ty.is_closure() => // The suggestion doesn't make sense for closures. - { - let ty = ty_to_string(ty); - (format!(" for `{}`", ty), - format!("the explicit type `{}`, with the type parameters specified", ty)) - } - Some(ty) if &ty.to_string() != "_" && - ty.to_string() != name && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 - (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) && - !ty.is_closure() => // The suggestion doesn't make sense for closures. - { - let ty = ty_to_string(ty); - (format!(" for `{}`", ty), - format!( - "the explicit type `{}`, where the type parameter `{}` is specified", - ty, - name, - )) - } + let mut err = struct_span_err!( + self.tcx.sess, + err_span, + E0282, + "type annotations needed{}", + ty_msg, + ); + + let suffix = match local_visitor.found_ty { Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => { - let msg = " for the closure".to_string(); let fn_sig = substs.closure_sig(*def_id, self.tcx); + let ret = fn_sig.output().skip_binder().to_string(); + + if let Some(ExprKind::Closure(_, decl, body_id, ..)) = local_visitor.found_closure { + let (arrow, post) = match decl.output { + FunctionRetTy::DefaultReturn(_) => ("-> ", " "), + _ => ("", ""), + }; + if let Some(body) = self.tcx.hir().krate().bodies.get(body_id) { + let suggestion = match body.value.node { + ExprKind::Block(..) => { + vec![(decl.output.span(), format!("{}{}{}", arrow, ret, post))] + } + _ => { + vec![ + (decl.output.span(), format!("{}{}{}{{ ", arrow, ret, post)), + (body.value.span.shrink_to_hi(), " }".to_string()), + ] + } + }; + err.multipart_suggestion( + "give this closure an explicit return type without `_` placeholders", + suggestion, + Applicability::HasPlaceholders, + ); + err.span_label(span, InferCtxt::missing_type_msg(&name)); + return err; + } + } + + // This shouldn't be reachable, but just in case we leave a reasonable fallback. let args = fn_sig.inputs() .skip_binder() .iter() @@ -189,18 +249,32 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // This suggestion is incomplete, as the user will get further type inference // errors due to the `_` placeholders and the introduction of `Box`, but it does // nudge them in the right direction. - (msg, format!( - "a boxed closure type like `Box {}>`", - args, - fn_sig.output().skip_binder().to_string(), - )) + format!("a boxed closure type like `Box {}>`", args, ret) + } + Some(ty) if &ty.to_string() != "_" && + name == "_" && + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 + (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) => + { + let ty = ty_to_string(ty); + format!("the explicit type `{}`, with the type parameters specified", ty) } - _ => (String::new(), "a type".to_owned()), + Some(ty) if &ty.to_string() != "_" && + ty.to_string() != name && + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 + (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) => + { + let ty = ty_to_string(ty); + format!( + "the explicit type `{}`, where the type parameter `{}` is specified", + ty, + name, + ) + } + _ => "a type".to_string(), }; - let mut labels = vec![(span, InferCtxt::missing_type_msg(&name))]; if let Some(pattern) = local_visitor.found_arg_pattern { - err_span = pattern.span; // We don't want to show the default label for closures. // // So, before clearing, the output would look something like this: @@ -217,39 +291,36 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { // ^ consider giving this closure parameter the type `[_; 0]` // with the type parameter `_` specified // ``` - labels.clear(); - labels.push(( + err.span_label( pattern.span, format!("consider giving this closure parameter {}", suffix), - )); + ); } else if let Some(pattern) = local_visitor.found_local_pattern { if let Some(simple_ident) = pattern.simple_ident() { match pattern.span.desugaring_kind() { - None => labels.push(( - pattern.span, - format!("consider giving `{}` {}", simple_ident, suffix), - )), - Some(DesugaringKind::ForLoop) => labels.push(( - pattern.span, - "the element type for this iterator is not specified".to_owned(), - )), + None => { + err.span_label( + pattern.span, + format!("consider giving `{}` {}", simple_ident, suffix), + ); + } + Some(DesugaringKind::ForLoop) => { + err.span_label( + pattern.span, + "the element type for this iterator is not specified".to_string(), + ); + } _ => {} } } else { - labels.push((pattern.span, format!("consider giving this pattern {}", suffix))); + err.span_label(pattern.span, format!("consider giving this pattern {}", suffix)); } - }; - - let mut err = struct_span_err!( - self.tcx.sess, - err_span, - E0282, - "type annotations needed{}", - ty_msg, - ); - - for (target_span, label_message) in labels { - err.span_label(target_span, label_message); + } + if !err.span.span_labels().iter().any(|span_label| { + span_label.label.is_some() && span_label.span == span + }) && local_visitor.found_arg_pattern.is_none() + { // Avoid multiple labels pointing at `span`. + err.span_label(span, InferCtxt::missing_type_msg(&name)); } err diff --git a/src/test/ui/error-codes/E0282.stderr b/src/test/ui/error-codes/E0282.stderr index 3a5040eb6daa6..0f610a5e42f65 100644 --- a/src/test/ui/error-codes/E0282.stderr +++ b/src/test/ui/error-codes/E0282.stderr @@ -2,10 +2,7 @@ error[E0282]: type annotations needed --> $DIR/E0282.rs:2:9 | LL | let x = "hello".chars().rev().collect(); - | ^ - | | - | cannot infer type - | consider giving `x` a type + | ^ consider giving `x` a type error: aborting due to previous error diff --git a/src/test/ui/for/for-loop-unconstrained-element-type.stderr b/src/test/ui/for/for-loop-unconstrained-element-type.stderr index 02fdb808da449..0672014a92929 100644 --- a/src/test/ui/for/for-loop-unconstrained-element-type.stderr +++ b/src/test/ui/for/for-loop-unconstrained-element-type.stderr @@ -2,10 +2,7 @@ error[E0282]: type annotations needed --> $DIR/for-loop-unconstrained-element-type.rs:8:14 | LL | for i in Vec::new() { } - | ^^^^^^^^^^ - | | - | cannot infer type - | the element type for this iterator is not specified + | ^^^^^^^^^^ the element type for this iterator is not specified error: aborting due to previous error diff --git a/src/test/ui/inference/cannot-infer-closure.stderr b/src/test/ui/inference/cannot-infer-closure.stderr index fb78dedc887ff..5f30b5d993c1e 100644 --- a/src/test/ui/inference/cannot-infer-closure.stderr +++ b/src/test/ui/inference/cannot-infer-closure.stderr @@ -1,10 +1,12 @@ -error[E0282]: type annotations needed for the closure +error[E0282]: type annotations needed for the closure `fn((), ()) -> std::result::Result<(), _>` --> $DIR/cannot-infer-closure.rs:3:9 | -LL | let x = |a: (), b: ()| { - | - consider giving `x` a boxed closure type like `Box std::result::Result<(), _>>` LL | Err(a)?; | ^^^^^^^ cannot infer type +help: give this closure an explicit return type without `_` placeholders + | +LL | let x = |a: (), b: ()| -> std::result::Result<(), _> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-18159.stderr b/src/test/ui/issues/issue-18159.stderr index 6048344125c5f..9b890be3c789b 100644 --- a/src/test/ui/issues/issue-18159.stderr +++ b/src/test/ui/issues/issue-18159.stderr @@ -2,10 +2,7 @@ error[E0282]: type annotations needed --> $DIR/issue-18159.rs:2:9 | LL | let x; - | ^ - | | - | cannot infer type - | consider giving `x` a type + | ^ consider giving `x` a type error: aborting due to previous error diff --git a/src/test/ui/match/match-unresolved-one-arm.stderr b/src/test/ui/match/match-unresolved-one-arm.stderr index ad8569b4802fb..77df9921b754a 100644 --- a/src/test/ui/match/match-unresolved-one-arm.stderr +++ b/src/test/ui/match/match-unresolved-one-arm.stderr @@ -2,10 +2,7 @@ error[E0282]: type annotations needed --> $DIR/match-unresolved-one-arm.rs:4:9 | LL | let x = match () { - | ^ - | | - | cannot infer type - | consider giving `x` a type + | ^ consider giving `x` a type error: aborting due to previous error diff --git a/src/test/ui/suggestions/suggest-closure-return-type-1.rs b/src/test/ui/suggestions/suggest-closure-return-type-1.rs new file mode 100644 index 0000000000000..910f273b9722d --- /dev/null +++ b/src/test/ui/suggestions/suggest-closure-return-type-1.rs @@ -0,0 +1,3 @@ +fn main() { + let _v = || -> _ { [] }; //~ ERROR type annotations needed for the closure +} diff --git a/src/test/ui/suggestions/suggest-closure-return-type-1.stderr b/src/test/ui/suggestions/suggest-closure-return-type-1.stderr new file mode 100644 index 0000000000000..de2d29f1270c6 --- /dev/null +++ b/src/test/ui/suggestions/suggest-closure-return-type-1.stderr @@ -0,0 +1,13 @@ +error[E0282]: type annotations needed for the closure `fn() -> [_; 0]` + --> $DIR/suggest-closure-return-type-1.rs:2:24 + | +LL | let _v = || -> _ { [] }; + | ^^ cannot infer type +help: give this closure an explicit return type without `_` placeholders + | +LL | let _v = || -> [_; 0] { [] }; + | ^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/src/test/ui/suggestions/suggest-closure-return-type-2.rs b/src/test/ui/suggestions/suggest-closure-return-type-2.rs new file mode 100644 index 0000000000000..6955b37ad97e4 --- /dev/null +++ b/src/test/ui/suggestions/suggest-closure-return-type-2.rs @@ -0,0 +1,3 @@ +fn main() { + let _v = || { [] }; //~ ERROR type annotations needed for the closure +} diff --git a/src/test/ui/suggestions/suggest-closure-return-type-2.stderr b/src/test/ui/suggestions/suggest-closure-return-type-2.stderr new file mode 100644 index 0000000000000..9dbd822fbb5de --- /dev/null +++ b/src/test/ui/suggestions/suggest-closure-return-type-2.stderr @@ -0,0 +1,13 @@ +error[E0282]: type annotations needed for the closure `fn() -> [_; 0]` + --> $DIR/suggest-closure-return-type-2.rs:2:19 + | +LL | let _v = || { [] }; + | ^^ cannot infer type +help: give this closure an explicit return type without `_` placeholders + | +LL | let _v = || -> [_; 0] { [] }; + | ^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. diff --git a/src/test/ui/suggestions/suggest-closure-return-type-3.rs b/src/test/ui/suggestions/suggest-closure-return-type-3.rs new file mode 100644 index 0000000000000..ec6c094027e1e --- /dev/null +++ b/src/test/ui/suggestions/suggest-closure-return-type-3.rs @@ -0,0 +1,3 @@ +fn main() { + let _v = || []; //~ ERROR type annotations needed for the closure +} diff --git a/src/test/ui/suggestions/suggest-closure-return-type-3.stderr b/src/test/ui/suggestions/suggest-closure-return-type-3.stderr new file mode 100644 index 0000000000000..ad0d4e41f7874 --- /dev/null +++ b/src/test/ui/suggestions/suggest-closure-return-type-3.stderr @@ -0,0 +1,13 @@ +error[E0282]: type annotations needed for the closure `fn() -> [_; 0]` + --> $DIR/suggest-closure-return-type-3.rs:2:17 + | +LL | let _v = || []; + | ^^ cannot infer type +help: give this closure an explicit return type without `_` placeholders + | +LL | let _v = || -> [_; 0] { [] }; + | ^^^^^^^^^^^ ^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0282`. From 9348af8396c961f8bb79cc360c091d74ea4ba34a Mon Sep 17 00:00:00 2001 From: Caio Date: Tue, 13 Aug 2019 22:22:51 -0300 Subject: [PATCH 33/37] Add NodeId for Arm, Field and FieldPat --- src/libsyntax/ast.rs | 3 +++ src/libsyntax/ext/build.rs | 2 ++ src/libsyntax/mut_visit.rs | 12 +++++++++--- src/libsyntax/parse/parser/expr.rs | 3 +++ src/libsyntax/parse/parser/pat.rs | 1 + src/libsyntax_ext/deriving/generic/mod.rs | 1 + 6 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 052eb55b40811..aadf7ec5588b4 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -608,6 +608,7 @@ pub struct FieldPat { pub pat: P, pub is_shorthand: bool, pub attrs: ThinVec, + pub id: NodeId, } #[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, Copy)] @@ -925,6 +926,7 @@ pub struct Arm { pub guard: Option>, pub body: P, pub span: Span, + pub id: NodeId, } #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] @@ -934,6 +936,7 @@ pub struct Field { pub span: Span, pub is_shorthand: bool, pub attrs: ThinVec, + pub id: NodeId, } pub type SpannedIdent = Spanned; diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 22962499a2b75..aab782d612e1b 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -403,6 +403,7 @@ impl<'a> ExtCtxt<'a> { span, is_shorthand: false, attrs: ThinVec::new(), + id: ast::DUMMY_NODE_ID, } } pub fn expr_struct( @@ -612,6 +613,7 @@ impl<'a> ExtCtxt<'a> { guard: None, body: expr, span, + id: ast::DUMMY_NODE_ID, } } diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index be04c6a76b06d..f910aaaf8fa7b 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -383,10 +383,11 @@ pub fn noop_visit_use_tree(use_tree: &mut UseTree, vis: &mut T) { } pub fn noop_visit_arm( - Arm { attrs, pats, guard, body, span }: &mut Arm, + Arm { attrs, pats, guard, body, span, id }: &mut Arm, vis: &mut T, ) { visit_attrs(attrs, vis); + vis.visit_id(id); visit_vec(pats, |pat| vis.visit_pat(pat)); visit_opt(guard, |guard| vis.visit_expr(guard)); vis.visit_expr(body); @@ -808,9 +809,10 @@ pub fn noop_visit_struct_field(f: &mut StructField, visitor: &mut } pub fn noop_visit_field(f: &mut Field, vis: &mut T) { - let Field { ident, expr, span, is_shorthand: _, attrs } = f; + let Field { ident, expr, span, is_shorthand: _, attrs, id } = f; vis.visit_ident(ident); vis.visit_expr(expr); + vis.visit_id(id); vis.visit_span(span); visit_thin_attrs(attrs, vis); } @@ -1040,8 +1042,12 @@ pub fn noop_visit_pat(pat: &mut P, vis: &mut T) { } PatKind::Struct(path, fields, _etc) => { vis.visit_path(path); - for Spanned { node: FieldPat { ident, pat, is_shorthand: _, attrs }, span } in fields { + for Spanned { + node: FieldPat { ident, pat, is_shorthand: _, attrs, id }, + span + } in fields { vis.visit_ident(ident); + vis.visit_id(id); vis.visit_pat(pat); visit_thin_attrs(attrs, vis); vis.visit_span(span); diff --git a/src/libsyntax/parse/parser/expr.rs b/src/libsyntax/parse/parser/expr.rs index 4432c1329cbfe..823dca2c9e765 100644 --- a/src/libsyntax/parse/parser/expr.rs +++ b/src/libsyntax/parse/parser/expr.rs @@ -1444,6 +1444,7 @@ impl<'a> Parser<'a> { guard, body: expr, span: lo.to(hi), + id: ast::DUMMY_NODE_ID, }) } @@ -1599,6 +1600,7 @@ impl<'a> Parser<'a> { expr: self.mk_expr(self.token.span, ExprKind::Err, ThinVec::new()), is_shorthand: false, attrs: ThinVec::new(), + id: ast::DUMMY_NODE_ID, }); } } @@ -1684,6 +1686,7 @@ impl<'a> Parser<'a> { expr, is_shorthand, attrs: attrs.into(), + id: ast::DUMMY_NODE_ID, }) } diff --git a/src/libsyntax/parse/parser/pat.rs b/src/libsyntax/parse/parser/pat.rs index 5cc428a4df1de..5a1b41645099b 100644 --- a/src/libsyntax/parse/parser/pat.rs +++ b/src/libsyntax/parse/parser/pat.rs @@ -620,6 +620,7 @@ impl<'a> Parser<'a> { pat: subpat, is_shorthand, attrs: attrs.into(), + id: ast::DUMMY_NODE_ID, } }) } diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 7e6d9126c8740..b21a6e7bc78a0 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -1613,6 +1613,7 @@ impl<'a> TraitDef<'a> { source_map::Spanned { span: pat.span.with_ctxt(self.span.ctxt()), node: ast::FieldPat { + id: ast::DUMMY_NODE_ID, ident: ident.unwrap(), pat, is_shorthand: false, From 264640cde5ed30bdc0f3ef138472eb0de4d2f3ea Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 14 Aug 2019 18:04:33 +0200 Subject: [PATCH 34/37] move test that shouldn't be in test/run-pass/ --- src/test/{run-pass => ui}/generator/niche-in-generator.rs | 2 ++ 1 file changed, 2 insertions(+) rename src/test/{run-pass => ui}/generator/niche-in-generator.rs (95%) diff --git a/src/test/run-pass/generator/niche-in-generator.rs b/src/test/ui/generator/niche-in-generator.rs similarity index 95% rename from src/test/run-pass/generator/niche-in-generator.rs rename to src/test/ui/generator/niche-in-generator.rs index 9a644ed44a670..42bee81f524c5 100644 --- a/src/test/run-pass/generator/niche-in-generator.rs +++ b/src/test/ui/generator/niche-in-generator.rs @@ -1,5 +1,7 @@ // Test that niche finding works with captured generator upvars. +// run-pass + #![feature(generators)] use std::mem::size_of_val; From 6a42b0b28db4c18fd9e0c0e6e69ecbe0c5d9b107 Mon Sep 17 00:00:00 2001 From: Caio Date: Tue, 13 Aug 2019 21:40:21 -0300 Subject: [PATCH 35/37] Merge Variant and Variant_ --- src/librustc/hir/check_attr.rs | 2 +- src/librustc/hir/intravisit.rs | 12 +++--- src/librustc/hir/lowering/item.rs | 14 +++--- src/librustc/hir/map/collector.rs | 8 ++-- src/librustc/hir/map/def_collector.rs | 6 +-- src/librustc/hir/map/mod.rs | 10 ++--- src/librustc/hir/mod.rs | 6 +-- src/librustc/hir/print.rs | 6 +-- src/librustc/ich/impls_hir.rs | 2 +- src/librustc/lint/context.rs | 4 +- src/librustc/lint/mod.rs | 2 +- src/librustc/middle/dead.rs | 12 +++--- src/librustc/middle/stability.rs | 8 ++-- src/librustc_lint/builtin.rs | 4 +- src/librustc_lint/nonstandard_style.rs | 2 +- src/librustc_lint/types.rs | 2 +- src/librustc_metadata/encoder.rs | 2 +- src/librustc_passes/ast_validation.rs | 2 +- src/librustc_privacy/lib.rs | 14 +++--- src/librustc_resolve/build_reduced_graph.rs | 10 ++--- src/librustc_save_analysis/dump_visitor.rs | 22 +++++----- src/librustc_save_analysis/lib.rs | 4 +- src/librustc_save_analysis/sig.rs | 4 +- src/librustc_typeck/check/mod.rs | 10 ++--- src/librustc_typeck/check/wfcheck.rs | 2 +- src/librustc_typeck/collect.rs | 47 ++++++++++----------- src/librustc_typeck/variance/constraints.rs | 4 +- src/librustc_typeck/variance/terms.rs | 4 +- src/librustdoc/test.rs | 2 +- src/librustdoc/visit_ast.rs | 8 ++-- src/libsyntax/ast.rs | 6 +-- src/libsyntax/attr/mod.rs | 2 +- src/libsyntax/config.rs | 2 +- src/libsyntax/ext/build.rs | 16 +++---- src/libsyntax/feature_gate.rs | 2 +- src/libsyntax/mut_visit.rs | 2 +- src/libsyntax/parse/diagnostics.rs | 11 +++-- src/libsyntax/parse/parser/item.rs | 5 ++- src/libsyntax/print/pprust.rs | 6 +-- src/libsyntax/print/pprust/tests.rs | 5 ++- src/libsyntax/visit.rs | 8 ++-- src/libsyntax_ext/deriving/clone.rs | 6 +-- src/libsyntax_ext/deriving/cmp/eq.rs | 2 +- src/libsyntax_ext/deriving/debug.rs | 2 +- src/libsyntax_ext/deriving/encodable.rs | 2 +- src/libsyntax_ext/deriving/generic/mod.rs | 16 +++---- 46 files changed, 162 insertions(+), 166 deletions(-) diff --git a/src/librustc/hir/check_attr.rs b/src/librustc/hir/check_attr.rs index 22124d4ee4120..eae956c978a8e 100644 --- a/src/librustc/hir/check_attr.rs +++ b/src/librustc/hir/check_attr.rs @@ -336,7 +336,7 @@ impl Visitor<'tcx> for CheckAttrVisitor<'tcx> { fn is_c_like_enum(item: &hir::Item) -> bool { if let hir::ItemKind::Enum(ref def, _) = item.node { for variant in &def.variants { - match variant.node.data { + match variant.data { hir::VariantData::Unit(..) => { /* continue */ } _ => { return false; } } diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index b5c760bc9a08e..71d7464540a4c 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -577,15 +577,15 @@ pub fn walk_variant<'v, V: Visitor<'v>>(visitor: &mut V, variant: &'v Variant, generics: &'v Generics, parent_item_id: HirId) { - visitor.visit_ident(variant.node.ident); - visitor.visit_id(variant.node.id); - visitor.visit_variant_data(&variant.node.data, - variant.node.ident.name, + visitor.visit_ident(variant.ident); + visitor.visit_id(variant.id); + visitor.visit_variant_data(&variant.data, + variant.ident.name, generics, parent_item_id, variant.span); - walk_list!(visitor, visit_anon_const, &variant.node.disr_expr); - walk_list!(visitor, visit_attribute, &variant.node.attrs); + walk_list!(visitor, visit_anon_const, &variant.disr_expr); + walk_list!(visitor, visit_attribute, &variant.attrs); } pub fn walk_ty<'v, V: Visitor<'v>>(visitor: &mut V, typ: &'v Ty) { diff --git a/src/librustc/hir/lowering/item.rs b/src/librustc/hir/lowering/item.rs index 6b717e75199c7..abe8b841cbd88 100644 --- a/src/librustc/hir/lowering/item.rs +++ b/src/librustc/hir/lowering/item.rs @@ -733,14 +733,12 @@ impl LoweringContext<'_> { } fn lower_variant(&mut self, v: &Variant) -> hir::Variant { - Spanned { - node: hir::VariantKind { - ident: v.node.ident, - id: self.lower_node_id(v.node.id), - attrs: self.lower_attrs(&v.node.attrs), - data: self.lower_variant_data(&v.node.data), - disr_expr: v.node.disr_expr.as_ref().map(|e| self.lower_anon_const(e)), - }, + hir::Variant { + attrs: self.lower_attrs(&v.attrs), + data: self.lower_variant_data(&v.data), + disr_expr: v.disr_expr.as_ref().map(|e| self.lower_anon_const(e)), + id: self.lower_node_id(v.id), + ident: v.ident, span: v.span, } } diff --git a/src/librustc/hir/map/collector.rs b/src/librustc/hir/map/collector.rs index b6807f7d3bbd2..effe2c0cc6a76 100644 --- a/src/librustc/hir/map/collector.rs +++ b/src/librustc/hir/map/collector.rs @@ -544,11 +544,11 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { } fn visit_variant(&mut self, v: &'hir Variant, g: &'hir Generics, item_id: HirId) { - self.insert(v.span, v.node.id, Node::Variant(v)); - self.with_parent(v.node.id, |this| { + self.insert(v.span, v.id, Node::Variant(v)); + self.with_parent(v.id, |this| { // Register the constructor of this variant. - if let Some(ctor_hir_id) = v.node.data.ctor_hir_id() { - this.insert(v.span, ctor_hir_id, Node::Ctor(&v.node.data)); + if let Some(ctor_hir_id) = v.data.ctor_hir_id() { + this.insert(v.span, ctor_hir_id, Node::Ctor(&v.data)); } intravisit::walk_variant(this, v, g, item_id); }); diff --git a/src/librustc/hir/map/def_collector.rs b/src/librustc/hir/map/def_collector.rs index 2964b130ddd31..df9bd3a35a621 100644 --- a/src/librustc/hir/map/def_collector.rs +++ b/src/librustc/hir/map/def_collector.rs @@ -155,11 +155,11 @@ impl<'a> visit::Visitor<'a> for DefCollector<'a> { } fn visit_variant(&mut self, v: &'a Variant, g: &'a Generics, item_id: NodeId) { - let def = self.create_def(v.node.id, - DefPathData::TypeNs(v.node.ident.as_interned_str()), + let def = self.create_def(v.id, + DefPathData::TypeNs(v.ident.as_interned_str()), v.span); self.with_parent(def, |this| { - if let Some(ctor_hir_id) = v.node.data.ctor_id() { + if let Some(ctor_hir_id) = v.data.ctor_id() { this.create_def(ctor_hir_id, DefPathData::Ctor, v.span); } visit::walk_variant(this, v, g, item_id) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index b85738dd29a6d..59e298ea13885 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -885,7 +885,7 @@ impl<'hir> Map<'hir> { _ => bug!("struct ID bound to non-struct {}", self.node_to_string(id)) } } - Some(Node::Variant(variant)) => &variant.node.data, + Some(Node::Variant(variant)) => &variant.data, Some(Node::Ctor(data)) => data, _ => bug!("expected struct or variant, found {}", self.node_to_string(id)) } @@ -918,7 +918,7 @@ impl<'hir> Map<'hir> { Node::ForeignItem(fi) => fi.ident.name, Node::ImplItem(ii) => ii.ident.name, Node::TraitItem(ti) => ti.ident.name, - Node::Variant(v) => v.node.ident.name, + Node::Variant(v) => v.ident.name, Node::Field(f) => f.ident.name, Node::Lifetime(lt) => lt.name.ident().name, Node::GenericParam(param) => param.name.ident().name, @@ -939,7 +939,7 @@ impl<'hir> Map<'hir> { Some(Node::ForeignItem(fi)) => Some(&fi.attrs[..]), Some(Node::TraitItem(ref ti)) => Some(&ti.attrs[..]), Some(Node::ImplItem(ref ii)) => Some(&ii.attrs[..]), - Some(Node::Variant(ref v)) => Some(&v.node.attrs[..]), + Some(Node::Variant(ref v)) => Some(&v.attrs[..]), Some(Node::Field(ref f)) => Some(&f.attrs[..]), Some(Node::Expr(ref e)) => Some(&*e.attrs), Some(Node::Stmt(ref s)) => Some(s.node.attrs()), @@ -1133,7 +1133,7 @@ impl Named for Spanned { fn name(&self) -> Name { self.node.name() } impl Named for Item { fn name(&self) -> Name { self.ident.name } } impl Named for ForeignItem { fn name(&self) -> Name { self.ident.name } } -impl Named for VariantKind { fn name(&self) -> Name { self.ident.name } } +impl Named for Variant { fn name(&self) -> Name { self.ident.name } } impl Named for StructField { fn name(&self) -> Name { self.ident.name } } impl Named for TraitItem { fn name(&self) -> Name { self.ident.name } } impl Named for ImplItem { fn name(&self) -> Name { self.ident.name } } @@ -1310,7 +1310,7 @@ fn hir_id_to_string(map: &Map<'_>, id: HirId, include_id: bool) -> String { } Some(Node::Variant(ref variant)) => { format!("variant {} in {}{}", - variant.node.ident, + variant.ident, path_str(), id_str) } Some(Node::Field(ref field)) => { diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 3d049fe4ccdad..56ab37e138f39 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -2193,7 +2193,7 @@ pub struct EnumDef { } #[derive(RustcEncodable, RustcDecodable, Debug, HashStable)] -pub struct VariantKind { +pub struct Variant { /// Name of the variant. #[stable_hasher(project(name))] pub ident: Ident, @@ -2205,10 +2205,10 @@ pub struct VariantKind { pub data: VariantData, /// Explicit discriminant (e.g., `Foo = 1`). pub disr_expr: Option, + /// Span + pub span: Span } -pub type Variant = Spanned; - #[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable)] pub enum UseKind { /// One import, e.g., `use foo::bar` or `use foo::bar as baz`. diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index 11ba512053084..0cbfe4d75f1c6 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -737,7 +737,7 @@ impl<'a> State<'a> { for v in variants { self.space_if_not_bol(); self.maybe_print_comment(v.span.lo()); - self.print_outer_attributes(&v.node.attrs); + self.print_outer_attributes(&v.attrs); self.ibox(INDENT_UNIT); self.print_variant(v); self.s.word(","); @@ -829,8 +829,8 @@ impl<'a> State<'a> { pub fn print_variant(&mut self, v: &hir::Variant) { self.head(""); let generics = hir::Generics::empty(); - self.print_struct(&v.node.data, &generics, v.node.ident.name, v.span, false); - if let Some(ref d) = v.node.disr_expr { + self.print_struct(&v.data, &generics, v.ident.name, v.span, false); + if let Some(ref d) = v.disr_expr { self.s.space(); self.word_space("="); self.print_anon_const(d); diff --git a/src/librustc/ich/impls_hir.rs b/src/librustc/ich/impls_hir.rs index 30d76f240d1fa..1fd4e00de497f 100644 --- a/src/librustc/ich/impls_hir.rs +++ b/src/librustc/ich/impls_hir.rs @@ -304,7 +304,7 @@ impl<'a> HashStable> for hir::Mod { } } -impl_stable_hash_for_spanned!(hir::VariantKind); +impl_stable_hash_for_spanned!(hir::Variant); impl<'a> HashStable> for hir::Item { diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs index de812410e8bd8..a05023718f8f6 100644 --- a/src/librustc/lint/context.rs +++ b/src/librustc/lint/context.rs @@ -1060,7 +1060,7 @@ for LateContextAndPass<'a, 'tcx, T> { v: &'tcx hir::Variant, g: &'tcx hir::Generics, item_id: hir::HirId) { - self.with_lint_attrs(v.node.id, &v.node.attrs, |cx| { + self.with_lint_attrs(v.id, &v.attrs, |cx| { lint_callback!(cx, check_variant, v, g); hir_visit::walk_variant(cx, v, g, item_id); lint_callback!(cx, check_variant_post, v, g); @@ -1236,7 +1236,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T> } fn visit_variant(&mut self, v: &'a ast::Variant, g: &'a ast::Generics, item_id: ast::NodeId) { - self.with_lint_attrs(item_id, &v.node.attrs, |cx| { + self.with_lint_attrs(item_id, &v.attrs, |cx| { run_early_pass!(cx, check_variant, v, g); ast_visit::walk_variant(cx, v, g, item_id); run_early_pass!(cx, check_variant_post, v, g); diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 8ddf4603490a1..8cb5b1e26d947 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -846,7 +846,7 @@ impl intravisit::Visitor<'tcx> for LintLevelMapBuilder<'tcx> { v: &'tcx hir::Variant, g: &'tcx hir::Generics, item_id: hir::HirId) { - self.with_lint_attrs(v.node.id, &v.node.attrs, |builder| { + self.with_lint_attrs(v.id, &v.attrs, |builder| { intravisit::walk_variant(builder, v, g, item_id); }) } diff --git a/src/librustc/middle/dead.rs b/src/librustc/middle/dead.rs index 55fa261f1ed57..1f3adf0092367 100644 --- a/src/librustc/middle/dead.rs +++ b/src/librustc/middle/dead.rs @@ -366,12 +366,12 @@ impl<'v, 'k, 'tcx> ItemLikeVisitor<'v> for LifeSeeder<'k, 'tcx> { match item.node { hir::ItemKind::Enum(ref enum_def, _) => { if allow_dead_code { - self.worklist.extend(enum_def.variants.iter().map(|variant| variant.node.id)); + self.worklist.extend(enum_def.variants.iter().map(|variant| variant.id)); } for variant in &enum_def.variants { - if let Some(ctor_hir_id) = variant.node.data.ctor_hir_id() { - self.struct_constructors.insert(ctor_hir_id, variant.node.id); + if let Some(ctor_hir_id) = variant.data.ctor_hir_id() { + self.struct_constructors.insert(ctor_hir_id, variant.id); } } } @@ -497,7 +497,7 @@ impl DeadVisitor<'tcx> { && !has_allow_dead_code_or_lang_attr(self.tcx, field.hir_id, &field.attrs) } - fn should_warn_about_variant(&mut self, variant: &hir::VariantKind) -> bool { + fn should_warn_about_variant(&mut self, variant: &hir::Variant) -> bool { !self.symbol_is_live(variant.id) && !has_allow_dead_code_or_lang_attr(self.tcx, variant.id, @@ -596,8 +596,8 @@ impl Visitor<'tcx> for DeadVisitor<'tcx> { variant: &'tcx hir::Variant, g: &'tcx hir::Generics, id: hir::HirId) { - if self.should_warn_about_variant(&variant.node) { - self.warn_dead_code(variant.node.id, variant.span, variant.node.ident.name, + if self.should_warn_about_variant(&variant) { + self.warn_dead_code(variant.id, variant.span, variant.ident.name, "variant", "constructed"); } else { intravisit::walk_variant(self, variant, g, id); diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index 5ab762ab225f9..d02259bf3010b 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -290,10 +290,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Annotator<'a, 'tcx> { } fn visit_variant(&mut self, var: &'tcx Variant, g: &'tcx Generics, item_id: HirId) { - self.annotate(var.node.id, &var.node.attrs, var.span, AnnotationKind::Required, + self.annotate(var.id, &var.attrs, var.span, AnnotationKind::Required, |v| { - if let Some(ctor_hir_id) = var.node.data.ctor_hir_id() { - v.annotate(ctor_hir_id, &var.node.attrs, var.span, AnnotationKind::Required, + if let Some(ctor_hir_id) = var.data.ctor_hir_id() { + v.annotate(ctor_hir_id, &var.attrs, var.span, AnnotationKind::Required, |_| {}); } @@ -372,7 +372,7 @@ impl<'a, 'tcx> Visitor<'tcx> for MissingStabilityAnnotations<'a, 'tcx> { } fn visit_variant(&mut self, var: &'tcx Variant, g: &'tcx Generics, item_id: HirId) { - self.check_missing_stability(var.node.id, var.span, "variant"); + self.check_missing_stability(var.id, var.span, "variant"); intravisit::walk_variant(self, var, g, item_id); } diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index bb2a5cab7d9bf..a83d490511562 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -484,8 +484,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDoc { fn check_variant(&mut self, cx: &LateContext<'_, '_>, v: &hir::Variant, _: &hir::Generics) { self.check_missing_docs_attrs(cx, - Some(v.node.id), - &v.node.attrs, + Some(v.id), + &v.attrs, v.span, "a variant"); } diff --git a/src/librustc_lint/nonstandard_style.rs b/src/librustc_lint/nonstandard_style.rs index 8f7fe6680cb14..acd17f7663234 100644 --- a/src/librustc_lint/nonstandard_style.rs +++ b/src/librustc_lint/nonstandard_style.rs @@ -147,7 +147,7 @@ impl EarlyLintPass for NonCamelCaseTypes { } fn check_variant(&mut self, cx: &EarlyContext<'_>, v: &ast::Variant, _: &ast::Generics) { - self.check_case(cx, "variant", &v.node.ident); + self.check_case(cx, "variant", &v.ident); } fn check_generic_param(&mut self, cx: &EarlyContext<'_>, param: &ast::GenericParam) { diff --git a/src/librustc_lint/types.rs b/src/librustc_lint/types.rs index e86230437f277..217e10ab24f55 100644 --- a/src/librustc_lint/types.rs +++ b/src/librustc_lint/types.rs @@ -976,7 +976,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VariantSizeDifferences { let bytes = variant_layout.size.bytes().saturating_sub(discr_size); debug!("- variant `{}` is {} bytes large", - variant.node.ident, + variant.ident, bytes); bytes }) diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index d73a4966bca8c..fb675d7d841e1 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -1676,7 +1676,7 @@ impl Visitor<'tcx> for EncodeContext<'tcx> { id: hir::HirId) { intravisit::walk_variant(self, v, g, id); - if let Some(ref discr) = v.node.disr_expr { + if let Some(ref discr) = v.disr_expr { let def_id = self.tcx.hir().local_def_id(discr.hir_id); self.record(def_id, EncodeContext::encode_info_for_anon_const, def_id); } diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 3c31bcef32b7a..489806f75a5a8 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -602,7 +602,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } ItemKind::Enum(ref def, _) => { for variant in &def.variants { - for field in variant.node.data.fields() { + for field in variant.data.fields() { self.invalid_visibility(&field.vis, None); } } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 673762ee4c6f7..852b4898f4ab2 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -687,11 +687,11 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { match item.node { hir::ItemKind::Enum(ref def, _) => { for variant in &def.variants { - let variant_level = self.update(variant.node.id, item_level); - if let Some(ctor_hir_id) = variant.node.data.ctor_hir_id() { + let variant_level = self.update(variant.id, item_level); + if let Some(ctor_hir_id) = variant.data.ctor_hir_id() { self.update(ctor_hir_id, item_level); } - for field in variant.node.data.fields() { + for field in variant.data.fields() { self.update(field.hir_id, variant_level); } } @@ -810,9 +810,9 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { self.reach(item.hir_id, item_level).generics().predicates(); } for variant in &def.variants { - let variant_level = self.get(variant.node.id); + let variant_level = self.get(variant.id); if variant_level.is_some() { - for field in variant.node.data.fields() { + for field in variant.data.fields() { self.reach(field.hir_id, variant_level).ty(); } // Corner case: if the variant is reachable, but its @@ -1647,7 +1647,7 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { v: &'tcx hir::Variant, g: &'tcx hir::Generics, item_id: hir::HirId) { - if self.access_levels.is_reachable(v.node.id) { + if self.access_levels.is_reachable(v.id) { self.in_variant = true; intravisit::walk_variant(self, v, g, item_id); self.in_variant = false; @@ -1898,7 +1898,7 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> self.check(item.hir_id, item_visibility).generics().predicates(); for variant in &def.variants { - for field in variant.node.data.fields() { + for field in variant.data.fields() { self.check(field.hir_id, item_visibility).ty(); } } diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 5dd7bc3054829..0a32b0c6e95db 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -799,17 +799,17 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { parent: Module<'a>, vis: ty::Visibility, expn_id: ExpnId) { - let ident = variant.node.ident; + let ident = variant.ident; // Define a name in the type namespace. - let def_id = self.r.definitions.local_def_id(variant.node.id); + let def_id = self.r.definitions.local_def_id(variant.id); let res = Res::Def(DefKind::Variant, def_id); self.r.define(parent, ident, TypeNS, (res, vis, variant.span, expn_id)); // If the variant is marked as non_exhaustive then lower the visibility to within the // crate. let mut ctor_vis = vis; - let has_non_exhaustive = attr::contains_name(&variant.node.attrs, sym::non_exhaustive); + let has_non_exhaustive = attr::contains_name(&variant.attrs, sym::non_exhaustive); if has_non_exhaustive && vis == ty::Visibility::Public { ctor_vis = ty::Visibility::Restricted(DefId::local(CRATE_DEF_INDEX)); } @@ -819,9 +819,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { // value namespace, they are reserved for possible future use. // It's ok to use the variant's id as a ctor id since an // error will be reported on any use of such resolution anyway. - let ctor_node_id = variant.node.data.ctor_id().unwrap_or(variant.node.id); + let ctor_node_id = variant.data.ctor_id().unwrap_or(variant.id); let ctor_def_id = self.r.definitions.local_def_id(ctor_node_id); - let ctor_kind = CtorKind::from_ast(&variant.node.data); + let ctor_kind = CtorKind::from_ast(&variant.data); let ctor_res = Res::Def(DefKind::Ctor(CtorOf::Variant, ctor_kind), ctor_def_id); self.r.define(parent, ident, ValueNS, (ctor_res, ctor_vis, variant.span, expn_id)); } diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs index c32d4885c4ad9..d93c12513c5a1 100644 --- a/src/librustc_save_analysis/dump_visitor.rs +++ b/src/librustc_save_analysis/dump_visitor.rs @@ -557,11 +557,11 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { let access = access_from!(self.save_ctxt, item, hir_id); for variant in &enum_definition.variants { - let name = variant.node.ident.name.to_string(); + let name = variant.ident.name.to_string(); let qualname = format!("{}::{}", enum_data.qualname, name); - let name_span = variant.node.ident.span; + let name_span = variant.ident.span; - match variant.node.data { + match variant.data { ast::VariantData::Struct(ref fields, ..) => { let fields_str = fields .iter() @@ -574,7 +574,7 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { let value = format!("{}::{} {{ {} }}", enum_data.name, name, fields_str); if !self.span.filter_generated(name_span) { let span = self.span_from_span(name_span); - let id = id_from_node_id(variant.node.id, &self.save_ctxt); + let id = id_from_node_id(variant.id, &self.save_ctxt); let parent = Some(id_from_node_id(item.id, &self.save_ctxt)); self.dumper.dump_def( @@ -589,10 +589,10 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { parent, children: vec![], decl_id: None, - docs: self.save_ctxt.docs_for_attrs(&variant.node.attrs), + docs: self.save_ctxt.docs_for_attrs(&variant.attrs), sig: sig::variant_signature(variant, &self.save_ctxt), attributes: lower_attributes( - variant.node.attrs.clone(), + variant.attrs.clone(), &self.save_ctxt, ), }, @@ -612,7 +612,7 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { } if !self.span.filter_generated(name_span) { let span = self.span_from_span(name_span); - let id = id_from_node_id(variant.node.id, &self.save_ctxt); + let id = id_from_node_id(variant.id, &self.save_ctxt); let parent = Some(id_from_node_id(item.id, &self.save_ctxt)); self.dumper.dump_def( @@ -627,10 +627,10 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { parent, children: vec![], decl_id: None, - docs: self.save_ctxt.docs_for_attrs(&variant.node.attrs), + docs: self.save_ctxt.docs_for_attrs(&variant.attrs), sig: sig::variant_signature(variant, &self.save_ctxt), attributes: lower_attributes( - variant.node.attrs.clone(), + variant.attrs.clone(), &self.save_ctxt, ), }, @@ -640,8 +640,8 @@ impl<'l, 'tcx> DumpVisitor<'l, 'tcx> { } - for field in variant.node.data.fields() { - self.process_struct_field_def(field, variant.node.id); + for field in variant.data.fields() { + self.process_struct_field_def(field, variant.id); self.visit_ty(&field.ty); } } diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs index c699a8834e0e0..9da6cd800570e 100644 --- a/src/librustc_save_analysis/lib.rs +++ b/src/librustc_save_analysis/lib.rs @@ -277,7 +277,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> { filter!(self.span_utils, item.ident.span); let variants_str = def.variants .iter() - .map(|v| v.node.ident.to_string()) + .map(|v| v.ident.to_string()) .collect::>() .join(", "); let value = format!("{}::{{{}}}", name, variants_str); @@ -291,7 +291,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> { parent: None, children: def.variants .iter() - .map(|v| id_from_node_id(v.node.id, self)) + .map(|v| id_from_node_id(v.id, self)) .collect(), decl_id: None, docs: self.docs_for_attrs(&item.attrs), diff --git a/src/librustc_save_analysis/sig.rs b/src/librustc_save_analysis/sig.rs index c212cda2d6679..b34506a4f1d37 100644 --- a/src/librustc_save_analysis/sig.rs +++ b/src/librustc_save_analysis/sig.rs @@ -65,7 +65,7 @@ pub fn variant_signature(variant: &ast::Variant, scx: &SaveContext<'_, '_>) -> O if !scx.config.signatures { return None; } - variant.node.make(0, None, scx).ok() + variant.make(0, None, scx).ok() } pub fn method_signature( @@ -699,7 +699,7 @@ impl Sig for ast::StructField { } -impl Sig for ast::Variant_ { +impl Sig for ast::Variant { fn make(&self, offset: usize, parent_id: Option, scx: &SaveContext<'_, '_>) -> Result { let mut text = self.ident.to_string(); match self.data { diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 4fb28db6e94fa..9559f47383a59 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1968,19 +1968,19 @@ pub fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, vs: &'tcx [hir::Variant], i } for v in vs { - if let Some(ref e) = v.node.disr_expr { + if let Some(ref e) = v.disr_expr { tcx.typeck_tables_of(tcx.hir().local_def_id(e.hir_id)); } } if tcx.adt_def(def_id).repr.int.is_none() && tcx.features().arbitrary_enum_discriminant { let is_unit = - |var: &hir::Variant| match var.node.data { + |var: &hir::Variant| match var.data { hir::VariantData::Unit(..) => true, _ => false }; - let has_disr = |var: &hir::Variant| var.node.disr_expr.is_some(); + let has_disr = |var: &hir::Variant| var.disr_expr.is_some(); let has_non_units = vs.iter().any(|var| !is_unit(var)); let disr_units = vs.iter().any(|var| is_unit(&var) && has_disr(&var)); let disr_non_unit = vs.iter().any(|var| !is_unit(&var) && has_disr(&var)); @@ -1999,11 +1999,11 @@ pub fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, vs: &'tcx [hir::Variant], i let variant_did = def.variants[VariantIdx::new(i)].def_id; let variant_i_hir_id = tcx.hir().as_local_hir_id(variant_did).unwrap(); let variant_i = tcx.hir().expect_variant(variant_i_hir_id); - let i_span = match variant_i.node.disr_expr { + let i_span = match variant_i.disr_expr { Some(ref expr) => tcx.hir().span(expr.hir_id), None => tcx.hir().span(variant_i_hir_id) }; - let span = match v.node.disr_expr { + let span = match v.disr_expr { Some(ref expr) => tcx.hir().span(expr.hir_id), None => v.span }; diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index c1d8fde3be15b..b2c0e34d6fad7 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -1119,7 +1119,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { fn enum_variants(&self, enum_def: &hir::EnumDef) -> Vec> { enum_def.variants.iter() - .map(|variant| self.non_enum_variant(&variant.node.data)) + .map(|variant| self.non_enum_variant(&variant.data)) .collect() } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 0f0568907c646..5ff64224c5aa0 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -35,7 +35,6 @@ use rustc_target::spec::abi; use syntax::ast; use syntax::ast::{Ident, MetaItemKind}; use syntax::attr::{InlineAttr, OptimizeAttr, list_contains_name, mark_used}; -use syntax::source_map::Spanned; use syntax::feature_gate; use syntax::symbol::{InternedString, kw, Symbol, sym}; use syntax_pos::{Span, DUMMY_SP}; @@ -520,7 +519,11 @@ fn convert_variant_ctor(tcx: TyCtxt<'_>, ctor_id: hir::HirId) { tcx.predicates_of(def_id); } -fn convert_enum_variant_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, variants: &[hir::Variant]) { +fn convert_enum_variant_types<'tcx>( + tcx: TyCtxt<'tcx>, + def_id: DefId, + variants: &[hir::Variant] +) { let def = tcx.adt_def(def_id); let repr_type = def.repr.discr_type(); let initial = repr_type.initial_discriminant(tcx); @@ -530,7 +533,7 @@ fn convert_enum_variant_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, variants: for variant in variants { let wrapped_discr = prev_discr.map_or(initial, |d| d.wrap_incr(tcx)); prev_discr = Some( - if let Some(ref e) = variant.node.disr_expr { + if let Some(ref e) = variant.disr_expr { let expr_did = tcx.hir().local_def_id(e.hir_id); def.eval_explicit_discr(tcx, expr_did) } else if let Some(discr) = repr_type.disr_incr(tcx, prev_discr) { @@ -546,14 +549,14 @@ fn convert_enum_variant_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, variants: format!("overflowed on value after {}", prev_discr.unwrap()), ).note(&format!( "explicitly set `{} = {}` if that is desired outcome", - variant.node.ident, wrapped_discr + variant.ident, wrapped_discr )) .emit(); None }.unwrap_or(wrapped_discr), ); - for f in variant.node.data.fields() { + for f in variant.data.fields() { let def_id = tcx.hir().local_def_id(f.hir_id); tcx.generics_of(def_id); tcx.type_of(def_id); @@ -562,7 +565,7 @@ fn convert_enum_variant_types<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, variants: // Convert the ctor, if any. This also registers the variant as // an item. - if let Some(ctor_hir_id) = variant.node.data.ctor_hir_id() { + if let Some(ctor_hir_id) = variant.data.ctor_hir_id() { convert_variant_ctor(tcx, ctor_hir_id); } } @@ -641,11 +644,11 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::AdtDef { let variants = def.variants .iter() .map(|v| { - let variant_did = Some(tcx.hir().local_def_id(v.node.id)); - let ctor_did = v.node.data.ctor_hir_id() + let variant_did = Some(tcx.hir().local_def_id(v.id)); + let ctor_did = v.data.ctor_hir_id() .map(|hir_id| tcx.hir().local_def_id(hir_id)); - let discr = if let Some(ref e) = v.node.disr_expr { + let discr = if let Some(ref e) = v.disr_expr { distance_from_explicit = 0; ty::VariantDiscr::Explicit(tcx.hir().local_def_id(e.hir_id)) } else { @@ -653,8 +656,8 @@ fn adt_def(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::AdtDef { }; distance_from_explicit += 1; - convert_variant(tcx, variant_did, ctor_did, v.node.ident, discr, - &v.node.data, AdtKind::Enum, def_id) + convert_variant(tcx, variant_did, ctor_did, v.ident, discr, + &v.data, AdtKind::Enum, def_id) }) .collect(); @@ -1314,10 +1317,9 @@ pub fn checked_type_of(tcx: TyCtxt<'_>, def_id: DefId, fail: bool) -> Option tcx.mk_foreign(def_id), }, - Node::Ctor(&ref def) | Node::Variant(&Spanned { - node: hir::VariantKind { data: ref def, .. }, - .. - }) => match *def { + Node::Ctor(&ref def) | Node::Variant( + hir::Variant { data: ref def, .. } + ) => match *def { VariantData::Unit(..) | VariantData::Struct(..) => { tcx.type_of(tcx.hir().get_parent_did(hir_id)) } @@ -1363,12 +1365,8 @@ pub fn checked_type_of(tcx: TyCtxt<'_>, def_id: DefId, fail: bool) -> Option { @@ -1809,10 +1807,9 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> { compute_sig_of_foreign_fn_decl(tcx, def_id, fn_decl, abi) } - Ctor(data) | Variant(Spanned { - node: hir::VariantKind { data, .. }, - .. - }) if data.ctor_hir_id().is_some() => { + Ctor(data) | Variant( + hir::Variant { data, .. } + ) if data.ctor_hir_id().is_some() => { let ty = tcx.type_of(tcx.hir().get_parent_did(hir_id)); let inputs = data.fields() .iter() diff --git a/src/librustc_typeck/variance/constraints.rs b/src/librustc_typeck/variance/constraints.rs index b75a0912657fa..7ed9d6606f646 100644 --- a/src/librustc_typeck/variance/constraints.rs +++ b/src/librustc_typeck/variance/constraints.rs @@ -82,8 +82,8 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for ConstraintContext<'a, 'tcx> { self.visit_node_helper(item.hir_id); for variant in &enum_def.variants { - if let hir::VariantData::Tuple(..) = variant.node.data { - self.visit_node_helper(variant.node.data.ctor_hir_id().unwrap()); + if let hir::VariantData::Tuple(..) = variant.data { + self.visit_node_helper(variant.data.ctor_hir_id().unwrap()); } } } diff --git a/src/librustc_typeck/variance/terms.rs b/src/librustc_typeck/variance/terms.rs index 7af7c79bb3c0d..e10837e52ad04 100644 --- a/src/librustc_typeck/variance/terms.rs +++ b/src/librustc_typeck/variance/terms.rs @@ -145,8 +145,8 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for TermsContext<'a, 'tcx> { self.add_inferreds_for_item(item.hir_id); for variant in &enum_def.variants { - if let hir::VariantData::Tuple(..) = variant.node.data { - self.add_inferreds_for_item(variant.node.data.ctor_hir_id().unwrap()); + if let hir::VariantData::Tuple(..) = variant.data { + self.add_inferreds_for_item(variant.data.ctor_hir_id().unwrap()); } } } diff --git a/src/librustdoc/test.rs b/src/librustdoc/test.rs index 462e21b8f6b55..83a8d3fc10999 100644 --- a/src/librustdoc/test.rs +++ b/src/librustdoc/test.rs @@ -951,7 +951,7 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirCollector<'a, 'hir> { v: &'hir hir::Variant, g: &'hir hir::Generics, item_id: hir::HirId) { - self.visit_testable(v.node.ident.to_string(), &v.node.attrs, |this| { + self.visit_testable(v.ident.to_string(), &v.attrs, |this| { intravisit::walk_variant(this, v, g, item_id); }); } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 35b6d9972da06..903ed3aae1470 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -130,10 +130,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { Enum { name, variants: def.variants.iter().map(|v| Variant { - name: v.node.ident.name, - id: v.node.id, - attrs: &v.node.attrs, - def: &v.node.data, + name: v.ident.name, + id: v.id, + attrs: &v.attrs, + def: &v.data, whence: v.span, }).collect(), vis: &it.vis, diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 052eb55b40811..a65f0b2ff51c8 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -2038,7 +2038,7 @@ pub struct EnumDef { } #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] -pub struct Variant_ { +pub struct Variant { /// Name of the variant. pub ident: Ident, /// Attributes of the variant. @@ -2049,10 +2049,10 @@ pub struct Variant_ { pub data: VariantData, /// Explicit discriminant, e.g., `Foo = 1`. pub disr_expr: Option, + /// Span + pub span: Span, } -pub type Variant = Spanned; - /// Part of `use` item to the right of its prefix. #[derive(Clone, RustcEncodable, RustcDecodable, Debug)] pub enum UseTreeKind { diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index a9d3227b3a8f4..70b1d3fc73b9a 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -712,7 +712,7 @@ macro_rules! derive_has_attrs { derive_has_attrs! { Item, Expr, Local, ast::ForeignItem, ast::StructField, ast::ImplItem, ast::TraitItem, ast::Arm, - ast::Field, ast::FieldPat, ast::Variant_, ast::Arg + ast::Field, ast::FieldPat, ast::Variant, ast::Arg } pub fn inject(mut krate: ast::Crate, parse_sess: &ParseSess, attrs: &[String]) -> ast::Crate { diff --git a/src/libsyntax/config.rs b/src/libsyntax/config.rs index 1ab367f73c1b3..7eeea4e7bdfe1 100644 --- a/src/libsyntax/config.rs +++ b/src/libsyntax/config.rs @@ -260,7 +260,7 @@ impl<'a> StripUnconfigured<'a> { ast::ItemKind::Enum(ast::EnumDef { variants }, _generics) => { variants.flat_map_in_place(|variant| self.configure(variant)); for variant in variants { - self.configure_variant_data(&mut variant.node.data); + self.configure_variant_data(&mut variant.data); } } _ => {} diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index 22962499a2b75..3dfcd82570491 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -781,14 +781,14 @@ impl<'a> ExtCtxt<'a> { ast::VariantData::Tuple(fields, ast::DUMMY_NODE_ID) }; - respan(span, - ast::Variant_ { - ident, - id: ast::DUMMY_NODE_ID, - attrs: Vec::new(), - data: vdata, - disr_expr: None, - }) + ast::Variant { + attrs: Vec::new(), + data: vdata, + disr_expr: None, + id: ast::DUMMY_NODE_ID, + ident, + span, + } } pub fn item_enum_poly(&self, span: Span, name: Ident, diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 08a113b53d032..8a56ae13b6f6e 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -1956,7 +1956,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { ast::ItemKind::Enum(ast::EnumDef{ref variants, ..}, ..) => { for variant in variants { - match (&variant.node.data, &variant.node.disr_expr) { + match (&variant.data, &variant.disr_expr) { (ast::VariantData::Unit(..), _) => {}, (_, Some(disr_expr)) => gate_feature_post!( diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs index be04c6a76b06d..36327fe10dc5d 100644 --- a/src/libsyntax/mut_visit.rs +++ b/src/libsyntax/mut_visit.rs @@ -455,7 +455,7 @@ pub fn noop_visit_foreign_mod(foreign_mod: &mut ForeignMod, vis: } pub fn noop_visit_variant(variant: &mut Variant, vis: &mut T) { - let Spanned { node: Variant_ { ident, attrs, id, data, disr_expr }, span } = variant; + let Variant { ident, attrs, id, data, disr_expr, span } = variant; vis.visit_ident(ident); visit_attrs(attrs, vis); vis.visit_id(id); diff --git a/src/libsyntax/parse/diagnostics.rs b/src/libsyntax/parse/diagnostics.rs index 730efb5ef013c..1fbf28fb83016 100644 --- a/src/libsyntax/parse/diagnostics.rs +++ b/src/libsyntax/parse/diagnostics.rs @@ -8,7 +8,6 @@ use crate::parse::parser::{BlockMode, PathStyle, SemiColonMode, TokenType, Token use crate::parse::token::{self, TokenKind}; use crate::print::pprust; use crate::ptr::P; -use crate::source_map::Spanned; use crate::symbol::{kw, sym}; use crate::ThinVec; use crate::util::parser::AssocOp; @@ -592,18 +591,18 @@ impl<'a> Parser<'a> { crate fn maybe_report_invalid_custom_discriminants( sess: &ParseSess, - variants: &[Spanned], + variants: &[ast::Variant], ) { - let has_fields = variants.iter().any(|variant| match variant.node.data { + let has_fields = variants.iter().any(|variant| match variant.data { VariantData::Tuple(..) | VariantData::Struct(..) => true, VariantData::Unit(..) => false, }); - let discriminant_spans = variants.iter().filter(|variant| match variant.node.data { + let discriminant_spans = variants.iter().filter(|variant| match variant.data { VariantData::Tuple(..) | VariantData::Struct(..) => false, VariantData::Unit(..) => true, }) - .filter_map(|variant| variant.node.disr_expr.as_ref().map(|c| c.value.span)) + .filter_map(|variant| variant.disr_expr.as_ref().map(|c| c.value.span)) .collect::>(); if !discriminant_spans.is_empty() && has_fields { @@ -618,7 +617,7 @@ impl<'a> Parser<'a> { err.span_label(sp, "disallowed custom discriminant"); } for variant in variants.iter() { - match &variant.node.data { + match &variant.data { VariantData::Struct(..) => { err.span_label( variant.span, diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs index e85ef9cc97419..60873ecb13468 100644 --- a/src/libsyntax/parse/parser/item.rs +++ b/src/libsyntax/parse/parser/item.rs @@ -1564,14 +1564,15 @@ impl<'a> Parser<'a> { None }; - let vr = ast::Variant_ { + let vr = ast::Variant { ident, id: ast::DUMMY_NODE_ID, attrs: variant_attrs, data: struct_def, disr_expr, + span: vlo.to(self.prev_span), }; - variants.push(respan(vlo.to(self.prev_span), vr)); + variants.push(vr); if !self.eat(&token::Comma) { if self.token.is_ident() && !self.token.is_reserved_ident() { diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index bda761244d5ca..8b97ec3da0bc9 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -1402,7 +1402,7 @@ impl<'a> State<'a> { for v in variants { self.space_if_not_bol(); self.maybe_print_comment(v.span.lo()); - self.print_outer_attributes(&v.node.attrs); + self.print_outer_attributes(&v.attrs); self.ibox(INDENT_UNIT); self.print_variant(v); self.s.word(","); @@ -1492,8 +1492,8 @@ impl<'a> State<'a> { crate fn print_variant(&mut self, v: &ast::Variant) { self.head(""); let generics = ast::Generics::default(); - self.print_struct(&v.node.data, &generics, v.node.ident, v.span, false); - match v.node.disr_expr { + self.print_struct(&v.data, &generics, v.ident, v.span, false); + match v.disr_expr { Some(ref d) => { self.s.space(); self.word_space("="); diff --git a/src/libsyntax/print/pprust/tests.rs b/src/libsyntax/print/pprust/tests.rs index 082a430e0ede1..25214673e6935 100644 --- a/src/libsyntax/print/pprust/tests.rs +++ b/src/libsyntax/print/pprust/tests.rs @@ -54,14 +54,15 @@ fn test_variant_to_string() { with_default_globals(|| { let ident = ast::Ident::from_str("principal_skinner"); - let var = source_map::respan(syntax_pos::DUMMY_SP, ast::Variant_ { + let var = ast::Variant { ident, attrs: Vec::new(), id: ast::DUMMY_NODE_ID, // making this up as I go.... ? data: ast::VariantData::Unit(ast::DUMMY_NODE_ID), disr_expr: None, - }); + span: syntax_pos::DUMMY_SP, + }; let varstr = variant_to_string(&var); assert_eq!(varstr, "principal_skinner"); diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs index 596c5b46b986f..41b8ef16665a4 100644 --- a/src/libsyntax/visit.rs +++ b/src/libsyntax/visit.rs @@ -311,11 +311,11 @@ pub fn walk_variant<'a, V>(visitor: &mut V, item_id: NodeId) where V: Visitor<'a>, { - visitor.visit_ident(variant.node.ident); - visitor.visit_variant_data(&variant.node.data, variant.node.ident, + visitor.visit_ident(variant.ident); + visitor.visit_variant_data(&variant.data, variant.ident, generics, item_id, variant.span); - walk_list!(visitor, visit_anon_const, &variant.node.disr_expr); - walk_list!(visitor, visit_attribute, &variant.node.attrs); + walk_list!(visitor, visit_anon_const, &variant.disr_expr); + walk_list!(visitor, visit_attribute, &variant.attrs); } pub fn walk_ty<'a, V: Visitor<'a>>(visitor: &mut V, typ: &'a Ty) { diff --git a/src/libsyntax_ext/deriving/clone.rs b/src/libsyntax_ext/deriving/clone.rs index 3b1edf90d6bf2..d80da566185cc 100644 --- a/src/libsyntax_ext/deriving/clone.rs +++ b/src/libsyntax_ext/deriving/clone.rs @@ -138,7 +138,7 @@ fn cs_clone_shallow(name: &str, } StaticEnum(enum_def, ..) => { for variant in &enum_def.variants { - process_variant(cx, &mut stmts, &variant.node.data); + process_variant(cx, &mut stmts, &variant.data); } } _ => cx.span_bug(trait_span, &format!("unexpected substructure in \ @@ -170,9 +170,9 @@ fn cs_clone(name: &str, vdata = vdata_; } EnumMatching(.., variant, ref af) => { - ctor_path = cx.path(trait_span, vec![substr.type_ident, variant.node.ident]); + ctor_path = cx.path(trait_span, vec![substr.type_ident, variant.ident]); all_fields = af; - vdata = &variant.node.data; + vdata = &variant.data; } EnumNonMatchingCollapsed(..) => { cx.span_bug(trait_span, diff --git a/src/libsyntax_ext/deriving/cmp/eq.rs b/src/libsyntax_ext/deriving/cmp/eq.rs index 1909729f4a941..5d7c4a84389c6 100644 --- a/src/libsyntax_ext/deriving/cmp/eq.rs +++ b/src/libsyntax_ext/deriving/cmp/eq.rs @@ -75,7 +75,7 @@ fn cs_total_eq_assert(cx: &mut ExtCtxt<'_>, } StaticEnum(enum_def, ..) => { for variant in &enum_def.variants { - process_variant(cx, &mut stmts, &variant.node.data); + process_variant(cx, &mut stmts, &variant.data); } } _ => cx.span_bug(trait_span, "unexpected substructure in `derive(Eq)`") diff --git a/src/libsyntax_ext/deriving/debug.rs b/src/libsyntax_ext/deriving/debug.rs index 0f709630bf41e..15e93f2843a12 100644 --- a/src/libsyntax_ext/deriving/debug.rs +++ b/src/libsyntax_ext/deriving/debug.rs @@ -53,7 +53,7 @@ fn show_substructure(cx: &mut ExtCtxt<'_>, span: Span, substr: &Substructure<'_> // based on the "shape". let (ident, vdata, fields) = match substr.fields { Struct(vdata, fields) => (substr.type_ident, *vdata, fields), - EnumMatching(_, _, v, fields) => (v.node.ident, &v.node.data, fields), + EnumMatching(_, _, v, fields) => (v.ident, &v.data, fields), EnumNonMatchingCollapsed(..) | StaticStruct(..) | StaticEnum(..) => cx.span_bug(span, "nonsensical .fields in `#[derive(Debug)]`"), diff --git a/src/libsyntax_ext/deriving/encodable.rs b/src/libsyntax_ext/deriving/encodable.rs index 6d0d3b96a56d6..52e74a7c57e8c 100644 --- a/src/libsyntax_ext/deriving/encodable.rs +++ b/src/libsyntax_ext/deriving/encodable.rs @@ -238,7 +238,7 @@ fn encodable_substructure(cx: &mut ExtCtxt<'_>, } let blk = cx.lambda_stmts_1(trait_span, stmts, blkarg); - let name = cx.expr_str(trait_span, variant.node.ident.name); + let name = cx.expr_str(trait_span, variant.ident.name); let call = cx.expr_method_call(trait_span, blkencoder, cx.ident_of("emit_enum_variant"), diff --git a/src/libsyntax_ext/deriving/generic/mod.rs b/src/libsyntax_ext/deriving/generic/mod.rs index 7e6d9126c8740..91e73810d66e2 100644 --- a/src/libsyntax_ext/deriving/generic/mod.rs +++ b/src/libsyntax_ext/deriving/generic/mod.rs @@ -758,7 +758,7 @@ impl<'a> TraitDef<'a> { let mut field_tys = Vec::new(); for variant in &enum_def.variants { - field_tys.extend(variant.node + field_tys.extend(variant .data .fields() .iter() @@ -1220,7 +1220,7 @@ impl<'a> MethodDef<'a> { let catch_all_substructure = EnumNonMatchingCollapsed(self_arg_idents, &variants[..], &vi_idents[..]); - let first_fieldless = variants.iter().find(|v| v.node.data.fields().is_empty()); + let first_fieldless = variants.iter().find(|v| v.data.fields().is_empty()); // These arms are of the form: // (Variant1, Variant1, ...) => Body1 @@ -1229,7 +1229,7 @@ impl<'a> MethodDef<'a> { // where each tuple has length = self_args.len() let mut match_arms: Vec = variants.iter() .enumerate() - .filter(|&(_, v)| !(self.unify_fieldless_variants && v.node.data.fields().is_empty())) + .filter(|&(_, v)| !(self.unify_fieldless_variants && v.data.fields().is_empty())) .map(|(index, variant)| { let mk_self_pat = |cx: &mut ExtCtxt<'_>, self_arg_name: &str| { let (p, idents) = trait_.create_enum_variant_pattern(cx, @@ -1513,8 +1513,8 @@ impl<'a> MethodDef<'a> { .iter() .map(|v| { let sp = v.span.with_ctxt(trait_.span.ctxt()); - let summary = trait_.summarise_struct(cx, &v.node.data); - (v.node.ident, sp, summary) + let summary = trait_.summarise_struct(cx, &v.data); + (v.ident, sp, summary) }) .collect(); self.call_substructure_method(cx, @@ -1643,9 +1643,9 @@ impl<'a> TraitDef<'a> { mutbl: ast::Mutability) -> (P, Vec<(Span, Option, P, &'a [ast::Attribute])>) { let sp = variant.span.with_ctxt(self.span.ctxt()); - let variant_path = cx.path(sp, vec![enum_ident, variant.node.ident]); + let variant_path = cx.path(sp, vec![enum_ident, variant.ident]); let use_temporaries = false; // enums can't be repr(packed) - self.create_struct_pattern(cx, variant_path, &variant.node.data, prefix, mutbl, + self.create_struct_pattern(cx, variant_path, &variant.data, prefix, mutbl, use_temporaries) } } @@ -1776,7 +1776,7 @@ pub fn is_type_without_fields(item: &Annotatable) -> bool { if let Annotatable::Item(ref item) = *item { match item.node { ast::ItemKind::Enum(ref enum_def, _) => { - enum_def.variants.iter().all(|v| v.node.data.fields().is_empty()) + enum_def.variants.iter().all(|v| v.data.fields().is_empty()) } ast::ItemKind::Struct(ref variant_data, _) => variant_data.fields().is_empty(), _ => false, From 24693d70d650b717770deb2331a50b56d1469158 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 14 Aug 2019 20:07:37 +0200 Subject: [PATCH 36/37] Adjust tracking issues for `MaybeUninit` gates --- src/libcore/mem/maybe_uninit.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libcore/mem/maybe_uninit.rs b/src/libcore/mem/maybe_uninit.rs index 1bbea02e0c7c9..c48fcfe99d290 100644 --- a/src/libcore/mem/maybe_uninit.rs +++ b/src/libcore/mem/maybe_uninit.rs @@ -312,7 +312,7 @@ impl MaybeUninit { /// without dropping it, so be careful not to use this twice unless you want to /// skip running the destructor. For your convenience, this also returns a mutable /// reference to the (now safely initialized) contents of `self`. - #[unstable(feature = "maybe_uninit_extra", issue = "53491")] + #[unstable(feature = "maybe_uninit_extra", issue = "63567")] #[inline(always)] pub fn write(&mut self, val: T) -> &mut T { unsafe { @@ -502,7 +502,7 @@ impl MaybeUninit { /// // We now created two copies of the same vector, leading to a double-free when /// // they both get dropped! /// ``` - #[unstable(feature = "maybe_uninit_extra", issue = "53491")] + #[unstable(feature = "maybe_uninit_extra", issue = "63567")] #[inline(always)] pub unsafe fn read(&self) -> T { intrinsics::panic_if_uninhabited::(); @@ -516,7 +516,7 @@ impl MaybeUninit { /// It is up to the caller to guarantee that the `MaybeUninit` really is in an initialized /// state. Calling this when the content is not yet fully initialized causes undefined /// behavior. - #[unstable(feature = "maybe_uninit_ref", issue = "53491")] + #[unstable(feature = "maybe_uninit_ref", issue = "63568")] #[inline(always)] pub unsafe fn get_ref(&self) -> &T { &*self.value @@ -532,21 +532,21 @@ impl MaybeUninit { // FIXME(#53491): We currently rely on the above being incorrect, i.e., we have references // to uninitialized data (e.g., in `libcore/fmt/float.rs`). We should make // a final decision about the rules before stabilization. - #[unstable(feature = "maybe_uninit_ref", issue = "53491")] + #[unstable(feature = "maybe_uninit_ref", issue = "63568")] #[inline(always)] pub unsafe fn get_mut(&mut self) -> &mut T { &mut *self.value } /// Gets a pointer to the first element of the array. - #[unstable(feature = "maybe_uninit_slice", issue = "53491")] + #[unstable(feature = "maybe_uninit_slice", issue = "63569")] #[inline(always)] pub fn first_ptr(this: &[MaybeUninit]) -> *const T { this as *const [MaybeUninit] as *const T } /// Gets a mutable pointer to the first element of the array. - #[unstable(feature = "maybe_uninit_slice", issue = "53491")] + #[unstable(feature = "maybe_uninit_slice", issue = "63569")] #[inline(always)] pub fn first_ptr_mut(this: &mut [MaybeUninit]) -> *mut T { this as *mut [MaybeUninit] as *mut T From 6c3a98e029048ff316d6c8ae2f69b3db08cae90e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 14 Aug 2019 12:14:25 -0700 Subject: [PATCH 37/37] review comments --- .../infer/error_reporting/need_type_info.rs | 137 ++++++++++-------- 1 file changed, 73 insertions(+), 64 deletions(-) diff --git a/src/librustc/infer/error_reporting/need_type_info.rs b/src/librustc/infer/error_reporting/need_type_info.rs index 16fed3d42d679..3267505708b81 100644 --- a/src/librustc/infer/error_reporting/need_type_info.rs +++ b/src/librustc/infer/error_reporting/need_type_info.rs @@ -24,8 +24,8 @@ impl<'a, 'tcx> FindLocalByTypeVisitor<'a, 'tcx> { infcx: &'a InferCtxt<'a, 'tcx>, target_ty: Ty<'tcx>, hir_map: &'a hir::map::Map<'tcx>, - ) -> FindLocalByTypeVisitor<'a, 'tcx> { - FindLocalByTypeVisitor { + ) -> Self { + Self { infcx, target_ty, hir_map, @@ -101,6 +101,50 @@ impl<'a, 'tcx> Visitor<'tcx> for FindLocalByTypeVisitor<'a, 'tcx> { } } +/// Suggest giving an appropriate return type to a closure expression. +fn closure_return_type_suggestion( + span: Span, + err: &mut DiagnosticBuilder<'_>, + output: &FunctionRetTy, + body: &Body, + name: &str, + ret: &str, +) { + let (arrow, post) = match output { + FunctionRetTy::DefaultReturn(_) => ("-> ", " "), + _ => ("", ""), + }; + let suggestion = match body.value.node { + ExprKind::Block(..) => { + vec![(output.span(), format!("{}{}{}", arrow, ret, post))] + } + _ => { + vec![ + (output.span(), format!("{}{}{}{{ ", arrow, ret, post)), + (body.value.span.shrink_to_hi(), " }".to_string()), + ] + } + }; + err.multipart_suggestion( + "give this closure an explicit return type without `_` placeholders", + suggestion, + Applicability::HasPlaceholders, + ); + err.span_label(span, InferCtxt::missing_type_msg(&name)); +} + +/// Given a closure signature, return a `String` containing a list of all its argument types. +fn closure_args(fn_sig: &ty::PolyFnSig<'_>) -> String { + fn_sig.inputs() + .skip_binder() + .iter() + .next() + .map(|args| args.tuple_fields() + .map(|arg| arg.to_string()) + .collect::>().join(", ")) + .unwrap_or_default() +} + impl<'a, 'tcx> InferCtxt<'a, 'tcx> { pub fn extract_type_name( &self, @@ -160,24 +204,20 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { span }; + let is_named_and_not_impl_trait = |ty: Ty<'_>| { + &ty.to_string() != "_" && + // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 + (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) + }; + let ty_msg = match local_visitor.found_ty { Some(ty::TyS { sty: ty::Closure(def_id, substs), .. }) => { let fn_sig = substs.closure_sig(*def_id, self.tcx); - let args = fn_sig.inputs() - .skip_binder() - .iter() - .next() - .map(|args| args.tuple_fields() - .map(|arg| arg.to_string()) - .collect::>().join(", ")) - .unwrap_or_default(); + let args = closure_args(&fn_sig); let ret = fn_sig.output().skip_binder().to_string(); format!(" for the closure `fn({}) -> {}`", args, ret) } - Some(ty) if &ty.to_string() != "_" && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 - (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) => - { + Some(ty) if is_named_and_not_impl_trait(ty) => { let ty = ty_to_string(ty); format!(" for `{}`", ty) } @@ -211,59 +251,33 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let ret = fn_sig.output().skip_binder().to_string(); if let Some(ExprKind::Closure(_, decl, body_id, ..)) = local_visitor.found_closure { - let (arrow, post) = match decl.output { - FunctionRetTy::DefaultReturn(_) => ("-> ", " "), - _ => ("", ""), - }; if let Some(body) = self.tcx.hir().krate().bodies.get(body_id) { - let suggestion = match body.value.node { - ExprKind::Block(..) => { - vec![(decl.output.span(), format!("{}{}{}", arrow, ret, post))] - } - _ => { - vec![ - (decl.output.span(), format!("{}{}{}{{ ", arrow, ret, post)), - (body.value.span.shrink_to_hi(), " }".to_string()), - ] - } - }; - err.multipart_suggestion( - "give this closure an explicit return type without `_` placeholders", - suggestion, - Applicability::HasPlaceholders, + closure_return_type_suggestion( + span, + &mut err, + &decl.output, + &body, + &name, + &ret, ); - err.span_label(span, InferCtxt::missing_type_msg(&name)); + // We don't want to give the other suggestions when the problem is the + // closure return type. return err; } } // This shouldn't be reachable, but just in case we leave a reasonable fallback. - let args = fn_sig.inputs() - .skip_binder() - .iter() - .next() - .map(|args| args.tuple_fields() - .map(|arg| arg.to_string()) - .collect::>().join(", ")) - .unwrap_or_default(); + let args = closure_args(&fn_sig); // This suggestion is incomplete, as the user will get further type inference // errors due to the `_` placeholders and the introduction of `Box`, but it does // nudge them in the right direction. format!("a boxed closure type like `Box {}>`", args, ret) } - Some(ty) if &ty.to_string() != "_" && - name == "_" && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 - (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) => - { + Some(ty) if is_named_and_not_impl_trait(ty) && name == "_" => { let ty = ty_to_string(ty); format!("the explicit type `{}`, with the type parameters specified", ty) } - Some(ty) if &ty.to_string() != "_" && - ty.to_string() != name && - // FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527 - (!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) => - { + Some(ty) if is_named_and_not_impl_trait(ty) && ty.to_string() != name => { let ty = ty_to_string(ty); format!( "the explicit type `{}`, where the type parameter `{}` is specified", @@ -296,25 +310,20 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { format!("consider giving this closure parameter {}", suffix), ); } else if let Some(pattern) = local_visitor.found_local_pattern { - if let Some(simple_ident) = pattern.simple_ident() { + let msg = if let Some(simple_ident) = pattern.simple_ident() { match pattern.span.desugaring_kind() { None => { - err.span_label( - pattern.span, - format!("consider giving `{}` {}", simple_ident, suffix), - ); + format!("consider giving `{}` {}", simple_ident, suffix) } Some(DesugaringKind::ForLoop) => { - err.span_label( - pattern.span, - "the element type for this iterator is not specified".to_string(), - ); + "the element type for this iterator is not specified".to_string() } - _ => {} + _ => format!("this needs {}", suffix), } } else { - err.span_label(pattern.span, format!("consider giving this pattern {}", suffix)); - } + format!("consider giving this pattern {}", suffix) + }; + err.span_label(pattern.span, msg); } if !err.span.span_labels().iter().any(|span_label| { span_label.label.is_some() && span_label.span == span