From 7e94641ee956989d57492975a1214fc79dcd1fce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Sun, 7 Feb 2021 23:13:36 +0300 Subject: [PATCH 01/22] Fix SourceMap::start_point `start_point` needs to return the *first* character's span, but it would previously call `find_width_of_character_at_span` which returns the span of the *last* character. The implementation is now fixed. Other changes: - Docs for start_point, end_point, find_width_of_character_at_span updated - Minor simplification in find_width_of_character_at_span code Fixes #81800 --- compiler/rustc_span/src/source_map.rs | 42 +++++++++++++++++++-------- src/test/ui/span/issue-81800.rs | 2 ++ src/test/ui/span/issue-81800.stderr | 19 ++++++++++++ 3 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/span/issue-81800.rs create mode 100644 src/test/ui/span/issue-81800.stderr diff --git a/compiler/rustc_span/src/source_map.rs b/compiler/rustc_span/src/source_map.rs index 2b429372dcffb..8bcd468d41276 100644 --- a/compiler/rustc_span/src/source_map.rs +++ b/compiler/rustc_span/src/source_map.rs @@ -777,16 +777,35 @@ impl SourceMap { self.span_until_char(sp, '{') } - /// Returns a new span representing just the start point of this span. + /// Returns a new span representing just the first character of the given span. pub fn start_point(&self, sp: Span) -> Span { - let pos = sp.lo().0; - let width = self.find_width_of_character_at_span(sp, false); - let corrected_start_position = pos.checked_add(width).unwrap_or(pos); - let end_point = BytePos(cmp::max(corrected_start_position, sp.lo().0)); - sp.with_hi(end_point) + let width = { + let sp = sp.data(); + let local_begin = self.lookup_byte_offset(sp.lo); + let start_index = local_begin.pos.to_usize(); + let src = local_begin.sf.external_src.borrow(); + + let snippet = if let Some(ref src) = local_begin.sf.src { + Some(&src[start_index..]) + } else if let Some(src) = src.get_source() { + Some(&src[start_index..]) + } else { + None + }; + + match snippet { + None => 1, + Some(snippet) => match snippet.chars().next() { + None => 1, + Some(c) => c.len_utf8(), + }, + } + }; + + sp.with_hi(BytePos(sp.lo().0 + width as u32)) } - /// Returns a new span representing just the end point of this span. + /// Returns a new span representing just the last character of this span. pub fn end_point(&self, sp: Span) -> Span { let pos = sp.hi().0; @@ -815,7 +834,8 @@ impl SourceMap { Span::new(BytePos(start_of_next_point), end_of_next_point, sp.ctxt()) } - /// Finds the width of a character, either before or after the provided span. + /// Finds the width of the character, either before or after the end of provided span, + /// depending on the `forwards` parameter. fn find_width_of_character_at_span(&self, sp: Span, forwards: bool) -> u32 { let sp = sp.data(); if sp.lo == sp.hi { @@ -862,11 +882,9 @@ impl SourceMap { // We need to extend the snippet to the end of the src rather than to end_index so when // searching forwards for boundaries we've got somewhere to search. let snippet = if let Some(ref src) = local_begin.sf.src { - let len = src.len(); - &src[start_index..len] + &src[start_index..] } else if let Some(src) = src.get_source() { - let len = src.len(); - &src[start_index..len] + &src[start_index..] } else { return 1; }; diff --git a/src/test/ui/span/issue-81800.rs b/src/test/ui/span/issue-81800.rs new file mode 100644 index 0000000000000..6ac66fdcb65ad --- /dev/null +++ b/src/test/ui/span/issue-81800.rs @@ -0,0 +1,2 @@ +fn x˂- //~ ERROR: unknown start of token + //~^ ERROR: expected one of `#`, `>`, `const`, identifier, or lifetime, found `-` diff --git a/src/test/ui/span/issue-81800.stderr b/src/test/ui/span/issue-81800.stderr new file mode 100644 index 0000000000000..d37f13a6683b0 --- /dev/null +++ b/src/test/ui/span/issue-81800.stderr @@ -0,0 +1,19 @@ +error: unknown start of token: \u{2c2} + --> $DIR/issue-81800.rs:1:5 + | +LL | fn x˂- + | ^ + | +help: Unicode character '˂' (Modifier Letter Left Arrowhead) looks like '<' (Less-Than Sign), but it is not + | +LL | fn x<- + | ^ + +error: expected one of `#`, `>`, `const`, identifier, or lifetime, found `-` + --> $DIR/issue-81800.rs:1:6 + | +LL | fn x˂- + | ^ expected one of `#`, `>`, `const`, identifier, or lifetime + +error: aborting due to 2 previous errors + From 1eb79f3c375f4d7834a30b40c20a9c8ec1c31b4c Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 9 Feb 2021 20:56:45 +0000 Subject: [PATCH 02/22] Use longer lifetime in `try_report_from_nll` return type --- .../src/infer/error_reporting/nice_region_error/mod.rs | 2 +- .../error_reporting/nice_region_error/named_anon_conflict.rs | 2 +- .../error_reporting/nice_region_error/placeholder_error.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs index cc8f1816bc3f4..0599c78ebfd07 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/mod.rs @@ -43,7 +43,7 @@ impl<'cx, 'tcx> NiceRegionError<'cx, 'tcx> { self.infcx.tcx } - pub fn try_report_from_nll(&self) -> Option> { + pub fn try_report_from_nll(&self) -> Option> { // Due to the improved diagnostics returned by the MIR borrow checker, only a subset of // the nice region errors are required when running under the MIR borrow checker. self.try_report_named_anon_conflict().or_else(|| self.try_report_placeholder_conflict()) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/named_anon_conflict.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/named_anon_conflict.rs index e3c613b1d6a12..2f622231a081e 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/named_anon_conflict.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/named_anon_conflict.rs @@ -9,7 +9,7 @@ use rustc_middle::ty; impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { /// When given a `ConcreteFailure` for a function with parameters containing a named region and /// an anonymous region, emit an descriptive diagnostic error. - pub(super) fn try_report_named_anon_conflict(&self) -> Option> { + pub(super) fn try_report_named_anon_conflict(&self) -> Option> { let (span, sub, sup) = self.regions()?; debug!( diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs index e8e0326d97865..fc211a15afc05 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs @@ -16,7 +16,7 @@ use std::fmt::{self, Write}; impl NiceRegionError<'me, 'tcx> { /// When given a `ConcreteFailure` for a function with arguments containing a named region and /// an anonymous region, emit a descriptive diagnostic error. - pub(super) fn try_report_placeholder_conflict(&self) -> Option> { + pub(super) fn try_report_placeholder_conflict(&self) -> Option> { match &self.error { /////////////////////////////////////////////////////////////////////////// // NB. The ordering of cases in this match is very @@ -199,7 +199,7 @@ impl NiceRegionError<'me, 'tcx> { trait_def_id: DefId, expected_substs: SubstsRef<'tcx>, actual_substs: SubstsRef<'tcx>, - ) -> DiagnosticBuilder<'me> { + ) -> DiagnosticBuilder<'tcx> { debug!( "try_report_placeholders_trait(\ vid={:?}, \ From 638980a07f72e20fcbcdab12b74b2a1a8663741d Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 9 Feb 2021 21:03:00 +0000 Subject: [PATCH 03/22] Using tracing macros in placeholder_error.rs --- .../nice_region_error/placeholder_error.rs | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs index fc211a15afc05..8c933df8ca00d 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs @@ -190,6 +190,7 @@ impl NiceRegionError<'me, 'tcx> { // = note: Due to a where-clause on the function `all`, // = note: `T` must implement `...` for any two lifetimes `'1` and `'2`. // = note: However, the type `T` only implements `...` for some specific lifetime `'2`. + #[instrument(level = "debug", skip(self))] fn try_report_placeholders_trait( &self, vid: Option>, @@ -200,17 +201,6 @@ impl NiceRegionError<'me, 'tcx> { expected_substs: SubstsRef<'tcx>, actual_substs: SubstsRef<'tcx>, ) -> DiagnosticBuilder<'tcx> { - debug!( - "try_report_placeholders_trait(\ - vid={:?}, \ - sub_placeholder={:?}, \ - sup_placeholder={:?}, \ - trait_def_id={:?}, \ - expected_substs={:?}, \ - actual_substs={:?})", - vid, sub_placeholder, sup_placeholder, trait_def_id, expected_substs, actual_substs - ); - let span = cause.span(self.tcx()); let msg = format!( "implementation of `{}` is not general enough", @@ -285,17 +275,13 @@ impl NiceRegionError<'me, 'tcx> { let any_self_ty_has_vid = actual_self_ty_has_vid || expected_self_ty_has_vid; - debug!("try_report_placeholders_trait: actual_has_vid={:?}", actual_has_vid); - debug!("try_report_placeholders_trait: expected_has_vid={:?}", expected_has_vid); - debug!("try_report_placeholders_trait: has_sub={:?}", has_sub); - debug!("try_report_placeholders_trait: has_sup={:?}", has_sup); - debug!( - "try_report_placeholders_trait: actual_self_ty_has_vid={:?}", - actual_self_ty_has_vid - ); debug!( - "try_report_placeholders_trait: expected_self_ty_has_vid={:?}", - expected_self_ty_has_vid + ?actual_has_vid, + ?expected_has_vid, + ?has_sub, + ?has_sup, + ?actual_self_ty_has_vid, + ?expected_self_ty_has_vid, ); self.explain_actual_impl_that_was_found( From c2066cf0694c304c0fc43f17bebefb518b066619 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 9 Feb 2021 21:16:00 +0000 Subject: [PATCH 04/22] Remove unnecessary note on errors Seeing the trait definition doesn't help with implementation not general enough errors, so don't make the error message larger to show it. --- .../nice_region_error/placeholder_error.rs | 4 -- .../associated-types-eq-hr.stderr | 55 ++++--------------- .../ui/generator/auto-trait-regions.stderr | 12 ---- src/test/ui/hrtb/due-to-where-clause.stderr | 3 - .../ui/hrtb/hrtb-cache-issue-54302.stderr | 3 - src/test/ui/hrtb/hrtb-conflate-regions.stderr | 9 +-- ...b-exists-forall-trait-contravariant.stderr | 3 - .../hrtb-exists-forall-trait-invariant.stderr | 3 - src/test/ui/hrtb/hrtb-just-for-static.stderr | 18 ++---- src/test/ui/hrtb/issue-46989.stderr | 3 - src/test/ui/issues/issue-54302-cases.stderr | 36 +++--------- src/test/ui/issues/issue-54302.stderr | 3 - src/test/ui/issues/issue-55731.stderr | 9 +-- .../ui/where-clauses/where-for-self-2.stderr | 9 +-- 14 files changed, 28 insertions(+), 142 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs index 8c933df8ca00d..d99e79005fbb8 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs @@ -207,10 +207,6 @@ impl NiceRegionError<'me, 'tcx> { self.tcx().def_path_str(trait_def_id), ); let mut err = self.tcx().sess.struct_span_err(span, &msg); - err.span_label( - self.tcx().def_span(trait_def_id), - format!("trait `{}` defined here", self.tcx().def_path_str(trait_def_id)), - ); let leading_ellipsis = if let ObligationCauseCode::ItemObligation(def_id) = cause.code { err.span_label(span, "doesn't satisfy where-clause"); diff --git a/src/test/ui/associated-types/associated-types-eq-hr.stderr b/src/test/ui/associated-types/associated-types-eq-hr.stderr index 127ab8673556d..6188d9ca979cf 100644 --- a/src/test/ui/associated-types/associated-types-eq-hr.stderr +++ b/src/test/ui/associated-types/associated-types-eq-hr.stderr @@ -31,15 +31,8 @@ LL | bar::(); error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:96:5 | -LL | / pub trait TheTrait { -LL | | type A; -LL | | -LL | | fn get(&self, t: T) -> Self::A; -LL | | } - | |_- trait `TheTrait` defined here -... -LL | tuple_one::(); - | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough +LL | tuple_one::(); + | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` @@ -47,15 +40,8 @@ LL | tuple_one::(); error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:96:5 | -LL | / pub trait TheTrait { -LL | | type A; -LL | | -LL | | fn get(&self, t: T) -> Self::A; -LL | | } - | |_- trait `TheTrait` defined here -... -LL | tuple_one::(); - | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough +LL | tuple_one::(); + | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` @@ -63,15 +49,8 @@ LL | tuple_one::(); error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:102:5 | -LL | / pub trait TheTrait { -LL | | type A; -LL | | -LL | | fn get(&self, t: T) -> Self::A; -LL | | } - | |_- trait `TheTrait` defined here -... -LL | tuple_two::(); - | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough +LL | tuple_two::(); + | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` @@ -79,15 +58,8 @@ LL | tuple_two::(); error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:102:5 | -LL | / pub trait TheTrait { -LL | | type A; -LL | | -LL | | fn get(&self, t: T) -> Self::A; -LL | | } - | |_- trait `TheTrait` defined here -... -LL | tuple_two::(); - | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough +LL | tuple_two::(); + | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` @@ -95,15 +67,8 @@ LL | tuple_two::(); error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:112:5 | -LL | / pub trait TheTrait { -LL | | type A; -LL | | -LL | | fn get(&self, t: T) -> Self::A; -LL | | } - | |_- trait `TheTrait` defined here -... -LL | tuple_four::(); - | ^^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough +LL | tuple_four::(); + | ^^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` diff --git a/src/test/ui/generator/auto-trait-regions.stderr b/src/test/ui/generator/auto-trait-regions.stderr index 5ec462e10465f..5fe4193905c72 100644 --- a/src/test/ui/generator/auto-trait-regions.stderr +++ b/src/test/ui/generator/auto-trait-regions.stderr @@ -1,9 +1,6 @@ error: implementation of `Foo` is not general enough --> $DIR/auto-trait-regions.rs:31:5 | -LL | auto trait Foo {} - | ----------------- trait `Foo` defined here -... LL | assert_foo(gen); | ^^^^^^^^^^ implementation of `Foo` is not general enough | @@ -13,9 +10,6 @@ LL | assert_foo(gen); error: implementation of `Foo` is not general enough --> $DIR/auto-trait-regions.rs:31:5 | -LL | auto trait Foo {} - | ----------------- trait `Foo` defined here -... LL | assert_foo(gen); | ^^^^^^^^^^ implementation of `Foo` is not general enough | @@ -25,9 +19,6 @@ LL | assert_foo(gen); error: implementation of `Foo` is not general enough --> $DIR/auto-trait-regions.rs:50:5 | -LL | auto trait Foo {} - | ----------------- trait `Foo` defined here -... LL | assert_foo(gen); | ^^^^^^^^^^ implementation of `Foo` is not general enough | @@ -37,9 +28,6 @@ LL | assert_foo(gen); error: implementation of `Foo` is not general enough --> $DIR/auto-trait-regions.rs:50:5 | -LL | auto trait Foo {} - | ----------------- trait `Foo` defined here -... LL | assert_foo(gen); | ^^^^^^^^^^ implementation of `Foo` is not general enough | diff --git a/src/test/ui/hrtb/due-to-where-clause.stderr b/src/test/ui/hrtb/due-to-where-clause.stderr index e4096ec059a6e..520938a633514 100644 --- a/src/test/ui/hrtb/due-to-where-clause.stderr +++ b/src/test/ui/hrtb/due-to-where-clause.stderr @@ -3,9 +3,6 @@ error: implementation of `Foo` is not general enough | LL | test::(&mut 42); | ^^^^^^^^^^^^ implementation of `Foo` is not general enough -... -LL | trait Foo<'a> {} - | ---------------- trait `Foo` defined here | = note: `FooS<'_>` must implement `Foo<'0>`, for any lifetime `'0`... = note: ...but `FooS<'_>` actually implements `Foo<'1>`, for some specific lifetime `'1` diff --git a/src/test/ui/hrtb/hrtb-cache-issue-54302.stderr b/src/test/ui/hrtb/hrtb-cache-issue-54302.stderr index 003f32659351f..f014eab8601fa 100644 --- a/src/test/ui/hrtb/hrtb-cache-issue-54302.stderr +++ b/src/test/ui/hrtb/hrtb-cache-issue-54302.stderr @@ -1,9 +1,6 @@ error: implementation of `Deserialize` is not general enough --> $DIR/hrtb-cache-issue-54302.rs:19:5 | -LL | trait Deserialize<'de> {} - | ------------------------- trait `Deserialize` defined here -... LL | assert_deserialize_owned::<&'static str>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Deserialize` is not general enough | diff --git a/src/test/ui/hrtb/hrtb-conflate-regions.stderr b/src/test/ui/hrtb/hrtb-conflate-regions.stderr index 45573814d13c0..7a0ede5af201c 100644 --- a/src/test/ui/hrtb/hrtb-conflate-regions.stderr +++ b/src/test/ui/hrtb/hrtb-conflate-regions.stderr @@ -1,13 +1,8 @@ error: implementation of `Foo` is not general enough --> $DIR/hrtb-conflate-regions.rs:27:10 | -LL | / trait Foo { -LL | | fn foo(&self, x: X) { } -LL | | } - | |_- trait `Foo` defined here -... -LL | fn b() { want_foo2::(); } - | ^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough +LL | fn b() { want_foo2::(); } + | ^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `SomeStruct` must implement `Foo<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... = note: ...but `SomeStruct` actually implements `Foo<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` diff --git a/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr b/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr index fe8209d054c8a..2f946c7d9bfe8 100644 --- a/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr +++ b/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr @@ -1,9 +1,6 @@ error: implementation of `Trait` is not general enough --> $DIR/hrtb-exists-forall-trait-contravariant.rs:34:5 | -LL | trait Trait {} - | ----------------- trait `Trait` defined here -... LL | foo::<()>(); | ^^^^^^^^^ implementation of `Trait` is not general enough | diff --git a/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr b/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr index 8bd23aa9018df..ba244e0f2ebc7 100644 --- a/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr +++ b/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr @@ -1,9 +1,6 @@ error: implementation of `Trait` is not general enough --> $DIR/hrtb-exists-forall-trait-invariant.rs:28:5 | -LL | trait Trait {} - | ----------------- trait `Trait` defined here -... LL | foo::<()>(); | ^^^^^^^^^ implementation of `Trait` is not general enough | diff --git a/src/test/ui/hrtb/hrtb-just-for-static.stderr b/src/test/ui/hrtb/hrtb-just-for-static.stderr index 5e3014317f5bc..0d46a130e093e 100644 --- a/src/test/ui/hrtb/hrtb-just-for-static.stderr +++ b/src/test/ui/hrtb/hrtb-just-for-static.stderr @@ -1,13 +1,8 @@ error: implementation of `Foo` is not general enough --> $DIR/hrtb-just-for-static.rs:24:5 | -LL | / trait Foo { -LL | | fn foo(&self, x: X) { } -LL | | } - | |_- trait `Foo` defined here -... -LL | want_hrtb::() - | ^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough +LL | want_hrtb::() + | ^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `StaticInt` must implement `Foo<&'0 isize>`, for any lifetime `'0`... = note: ...but `StaticInt` actually implements `Foo<&'1 isize>`, for some specific lifetime `'1` @@ -15,13 +10,8 @@ LL | want_hrtb::() error: implementation of `Foo` is not general enough --> $DIR/hrtb-just-for-static.rs:30:5 | -LL | / trait Foo { -LL | | fn foo(&self, x: X) { } -LL | | } - | |_- trait `Foo` defined here -... -LL | want_hrtb::<&'a u32>() - | ^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough +LL | want_hrtb::<&'a u32>() + | ^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `Foo<&'0 isize>` would have to be implemented for the type `&'a u32`, for any lifetime `'0`... = note: ...but `Foo<&'1 isize>` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` diff --git a/src/test/ui/hrtb/issue-46989.stderr b/src/test/ui/hrtb/issue-46989.stderr index c85c37ff9239e..f3d906cae4cc3 100644 --- a/src/test/ui/hrtb/issue-46989.stderr +++ b/src/test/ui/hrtb/issue-46989.stderr @@ -1,9 +1,6 @@ error: implementation of `Foo` is not general enough --> $DIR/issue-46989.rs:38:5 | -LL | trait Foo {} - | ------------ trait `Foo` defined here -... LL | assert_foo::(); | ^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | diff --git a/src/test/ui/issues/issue-54302-cases.stderr b/src/test/ui/issues/issue-54302-cases.stderr index 3ed2779164301..baa75f28d37f3 100644 --- a/src/test/ui/issues/issue-54302-cases.stderr +++ b/src/test/ui/issues/issue-54302-cases.stderr @@ -1,13 +1,8 @@ error: implementation of `Foo` is not general enough --> $DIR/issue-54302-cases.rs:63:5 | -LL | / trait Foo<'x, T> { -LL | | fn foo(self) -> &'x T; -LL | | } - | |_- trait `Foo` defined here -... -LL | >::ref_foo(a) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough +LL | >::ref_foo(a) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `Foo<'static, u32>` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Foo<'_, u32>` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` @@ -15,13 +10,8 @@ LL | >::ref_foo(a) error: implementation of `Foo` is not general enough --> $DIR/issue-54302-cases.rs:69:5 | -LL | / trait Foo<'x, T> { -LL | | fn foo(self) -> &'x T; -LL | | } - | |_- trait `Foo` defined here -... -LL | >::ref_foo(a) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough +LL | >::ref_foo(a) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `Foo<'static, i32>` would have to be implemented for the type `&'0 i32`, for any lifetime `'0`... = note: ...but `Foo<'_, i32>` is actually implemented for the type `&'1 i32`, for some specific lifetime `'1` @@ -29,13 +19,8 @@ LL | >::ref_foo(a) error: implementation of `Foo` is not general enough --> $DIR/issue-54302-cases.rs:75:5 | -LL | / trait Foo<'x, T> { -LL | | fn foo(self) -> &'x T; -LL | | } - | |_- trait `Foo` defined here -... -LL | >::ref_foo(a) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough +LL | >::ref_foo(a) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `Foo<'static, u64>` would have to be implemented for the type `&'0 u64`, for any lifetime `'0`... = note: ...but `Foo<'_, u64>` is actually implemented for the type `&'1 u64`, for some specific lifetime `'1` @@ -43,13 +28,8 @@ LL | >::ref_foo(a) error: implementation of `Foo` is not general enough --> $DIR/issue-54302-cases.rs:81:5 | -LL | / trait Foo<'x, T> { -LL | | fn foo(self) -> &'x T; -LL | | } - | |_- trait `Foo` defined here -... -LL | >::ref_foo(a) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough +LL | >::ref_foo(a) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `Foo<'static, i64>` would have to be implemented for the type `&'0 i64`, for any lifetime `'0`... = note: ...but `Foo<'_, i64>` is actually implemented for the type `&'1 i64`, for some specific lifetime `'1` diff --git a/src/test/ui/issues/issue-54302.stderr b/src/test/ui/issues/issue-54302.stderr index 1b3f57ba188a3..26c46571f9cb2 100644 --- a/src/test/ui/issues/issue-54302.stderr +++ b/src/test/ui/issues/issue-54302.stderr @@ -1,9 +1,6 @@ error: implementation of `Deserialize` is not general enough --> $DIR/issue-54302.rs:13:5 | -LL | trait Deserialize<'de> {} - | ------------------------- trait `Deserialize` defined here -... LL | assert_deserialize_owned::<&'static str>(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Deserialize` is not general enough | diff --git a/src/test/ui/issues/issue-55731.stderr b/src/test/ui/issues/issue-55731.stderr index f44c842187cc2..de327cd3cc23c 100644 --- a/src/test/ui/issues/issue-55731.stderr +++ b/src/test/ui/issues/issue-55731.stderr @@ -1,13 +1,8 @@ error: implementation of `DistributedIteratorMulti` is not general enough --> $DIR/issue-55731.rs:48:5 | -LL | / trait DistributedIteratorMulti { -LL | | type Item; -LL | | } - | |_- trait `DistributedIteratorMulti` defined here -... -LL | multi(Map { - | ^^^^^ implementation of `DistributedIteratorMulti` is not general enough +LL | multi(Map { + | ^^^^^ implementation of `DistributedIteratorMulti` is not general enough | = note: `DistributedIteratorMulti<&'0 ()>` would have to be implemented for the type `Cloned<&()>`, for any lifetime `'0`... = note: ...but `DistributedIteratorMulti<&'1 ()>` is actually implemented for the type `Cloned<&'1 ()>`, for some specific lifetime `'1` diff --git a/src/test/ui/where-clauses/where-for-self-2.stderr b/src/test/ui/where-clauses/where-for-self-2.stderr index 30eb78b2da4f7..4f8b19291db40 100644 --- a/src/test/ui/where-clauses/where-for-self-2.stderr +++ b/src/test/ui/where-clauses/where-for-self-2.stderr @@ -1,13 +1,8 @@ error: implementation of `Bar` is not general enough --> $DIR/where-for-self-2.rs:23:5 | -LL | / trait Bar { -LL | | fn bar(&self); -LL | | } - | |_- trait `Bar` defined here -... -LL | foo(&X); - | ^^^ implementation of `Bar` is not general enough +LL | foo(&X); + | ^^^ implementation of `Bar` is not general enough | = note: `Bar` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... = note: ...but `Bar` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` From daab6db1a02b680a71976fcbeed577ff102b31e4 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 9 Feb 2021 22:28:09 +0000 Subject: [PATCH 05/22] Avoid repeating self type in placeholder error --- .../nice_region_error/placeholder_error.rs | 20 +++++++++++++++++-- .../associated-types-eq-hr.stderr | 10 +++++----- src/test/ui/hrtb/hrtb-conflate-regions.stderr | 2 +- ...b-exists-forall-trait-contravariant.stderr | 2 +- .../hrtb-exists-forall-trait-invariant.stderr | 2 +- src/test/ui/hrtb/hrtb-just-for-static.stderr | 2 +- 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs index d99e79005fbb8..09be0b076705c 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs @@ -370,6 +370,8 @@ impl NiceRegionError<'me, 'tcx> { value: trait_ref, }; + let same_self_type = actual_trait_ref.self_ty() == expected_trait_ref.self_ty(); + let mut expected_trait_ref = highlight_trait_ref(expected_trait_ref); expected_trait_ref.highlight.maybe_highlighting_region(sub_placeholder, has_sub); expected_trait_ref.highlight.maybe_highlighting_region(sup_placeholder, has_sup); @@ -385,7 +387,16 @@ impl NiceRegionError<'me, 'tcx> { } }; - let mut note = if passive_voice { + let mut note = if same_self_type { + let mut self_ty = expected_trait_ref.map(|tr| tr.self_ty()); + self_ty.highlight.maybe_highlighting_region(vid, actual_has_vid); + format!( + "{}`{}` must implement `{}`", + if leading_ellipsis { "..." } else { "" }, + self_ty, + expected_trait_ref.map(|tr| tr.print_only_trait_path()), + ) + } else if passive_voice { format!( "{}`{}` would have to be implemented for the type `{}`", if leading_ellipsis { "..." } else { "" }, @@ -431,7 +442,12 @@ impl NiceRegionError<'me, 'tcx> { None => true, }; - let mut note = if passive_voice { + let mut note = if same_self_type { + format!( + "...but it actually implements `{}`", + actual_trait_ref.map(|tr| tr.print_only_trait_path()), + ) + } else if passive_voice { format!( "...but `{}` is actually implemented for the type `{}`", actual_trait_ref.map(|tr| tr.print_only_trait_path()), diff --git a/src/test/ui/associated-types/associated-types-eq-hr.stderr b/src/test/ui/associated-types/associated-types-eq-hr.stderr index 6188d9ca979cf..6897b31fe4685 100644 --- a/src/test/ui/associated-types/associated-types-eq-hr.stderr +++ b/src/test/ui/associated-types/associated-types-eq-hr.stderr @@ -35,7 +35,7 @@ LL | tuple_one::(); | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... - = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` + = note: ...but it actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:96:5 @@ -44,7 +44,7 @@ LL | tuple_one::(); | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... - = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` + = note: ...but it actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:102:5 @@ -53,7 +53,7 @@ LL | tuple_two::(); | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... - = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` + = note: ...but it actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:102:5 @@ -62,7 +62,7 @@ LL | tuple_two::(); | ^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... - = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` + = note: ...but it actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` error: implementation of `TheTrait` is not general enough --> $DIR/associated-types-eq-hr.rs:112:5 @@ -71,7 +71,7 @@ LL | tuple_four::(); | ^^^^^^^^^^^^^^^^^^^ implementation of `TheTrait` is not general enough | = note: `Tuple` must implement `TheTrait<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... - = note: ...but `Tuple` actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` + = note: ...but it actually implements `TheTrait<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` error: aborting due to 7 previous errors diff --git a/src/test/ui/hrtb/hrtb-conflate-regions.stderr b/src/test/ui/hrtb/hrtb-conflate-regions.stderr index 7a0ede5af201c..b1d4c0bf37505 100644 --- a/src/test/ui/hrtb/hrtb-conflate-regions.stderr +++ b/src/test/ui/hrtb/hrtb-conflate-regions.stderr @@ -5,7 +5,7 @@ LL | fn b() { want_foo2::(); } | ^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `SomeStruct` must implement `Foo<(&'0 isize, &'1 isize)>`, for any two lifetimes `'0` and `'1`... - = note: ...but `SomeStruct` actually implements `Foo<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` + = note: ...but it actually implements `Foo<(&'2 isize, &'2 isize)>`, for some specific lifetime `'2` error: aborting due to previous error diff --git a/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr b/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr index 2f946c7d9bfe8..613f4dc4951ec 100644 --- a/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr +++ b/src/test/ui/hrtb/hrtb-exists-forall-trait-contravariant.stderr @@ -5,7 +5,7 @@ LL | foo::<()>(); | ^^^^^^^^^ implementation of `Trait` is not general enough | = note: `()` must implement `Trait fn(&'b u32)>` - = note: ...but `()` actually implements `Trait`, for some specific lifetime `'0` + = note: ...but it actually implements `Trait`, for some specific lifetime `'0` error: aborting due to previous error diff --git a/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr b/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr index ba244e0f2ebc7..b487ce3e0ffa1 100644 --- a/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr +++ b/src/test/ui/hrtb/hrtb-exists-forall-trait-invariant.stderr @@ -5,7 +5,7 @@ LL | foo::<()>(); | ^^^^^^^^^ implementation of `Trait` is not general enough | = note: `()` must implement `Trait fn(Cell<&'b u32>)>` - = note: ...but `()` actually implements `Trait)>`, for some specific lifetime `'0` + = note: ...but it actually implements `Trait)>`, for some specific lifetime `'0` error: aborting due to previous error diff --git a/src/test/ui/hrtb/hrtb-just-for-static.stderr b/src/test/ui/hrtb/hrtb-just-for-static.stderr index 0d46a130e093e..38d7c12d4ec03 100644 --- a/src/test/ui/hrtb/hrtb-just-for-static.stderr +++ b/src/test/ui/hrtb/hrtb-just-for-static.stderr @@ -5,7 +5,7 @@ LL | want_hrtb::() | ^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `StaticInt` must implement `Foo<&'0 isize>`, for any lifetime `'0`... - = note: ...but `StaticInt` actually implements `Foo<&'1 isize>`, for some specific lifetime `'1` + = note: ...but it actually implements `Foo<&'1 isize>`, for some specific lifetime `'1` error: implementation of `Foo` is not general enough --> $DIR/hrtb-just-for-static.rs:30:5 From 94c11dfe78c9c2e81ababe51b04231db4c90d07f Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 9 Feb 2021 22:59:32 +0000 Subject: [PATCH 06/22] Report "nice" placeholder errors more often If we have a cause containing `ValuePairs::PolyTraitRefs` but neither TraitRef has any escaping bound regions then we report the same error as for `ValuePairs::TraitRefs`. --- .../nice_region_error/placeholder_error.rs | 142 +++++++++--------- .../hrtb/hrtb-perfect-forwarding.nll.stderr | 38 ++--- src/test/ui/hrtb/hrtb-perfect-forwarding.rs | 39 +++-- .../ui/hrtb/hrtb-perfect-forwarding.stderr | 41 ++--- src/test/ui/issues/issue-57843.nll.stderr | 2 +- src/test/ui/issues/issue-57843.rs | 6 +- src/test/ui/issues/issue-57843.stderr | 16 +- src/test/ui/lifetimes/issue-79187.rs | 2 +- src/test/ui/lifetimes/issue-79187.stderr | 19 +-- .../closure-arg-type-mismatch.rs | 4 +- .../closure-arg-type-mismatch.stderr | 36 +---- src/test/ui/rfc1623.rs | 2 +- src/test/ui/rfc1623.stderr | 9 +- .../issue-57611-trait-alias.nll.stderr | 13 +- .../issue-57611-trait-alias.rs | 6 +- .../issue-57611-trait-alias.stderr | 39 ++--- .../unboxed-closures/issue-30906.nll.stderr | 2 +- src/test/ui/unboxed-closures/issue-30906.rs | 12 +- .../ui/unboxed-closures/issue-30906.stderr | 16 +- 19 files changed, 181 insertions(+), 263 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs index 09be0b076705c..285666b7acb99 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs @@ -30,157 +30,153 @@ impl NiceRegionError<'me, 'tcx> { Some(RegionResolutionError::SubSupConflict( vid, _, - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), sub_placeholder @ ty::RePlaceholder(_), _, sup_placeholder @ ty::RePlaceholder(_), - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( Some(self.tcx().mk_region(ty::ReVar(*vid))), cause, Some(sub_placeholder), Some(sup_placeholder), - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), Some(RegionResolutionError::SubSupConflict( vid, _, - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), sub_placeholder @ ty::RePlaceholder(_), _, _, - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( Some(self.tcx().mk_region(ty::ReVar(*vid))), cause, Some(sub_placeholder), None, - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), Some(RegionResolutionError::SubSupConflict( vid, _, - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), _, _, sup_placeholder @ ty::RePlaceholder(_), - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( Some(self.tcx().mk_region(ty::ReVar(*vid))), cause, None, Some(*sup_placeholder), - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), Some(RegionResolutionError::SubSupConflict( vid, _, _, _, - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), sup_placeholder @ ty::RePlaceholder(_), - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( Some(self.tcx().mk_region(ty::ReVar(*vid))), cause, None, Some(*sup_placeholder), - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), Some(RegionResolutionError::UpperBoundUniverseConflict( vid, _, _, - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), sup_placeholder @ ty::RePlaceholder(_), - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( Some(self.tcx().mk_region(ty::ReVar(*vid))), cause, None, Some(*sup_placeholder), - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), Some(RegionResolutionError::ConcreteFailure( - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), sub_region @ ty::RePlaceholder(_), sup_region @ ty::RePlaceholder(_), - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( None, cause, Some(*sub_region), Some(*sup_region), - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), Some(RegionResolutionError::ConcreteFailure( - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), sub_region @ ty::RePlaceholder(_), sup_region, - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( Some(sup_region), cause, Some(*sub_region), None, - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), Some(RegionResolutionError::ConcreteFailure( - SubregionOrigin::Subtype(box TypeTrace { - cause, - values: ValuePairs::TraitRefs(ExpectedFound { expected, found }), - }), + SubregionOrigin::Subtype(box TypeTrace { cause, values }), sub_region, sup_region @ ty::RePlaceholder(_), - )) if expected.def_id == found.def_id => Some(self.try_report_placeholders_trait( + )) => self.try_report_trait_placeholder_mismatch( Some(sub_region), cause, None, Some(*sup_region), - expected.def_id, - expected.substs, - found.substs, - )), + values, + ), _ => None, } } + fn try_report_trait_placeholder_mismatch( + &self, + vid: Option>, + cause: &ObligationCause<'tcx>, + sub_placeholder: Option>, + sup_placeholder: Option>, + value_pairs: &ValuePairs<'tcx>, + ) -> Option> { + let (expected_substs, found_substs, trait_def_id) = match value_pairs { + ValuePairs::TraitRefs(ExpectedFound { expected, found }) + if expected.def_id == found.def_id => + { + (expected.substs, found.substs, expected.def_id) + } + ValuePairs::PolyTraitRefs(ExpectedFound { expected, found }) + if expected.def_id() == found.def_id() => + { + // It's possible that the placeholders come from a binder + // outside of this value pair. Use `no_bound_vars` as a + // simple heuristic for that. + (expected.no_bound_vars()?.substs, found.no_bound_vars()?.substs, expected.def_id()) + } + _ => return None, + }; + + Some(self.report_trait_placeholder_mismatch( + vid, + cause, + sub_placeholder, + sup_placeholder, + trait_def_id, + expected_substs, + found_substs, + )) + } + // error[E0308]: implementation of `Foo` does not apply to enough lifetimes // --> /home/nmatsakis/tmp/foo.rs:12:5 // | @@ -191,7 +187,7 @@ impl NiceRegionError<'me, 'tcx> { // = note: `T` must implement `...` for any two lifetimes `'1` and `'2`. // = note: However, the type `T` only implements `...` for some specific lifetime `'2`. #[instrument(level = "debug", skip(self))] - fn try_report_placeholders_trait( + fn report_trait_placeholder_mismatch( &self, vid: Option>, cause: &ObligationCause<'tcx>, diff --git a/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr b/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr index edce0e6702ee1..c3dd794957540 100644 --- a/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr +++ b/src/test/ui/hrtb/hrtb-perfect-forwarding.nll.stderr @@ -1,11 +1,11 @@ warning: function cannot return without recursing - --> $DIR/hrtb-perfect-forwarding.rs:22:1 + --> $DIR/hrtb-perfect-forwarding.rs:16:1 | -LL | / fn no_hrtb<'b,T>(mut t: T) -LL | | where T : Bar<&'b isize> +LL | / fn no_hrtb<'b, T>(mut t: T) +LL | | where +LL | | T: Bar<&'b isize>, LL | | { -LL | | // OK -- `T : Bar<&'b isize>`, and thus the impl above ensures that -LL | | // `&mut T : Bar<&'b isize>`. +... | LL | | no_hrtb(&mut t); | | --------------- recursive call site LL | | } @@ -15,12 +15,12 @@ LL | | } = help: a `loop` may express intention better if this is on purpose warning: function cannot return without recursing - --> $DIR/hrtb-perfect-forwarding.rs:30:1 + --> $DIR/hrtb-perfect-forwarding.rs:25:1 | LL | / fn bar_hrtb(mut t: T) -LL | | where T : for<'b> Bar<&'b isize> +LL | | where +LL | | T: for<'b> Bar<&'b isize>, LL | | { -LL | | // OK -- `T : for<'b> Bar<&'b isize>`, and thus the impl above ... | LL | | bar_hrtb(&mut t); | | ---------------- recursive call site @@ -30,25 +30,26 @@ LL | | } = help: a `loop` may express intention better if this is on purpose warning: function cannot return without recursing - --> $DIR/hrtb-perfect-forwarding.rs:39:1 + --> $DIR/hrtb-perfect-forwarding.rs:35:1 | -LL | / fn foo_hrtb_bar_not<'b,T>(mut t: T) -LL | | where T : for<'a> Foo<&'a isize> + Bar<&'b isize> +LL | / fn foo_hrtb_bar_not<'b, T>(mut t: T) +LL | | where +LL | | T: for<'a> Foo<&'a isize> + Bar<&'b isize>, LL | | { -LL | | // Not OK -- The forwarding impl for `Foo` requires that `Bar` also ... | LL | | foo_hrtb_bar_not(&mut t); | | ------------------------ recursive call site LL | | +LL | | LL | | } | |_^ cannot return without recursing | = help: a `loop` may express intention better if this is on purpose error: lifetime may not live long enough - --> $DIR/hrtb-perfect-forwarding.rs:46:5 + --> $DIR/hrtb-perfect-forwarding.rs:43:5 | -LL | fn foo_hrtb_bar_not<'b,T>(mut t: T) +LL | fn foo_hrtb_bar_not<'b, T>(mut t: T) | -- lifetime `'b` defined here ... LL | foo_hrtb_bar_not(&mut t); @@ -57,18 +58,19 @@ LL | foo_hrtb_bar_not(&mut t); = help: consider replacing `'b` with `'static` error: higher-ranked subtype error - --> $DIR/hrtb-perfect-forwarding.rs:46:5 + --> $DIR/hrtb-perfect-forwarding.rs:43:5 | LL | foo_hrtb_bar_not(&mut t); | ^^^^^^^^^^^^^^^^^^^^^^^^ warning: function cannot return without recursing - --> $DIR/hrtb-perfect-forwarding.rs:50:1 + --> $DIR/hrtb-perfect-forwarding.rs:48:1 | LL | / fn foo_hrtb_bar_hrtb(mut t: T) -LL | | where T : for<'a> Foo<&'a isize> + for<'b> Bar<&'b isize> +LL | | where +LL | | T: for<'a> Foo<&'a isize> + for<'b> Bar<&'b isize>, LL | | { -LL | | // OK -- now we have `T : for<'b> Bar&'b isize>`. +LL | | // OK -- now we have `T : for<'b> Bar<&'b isize>`. LL | | foo_hrtb_bar_hrtb(&mut t); | | ------------------------- recursive call site LL | | } diff --git a/src/test/ui/hrtb/hrtb-perfect-forwarding.rs b/src/test/ui/hrtb/hrtb-perfect-forwarding.rs index 0303a764c12de..441a788359e03 100644 --- a/src/test/ui/hrtb/hrtb-perfect-forwarding.rs +++ b/src/test/ui/hrtb/hrtb-perfect-forwarding.rs @@ -2,25 +2,20 @@ // is being applied to `for<'a> Foo<&'a mut X>`. Issue #19730. trait Foo { - fn foo(&mut self, x: X) { } + fn foo(&mut self, x: X) {} } trait Bar { - fn bar(&mut self, x: X) { } + fn bar(&mut self, x: X) {} } -impl<'a,X,F> Foo for &'a mut F - where F : Foo + Bar -{ -} +impl<'a, X, F> Foo for &'a mut F where F: Foo + Bar {} -impl<'a,X,F> Bar for &'a mut F - where F : Bar -{ -} +impl<'a, X, F> Bar for &'a mut F where F: Bar {} -fn no_hrtb<'b,T>(mut t: T) - where T : Bar<&'b isize> +fn no_hrtb<'b, T>(mut t: T) +where + T: Bar<&'b isize>, { // OK -- `T : Bar<&'b isize>`, and thus the impl above ensures that // `&mut T : Bar<&'b isize>`. @@ -28,7 +23,8 @@ fn no_hrtb<'b,T>(mut t: T) } fn bar_hrtb(mut t: T) - where T : for<'b> Bar<&'b isize> +where + T: for<'b> Bar<&'b isize>, { // OK -- `T : for<'b> Bar<&'b isize>`, and thus the impl above // ensures that `&mut T : for<'b> Bar<&'b isize>`. This is an @@ -36,22 +32,25 @@ fn bar_hrtb(mut t: T) bar_hrtb(&mut t); } -fn foo_hrtb_bar_not<'b,T>(mut t: T) - where T : for<'a> Foo<&'a isize> + Bar<&'b isize> +fn foo_hrtb_bar_not<'b, T>(mut t: T) +where + T: for<'a> Foo<&'a isize> + Bar<&'b isize>, { // Not OK -- The forwarding impl for `Foo` requires that `Bar` also // be implemented. Thus to satisfy `&mut T : for<'a> Foo<&'a // isize>`, we require `T : for<'a> Bar<&'a isize>`, but the where // clause only specifies `T : Bar<&'b isize>`. - foo_hrtb_bar_not(&mut t); //~ ERROR mismatched types - //~| ERROR mismatched types + foo_hrtb_bar_not(&mut t); + //~^ ERROR implementation of `Bar` is not general enough + //~| ERROR implementation of `Bar` is not general enough } fn foo_hrtb_bar_hrtb(mut t: T) - where T : for<'a> Foo<&'a isize> + for<'b> Bar<&'b isize> +where + T: for<'a> Foo<&'a isize> + for<'b> Bar<&'b isize>, { - // OK -- now we have `T : for<'b> Bar&'b isize>`. + // OK -- now we have `T : for<'b> Bar<&'b isize>`. foo_hrtb_bar_hrtb(&mut t); } -fn main() { } +fn main() {} diff --git a/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr b/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr index ed810d443bef7..e265f53cd2a7f 100644 --- a/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr +++ b/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr @@ -1,41 +1,20 @@ -error[E0308]: mismatched types - --> $DIR/hrtb-perfect-forwarding.rs:46:5 +error: implementation of `Bar` is not general enough + --> $DIR/hrtb-perfect-forwarding.rs:43:5 | LL | foo_hrtb_bar_not(&mut t); - | ^^^^^^^^^^^^^^^^ lifetime mismatch + | ^^^^^^^^^^^^^^^^ implementation of `Bar` is not general enough | - = note: expected type `Bar<&'a isize>` - found type `Bar<&'b isize>` -note: the required lifetime does not necessarily outlive the lifetime `'b` as defined on the function body at 39:21 - --> $DIR/hrtb-perfect-forwarding.rs:39:21 - | -LL | fn foo_hrtb_bar_not<'b,T>(mut t: T) - | ^^ -note: the lifetime requirement is introduced here - --> $DIR/hrtb-perfect-forwarding.rs:40:15 - | -LL | where T : for<'a> Foo<&'a isize> + Bar<&'b isize> - | ^^^^^^^^^^^^^^^^^^^^^^ + = note: `T` must implement `Bar<&'0 isize>`, for any lifetime `'0`... + = note: ...but it actually implements `Bar<&'1 isize>`, for some specific lifetime `'1` -error[E0308]: mismatched types - --> $DIR/hrtb-perfect-forwarding.rs:46:5 +error: implementation of `Bar` is not general enough + --> $DIR/hrtb-perfect-forwarding.rs:43:5 | LL | foo_hrtb_bar_not(&mut t); - | ^^^^^^^^^^^^^^^^ lifetime mismatch - | - = note: expected type `Bar<&'a isize>` - found type `Bar<&'b isize>` -note: the lifetime `'b` as defined on the function body at 39:21 doesn't meet the lifetime requirements - --> $DIR/hrtb-perfect-forwarding.rs:39:21 - | -LL | fn foo_hrtb_bar_not<'b,T>(mut t: T) - | ^^ -note: the lifetime requirement is introduced here - --> $DIR/hrtb-perfect-forwarding.rs:40:15 + | ^^^^^^^^^^^^^^^^ implementation of `Bar` is not general enough | -LL | where T : for<'a> Foo<&'a isize> + Bar<&'b isize> - | ^^^^^^^^^^^^^^^^^^^^^^ + = note: `T` must implement `Bar<&'0 isize>`, for any lifetime `'0`... + = note: ...but it actually implements `Bar<&'1 isize>`, for some specific lifetime `'1` error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/issues/issue-57843.nll.stderr b/src/test/ui/issues/issue-57843.nll.stderr index 70310780b4330..70d16cc9a1da6 100644 --- a/src/test/ui/issues/issue-57843.nll.stderr +++ b/src/test/ui/issues/issue-57843.nll.stderr @@ -1,5 +1,5 @@ error: higher-ranked subtype error - --> $DIR/issue-57843.rs:23:9 + --> $DIR/issue-57843.rs:25:9 | LL | Foo(Box::new(|_| ())); | ^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/issues/issue-57843.rs b/src/test/ui/issues/issue-57843.rs index 466082552667b..11629690ecc5e 100644 --- a/src/test/ui/issues/issue-57843.rs +++ b/src/test/ui/issues/issue-57843.rs @@ -11,7 +11,9 @@ trait ClonableFn { } impl ClonableFn for F -where F: Fn(T) + Clone { +where + F: Fn(T) + Clone, +{ fn clone(&self) -> Box { Box::new(self.clone()) } @@ -20,5 +22,5 @@ where F: Fn(T) + Clone { struct Foo(Box ClonableFn<&'a bool>>); fn main() { - Foo(Box::new(|_| ())); //~ ERROR mismatched types + Foo(Box::new(|_| ())); //~ ERROR implementation of `FnOnce` is not general enough } diff --git a/src/test/ui/issues/issue-57843.stderr b/src/test/ui/issues/issue-57843.stderr index 7699e97da99ad..acd48b6c7e5ba 100644 --- a/src/test/ui/issues/issue-57843.stderr +++ b/src/test/ui/issues/issue-57843.stderr @@ -1,17 +1,11 @@ -error[E0308]: mismatched types - --> $DIR/issue-57843.rs:23:9 +error: implementation of `FnOnce` is not general enough + --> $DIR/issue-57843.rs:25:9 | LL | Foo(Box::new(|_| ())); - | ^^^^^^^^^^^^^^^^ one type is more general than the other + | ^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: expected type `FnOnce<(&'a bool,)>` - found type `FnOnce<(&bool,)>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-57843.rs:23:18 - | -LL | Foo(Box::new(|_| ())); - | ^^^^^^ + = note: `[closure@$DIR/issue-57843.rs:25:18: 25:24]` must implement `FnOnce<(&'1 bool,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 bool,)>`, for some specific lifetime `'2` error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/lifetimes/issue-79187.rs b/src/test/ui/lifetimes/issue-79187.rs index bf331d8a6068e..2ee69ee2234ee 100644 --- a/src/test/ui/lifetimes/issue-79187.rs +++ b/src/test/ui/lifetimes/issue-79187.rs @@ -2,5 +2,5 @@ fn thing(x: impl FnOnce(&u32)) {} fn main() { let f = |_| (); - thing(f); //~ERROR mismatched types + thing(f); //~ERROR implementation of `FnOnce` is not general enough } diff --git a/src/test/ui/lifetimes/issue-79187.stderr b/src/test/ui/lifetimes/issue-79187.stderr index 63f501e08fce4..3627ab5ed1e97 100644 --- a/src/test/ui/lifetimes/issue-79187.stderr +++ b/src/test/ui/lifetimes/issue-79187.stderr @@ -1,22 +1,11 @@ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/issue-79187.rs:5:5 | LL | thing(f); - | ^^^^^ lifetime mismatch + | ^^^^^ implementation of `FnOnce` is not general enough | - = note: expected type `FnOnce<(&u32,)>` - found type `FnOnce<(&u32,)>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-79187.rs:4:13 - | -LL | let f = |_| (); - | ^^^^^^ -note: the lifetime requirement is introduced here - --> $DIR/issue-79187.rs:1:18 - | -LL | fn thing(x: impl FnOnce(&u32)) {} - | ^^^^^^^^^^^^ + = note: `[closure@$DIR/issue-79187.rs:4:13: 4:19]` must implement `FnOnce<(&'1 u32,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 u32,)>`, for some specific lifetime `'2` error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/mismatched_types/closure-arg-type-mismatch.rs b/src/test/ui/mismatched_types/closure-arg-type-mismatch.rs index e278049c8cc42..2d485d14a8d6b 100644 --- a/src/test/ui/mismatched_types/closure-arg-type-mismatch.rs +++ b/src/test/ui/mismatched_types/closure-arg-type-mismatch.rs @@ -8,8 +8,8 @@ fn main() { fn baz(_: F) {} fn _test<'a>(f: fn(*mut &'a u32)) { baz(f); - //~^ ERROR mismatched types - //~| ERROR mismatched types + //~^ ERROR implementation of `FnOnce` is not general enough + //~| ERROR implementation of `FnOnce` is not general enough //~| ERROR mismatched types //~| ERROR mismatched types } diff --git a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr index 02ba60f7f4b73..67c5a68ed83c6 100644 --- a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr +++ b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr @@ -41,24 +41,14 @@ note: the lifetime requirement is introduced here LL | fn baz(_: F) {} | ^^^^^^^^^^^^^ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/closure-arg-type-mismatch.rs:10:5 | LL | baz(f); - | ^^^ lifetime mismatch - | - = note: expected type `FnOnce<(*mut &u32,)>` - found type `FnOnce<(*mut &'a u32,)>` -note: the required lifetime does not necessarily outlive the lifetime `'a` as defined on the function body at 9:10 - --> $DIR/closure-arg-type-mismatch.rs:9:10 - | -LL | fn _test<'a>(f: fn(*mut &'a u32)) { - | ^^ -note: the lifetime requirement is introduced here - --> $DIR/closure-arg-type-mismatch.rs:8:11 + | ^^^ implementation of `FnOnce` is not general enough | -LL | fn baz(_: F) {} - | ^^^^^^^^^^^^^ + = note: `fn(*mut &'2 u32)` must implement `FnOnce<(*mut &'1 u32,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(*mut &'2 u32,)>`, for some specific lifetime `'2` error[E0308]: mismatched types --> $DIR/closure-arg-type-mismatch.rs:10:5 @@ -79,24 +69,14 @@ note: the lifetime requirement is introduced here LL | fn baz(_: F) {} | ^^^^^^^^^^^^^ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/closure-arg-type-mismatch.rs:10:5 | LL | baz(f); - | ^^^ lifetime mismatch - | - = note: expected type `FnOnce<(*mut &u32,)>` - found type `FnOnce<(*mut &'a u32,)>` -note: the lifetime `'a` as defined on the function body at 9:10 doesn't meet the lifetime requirements - --> $DIR/closure-arg-type-mismatch.rs:9:10 - | -LL | fn _test<'a>(f: fn(*mut &'a u32)) { - | ^^ -note: the lifetime requirement is introduced here - --> $DIR/closure-arg-type-mismatch.rs:8:11 + | ^^^ implementation of `FnOnce` is not general enough | -LL | fn baz(_: F) {} - | ^^^^^^^^^^^^^ + = note: `fn(*mut &'2 u32)` must implement `FnOnce<(*mut &'1 u32,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(*mut &'2 u32,)>`, for some specific lifetime `'2` error: aborting due to 7 previous errors diff --git a/src/test/ui/rfc1623.rs b/src/test/ui/rfc1623.rs index aa6b1c0012c93..9ff4813d11286 100644 --- a/src/test/ui/rfc1623.rs +++ b/src/test/ui/rfc1623.rs @@ -22,7 +22,7 @@ static SOME_STRUCT: &SomeStruct = &SomeStruct { foo: &Foo { bools: &[false, true] }, bar: &Bar { bools: &[true, true] }, f: &id, - //~^ ERROR mismatched types + //~^ ERROR implementation of `FnOnce` is not general enough }; // very simple test for a 'static static with default lifetime diff --git a/src/test/ui/rfc1623.stderr b/src/test/ui/rfc1623.stderr index 2835e47fa4515..e95e68c8e6d26 100644 --- a/src/test/ui/rfc1623.stderr +++ b/src/test/ui/rfc1623.stderr @@ -1,12 +1,11 @@ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/rfc1623.rs:24:8 | LL | f: &id, - | ^^^ one type is more general than the other + | ^^^ implementation of `FnOnce` is not general enough | - = note: expected type `FnOnce<(&'a Foo<'b>,)>` - found type `FnOnce<(&Foo<'_>,)>` + = note: `fn(&'2 Foo<'_>) -> &'2 Foo<'_> {id::<&'2 Foo<'_>>}` must implement `FnOnce<(&'1 Foo<'b>,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 Foo<'_>,)>`, for some specific lifetime `'2` error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr index be1dd1a8524c8..59b14eedc24ba 100644 --- a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr @@ -24,19 +24,14 @@ note: this closure does not fulfill the lifetime requirements LL | |x| x | ^^^^^ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/issue-57611-trait-alias.rs:17:16 | LL | type Bar = impl Baz; - | ^^^^^^^^^^^^^^^^^^^^ one type is more general than the other + | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: expected type `FnOnce<(&X,)>` - found type `FnOnce<(&'static X,)>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-57611-trait-alias.rs:25:9 - | -LL | |x| x - | ^^^^^ + = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error: aborting due to 4 previous errors diff --git a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.rs b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.rs index 3372b81404ecb..561528c2abbd5 100644 --- a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.rs +++ b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.rs @@ -15,9 +15,9 @@ struct X; impl Foo for X { type Bar = impl Baz; - //~^ ERROR mismatched types - //~| ERROR mismatched types - //~| ERROR mismatched types + //~^ ERROR implementation of `FnOnce` is not general enough + //~| ERROR implementation of `FnOnce` is not general enough + //~| ERROR implementation of `FnOnce` is not general enough //~| ERROR mismatched types //~| ERROR mismatched types diff --git a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr index 9d9293e958eeb..59c91d52cca98 100644 --- a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr @@ -1,16 +1,11 @@ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/issue-57611-trait-alias.rs:17:16 | LL | type Bar = impl Baz; - | ^^^^^^^^^^^^^^^^^^^^ one type is more general than the other - | - = note: expected type `FnOnce<(&X,)>` - found type `FnOnce<(&X,)>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-57611-trait-alias.rs:25:9 + | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | -LL | |x| x - | ^^^^^ + = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error[E0308]: mismatched types --> $DIR/issue-57611-trait-alias.rs:17:16 @@ -26,19 +21,14 @@ note: this closure does not fulfill the lifetime requirements LL | |x| x | ^^^^^ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/issue-57611-trait-alias.rs:17:16 | LL | type Bar = impl Baz; - | ^^^^^^^^^^^^^^^^^^^^ one type is more general than the other + | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: expected type `FnOnce<(&X,)>` - found type `FnOnce<(&' X,)>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-57611-trait-alias.rs:25:9 - | -LL | |x| x - | ^^^^^ + = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error[E0308]: mismatched types --> $DIR/issue-57611-trait-alias.rs:17:16 @@ -54,19 +44,14 @@ note: this closure does not fulfill the lifetime requirements LL | |x| x | ^^^^^ -error[E0308]: mismatched types +error: implementation of `FnOnce` is not general enough --> $DIR/issue-57611-trait-alias.rs:17:16 | LL | type Bar = impl Baz; - | ^^^^^^^^^^^^^^^^^^^^ one type is more general than the other + | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: expected type `FnOnce<(&X,)>` - found type `FnOnce<(&' X,)>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-57611-trait-alias.rs:25:9 - | -LL | |x| x - | ^^^^^ + = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error: aborting due to 5 previous errors diff --git a/src/test/ui/unboxed-closures/issue-30906.nll.stderr b/src/test/ui/unboxed-closures/issue-30906.nll.stderr index 5a2cbab9a1500..2db392e8b8b9f 100644 --- a/src/test/ui/unboxed-closures/issue-30906.nll.stderr +++ b/src/test/ui/unboxed-closures/issue-30906.nll.stderr @@ -1,5 +1,5 @@ error: higher-ranked subtype error - --> $DIR/issue-30906.rs:15:5 + --> $DIR/issue-30906.rs:18:5 | LL | test(Compose(f, |_| {})); | ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/unboxed-closures/issue-30906.rs b/src/test/ui/unboxed-closures/issue-30906.rs index 03cce83277515..e2d219e470384 100644 --- a/src/test/ui/unboxed-closures/issue-30906.rs +++ b/src/test/ui/unboxed-closures/issue-30906.rs @@ -2,9 +2,12 @@ fn test FnOnce<(&'x str,)>>(_: F) {} -struct Compose(F,G); -impl FnOnce<(T,)> for Compose -where F: FnOnce<(T,)>, G: FnOnce<(F::Output,)> { +struct Compose(F, G); +impl FnOnce<(T,)> for Compose +where + F: FnOnce<(T,)>, + G: FnOnce<(F::Output,)>, +{ type Output = G::Output; extern "rust-call" fn call_once(self, (x,): (T,)) -> G::Output { (self.1)((self.0)(x)) @@ -12,7 +15,8 @@ where F: FnOnce<(T,)>, G: FnOnce<(F::Output,)> { } fn bad(f: fn(&'static str) -> T) { - test(Compose(f, |_| {})); //~ ERROR: mismatched types + test(Compose(f, |_| {})); + //~^ ERROR: implementation of `FnOnce` is not general enough } fn main() {} diff --git a/src/test/ui/unboxed-closures/issue-30906.stderr b/src/test/ui/unboxed-closures/issue-30906.stderr index ecf3a96b5a8dc..35b1e454c02b4 100644 --- a/src/test/ui/unboxed-closures/issue-30906.stderr +++ b/src/test/ui/unboxed-closures/issue-30906.stderr @@ -1,17 +1,11 @@ -error[E0308]: mismatched types - --> $DIR/issue-30906.rs:15:5 +error: implementation of `FnOnce` is not general enough + --> $DIR/issue-30906.rs:18:5 | LL | test(Compose(f, |_| {})); - | ^^^^ lifetime mismatch + | ^^^^ implementation of `FnOnce` is not general enough | - = note: expected type `FnOnce<(&'x str,)>` - found type `FnOnce<(&str,)>` -note: the lifetime requirement is introduced here - --> $DIR/issue-30906.rs:3:12 - | -LL | fn test FnOnce<(&'x str,)>>(_: F) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `fn(&'2 str) -> T` must implement `FnOnce<(&'1 str,)>`, for any lifetime `'1`... + = note: ...but it actually implements `FnOnce<(&'2 str,)>`, for some specific lifetime `'2` error: aborting due to previous error -For more information about this error, try `rustc --explain E0308`. From 9337d4fde89ded13243d67bec0b03f7fc553cbf9 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Tue, 9 Feb 2021 23:00:33 +0000 Subject: [PATCH 07/22] Print closure signatures when reporting placeholder errors --- .../nice_region_error/placeholder_error.rs | 38 ++++++++++++++++--- src/test/ui/issues/issue-57843.stderr | 2 +- src/test/ui/lifetimes/issue-79187.stderr | 2 +- .../issue-57611-trait-alias.nll.stderr | 2 +- .../issue-57611-trait-alias.stderr | 6 +-- 5 files changed, 38 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs index 285666b7acb99..46986d8d58b64 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs @@ -386,12 +386,38 @@ impl NiceRegionError<'me, 'tcx> { let mut note = if same_self_type { let mut self_ty = expected_trait_ref.map(|tr| tr.self_ty()); self_ty.highlight.maybe_highlighting_region(vid, actual_has_vid); - format!( - "{}`{}` must implement `{}`", - if leading_ellipsis { "..." } else { "" }, - self_ty, - expected_trait_ref.map(|tr| tr.print_only_trait_path()), - ) + + if self_ty.value.is_closure() + && self + .tcx() + .fn_trait_kind_from_lang_item(expected_trait_ref.value.def_id) + .is_some() + { + let closure_sig = self_ty.map(|closure| { + if let ty::Closure(_, substs) = closure.kind() { + self.tcx().signature_unclosure( + substs.as_closure().sig(), + rustc_hir::Unsafety::Normal, + ) + } else { + bug!("type is not longer closure"); + } + }); + + format!( + "{}closure with signature `{}` must implement `{}`", + if leading_ellipsis { "..." } else { "" }, + closure_sig, + expected_trait_ref.map(|tr| tr.print_only_trait_path()), + ) + } else { + format!( + "{}`{}` must implement `{}`", + if leading_ellipsis { "..." } else { "" }, + self_ty, + expected_trait_ref.map(|tr| tr.print_only_trait_path()), + ) + } } else if passive_voice { format!( "{}`{}` would have to be implemented for the type `{}`", diff --git a/src/test/ui/issues/issue-57843.stderr b/src/test/ui/issues/issue-57843.stderr index acd48b6c7e5ba..2ab49ec61cf59 100644 --- a/src/test/ui/issues/issue-57843.stderr +++ b/src/test/ui/issues/issue-57843.stderr @@ -4,7 +4,7 @@ error: implementation of `FnOnce` is not general enough LL | Foo(Box::new(|_| ())); | ^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: `[closure@$DIR/issue-57843.rs:25:18: 25:24]` must implement `FnOnce<(&'1 bool,)>`, for any lifetime `'1`... + = note: closure with signature `fn(&'2 bool)` must implement `FnOnce<(&'1 bool,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 bool,)>`, for some specific lifetime `'2` error: aborting due to previous error diff --git a/src/test/ui/lifetimes/issue-79187.stderr b/src/test/ui/lifetimes/issue-79187.stderr index 3627ab5ed1e97..3c0439fb660e5 100644 --- a/src/test/ui/lifetimes/issue-79187.stderr +++ b/src/test/ui/lifetimes/issue-79187.stderr @@ -4,7 +4,7 @@ error: implementation of `FnOnce` is not general enough LL | thing(f); | ^^^^^ implementation of `FnOnce` is not general enough | - = note: `[closure@$DIR/issue-79187.rs:4:13: 4:19]` must implement `FnOnce<(&'1 u32,)>`, for any lifetime `'1`... + = note: closure with signature `fn(&'2 u32)` must implement `FnOnce<(&'1 u32,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 u32,)>`, for some specific lifetime `'2` error: aborting due to previous error diff --git a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr index 59b14eedc24ba..a5409582288f2 100644 --- a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr @@ -30,7 +30,7 @@ error: implementation of `FnOnce` is not general enough LL | type Bar = impl Baz; | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: closure with signature `fn(&'2 X) -> &'2 X` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error: aborting due to 4 previous errors diff --git a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr index 59c91d52cca98..91c9d459ad8ce 100644 --- a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.stderr @@ -4,7 +4,7 @@ error: implementation of `FnOnce` is not general enough LL | type Bar = impl Baz; | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: closure with signature `fn(&'2 X) -> &X` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error[E0308]: mismatched types @@ -27,7 +27,7 @@ error: implementation of `FnOnce` is not general enough LL | type Bar = impl Baz; | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: closure with signature `fn(&'2 X) -> &'2 X` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error[E0308]: mismatched types @@ -50,7 +50,7 @@ error: implementation of `FnOnce` is not general enough LL | type Bar = impl Baz; | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: `[closure@$DIR/issue-57611-trait-alias.rs:25:9: 25:14]` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... + = note: closure with signature `fn(&'2 X) -> &'2 X` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` error: aborting due to 5 previous errors From 615fd141bde1e4319f95c3ea2981a1f5a8aa5dbd Mon Sep 17 00:00:00 2001 From: Nam Nguyen Date: Mon, 8 Feb 2021 11:52:59 -0800 Subject: [PATCH 08/22] Set the kind for local variables created by &str and slice arguments to LocalVariable --- .../rustc_codegen_ssa/src/mir/debuginfo.rs | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs index f1eae605da018..ea59e1831188b 100644 --- a/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs +++ b/compiler/rustc_codegen_ssa/src/mir/debuginfo.rs @@ -320,6 +320,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { ) -> Option>>> { let full_debug_info = self.cx.sess().opts.debuginfo == DebugInfo::Full; + let target_is_msvc = self.cx.sess().target.is_like_msvc; + if !full_debug_info && self.cx.sess().fewer_names() { return None; } @@ -341,11 +343,29 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { && var.source_info.scope == mir::OUTERMOST_SOURCE_SCOPE { let arg_index = place.local.index() - 1; - - // FIXME(eddyb) shouldn't `ArgumentVariable` indices be - // offset in closures to account for the hidden environment? - // Also, is this `+ 1` needed at all? - VariableKind::ArgumentVariable(arg_index + 1) + if target_is_msvc { + // Rust compiler decomposes every &str or slice argument into two components: + // a pointer to the memory address where the data is stored and a usize representing + // the length of the str (or slice). These components will later be used to reconstruct + // the original argument inside the body of the function that owns it (see the + // definition of debug_introduce_local for more details). + // + // Since the original argument is declared inside a function rather than being passed + // in as an argument, it must be marked as a LocalVariable for MSVC debuggers to visualize + // its data correctly. (See issue #81894 for an in-depth description of the problem). + match *var_ty.kind() { + ty::Ref(_, inner_type, _) => match *inner_type.kind() { + ty::Slice(_) | ty::Str => VariableKind::LocalVariable, + _ => VariableKind::ArgumentVariable(arg_index + 1), + }, + _ => VariableKind::ArgumentVariable(arg_index + 1), + } + } else { + // FIXME(eddyb) shouldn't `ArgumentVariable` indices be + // offset in closures to account for the hidden environment? + // Also, is this `+ 1` needed at all? + VariableKind::ArgumentVariable(arg_index + 1) + } } else { VariableKind::LocalVariable }; From f852160a99721f89a6bc206bae337291c485069e Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 10 Feb 2021 20:00:36 +0000 Subject: [PATCH 09/22] Keep existing names of regions in placeholder_error --- .../nice_region_error/placeholder_error.rs | 8 ++++---- src/test/ui/generator/auto-trait-regions.stderr | 8 ++++---- src/test/ui/hrtb/hrtb-just-for-static.stderr | 2 +- src/test/ui/hrtb/hrtb-perfect-forwarding.stderr | 4 ++-- .../ui/mismatched_types/closure-arg-type-mismatch.stderr | 8 ++++---- .../issue-57611-trait-alias.nll.stderr | 4 ++-- src/test/ui/where-clauses/where-for-self-2.stderr | 4 ++-- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs index 46986d8d58b64..4aecc2f40b874 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/placeholder_error.rs @@ -118,9 +118,9 @@ impl NiceRegionError<'me, 'tcx> { sub_region @ ty::RePlaceholder(_), sup_region, )) => self.try_report_trait_placeholder_mismatch( - Some(sup_region), + (!sup_region.has_name()).then_some(sup_region), cause, - Some(*sub_region), + Some(sub_region), None, values, ), @@ -130,10 +130,10 @@ impl NiceRegionError<'me, 'tcx> { sub_region, sup_region @ ty::RePlaceholder(_), )) => self.try_report_trait_placeholder_mismatch( - Some(sub_region), + (!sub_region.has_name()).then_some(sub_region), cause, None, - Some(*sup_region), + Some(sup_region), values, ), diff --git a/src/test/ui/generator/auto-trait-regions.stderr b/src/test/ui/generator/auto-trait-regions.stderr index 5fe4193905c72..da3d3249f0e7e 100644 --- a/src/test/ui/generator/auto-trait-regions.stderr +++ b/src/test/ui/generator/auto-trait-regions.stderr @@ -4,8 +4,8 @@ error: implementation of `Foo` is not general enough LL | assert_foo(gen); | ^^^^^^^^^^ implementation of `Foo` is not general enough | - = note: `Foo` would have to be implemented for the type `&'0 OnlyFooIfStaticRef`, for any lifetime `'0`... - = note: ...but `Foo` is actually implemented for the type `&'1 OnlyFooIfStaticRef`, for some specific lifetime `'1` + = note: `&'0 OnlyFooIfStaticRef` must implement `Foo`, for any lifetime `'0`... + = note: ...but `Foo` is actually implemented for the type `&'static OnlyFooIfStaticRef` error: implementation of `Foo` is not general enough --> $DIR/auto-trait-regions.rs:31:5 @@ -13,8 +13,8 @@ error: implementation of `Foo` is not general enough LL | assert_foo(gen); | ^^^^^^^^^^ implementation of `Foo` is not general enough | - = note: `Foo` would have to be implemented for the type `&'0 OnlyFooIfStaticRef`, for any lifetime `'0`... - = note: ...but `Foo` is actually implemented for the type `&'1 OnlyFooIfStaticRef`, for some specific lifetime `'1` + = note: `&'0 OnlyFooIfStaticRef` must implement `Foo`, for any lifetime `'0`... + = note: ...but `Foo` is actually implemented for the type `&'static OnlyFooIfStaticRef` error: implementation of `Foo` is not general enough --> $DIR/auto-trait-regions.rs:50:5 diff --git a/src/test/ui/hrtb/hrtb-just-for-static.stderr b/src/test/ui/hrtb/hrtb-just-for-static.stderr index 38d7c12d4ec03..ffc83aab4af4d 100644 --- a/src/test/ui/hrtb/hrtb-just-for-static.stderr +++ b/src/test/ui/hrtb/hrtb-just-for-static.stderr @@ -5,7 +5,7 @@ LL | want_hrtb::() | ^^^^^^^^^^^^^^^^^^^^^^ implementation of `Foo` is not general enough | = note: `StaticInt` must implement `Foo<&'0 isize>`, for any lifetime `'0`... - = note: ...but it actually implements `Foo<&'1 isize>`, for some specific lifetime `'1` + = note: ...but it actually implements `Foo<&'static isize>` error: implementation of `Foo` is not general enough --> $DIR/hrtb-just-for-static.rs:30:5 diff --git a/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr b/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr index e265f53cd2a7f..07ff9b96e44ff 100644 --- a/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr +++ b/src/test/ui/hrtb/hrtb-perfect-forwarding.stderr @@ -5,7 +5,7 @@ LL | foo_hrtb_bar_not(&mut t); | ^^^^^^^^^^^^^^^^ implementation of `Bar` is not general enough | = note: `T` must implement `Bar<&'0 isize>`, for any lifetime `'0`... - = note: ...but it actually implements `Bar<&'1 isize>`, for some specific lifetime `'1` + = note: ...but it actually implements `Bar<&'b isize>` error: implementation of `Bar` is not general enough --> $DIR/hrtb-perfect-forwarding.rs:43:5 @@ -14,7 +14,7 @@ LL | foo_hrtb_bar_not(&mut t); | ^^^^^^^^^^^^^^^^ implementation of `Bar` is not general enough | = note: `T` must implement `Bar<&'0 isize>`, for any lifetime `'0`... - = note: ...but it actually implements `Bar<&'1 isize>`, for some specific lifetime `'1` + = note: ...but it actually implements `Bar<&'b isize>` error: aborting due to 2 previous errors diff --git a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr index 67c5a68ed83c6..521de3742b03f 100644 --- a/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr +++ b/src/test/ui/mismatched_types/closure-arg-type-mismatch.stderr @@ -47,8 +47,8 @@ error: implementation of `FnOnce` is not general enough LL | baz(f); | ^^^ implementation of `FnOnce` is not general enough | - = note: `fn(*mut &'2 u32)` must implement `FnOnce<(*mut &'1 u32,)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(*mut &'2 u32,)>`, for some specific lifetime `'2` + = note: `fn(*mut &'a u32)` must implement `FnOnce<(*mut &'0 u32,)>`, for any lifetime `'0`... + = note: ...but it actually implements `FnOnce<(*mut &'a u32,)>` error[E0308]: mismatched types --> $DIR/closure-arg-type-mismatch.rs:10:5 @@ -75,8 +75,8 @@ error: implementation of `FnOnce` is not general enough LL | baz(f); | ^^^ implementation of `FnOnce` is not general enough | - = note: `fn(*mut &'2 u32)` must implement `FnOnce<(*mut &'1 u32,)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(*mut &'2 u32,)>`, for some specific lifetime `'2` + = note: `fn(*mut &'a u32)` must implement `FnOnce<(*mut &'0 u32,)>`, for any lifetime `'0`... + = note: ...but it actually implements `FnOnce<(*mut &'a u32,)>` error: aborting due to 7 previous errors diff --git a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr index a5409582288f2..998e178966a22 100644 --- a/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr +++ b/src/test/ui/type-alias-impl-trait/issue-57611-trait-alias.nll.stderr @@ -30,8 +30,8 @@ error: implementation of `FnOnce` is not general enough LL | type Bar = impl Baz; | ^^^^^^^^^^^^^^^^^^^^ implementation of `FnOnce` is not general enough | - = note: closure with signature `fn(&'2 X) -> &'2 X` must implement `FnOnce<(&'1 X,)>`, for any lifetime `'1`... - = note: ...but it actually implements `FnOnce<(&'2 X,)>`, for some specific lifetime `'2` + = note: closure with signature `fn(&'static X) -> &'static X` must implement `FnOnce<(&'0 X,)>`, for any lifetime `'0`... + = note: ...but it actually implements `FnOnce<(&'static X,)>` error: aborting due to 4 previous errors diff --git a/src/test/ui/where-clauses/where-for-self-2.stderr b/src/test/ui/where-clauses/where-for-self-2.stderr index 4f8b19291db40..6da46e20c09c0 100644 --- a/src/test/ui/where-clauses/where-for-self-2.stderr +++ b/src/test/ui/where-clauses/where-for-self-2.stderr @@ -4,8 +4,8 @@ error: implementation of `Bar` is not general enough LL | foo(&X); | ^^^ implementation of `Bar` is not general enough | - = note: `Bar` would have to be implemented for the type `&'0 u32`, for any lifetime `'0`... - = note: ...but `Bar` is actually implemented for the type `&'1 u32`, for some specific lifetime `'1` + = note: `&'0 u32` must implement `Bar`, for any lifetime `'0`... + = note: ...but `Bar` is actually implemented for the type `&'static u32` error: aborting due to previous error From 05704ec79aa729044aef70449d6661b658f0e37e Mon Sep 17 00:00:00 2001 From: csmoe Date: Fri, 12 Feb 2021 16:50:25 +0800 Subject: [PATCH 10/22] add testcase for issue 78600 --- src/test/ui/async-await/issues/issue-78600.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 src/test/ui/async-await/issues/issue-78600.rs diff --git a/src/test/ui/async-await/issues/issue-78600.rs b/src/test/ui/async-await/issues/issue-78600.rs new file mode 100644 index 0000000000000..8aaeaecf3e1da --- /dev/null +++ b/src/test/ui/async-await/issues/issue-78600.rs @@ -0,0 +1,12 @@ +// edition:2018 + +struct S<'a>(&'a i32); + +impl<'a> S<'a> { + async fn new(i: &'a i32) -> Result { + //~^ ERROR: `async fn` + Ok(S(&22)) + } +} + +fn main() {} From ed40b95925ddbe3596eebbca64f776f0f7c57d95 Mon Sep 17 00:00:00 2001 From: csmoe Date: Fri, 12 Feb 2021 16:50:45 +0800 Subject: [PATCH 11/22] spell out nested self type --- compiler/rustc_typeck/src/check/check.rs | 47 +++++++++++++++---- .../ui/async-await/issues/issue-78600.stderr | 11 +++++ 2 files changed, 49 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/async-await/issues/issue-78600.stderr diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 012ccb1af46b1..990d19914381d 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -7,6 +7,7 @@ use rustc_attr as attr; use rustc_errors::{Applicability, ErrorReported}; use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; +use rustc_hir::intravisit::Visitor; use rustc_hir::lang_items::LangItem; use rustc_hir::{ItemKind, Node}; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; @@ -557,6 +558,8 @@ pub(super) fn check_opaque_for_inheriting_lifetimes( ); if let Some(ty) = prohibit_opaque.break_value() { + let mut visitor = SelfTySpanVisitor { tcx, selfty_spans: vec![] }; + visitor.visit_item(&item); let is_async = match item.kind { ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) => { matches!(origin, hir::OpaqueTyOrigin::AsyncFn) @@ -573,15 +576,13 @@ pub(super) fn check_opaque_for_inheriting_lifetimes( if is_async { "async fn" } else { "impl Trait" }, ); - if let Ok(snippet) = tcx.sess.source_map().span_to_snippet(span) { - if snippet == "Self" { - err.span_suggestion( - span, - "consider spelling out the type instead", - format!("{:?}", ty), - Applicability::MaybeIncorrect, - ); - } + for span in visitor.selfty_spans { + err.span_suggestion( + span, + "consider spelling out the type instead", + format!("{:?}", ty), + Applicability::MaybeIncorrect, + ); } err.emit(); } @@ -1590,3 +1591,31 @@ fn opaque_type_cycle_error(tcx: TyCtxt<'tcx>, def_id: LocalDefId, span: Span) { } err.emit(); } + +struct SelfTySpanVisitor<'tcx> { + tcx: TyCtxt<'tcx>, + selfty_spans: Vec, +} + +impl Visitor<'tcx> for SelfTySpanVisitor<'tcx> { + type Map = rustc_middle::hir::map::Map<'tcx>; + + fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap { + hir::intravisit::NestedVisitorMap::OnlyBodies(self.tcx.hir()) + } + + fn visit_ty(&mut self, arg: &'tcx hir::Ty<'tcx>) { + match arg.kind { + hir::TyKind::Path(hir::QPath::Resolved(None, path)) => match &path.segments { + [segment] + if segment.res.map(|res| matches!(res, Res::SelfTy(_, _))).unwrap_or(false) => + { + self.selfty_spans.push(path.span); + } + _ => {} + }, + _ => {} + } + hir::intravisit::walk_ty(self, arg); + } +} diff --git a/src/test/ui/async-await/issues/issue-78600.stderr b/src/test/ui/async-await/issues/issue-78600.stderr new file mode 100644 index 0000000000000..8e3dc0406fafc --- /dev/null +++ b/src/test/ui/async-await/issues/issue-78600.stderr @@ -0,0 +1,11 @@ +error[E0760]: `async fn` return type cannot contain a projection or `Self` that references lifetimes from a parent scope + --> $DIR/issue-78600.rs:6:33 + | +LL | async fn new(i: &'a i32) -> Result { + | ^^^^^^^----^^^^^ + | | + | help: consider spelling out the type instead: `std::result::Result, ()>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0760`. From 5385a3d6a779b2e85d47c3fc317796ea3755c11a Mon Sep 17 00:00:00 2001 From: csmoe Date: Sat, 13 Feb 2021 14:45:53 +0800 Subject: [PATCH 12/22] spell the real selftype --- compiler/rustc_typeck/src/check/check.rs | 67 +++++++++---------- .../ui/async-await/issues/issue-78600.stderr | 2 +- 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 990d19914381d..21613bbc6f2e4 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -9,7 +9,7 @@ use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::intravisit::Visitor; use rustc_hir::lang_items::LangItem; -use rustc_hir::{ItemKind, Node}; +use rustc_hir::{def::Res, ItemKind, Node, PathSegment}; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt}; use rustc_middle::ty::fold::TypeFoldable; @@ -516,10 +516,11 @@ pub(super) fn check_opaque_for_inheriting_lifetimes( } } - #[derive(Debug)] struct ProhibitOpaqueVisitor<'tcx> { opaque_identity_ty: Ty<'tcx>, generics: &'tcx ty::Generics, + tcx: TyCtxt<'tcx>, + selftys: Vec<(Span, Option)>, } impl<'tcx> ty::fold::TypeVisitor<'tcx> for ProhibitOpaqueVisitor<'tcx> { @@ -536,6 +537,29 @@ pub(super) fn check_opaque_for_inheriting_lifetimes( } } + impl Visitor<'tcx> for ProhibitOpaqueVisitor<'tcx> { + type Map = rustc_middle::hir::map::Map<'tcx>; + + fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap { + hir::intravisit::NestedVisitorMap::OnlyBodies(self.tcx.hir()) + } + + fn visit_ty(&mut self, arg: &'tcx hir::Ty<'tcx>) { + match arg.kind { + hir::TyKind::Path(hir::QPath::Resolved(None, path)) => match &path.segments { + [PathSegment { res: Some(Res::SelfTy(_, impl_ref)), .. }] => { + let impl_ty_name = + impl_ref.map(|(def_id, _)| self.tcx.def_path_str(def_id)); + self.selftys.push((path.span, impl_ty_name)); + } + _ => {} + }, + _ => {} + } + hir::intravisit::walk_ty(self, arg); + } + } + if let ItemKind::OpaqueTy(hir::OpaqueTy { origin: hir::OpaqueTyOrigin::AsyncFn | hir::OpaqueTyOrigin::FnReturn, .. @@ -547,18 +571,19 @@ pub(super) fn check_opaque_for_inheriting_lifetimes( InternalSubsts::identity_for_item(tcx, def_id.to_def_id()), ), generics: tcx.generics_of(def_id), + tcx, + selftys: vec![], }; let prohibit_opaque = tcx .explicit_item_bounds(def_id) .iter() .try_for_each(|(predicate, _)| predicate.visit_with(&mut visitor)); debug!( - "check_opaque_for_inheriting_lifetimes: prohibit_opaque={:?}, visitor={:?}", - prohibit_opaque, visitor + "check_opaque_for_inheriting_lifetimes: prohibit_opaque={:?}, visitor.opaque_identity_ty={:?}, visitor.generics={:?}", + prohibit_opaque, visitor.opaque_identity_ty, visitor.generics ); if let Some(ty) = prohibit_opaque.break_value() { - let mut visitor = SelfTySpanVisitor { tcx, selfty_spans: vec![] }; visitor.visit_item(&item); let is_async = match item.kind { ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) => { @@ -576,11 +601,11 @@ pub(super) fn check_opaque_for_inheriting_lifetimes( if is_async { "async fn" } else { "impl Trait" }, ); - for span in visitor.selfty_spans { + for (span, name) in visitor.selftys { err.span_suggestion( span, "consider spelling out the type instead", - format!("{:?}", ty), + name.unwrap_or_else(|| format!("{:?}", ty)), Applicability::MaybeIncorrect, ); } @@ -1591,31 +1616,3 @@ fn opaque_type_cycle_error(tcx: TyCtxt<'tcx>, def_id: LocalDefId, span: Span) { } err.emit(); } - -struct SelfTySpanVisitor<'tcx> { - tcx: TyCtxt<'tcx>, - selfty_spans: Vec, -} - -impl Visitor<'tcx> for SelfTySpanVisitor<'tcx> { - type Map = rustc_middle::hir::map::Map<'tcx>; - - fn nested_visit_map(&mut self) -> hir::intravisit::NestedVisitorMap { - hir::intravisit::NestedVisitorMap::OnlyBodies(self.tcx.hir()) - } - - fn visit_ty(&mut self, arg: &'tcx hir::Ty<'tcx>) { - match arg.kind { - hir::TyKind::Path(hir::QPath::Resolved(None, path)) => match &path.segments { - [segment] - if segment.res.map(|res| matches!(res, Res::SelfTy(_, _))).unwrap_or(false) => - { - self.selfty_spans.push(path.span); - } - _ => {} - }, - _ => {} - } - hir::intravisit::walk_ty(self, arg); - } -} diff --git a/src/test/ui/async-await/issues/issue-78600.stderr b/src/test/ui/async-await/issues/issue-78600.stderr index 8e3dc0406fafc..92b66147106e1 100644 --- a/src/test/ui/async-await/issues/issue-78600.stderr +++ b/src/test/ui/async-await/issues/issue-78600.stderr @@ -4,7 +4,7 @@ error[E0760]: `async fn` return type cannot contain a projection or `Self` that LL | async fn new(i: &'a i32) -> Result { | ^^^^^^^----^^^^^ | | - | help: consider spelling out the type instead: `std::result::Result, ()>` + | help: consider spelling out the type instead: `S<'a>` error: aborting due to previous error From aee1e59e6f00876a881e9d8c2ff00e0f11f896eb Mon Sep 17 00:00:00 2001 From: mark Date: Sun, 7 Feb 2021 20:40:33 -0600 Subject: [PATCH 13/22] Simplify pattern grammar by allowing nested leading vert Along the way, we also implement a handful of diagnostics improvements and fixes, particularly with respect to the special handling of `||` in place of `|` and when there are leading verts in function params, which don't allow top-level or-patterns anyway. --- compiler/rustc_expand/src/expand.rs | 6 +- .../rustc_parse/src/parser/diagnostics.rs | 2 +- compiler/rustc_parse/src/parser/expr.rs | 8 +- compiler/rustc_parse/src/parser/mod.rs | 1 + .../rustc_parse/src/parser/nonterminal.rs | 4 +- compiler/rustc_parse/src/parser/pat.rs | 183 ++++++++++-------- compiler/rustc_parse/src/parser/stmt.rs | 2 +- .../feature-gate-or_patterns.stderr | 8 +- .../ui/or-patterns/fn-param-wrap-parens.fixed | 2 +- .../ui/or-patterns/fn-param-wrap-parens.rs | 2 +- .../or-patterns/fn-param-wrap-parens.stderr | 4 +- .../ui/or-patterns/multiple-pattern-typo.rs | 14 +- .../or-patterns/multiple-pattern-typo.stderr | 14 +- .../or-patterns/or-patterns-syntactic-fail.rs | 25 +-- .../or-patterns-syntactic-fail.stderr | 91 +-------- .../ui/or-patterns/remove-leading-vert.fixed | 26 +-- .../ui/or-patterns/remove-leading-vert.rs | 26 +-- .../ui/or-patterns/remove-leading-vert.stderr | 72 ++----- 18 files changed, 188 insertions(+), 302 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 870b5c92d8983..8c5395b12abce 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -22,7 +22,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_data_structures::sync::Lrc; use rustc_errors::{Applicability, PResult}; use rustc_feature::Features; -use rustc_parse::parser::{AttemptLocalParseRecovery, ForceCollect, Parser}; +use rustc_parse::parser::{AttemptLocalParseRecovery, ForceCollect, GateOr, Parser, RecoverComma}; use rustc_parse::validate_attr; use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS; use rustc_session::lint::BuiltinLintDiagnostics; @@ -914,7 +914,9 @@ pub fn parse_ast_fragment<'a>( } } AstFragmentKind::Ty => AstFragment::Ty(this.parse_ty()?), - AstFragmentKind::Pat => AstFragment::Pat(this.parse_pat(None)?), + AstFragmentKind::Pat => { + AstFragment::Pat(this.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::No)?) + } AstFragmentKind::Arms | AstFragmentKind::Fields | AstFragmentKind::FieldPats diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 5512e849c451d..0f49386dec07b 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1654,7 +1654,7 @@ impl<'a> Parser<'a> { } pub(super) fn recover_arg_parse(&mut self) -> PResult<'a, (P, P)> { - let pat = self.parse_pat(Some("argument name"))?; + let pat = self.parse_pat_no_top_alt(Some("argument name"))?; self.expect(&token::Colon)?; let ty = self.parse_ty()?; diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 20430ece05b06..59fd060aa2465 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1726,7 +1726,7 @@ impl<'a> Parser<'a> { let lo = self.token.span; let attrs = self.parse_outer_attributes()?; self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| { - let pat = this.parse_pat(PARAM_EXPECTED)?; + let pat = this.parse_pat_no_top_alt(PARAM_EXPECTED)?; let ty = if this.eat(&token::Colon) { this.parse_ty()? } else { @@ -1803,7 +1803,7 @@ impl<'a> Parser<'a> { /// The `let` token has already been eaten. fn parse_let_expr(&mut self, attrs: AttrVec) -> PResult<'a, P> { let lo = self.prev_token.span; - let pat = self.parse_top_pat(GateOr::No, RecoverComma::Yes)?; + let pat = self.parse_pat_allow_top_alt(None, GateOr::No, RecoverComma::Yes)?; self.expect(&token::Eq)?; let expr = self.with_res(self.restrictions | Restrictions::NO_STRUCT_LITERAL, |this| { this.parse_assoc_expr_with(1 + prec_let_scrutinee_needs_par(), None.into()) @@ -1866,7 +1866,7 @@ impl<'a> Parser<'a> { _ => None, }; - let pat = self.parse_top_pat(GateOr::Yes, RecoverComma::Yes)?; + let pat = self.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::Yes)?; if !self.eat_keyword(kw::In) { self.error_missing_in_for_loop(); } @@ -1977,7 +1977,7 @@ impl<'a> Parser<'a> { let attrs = self.parse_outer_attributes()?; self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| { let lo = this.token.span; - let pat = this.parse_top_pat(GateOr::No, RecoverComma::Yes)?; + let pat = this.parse_pat_allow_top_alt(None, GateOr::No, RecoverComma::Yes)?; let guard = if this.eat_keyword(kw::If) { let if_span = this.prev_token.span; let cond = this.parse_expr()?; diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 18013f1250bdb..ace4134b1f698 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -14,6 +14,7 @@ use crate::lexer::UnmatchedBrace; pub use attr_wrapper::AttrWrapper; pub use diagnostics::AttemptLocalParseRecovery; use diagnostics::Error; +pub use pat::{GateOr, RecoverComma}; pub use path::PathStyle; use rustc_ast::ptr::P; diff --git a/compiler/rustc_parse/src/parser/nonterminal.rs b/compiler/rustc_parse/src/parser/nonterminal.rs index 40dd938f000e3..a84ae5151442d 100644 --- a/compiler/rustc_parse/src/parser/nonterminal.rs +++ b/compiler/rustc_parse/src/parser/nonterminal.rs @@ -120,9 +120,9 @@ impl<'a> Parser<'a> { }, NonterminalKind::Pat2018 { .. } | NonterminalKind::Pat2021 { .. } => { token::NtPat(self.collect_tokens_no_attrs(|this| match kind { - NonterminalKind::Pat2018 { .. } => this.parse_pat(None), + NonterminalKind::Pat2018 { .. } => this.parse_pat_no_top_alt(None), NonterminalKind::Pat2021 { .. } => { - this.parse_top_pat(GateOr::Yes, RecoverComma::No) + this.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::No) } _ => unreachable!(), })?) diff --git a/compiler/rustc_parse/src/parser/pat.rs b/compiler/rustc_parse/src/parser/pat.rs index 317ef84742c21..8874548da784d 100644 --- a/compiler/rustc_parse/src/parser/pat.rs +++ b/compiler/rustc_parse/src/parser/pat.rs @@ -19,14 +19,14 @@ const WHILE_PARSING_OR_MSG: &str = "while parsing this or-pattern starting here" /// Whether or not an or-pattern should be gated when occurring in the current context. #[derive(PartialEq, Clone, Copy)] -pub(super) enum GateOr { +pub enum GateOr { Yes, No, } /// Whether or not to recover a `,` when parsing or-patterns. #[derive(PartialEq, Copy, Clone)] -pub(super) enum RecoverComma { +pub enum RecoverComma { Yes, No, } @@ -37,80 +37,57 @@ impl<'a> Parser<'a> { /// Corresponds to `pat` in RFC 2535 and does not admit or-patterns /// at the top level. Used when parsing the parameters of lambda expressions, /// functions, function pointers, and `pat` macro fragments. - pub fn parse_pat(&mut self, expected: Expected) -> PResult<'a, P> { + pub fn parse_pat_no_top_alt(&mut self, expected: Expected) -> PResult<'a, P> { self.parse_pat_with_range_pat(true, expected) } - /// Entry point to the main pattern parser. + /// Parses a pattern. + /// /// Corresponds to `top_pat` in RFC 2535 and allows or-pattern at the top level. - pub(super) fn parse_top_pat( + /// Used for parsing patterns in all cases when `pat` is not used. + /// + /// Note that after the FCP in , + /// a leading vert is allowed in nested or-patterns, too. This allows us to + /// simplify the grammar somewhat. + pub fn parse_pat_allow_top_alt( &mut self, + expected: Expected, gate_or: GateOr, rc: RecoverComma, ) -> PResult<'a, P> { // Allow a '|' before the pats (RFCs 1925, 2530, and 2535). - let gated_leading_vert = self.eat_or_separator(None) && gate_or == GateOr::Yes; - let leading_vert_span = self.prev_token.span; - - // Parse the possibly-or-pattern. - let pat = self.parse_pat_with_or(None, gate_or, rc)?; - - // If we parsed a leading `|` which should be gated, - // and no other gated or-pattern has been parsed thus far, - // then we should really gate the leading `|`. - // This complicated procedure is done purely for diagnostics UX. - if gated_leading_vert && self.sess.gated_spans.is_ungated(sym::or_patterns) { - self.sess.gated_spans.gate(sym::or_patterns, leading_vert_span); - } - - Ok(pat) - } - - /// Parse the pattern for a function or function pointer parameter. - /// Special recovery is provided for or-patterns and leading `|`. - pub(super) fn parse_fn_param_pat(&mut self) -> PResult<'a, P> { - self.recover_leading_vert(None, "not allowed in a parameter pattern"); - let pat = self.parse_pat_with_or(PARAM_EXPECTED, GateOr::No, RecoverComma::No)?; - - if let PatKind::Or(..) = &pat.kind { - self.ban_illegal_fn_param_or_pat(&pat); - } - - Ok(pat) - } + let leading_vert_span = + if self.eat_or_separator(None) { Some(self.prev_token.span) } else { None }; - /// Ban `A | B` immediately in a parameter pattern and suggest wrapping in parens. - fn ban_illegal_fn_param_or_pat(&self, pat: &Pat) { - let msg = "wrap the pattern in parenthesis"; - let fix = format!("({})", pprust::pat_to_string(pat)); - self.struct_span_err(pat.span, "an or-pattern parameter must be wrapped in parenthesis") - .span_suggestion(pat.span, msg, fix, Applicability::MachineApplicable) - .emit(); - } - - /// Parses a pattern, that may be a or-pattern (e.g. `Foo | Bar` in `Some(Foo | Bar)`). - /// Corresponds to `pat` in RFC 2535. - fn parse_pat_with_or( - &mut self, - expected: Expected, - gate_or: GateOr, - rc: RecoverComma, - ) -> PResult<'a, P> { // Parse the first pattern (`p_0`). - let first_pat = self.parse_pat(expected)?; + let first_pat = self.parse_pat_no_top_alt(expected)?; self.maybe_recover_unexpected_comma(first_pat.span, rc, gate_or)?; // If the next token is not a `|`, // this is not an or-pattern and we should exit here. if !self.check(&token::BinOp(token::Or)) && self.token != token::OrOr { + // If we parsed a leading `|` which should be gated, + // then we should really gate the leading `|`. + // This complicated procedure is done purely for diagnostics UX. + if let Some(leading_vert_span) = leading_vert_span { + if gate_or == GateOr::Yes && self.sess.gated_spans.is_ungated(sym::or_patterns) { + self.sess.gated_spans.gate(sym::or_patterns, leading_vert_span); + } + + // If there was a leading vert, treat this as an or-pattern. This improves + // diagnostics. + let span = leading_vert_span.to(self.prev_token.span); + return Ok(self.mk_pat(span, PatKind::Or(vec![first_pat]))); + } + return Ok(first_pat); } // Parse the patterns `p_1 | ... | p_n` where `n > 0`. - let lo = first_pat.span; + let lo = leading_vert_span.unwrap_or(first_pat.span); let mut pats = vec![first_pat]; while self.eat_or_separator(Some(lo)) { - let pat = self.parse_pat(expected).map_err(|mut err| { + let pat = self.parse_pat_no_top_alt(expected).map_err(|mut err| { err.span_label(lo, WHILE_PARSING_OR_MSG); err })?; @@ -127,6 +104,62 @@ impl<'a> Parser<'a> { Ok(self.mk_pat(or_pattern_span, PatKind::Or(pats))) } + /// Parse the pattern for a function or function pointer parameter. + pub(super) fn parse_fn_param_pat(&mut self) -> PResult<'a, P> { + // We actually do _not_ allow top-level or-patterns in function params, but we use + // `parse_pat_allow_top_alt` anyway so that we can detect when a user tries to use it. This + // allows us to print a better error message. + // + // In order to get good UX, we first recover in the case of a leading vert for an illegal + // top-level or-pat. Normally, this means recovering both `|` and `||`, but in this case, + // a leading `||` probably doesn't indicate an or-pattern attempt, so we handle that + // separately. + if let token::OrOr = self.token.kind { + let span = self.token.span; + let mut err = self.struct_span_err(span, "unexpected `||` before function parameter"); + err.span_suggestion( + span, + "remove the `||`", + String::new(), + Applicability::MachineApplicable, + ); + err.note("alternatives in or-patterns are separated with `|`, not `||`"); + err.emit(); + self.bump(); + } + + let pat = self.parse_pat_allow_top_alt(PARAM_EXPECTED, GateOr::No, RecoverComma::No)?; + + if let PatKind::Or(..) = &pat.kind { + self.ban_illegal_fn_param_or_pat(&pat); + } + + Ok(pat) + } + + /// Ban `A | B` immediately in a parameter pattern and suggest wrapping in parens. + fn ban_illegal_fn_param_or_pat(&self, pat: &Pat) { + // If all we have a leading vert, then print a special message. This is the case if + // `parse_pat_allow_top_alt` returns an or-pattern with one variant. + let (msg, fix) = match &pat.kind { + PatKind::Or(pats) if pats.len() == 1 => { + let msg = "remove the leading `|`"; + let fix = pprust::pat_to_string(pat); + (msg, fix) + } + + _ => { + let msg = "wrap the pattern in parentheses"; + let fix = format!("({})", pprust::pat_to_string(pat)); + (msg, fix) + } + }; + + self.struct_span_err(pat.span, "an or-pattern parameter must be wrapped in parentheses") + .span_suggestion(pat.span, msg, fix, Applicability::MachineApplicable) + .emit(); + } + /// Eat the or-pattern `|` separator. /// If instead a `||` token is encountered, recover and pretend we parsed `|`. fn eat_or_separator(&mut self, lo: Option) -> bool { @@ -179,7 +212,7 @@ impl<'a> Parser<'a> { /// We have parsed `||` instead of `|`. Error and suggest `|` instead. fn ban_unexpected_or_or(&mut self, lo: Option) { - let mut err = self.struct_span_err(self.token.span, "unexpected token `||` after pattern"); + let mut err = self.struct_span_err(self.token.span, "unexpected token `||` in pattern"); err.span_suggestion( self.token.span, "use a single `|` to separate multiple alternative patterns", @@ -244,7 +277,7 @@ impl<'a> Parser<'a> { /// sequence of patterns until `)` is reached. fn skip_pat_list(&mut self) -> PResult<'a, ()> { while !self.check(&token::CloseDelim(token::Paren)) { - self.parse_pat(None)?; + self.parse_pat_no_top_alt(None)?; if !self.eat(&token::Comma) { return Ok(()); } @@ -252,22 +285,6 @@ impl<'a> Parser<'a> { Ok(()) } - /// Recursive possibly-or-pattern parser with recovery for an erroneous leading `|`. - /// See `parse_pat_with_or` for details on parsing or-patterns. - fn parse_pat_with_or_inner(&mut self) -> PResult<'a, P> { - self.recover_leading_vert(None, "only allowed in a top-level pattern"); - self.parse_pat_with_or(None, GateOr::Yes, RecoverComma::No) - } - - /// Recover if `|` or `||` is here. - /// The user is thinking that a leading `|` is allowed in this position. - fn recover_leading_vert(&mut self, lo: Option, ctx: &str) { - if let token::BinOp(token::Or) | token::OrOr = self.token.kind { - self.ban_illegal_vert(lo, "leading", ctx); - self.bump(); - } - } - /// A `|` or possibly `||` token shouldn't be here. Ban it. fn ban_illegal_vert(&mut self, lo: Option, pos: &str, ctx: &str) { let span = self.token.span; @@ -305,8 +322,9 @@ impl<'a> Parser<'a> { self.parse_pat_tuple_or_parens()? } else if self.check(&token::OpenDelim(token::Bracket)) { // Parse `[pat, pat,...]` as a slice pattern. - let (pats, _) = - self.parse_delim_comma_seq(token::Bracket, |p| p.parse_pat_with_or_inner())?; + let (pats, _) = self.parse_delim_comma_seq(token::Bracket, |p| { + p.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::No) + })?; PatKind::Slice(pats) } else if self.check(&token::DotDot) && !self.is_pat_range_end_start(1) { // A rest pattern `..`. @@ -429,7 +447,7 @@ impl<'a> Parser<'a> { // At this point we attempt to parse `@ $pat_rhs` and emit an error. self.bump(); // `@` - let mut rhs = self.parse_pat(None)?; + let mut rhs = self.parse_pat_no_top_alt(None)?; let sp = lhs.span.to(rhs.span); if let PatKind::Ident(_, _, ref mut sub @ None) = rhs.kind { @@ -518,8 +536,9 @@ impl<'a> Parser<'a> { /// 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_with_or_inner())?; + let (fields, trailing_comma) = self.parse_paren_comma_seq(|p| { + p.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::No) + })?; // Here, `(pat,)` is a tuple pattern. // For backward compatibility, `(..)` is a tuple pattern as well. @@ -548,7 +567,7 @@ impl<'a> Parser<'a> { } // Parse the pattern we hope to be an identifier. - let mut pat = self.parse_pat(Some("identifier"))?; + let mut pat = self.parse_pat_no_top_alt(Some("identifier"))?; // If we don't have `mut $ident (@ pat)?`, error. if let PatKind::Ident(BindingMode::ByValue(m @ Mutability::Not), ..) = &mut pat.kind { @@ -793,7 +812,7 @@ impl<'a> Parser<'a> { 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"))?) + Some(self.parse_pat_no_top_alt(Some("binding pattern"))?) } else { None }; @@ -832,7 +851,9 @@ impl<'a> Parser<'a> { if qself.is_some() { return self.error_qpath_before_pat(&path, "("); } - let (fields, _) = self.parse_paren_comma_seq(|p| p.parse_pat_with_or_inner())?; + let (fields, _) = self.parse_paren_comma_seq(|p| { + p.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::No) + })?; Ok(PatKind::TupleStruct(path, fields)) } @@ -998,7 +1019,7 @@ impl<'a> Parser<'a> { // Parsing a pattern of the form `fieldname: pat`. let fieldname = self.parse_field_name()?; self.bump(); - let pat = self.parse_pat_with_or_inner()?; + let pat = self.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::No)?; hi = pat.span; (pat, fieldname, false) } else { diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index e36ebd5e48113..2e00ddfaacc81 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -220,7 +220,7 @@ impl<'a> Parser<'a> { /// Parses a local variable declaration. fn parse_local(&mut self, attrs: AttrVec) -> PResult<'a, P> { let lo = self.prev_token.span; - let pat = self.parse_top_pat(GateOr::Yes, RecoverComma::Yes)?; + let pat = self.parse_pat_allow_top_alt(None, GateOr::Yes, RecoverComma::Yes)?; let (err, ty) = if self.eat(&token::Colon) { // Save the state of the parser before parsing type normally, in case there is a `:` diff --git a/src/test/ui/or-patterns/feature-gate-or_patterns.stderr b/src/test/ui/or-patterns/feature-gate-or_patterns.stderr index a9e43a4575da0..c01d17c0b3d7e 100644 --- a/src/test/ui/or-patterns/feature-gate-or_patterns.stderr +++ b/src/test/ui/or-patterns/feature-gate-or_patterns.stderr @@ -8,10 +8,10 @@ LL | Some(0 | 1 | 2) => {} = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:28:11 + --> $DIR/feature-gate-or_patterns.rs:28:9 | LL | let | A | B; - | ^^^^^ + | ^^^^^^^ | = note: see issue #54883 for more information = help: add `#![feature(or_patterns)]` to the crate attributes to enable @@ -26,10 +26,10 @@ LL | let A | B; = help: add `#![feature(or_patterns)]` to the crate attributes to enable error[E0658]: or-patterns syntax is experimental - --> $DIR/feature-gate-or_patterns.rs:30:11 + --> $DIR/feature-gate-or_patterns.rs:30:9 | LL | for | A | B in 0 {} - | ^^^^^ + | ^^^^^^^ | = note: see issue #54883 for more information = help: add `#![feature(or_patterns)]` to the crate attributes to enable diff --git a/src/test/ui/or-patterns/fn-param-wrap-parens.fixed b/src/test/ui/or-patterns/fn-param-wrap-parens.fixed index 08730fe8b07b7..bbc75d2b411eb 100644 --- a/src/test/ui/or-patterns/fn-param-wrap-parens.fixed +++ b/src/test/ui/or-patterns/fn-param-wrap-parens.fixed @@ -11,4 +11,4 @@ enum E { A, B } use E::*; #[cfg(FALSE)] -fn fun1((A | B): E) {} //~ ERROR an or-pattern parameter must be wrapped in parenthesis +fn fun1((A | B): E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses diff --git a/src/test/ui/or-patterns/fn-param-wrap-parens.rs b/src/test/ui/or-patterns/fn-param-wrap-parens.rs index ed667e0e66067..65b93dcbf7467 100644 --- a/src/test/ui/or-patterns/fn-param-wrap-parens.rs +++ b/src/test/ui/or-patterns/fn-param-wrap-parens.rs @@ -11,4 +11,4 @@ enum E { A, B } use E::*; #[cfg(FALSE)] -fn fun1(A | B: E) {} //~ ERROR an or-pattern parameter must be wrapped in parenthesis +fn fun1(A | B: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses diff --git a/src/test/ui/or-patterns/fn-param-wrap-parens.stderr b/src/test/ui/or-patterns/fn-param-wrap-parens.stderr index 2c6e4d9838ddc..0e6424a430043 100644 --- a/src/test/ui/or-patterns/fn-param-wrap-parens.stderr +++ b/src/test/ui/or-patterns/fn-param-wrap-parens.stderr @@ -1,8 +1,8 @@ -error: an or-pattern parameter must be wrapped in parenthesis +error: an or-pattern parameter must be wrapped in parentheses --> $DIR/fn-param-wrap-parens.rs:14:9 | LL | fn fun1(A | B: E) {} - | ^^^^^ help: wrap the pattern in parenthesis: `(A | B)` + | ^^^^^ help: wrap the pattern in parentheses: `(A | B)` error: aborting due to previous error diff --git a/src/test/ui/or-patterns/multiple-pattern-typo.rs b/src/test/ui/or-patterns/multiple-pattern-typo.rs index 702c9573741e7..4d06101044f6c 100644 --- a/src/test/ui/or-patterns/multiple-pattern-typo.rs +++ b/src/test/ui/or-patterns/multiple-pattern-typo.rs @@ -4,41 +4,41 @@ fn main() { let x = 3; match x { - 1 | 2 || 3 => (), //~ ERROR unexpected token `||` after pattern + 1 | 2 || 3 => (), //~ ERROR unexpected token `||` in pattern _ => (), } match x { - (1 | 2 || 3) => (), //~ ERROR unexpected token `||` after pattern + (1 | 2 || 3) => (), //~ ERROR unexpected token `||` in pattern _ => (), } match (x,) { - (1 | 2 || 3,) => (), //~ ERROR unexpected token `||` after pattern + (1 | 2 || 3,) => (), //~ ERROR unexpected token `||` in pattern _ => (), } struct TS(u8); match TS(x) { - TS(1 | 2 || 3) => (), //~ ERROR unexpected token `||` after pattern + TS(1 | 2 || 3) => (), //~ ERROR unexpected token `||` in pattern _ => (), } struct NS { f: u8 } match (NS { f: x }) { - NS { f: 1 | 2 || 3 } => (), //~ ERROR unexpected token `||` after pattern + NS { f: 1 | 2 || 3 } => (), //~ ERROR unexpected token `||` in pattern _ => (), } match [x] { - [1 | 2 || 3] => (), //~ ERROR unexpected token `||` after pattern + [1 | 2 || 3] => (), //~ ERROR unexpected token `||` in pattern _ => (), } match x { - || 1 | 2 | 3 => (), //~ ERROR unexpected token `||` after pattern + || 1 | 2 | 3 => (), //~ ERROR unexpected token `||` in pattern _ => (), } } diff --git a/src/test/ui/or-patterns/multiple-pattern-typo.stderr b/src/test/ui/or-patterns/multiple-pattern-typo.stderr index cb32068ec0d5e..b0a82b3673b83 100644 --- a/src/test/ui/or-patterns/multiple-pattern-typo.stderr +++ b/src/test/ui/or-patterns/multiple-pattern-typo.stderr @@ -1,4 +1,4 @@ -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/multiple-pattern-typo.rs:7:15 | LL | 1 | 2 || 3 => (), @@ -6,7 +6,7 @@ LL | 1 | 2 || 3 => (), | | | while parsing this or-pattern starting here -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/multiple-pattern-typo.rs:12:16 | LL | (1 | 2 || 3) => (), @@ -14,7 +14,7 @@ LL | (1 | 2 || 3) => (), | | | while parsing this or-pattern starting here -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/multiple-pattern-typo.rs:17:16 | LL | (1 | 2 || 3,) => (), @@ -22,7 +22,7 @@ LL | (1 | 2 || 3,) => (), | | | while parsing this or-pattern starting here -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/multiple-pattern-typo.rs:24:18 | LL | TS(1 | 2 || 3) => (), @@ -30,7 +30,7 @@ LL | TS(1 | 2 || 3) => (), | | | while parsing this or-pattern starting here -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/multiple-pattern-typo.rs:31:23 | LL | NS { f: 1 | 2 || 3 } => (), @@ -38,7 +38,7 @@ LL | NS { f: 1 | 2 || 3 } => (), | | | while parsing this or-pattern starting here -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/multiple-pattern-typo.rs:36:16 | LL | [1 | 2 || 3] => (), @@ -46,7 +46,7 @@ LL | [1 | 2 || 3] => (), | | | while parsing this or-pattern starting here -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/multiple-pattern-typo.rs:41:9 | LL | || 1 | 2 | 3 => (), diff --git a/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs b/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs index efe90b3e3c60c..cbc24eb26fa47 100644 --- a/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs +++ b/src/test/ui/or-patterns/or-patterns-syntactic-fail.rs @@ -14,29 +14,8 @@ fn no_top_level_or_patterns() { // -------- This looks like an or-pattern but is in fact `|A| (B: E | ())`. // ...and for now neither do we allow or-patterns at the top level of functions. - fn fun1(A | B: E) {} //~ ERROR an or-pattern parameter must be wrapped in parenthesis + fn fun1(A | B: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses fn fun2(| A | B: E) {} - //~^ ERROR a leading `|` is not allowed in a parameter pattern - //~| ERROR an or-pattern parameter must be wrapped in parenthesis -} - -// We also do not allow a leading `|` when not in a top level position: - -fn no_leading_inner() { - struct TS(E); - struct NS { f: E } - - let ( | A | B) = E::A; //~ ERROR a leading `|` is only allowed in a top-level pattern - let ( | A | B,) = (E::B,); //~ ERROR a leading `|` is only allowed in a top-level pattern - let [ | A | B ] = [E::A]; //~ ERROR a leading `|` is only allowed in a top-level pattern - let TS( | A | B ); //~ ERROR a leading `|` is only allowed in a top-level pattern - let NS { f: | A | B }; //~ ERROR a leading `|` is only allowed in a top-level pattern - - let ( || A | B) = E::A; //~ ERROR a leading `|` is only allowed in a top-level pattern - let [ || A | B ] = [E::A]; //~ ERROR a leading `|` is only allowed in a top-level pattern - let TS( || A | B ); //~ ERROR a leading `|` is only allowed in a top-level pattern - let NS { f: || A | B }; //~ ERROR a leading `|` is only allowed in a top-level pattern - - let recovery_witness: String = 0; //~ ERROR mismatched types + //~^ ERROR an or-pattern parameter must be wrapped in parentheses } diff --git a/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr b/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr index 989aeb5200645..db4d827757b03 100644 --- a/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr +++ b/src/test/ui/or-patterns/or-patterns-syntactic-fail.stderr @@ -1,82 +1,14 @@ -error: an or-pattern parameter must be wrapped in parenthesis +error: an or-pattern parameter must be wrapped in parentheses --> $DIR/or-patterns-syntactic-fail.rs:17:13 | LL | fn fun1(A | B: E) {} - | ^^^^^ help: wrap the pattern in parenthesis: `(A | B)` + | ^^^^^ help: wrap the pattern in parentheses: `(A | B)` -error: a leading `|` is not allowed in a parameter pattern +error: an or-pattern parameter must be wrapped in parentheses --> $DIR/or-patterns-syntactic-fail.rs:19:13 | LL | fn fun2(| A | B: E) {} - | ^ help: remove the `|` - -error: an or-pattern parameter must be wrapped in parenthesis - --> $DIR/or-patterns-syntactic-fail.rs:19:15 - | -LL | fn fun2(| A | B: E) {} - | ^^^^^ help: wrap the pattern in parenthesis: `(A | B)` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:30:11 - | -LL | let ( | A | B) = E::A; - | ^ help: remove the `|` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:31:11 - | -LL | let ( | A | B,) = (E::B,); - | ^ help: remove the `|` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:32:11 - | -LL | let [ | A | B ] = [E::A]; - | ^ help: remove the `|` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:33:13 - | -LL | let TS( | A | B ); - | ^ help: remove the `|` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:34:17 - | -LL | let NS { f: | A | B }; - | ^ help: remove the `|` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:36:11 - | -LL | let ( || A | B) = E::A; - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:37:11 - | -LL | let [ || A | B ] = [E::A]; - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:38:13 - | -LL | let TS( || A | B ); - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/or-patterns-syntactic-fail.rs:39:17 - | -LL | let NS { f: || A | B }; - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` + | ^^^^^^^ help: wrap the pattern in parentheses: `(A | B)` error[E0369]: no implementation for `E | ()` --> $DIR/or-patterns-syntactic-fail.rs:13:22 @@ -88,17 +20,6 @@ LL | let _ = |A | B: E| (); | = note: an implementation of `std::ops::BitOr` might be missing for `E` -error[E0308]: mismatched types - --> $DIR/or-patterns-syntactic-fail.rs:41:36 - | -LL | let recovery_witness: String = 0; - | ------ ^ - | | | - | | expected struct `String`, found integer - | | help: try using a conversion method: `0.to_string()` - | expected due to this - -error: aborting due to 14 previous errors +error: aborting due to 3 previous errors -Some errors have detailed explanations: E0308, E0369. -For more information about an error, try `rustc --explain E0308`. +For more information about this error, try `rustc --explain E0369`. diff --git a/src/test/ui/or-patterns/remove-leading-vert.fixed b/src/test/ui/or-patterns/remove-leading-vert.fixed index 443ef398293d9..c8fac4faa2a66 100644 --- a/src/test/ui/or-patterns/remove-leading-vert.fixed +++ b/src/test/ui/or-patterns/remove-leading-vert.fixed @@ -9,17 +9,17 @@ fn main() {} #[cfg(FALSE)] fn leading() { - fn fun1( A: E) {} //~ ERROR a leading `|` is not allowed in a parameter pattern - fn fun2( A: E) {} //~ ERROR a leading `|` is not allowed in a parameter pattern - let ( A): E; //~ ERROR a leading `|` is only allowed in a top-level pattern - let ( A): (E); //~ ERROR a leading `|` is only allowed in a top-level pattern - let ( A,): (E,); //~ ERROR a leading `|` is only allowed in a top-level pattern - let [ A ]: [E; 1]; //~ ERROR a leading `|` is only allowed in a top-level pattern - let [ A ]: [E; 1]; //~ ERROR a leading `|` is only allowed in a top-level pattern - let TS( A ): TS; //~ ERROR a leading `|` is only allowed in a top-level pattern - let TS( A ): TS; //~ ERROR a leading `|` is only allowed in a top-level pattern - let NS { f: A }: NS; //~ ERROR a leading `|` is only allowed in a top-level pattern - let NS { f: A }: NS; //~ ERROR a leading `|` is only allowed in a top-level pattern + fn fun1( A: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses + fn fun2( A: E) {} //~ ERROR unexpected `||` before function parameter + let ( | A): E; + let ( | A): (E); //~ ERROR unexpected token `||` in pattern + let ( | A,): (E,); + let [ | A ]: [E; 1]; + let [ | A ]: [E; 1]; //~ ERROR unexpected token `||` in pattern + let TS( | A ): TS; + let TS( | A ): TS; //~ ERROR unexpected token `||` in pattern + let NS { f: | A }: NS; + let NS { f: | A }: NS; //~ ERROR unexpected token `||` in pattern } #[cfg(FALSE)] @@ -29,12 +29,12 @@ fn trailing() { let ( A | B ): E; //~ ERROR a trailing `|` is not allowed in an or-pattern let [ A | B ]: [E; 1]; //~ ERROR a trailing `|` is not allowed in an or-pattern let S { f: B }; //~ ERROR a trailing `|` is not allowed in an or-pattern - let ( A | B ): E; //~ ERROR unexpected token `||` after pattern + let ( A | B ): E; //~ ERROR unexpected token `||` in pattern //~^ ERROR a trailing `|` is not allowed in an or-pattern match A { A => {} //~ ERROR a trailing `|` is not allowed in an or-pattern A => {} //~ ERROR a trailing `|` is not allowed in an or-pattern - A | B => {} //~ ERROR unexpected token `||` after pattern + A | B => {} //~ ERROR unexpected token `||` in pattern //~^ ERROR a trailing `|` is not allowed in an or-pattern | A | B => {} //~^ ERROR a trailing `|` is not allowed in an or-pattern diff --git a/src/test/ui/or-patterns/remove-leading-vert.rs b/src/test/ui/or-patterns/remove-leading-vert.rs index 3c427a6f7b23e..2cf6b27ab1aac 100644 --- a/src/test/ui/or-patterns/remove-leading-vert.rs +++ b/src/test/ui/or-patterns/remove-leading-vert.rs @@ -9,17 +9,17 @@ fn main() {} #[cfg(FALSE)] fn leading() { - fn fun1( | A: E) {} //~ ERROR a leading `|` is not allowed in a parameter pattern - fn fun2( || A: E) {} //~ ERROR a leading `|` is not allowed in a parameter pattern - let ( | A): E; //~ ERROR a leading `|` is only allowed in a top-level pattern - let ( || A): (E); //~ ERROR a leading `|` is only allowed in a top-level pattern - let ( | A,): (E,); //~ ERROR a leading `|` is only allowed in a top-level pattern - let [ | A ]: [E; 1]; //~ ERROR a leading `|` is only allowed in a top-level pattern - let [ || A ]: [E; 1]; //~ ERROR a leading `|` is only allowed in a top-level pattern - let TS( | A ): TS; //~ ERROR a leading `|` is only allowed in a top-level pattern - let TS( || A ): TS; //~ ERROR a leading `|` is only allowed in a top-level pattern - let NS { f: | A }: NS; //~ ERROR a leading `|` is only allowed in a top-level pattern - let NS { f: || A }: NS; //~ ERROR a leading `|` is only allowed in a top-level pattern + fn fun1( | A: E) {} //~ ERROR an or-pattern parameter must be wrapped in parentheses + fn fun2( || A: E) {} //~ ERROR unexpected `||` before function parameter + let ( | A): E; + let ( || A): (E); //~ ERROR unexpected token `||` in pattern + let ( | A,): (E,); + let [ | A ]: [E; 1]; + let [ || A ]: [E; 1]; //~ ERROR unexpected token `||` in pattern + let TS( | A ): TS; + let TS( || A ): TS; //~ ERROR unexpected token `||` in pattern + let NS { f: | A }: NS; + let NS { f: || A }: NS; //~ ERROR unexpected token `||` in pattern } #[cfg(FALSE)] @@ -29,12 +29,12 @@ fn trailing() { let ( A | B | ): E; //~ ERROR a trailing `|` is not allowed in an or-pattern let [ A | B | ]: [E; 1]; //~ ERROR a trailing `|` is not allowed in an or-pattern let S { f: B | }; //~ ERROR a trailing `|` is not allowed in an or-pattern - let ( A || B | ): E; //~ ERROR unexpected token `||` after pattern + let ( A || B | ): E; //~ ERROR unexpected token `||` in pattern //~^ ERROR a trailing `|` is not allowed in an or-pattern match A { A | => {} //~ ERROR a trailing `|` is not allowed in an or-pattern A || => {} //~ ERROR a trailing `|` is not allowed in an or-pattern - A || B | => {} //~ ERROR unexpected token `||` after pattern + A || B | => {} //~ ERROR unexpected token `||` in pattern //~^ ERROR a trailing `|` is not allowed in an or-pattern | A | B | => {} //~^ ERROR a trailing `|` is not allowed in an or-pattern diff --git a/src/test/ui/or-patterns/remove-leading-vert.stderr b/src/test/ui/or-patterns/remove-leading-vert.stderr index 53025230a63c2..5c9efd44a187f 100644 --- a/src/test/ui/or-patterns/remove-leading-vert.stderr +++ b/src/test/ui/or-patterns/remove-leading-vert.stderr @@ -1,10 +1,10 @@ -error: a leading `|` is not allowed in a parameter pattern +error: an or-pattern parameter must be wrapped in parentheses --> $DIR/remove-leading-vert.rs:12:14 | LL | fn fun1( | A: E) {} - | ^ help: remove the `|` + | ^^^ help: remove the leading `|`: `A` -error: a leading `|` is not allowed in a parameter pattern +error: unexpected `||` before function parameter --> $DIR/remove-leading-vert.rs:13:14 | LL | fn fun2( || A: E) {} @@ -12,67 +12,29 @@ LL | fn fun2( || A: E) {} | = note: alternatives in or-patterns are separated with `|`, not `||` -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/remove-leading-vert.rs:14:11 - | -LL | let ( | A): E; - | ^ help: remove the `|` - -error: a leading `|` is only allowed in a top-level pattern +error: unexpected token `||` in pattern --> $DIR/remove-leading-vert.rs:15:11 | LL | let ( || A): (E); - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/remove-leading-vert.rs:16:11 - | -LL | let ( | A,): (E,); - | ^ help: remove the `|` + | ^^ help: use a single `|` to separate multiple alternative patterns: `|` -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/remove-leading-vert.rs:17:11 - | -LL | let [ | A ]: [E; 1]; - | ^ help: remove the `|` - -error: a leading `|` is only allowed in a top-level pattern +error: unexpected token `||` in pattern --> $DIR/remove-leading-vert.rs:18:11 | LL | let [ || A ]: [E; 1]; - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/remove-leading-vert.rs:19:13 - | -LL | let TS( | A ): TS; - | ^ help: remove the `|` + | ^^ help: use a single `|` to separate multiple alternative patterns: `|` -error: a leading `|` is only allowed in a top-level pattern +error: unexpected token `||` in pattern --> $DIR/remove-leading-vert.rs:20:13 | LL | let TS( || A ): TS; - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` - -error: a leading `|` is only allowed in a top-level pattern - --> $DIR/remove-leading-vert.rs:21:17 - | -LL | let NS { f: | A }: NS; - | ^ help: remove the `|` + | ^^ help: use a single `|` to separate multiple alternative patterns: `|` -error: a leading `|` is only allowed in a top-level pattern +error: unexpected token `||` in pattern --> $DIR/remove-leading-vert.rs:22:17 | LL | let NS { f: || A }: NS; - | ^^ help: remove the `||` - | - = note: alternatives in or-patterns are separated with `|`, not `||` + | ^^ help: use a single `|` to separate multiple alternative patterns: `|` error: a trailing `|` is not allowed in an or-pattern --> $DIR/remove-leading-vert.rs:27:13 @@ -114,7 +76,7 @@ LL | let S { f: B | }; | | | while parsing this or-pattern starting here -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/remove-leading-vert.rs:32:13 | LL | let ( A || B | ): E; @@ -148,7 +110,7 @@ LL | A || => {} | = note: alternatives in or-patterns are separated with `|`, not `||` -error: unexpected token `||` after pattern +error: unexpected token `||` in pattern --> $DIR/remove-leading-vert.rs:37:11 | LL | A || B | => {} @@ -168,9 +130,9 @@ error: a trailing `|` is not allowed in an or-pattern --> $DIR/remove-leading-vert.rs:39:17 | LL | | A | B | => {} - | - ^ help: remove the `|` - | | - | while parsing this or-pattern starting here + | - ^ help: remove the `|` + | | + | while parsing this or-pattern starting here error: a trailing `|` is not allowed in an or-pattern --> $DIR/remove-leading-vert.rs:43:11 @@ -196,5 +158,5 @@ LL | let a | ; | | | while parsing this or-pattern starting here -error: aborting due to 26 previous errors +error: aborting due to 21 previous errors From b86c5db96eecbf6f4b08cd3f29d5a5b4dae13aee Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Thu, 4 Feb 2021 02:12:31 -0500 Subject: [PATCH 14/22] Implement reborrow for closure captures --- compiler/rustc_typeck/src/check/upvar.rs | 103 +++++++++++------- .../2229_closure_analysis/by_value.rs | 3 +- .../2229_closure_analysis/by_value.stderr | 11 +- .../2229_closure_analysis/move_closure.rs | 8 +- .../2229_closure_analysis/move_closure.stderr | 8 +- .../run_pass/move_closure.rs | 41 +++++++ 6 files changed, 121 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index d097f863ad2f9..79c60ab3cfe0f 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -180,7 +180,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { debug!("seed place {:?}", place); let upvar_id = ty::UpvarId::new(*var_hir_id, local_def_id); - let capture_kind = self.init_capture_kind(capture_clause, upvar_id, span); + let capture_kind = + self.init_capture_kind_for_place(&place, capture_clause, upvar_id, span); let fake_info = ty::CaptureInfo { capture_kind_expr_id: None, path_expr_id: None, @@ -449,7 +450,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { base => bug!("Expected upvar, found={:?}", base), }; - let place = restrict_capture_precision(place, capture_info.capture_kind); + let place = restrict_capture_precision(place); let min_cap_list = match root_var_min_capture_list.get_mut(&var_hir_id) { None => { @@ -897,15 +898,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - fn init_capture_kind( + fn init_capture_kind_for_place( &self, + place: &Place<'tcx>, capture_clause: hir::CaptureBy, upvar_id: ty::UpvarId, closure_span: Span, ) -> ty::UpvarCapture<'tcx> { match capture_clause { - hir::CaptureBy::Value => ty::UpvarCapture::ByValue(None), - hir::CaptureBy::Ref => { + // In case of a move closure if the data is accessed through a reference we + // want to capture by ref to allow precise capture using reborrows. + // + // If the data will be moved out of this place, then the place will be truncated + // at the first Deref in `adjust_upvar_borrow_kind_for_consume` and then moved into + // the closure. + hir::CaptureBy::Value if !place.deref_tys().any(ty::TyS::is_ref) => { + ty::UpvarCapture::ByValue(None) + } + hir::CaptureBy::Value | hir::CaptureBy::Ref => { let origin = UpvarRegion(upvar_id, closure_span); let upvar_region = self.next_region_var(origin); let upvar_borrow = ty::UpvarBorrow { kind: ty::ImmBorrow, region: upvar_region }; @@ -1109,12 +1119,18 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { place_with_id, diag_expr_id, mode ); - // we only care about moves - match mode { - euv::Copy => { - return; - } - euv::Move => {} + let place = truncate_capture_for_move(place_with_id.place.clone()); + match (self.capture_clause, mode) { + // In non-move closures, we only care about moves + (hir::CaptureBy::Ref, euv::Copy) => return, + + (hir::CaptureBy::Ref, euv::Move) | (hir::CaptureBy::Value, euv::Move | euv::Copy) => {} + }; + + let place_with_id = PlaceWithHirId { place: place.clone(), hir_id: place_with_id.hir_id }; + + if !self.capture_information.contains_key(&place) { + self.init_capture_info_for_place(&place_with_id, diag_expr_id); } let tcx = self.fcx.tcx; @@ -1128,13 +1144,15 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { let usage_span = tcx.hir().span(diag_expr_id); - // To move out of an upvar, this must be a FnOnce closure - self.adjust_closure_kind( - upvar_id.closure_expr_id, - ty::ClosureKind::FnOnce, - usage_span, - place_with_id.place.clone(), - ); + if matches!(mode, euv::Move) { + // To move out of an upvar, this must be a FnOnce closure + self.adjust_closure_kind( + upvar_id.closure_expr_id, + ty::ClosureKind::FnOnce, + usage_span, + place.clone(), + ); + } let capture_info = ty::CaptureInfo { capture_kind_expr_id: Some(diag_expr_id), @@ -1317,8 +1335,12 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { if let PlaceBase::Upvar(upvar_id) = place_with_id.place.base { assert_eq!(self.closure_def_id.expect_local(), upvar_id.closure_expr_id); - let capture_kind = - self.fcx.init_capture_kind(self.capture_clause, upvar_id, self.closure_span); + let capture_kind = self.fcx.init_capture_kind_for_place( + &place_with_id.place, + self.capture_clause, + upvar_id, + self.closure_span, + ); let expr_id = Some(diag_expr_id); let capture_info = ty::CaptureInfo { @@ -1392,15 +1414,10 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> { } /// Truncate projections so that following rules are obeyed by the captured `place`: -/// -/// - No Derefs in move closure, this will result in value behind a reference getting moved. /// - No projections are applied to raw pointers, since these require unsafe blocks. We capture /// them completely. /// - No Index projections are captured, since arrays are captured completely. -fn restrict_capture_precision<'tcx>( - mut place: Place<'tcx>, - capture_kind: ty::UpvarCapture<'tcx>, -) -> Place<'tcx> { +fn restrict_capture_precision<'tcx>(mut place: Place<'tcx>) -> Place<'tcx> { if place.projections.is_empty() { // Nothing to do here return place; @@ -1412,7 +1429,6 @@ fn restrict_capture_precision<'tcx>( } let mut truncated_length = usize::MAX; - let mut first_deref_projection = usize::MAX; for (i, proj) in place.projections.iter().enumerate() { if proj.ty.is_unsafe_ptr() { @@ -1426,31 +1442,36 @@ fn restrict_capture_precision<'tcx>( truncated_length = truncated_length.min(i); break; } - ProjectionKind::Deref => { - // We only drop Derefs in case of move closures - // There might be an index projection or raw ptr ahead, so we don't stop here. - first_deref_projection = first_deref_projection.min(i); - } + ProjectionKind::Deref => {} ProjectionKind::Field(..) => {} // ignore ProjectionKind::Subslice => {} // We never capture this } } - let length = place - .projections - .len() - .min(truncated_length) - // In case of capture `ByValue` we want to not capture derefs - .min(match capture_kind { - ty::UpvarCapture::ByValue(..) => first_deref_projection, - ty::UpvarCapture::ByRef(..) => usize::MAX, - }); + let length = place.projections.len().min(truncated_length); place.projections.truncate(length); place } +/// Truncates a place so that the resultant capture doesn't move data out of a reference +fn truncate_capture_for_move(mut place: Place<'tcx>) -> Place<'tcx> { + for (i, proj) in place.projections.iter().enumerate() { + match proj.kind { + ProjectionKind::Deref => { + // We only drop Derefs in case of move closures + // There might be an index projection or raw ptr ahead, so we don't stop here. + place.projections.truncate(i); + return place; + } + _ => {} + } + } + + place +} + fn construct_place_string(tcx: TyCtxt<'_>, place: &Place<'tcx>) -> String { let variable_name = match place.base { PlaceBase::Upvar(upvar_id) => var_name(tcx, upvar_id.var_path.hir_id).to_string(), diff --git a/src/test/ui/closures/2229_closure_analysis/by_value.rs b/src/test/ui/closures/2229_closure_analysis/by_value.rs index 1007fb582e5ed..27c8fb1363f17 100644 --- a/src/test/ui/closures/2229_closure_analysis/by_value.rs +++ b/src/test/ui/closures/2229_closure_analysis/by_value.rs @@ -26,7 +26,8 @@ fn big_box() { //~^ First Pass analysis includes: //~| Min Capture analysis includes: let p = t.0.0; - //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue + //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + //~| NOTE: Capturing t[(0, 0)] -> ByValue //~| NOTE: Min Capture t[(0, 0)] -> ByValue println!("{} {:?}", t.1, p); //~^ NOTE: Capturing t[(1, 0)] -> ImmBorrow diff --git a/src/test/ui/closures/2229_closure_analysis/by_value.stderr b/src/test/ui/closures/2229_closure_analysis/by_value.stderr index fe04dbef6d8b5..944e4c40a78ef 100644 --- a/src/test/ui/closures/2229_closure_analysis/by_value.stderr +++ b/src/test/ui/closures/2229_closure_analysis/by_value.stderr @@ -28,13 +28,18 @@ LL | | LL | | }; | |_____^ | -note: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue +note: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + --> $DIR/by_value.rs:28:17 + | +LL | let p = t.0.0; + | ^^^^^ +note: Capturing t[(0, 0)] -> ByValue --> $DIR/by_value.rs:28:17 | LL | let p = t.0.0; | ^^^^^ note: Capturing t[(1, 0)] -> ImmBorrow - --> $DIR/by_value.rs:31:29 + --> $DIR/by_value.rs:32:29 | LL | println!("{} {:?}", t.1, p); | ^^^ @@ -57,7 +62,7 @@ note: Min Capture t[(0, 0)] -> ByValue LL | let p = t.0.0; | ^^^^^ note: Min Capture t[(1, 0)] -> ImmBorrow - --> $DIR/by_value.rs:31:29 + --> $DIR/by_value.rs:32:29 | LL | println!("{} {:?}", t.1, p); | ^^^ diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/move_closure.rs index 8bdc999ca3c3f..d57c2280438ba 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.rs @@ -18,8 +18,8 @@ fn simple_ref() { //~^ ERROR: First Pass analysis includes: //~| ERROR: Min Capture analysis includes: *ref_s += 10; - //~^ NOTE: Capturing ref_s[Deref] -> ByValue - //~| NOTE: Min Capture ref_s[] -> ByValue + //~^ NOTE: Capturing ref_s[Deref] -> UniqueImmBorrow + //~| NOTE: Min Capture ref_s[Deref] -> UniqueImmBorrow }; c(); } @@ -39,8 +39,8 @@ fn struct_contains_ref_to_another_struct() { //~^ ERROR: First Pass analysis includes: //~| ERROR: Min Capture analysis includes: t.0.0 = "new s".into(); - //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue - //~| NOTE: Min Capture t[(0, 0)] -> ByValue + //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> UniqueImmBorrow + //~| NOTE: Min Capture t[(0, 0),Deref,(0, 0)] -> UniqueImmBorrow }; c(); diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr index a745f14598ee2..554dc11f6badb 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr @@ -46,7 +46,7 @@ LL | | LL | | }; | |_____^ | -note: Capturing ref_s[Deref] -> ByValue +note: Capturing ref_s[Deref] -> UniqueImmBorrow --> $DIR/move_closure.rs:20:9 | LL | *ref_s += 10; @@ -64,7 +64,7 @@ LL | | LL | | }; | |_____^ | -note: Min Capture ref_s[] -> ByValue +note: Min Capture ref_s[Deref] -> UniqueImmBorrow --> $DIR/move_closure.rs:20:9 | LL | *ref_s += 10; @@ -82,7 +82,7 @@ LL | | LL | | }; | |_____^ | -note: Capturing t[(0, 0),Deref,(0, 0)] -> ByValue +note: Capturing t[(0, 0),Deref,(0, 0)] -> UniqueImmBorrow --> $DIR/move_closure.rs:41:9 | LL | t.0.0 = "new s".into(); @@ -100,7 +100,7 @@ LL | | LL | | }; | |_____^ | -note: Min Capture t[(0, 0)] -> ByValue +note: Min Capture t[(0, 0),Deref,(0, 0)] -> UniqueImmBorrow --> $DIR/move_closure.rs:41:9 | LL | t.0.0 = "new s".into(); diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs index 4007a5a48aaec..afaafbda01877 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs @@ -56,9 +56,50 @@ fn no_ref_nested() { c(); } +struct A<'a>(&'a mut String, &'a mut String); +// Test that reborrowing works as expected for move closures +// by attempting a disjoint capture through a reference. +fn disjoint_via_ref() { + let mut x = String::new(); + let mut y = String::new(); + + let mut a = A(&mut x, &mut y); + let a = &mut a; + + let mut c1 = move || { + a.0.truncate(0); + }; + + let mut c2 = move || { + a.1.truncate(0); + }; + + c1(); + c2(); +} + +// Test that even if a path is moved into the closure, the closure is not FnOnce +// if the path is not moved by the closure call. +fn data_moved_but_not_fn_once() { + let x = Box::new(10i32); + + let c = move || { + // *x has type i32 which is Copy. So even though the box `x` will be moved + // into the closure, `x` is never moved when the closure is called, i.e. the + // ownership stays with the closure and therefore we can call the function multiple times. + let _x = *x; + }; + + c(); + c(); +} + fn main() { simple_ref(); struct_contains_ref_to_another_struct(); no_ref(); no_ref_nested(); + + disjoint_via_ref(); + data_moved_but_not_fn_once(); } From e39c3c05a472a7b4ecb2e3f93f1b7f299ea18664 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Mon, 8 Feb 2021 23:31:26 -0500 Subject: [PATCH 15/22] Handle restricting closure origin --- compiler/rustc_typeck/src/check/upvar.rs | 5 +--- .../closure-origin-array-diagnostics.rs | 16 +++++++++++++ .../closure-origin-array-diagnostics.stderr | 23 +++++++++++++++++++ 3 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.stderr diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 79c60ab3cfe0f..12eacdf25e2f1 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -206,11 +206,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // If we have an origin, store it. if let Some(origin) = delegate.current_origin.clone() { let origin = if self.tcx.features().capture_disjoint_fields { - origin + (origin.0, restrict_capture_precision(origin.1)) } else { - // FIXME(project-rfc-2229#31): Once the changes to support reborrowing are - // made, make sure we are selecting and restricting - // the origin correctly. (origin.0, Place { projections: vec![], ..origin.1 }) }; diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.rs new file mode 100644 index 0000000000000..cd7c25620a7c4 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.rs @@ -0,0 +1,16 @@ +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +//~| `#[warn(incomplete_features)]` on by default +//~| see issue #53488 + +// Test that array access is not stored as part of closure kind origin + +fn expect_fn(_f: F) {} + +fn main() { + let s = [format!("s"), format!("s")]; + let c = || { //~ ERROR expected a closure that implements the `Fn` + let [_, _s] = s; + }; + expect_fn(c); +} diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.stderr new file mode 100644 index 0000000000000..bd9428771f4c5 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-array-diagnostics.stderr @@ -0,0 +1,23 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/closure-origin-array-diagnostics.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce` + --> $DIR/closure-origin-array-diagnostics.rs:12:13 + | +LL | let c = || { + | ^^ this closure implements `FnOnce`, not `Fn` +LL | let [_, _s] = s; + | - closure is `FnOnce` because it moves the variable `s` out of its environment +LL | }; +LL | expect_fn(c); + | --------- the requirement to implement `Fn` derives from here + +error: aborting due to previous error; 1 warning emitted + +For more information about this error, try `rustc --explain E0525`. From 1b86ad8485a1937ecd16b2dd5d9c5feb4eac93ec Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Thu, 11 Feb 2021 19:36:05 -0500 Subject: [PATCH 16/22] Treat read of COpy types via refs as not move in move-closure --- compiler/rustc_typeck/src/check/upvar.rs | 9 +- .../2229_closure_analysis/move_closure.rs | 93 +++++++-- .../2229_closure_analysis/move_closure.stderr | 189 ++++++++++++++++-- 3 files changed, 256 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 12eacdf25e2f1..fbbc1fba8771f 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -1116,14 +1116,21 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { place_with_id, diag_expr_id, mode ); - let place = truncate_capture_for_move(place_with_id.place.clone()); match (self.capture_clause, mode) { // In non-move closures, we only care about moves (hir::CaptureBy::Ref, euv::Copy) => return, + // We want to capture Copy types that read through a ref via a reborrow + (hir::CaptureBy::Value, euv::Copy) + if place_with_id.place.deref_tys().any(ty::TyS::is_ref) => + { + return; + } + (hir::CaptureBy::Ref, euv::Move) | (hir::CaptureBy::Value, euv::Move | euv::Copy) => {} }; + let place = truncate_capture_for_move(place_with_id.place.clone()); let place_with_id = PlaceWithHirId { place: place.clone(), hir_id: place_with_id.hir_id }; if !self.capture_information.contains_key(&place) { diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/move_closure.rs index d57c2280438ba..1c574da5f48bc 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.rs @@ -6,7 +6,25 @@ //~| NOTE: see issue #53488 #![feature(rustc_attrs)] -// Test we truncate derefs properly +fn simple_move_closure() { + struct S(String); + struct T(S); + + let t = T(S("s".into())); + let mut c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + t.0.0 = "new S".into(); + //~^ NOTE: Capturing t[(0, 0),(0, 0)] -> ByValue + //~| NOTE: Min Capture t[(0, 0),(0, 0)] -> ByValue + }; + c(); +} + +// Test move closure use reborrows when using references fn simple_ref() { let mut s = 10; let ref_s = &mut s; @@ -24,8 +42,8 @@ fn simple_ref() { c(); } -// Test we truncate derefs properly -fn struct_contains_ref_to_another_struct() { +// Test move closure use reborrows when using references +fn struct_contains_ref_to_another_struct_1() { struct S(String); struct T<'a>(&'a mut S); @@ -46,27 +64,78 @@ fn struct_contains_ref_to_another_struct() { c(); } -// Test that we don't reduce precision when there is nothing deref. -fn no_ref() { +// Test that we can use reborrows to read data of Copy types +// i.e. without truncating derefs +fn struct_contains_ref_to_another_struct_2() { + struct S(i32); + struct T<'a>(&'a S); + + let s = S(0); + let t = T(&s); + + let mut c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + let _t = t.0.0; + //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + //~| NOTE: Min Capture t[(0, 0),Deref,(0, 0)] -> ImmBorrow + }; + + c(); +} + +// Test that we can use truncate to move out of !Copy types +fn struct_contains_ref_to_another_struct_3() { struct S(String); - struct T(S); + struct T<'a>(&'a S); + + let s = S("s".into()); + let t = T(&s); - let t = T(S("s".into())); let mut c = #[rustc_capture_analysis] //~^ ERROR: attributes on expressions are experimental //~| NOTE: see issue #15701 move || { //~^ ERROR: First Pass analysis includes: //~| ERROR: Min Capture analysis includes: - t.0.0 = "new S".into(); - //~^ NOTE: Capturing t[(0, 0),(0, 0)] -> ByValue - //~| NOTE: Min Capture t[(0, 0),(0, 0)] -> ByValue + let _t = t.0.0; + //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + //~| NOTE: Capturing t[(0, 0)] -> ByValue + //~| NOTE: Min Capture t[(0, 0)] -> ByValue + }; + + c(); +} + +// Test that derefs of box are truncated in move closures +fn truncate_box_derefs() { + struct S(i32); + + let b = Box::new(S(10)); + + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + let _t = b.0; + //~^ NOTE: Capturing b[Deref,(0, 0)] -> ByValue + //~| NOTE: Capturing b[] -> ByValue + //~| NOTE: Min Capture b[] -> ByValue }; + c(); } fn main() { + simple_move_closure(); simple_ref(); - struct_contains_ref_to_another_struct(); - no_ref(); + struct_contains_ref_to_another_struct_1(); + struct_contains_ref_to_another_struct_2(); + struct_contains_ref_to_another_struct_3(); + truncate_box_derefs(); } diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr index 554dc11f6badb..b91ef4dd85c48 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr +++ b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr @@ -8,7 +8,7 @@ LL | let mut c = #[rustc_capture_analysis] = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable error[E0658]: attributes on expressions are experimental - --> $DIR/move_closure.rs:35:17 + --> $DIR/move_closure.rs:32:17 | LL | let mut c = #[rustc_capture_analysis] | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,7 +17,7 @@ LL | let mut c = #[rustc_capture_analysis] = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable error[E0658]: attributes on expressions are experimental - --> $DIR/move_closure.rs:55:17 + --> $DIR/move_closure.rs:53:17 | LL | let mut c = #[rustc_capture_analysis] | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -25,6 +25,33 @@ LL | let mut c = #[rustc_capture_analysis] = note: see issue #15701 for more information = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable +error[E0658]: attributes on expressions are experimental + --> $DIR/move_closure.rs:76:17 + | +LL | let mut c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +error[E0658]: attributes on expressions are experimental + --> $DIR/move_closure.rs:98:17 + | +LL | let mut c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + +error[E0658]: attributes on expressions are experimental + --> $DIR/move_closure.rs:119:13 + | +LL | let c = #[rustc_capture_analysis] + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: see issue #15701 for more information + = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable + warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes --> $DIR/move_closure.rs:3:12 | @@ -40,6 +67,42 @@ error: First Pass analysis includes: LL | / move || { LL | | LL | | +LL | | t.0.0 = "new S".into(); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Capturing t[(0, 0),(0, 0)] -> ByValue + --> $DIR/move_closure.rs:20:9 + | +LL | t.0.0 = "new S".into(); + | ^^^^^ + +error: Min Capture analysis includes: + --> $DIR/move_closure.rs:17:5 + | +LL | / move || { +LL | | +LL | | +LL | | t.0.0 = "new S".into(); +LL | | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture t[(0, 0),(0, 0)] -> ByValue + --> $DIR/move_closure.rs:20:9 + | +LL | t.0.0 = "new S".into(); + | ^^^^^ + +error: First Pass analysis includes: + --> $DIR/move_closure.rs:35:5 + | +LL | / move || { +LL | | +LL | | LL | | *ref_s += 10; LL | | LL | | @@ -47,13 +110,13 @@ LL | | }; | |_____^ | note: Capturing ref_s[Deref] -> UniqueImmBorrow - --> $DIR/move_closure.rs:20:9 + --> $DIR/move_closure.rs:38:9 | LL | *ref_s += 10; | ^^^^^^ error: Min Capture analysis includes: - --> $DIR/move_closure.rs:17:5 + --> $DIR/move_closure.rs:35:5 | LL | / move || { LL | | @@ -65,13 +128,13 @@ LL | | }; | |_____^ | note: Min Capture ref_s[Deref] -> UniqueImmBorrow - --> $DIR/move_closure.rs:20:9 + --> $DIR/move_closure.rs:38:9 | LL | *ref_s += 10; | ^^^^^^ error: First Pass analysis includes: - --> $DIR/move_closure.rs:38:5 + --> $DIR/move_closure.rs:56:5 | LL | / move || { LL | | @@ -83,13 +146,13 @@ LL | | }; | |_____^ | note: Capturing t[(0, 0),Deref,(0, 0)] -> UniqueImmBorrow - --> $DIR/move_closure.rs:41:9 + --> $DIR/move_closure.rs:59:9 | LL | t.0.0 = "new s".into(); | ^^^^^ error: Min Capture analysis includes: - --> $DIR/move_closure.rs:38:5 + --> $DIR/move_closure.rs:56:5 | LL | / move || { LL | | @@ -101,47 +164,129 @@ LL | | }; | |_____^ | note: Min Capture t[(0, 0),Deref,(0, 0)] -> UniqueImmBorrow - --> $DIR/move_closure.rs:41:9 + --> $DIR/move_closure.rs:59:9 | LL | t.0.0 = "new s".into(); | ^^^^^ error: First Pass analysis includes: - --> $DIR/move_closure.rs:58:5 + --> $DIR/move_closure.rs:79:5 | LL | / move || { LL | | LL | | -LL | | t.0.0 = "new S".into(); +LL | | let _t = t.0.0; LL | | LL | | LL | | }; | |_____^ | -note: Capturing t[(0, 0),(0, 0)] -> ByValue - --> $DIR/move_closure.rs:61:9 +note: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + --> $DIR/move_closure.rs:82:18 | -LL | t.0.0 = "new S".into(); - | ^^^^^ +LL | let _t = t.0.0; + | ^^^^^ error: Min Capture analysis includes: - --> $DIR/move_closure.rs:58:5 + --> $DIR/move_closure.rs:79:5 | LL | / move || { LL | | LL | | -LL | | t.0.0 = "new S".into(); +LL | | let _t = t.0.0; LL | | LL | | LL | | }; | |_____^ | -note: Min Capture t[(0, 0),(0, 0)] -> ByValue - --> $DIR/move_closure.rs:61:9 +note: Min Capture t[(0, 0),Deref,(0, 0)] -> ImmBorrow + --> $DIR/move_closure.rs:82:18 | -LL | t.0.0 = "new S".into(); - | ^^^^^ +LL | let _t = t.0.0; + | ^^^^^ + +error: First Pass analysis includes: + --> $DIR/move_closure.rs:101:5 + | +LL | / move || { +LL | | +LL | | +LL | | let _t = t.0.0; +... | +LL | | +LL | | }; + | |_____^ + | +note: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow + --> $DIR/move_closure.rs:104:18 + | +LL | let _t = t.0.0; + | ^^^^^ +note: Capturing t[(0, 0)] -> ByValue + --> $DIR/move_closure.rs:104:18 + | +LL | let _t = t.0.0; + | ^^^^^ + +error: Min Capture analysis includes: + --> $DIR/move_closure.rs:101:5 + | +LL | / move || { +LL | | +LL | | +LL | | let _t = t.0.0; +... | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture t[(0, 0)] -> ByValue + --> $DIR/move_closure.rs:104:18 + | +LL | let _t = t.0.0; + | ^^^^^ + +error: First Pass analysis includes: + --> $DIR/move_closure.rs:122:5 + | +LL | / move || { +LL | | +LL | | +LL | | let _t = b.0; +... | +LL | | +LL | | }; + | |_____^ + | +note: Capturing b[Deref,(0, 0)] -> ByValue + --> $DIR/move_closure.rs:125:18 + | +LL | let _t = b.0; + | ^^^ +note: Capturing b[] -> ByValue + --> $DIR/move_closure.rs:125:18 + | +LL | let _t = b.0; + | ^^^ + +error: Min Capture analysis includes: + --> $DIR/move_closure.rs:122:5 + | +LL | / move || { +LL | | +LL | | +LL | | let _t = b.0; +... | +LL | | +LL | | }; + | |_____^ + | +note: Min Capture b[] -> ByValue + --> $DIR/move_closure.rs:125:18 + | +LL | let _t = b.0; + | ^^^ -error: aborting due to 9 previous errors; 1 warning emitted +error: aborting due to 18 previous errors; 1 warning emitted For more information about this error, try `rustc --explain E0658`. From f99e152e5a02d75e5df1a10b33d4e8592cd25fee Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Sat, 13 Feb 2021 01:59:35 -0500 Subject: [PATCH 17/22] Use iter::position in truncate_capture_for_move --- compiler/rustc_typeck/src/check/upvar.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index fbbc1fba8771f..69c09528662d3 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -1461,16 +1461,10 @@ fn restrict_capture_precision<'tcx>(mut place: Place<'tcx>) -> Place<'tcx> { /// Truncates a place so that the resultant capture doesn't move data out of a reference fn truncate_capture_for_move(mut place: Place<'tcx>) -> Place<'tcx> { - for (i, proj) in place.projections.iter().enumerate() { - match proj.kind { - ProjectionKind::Deref => { - // We only drop Derefs in case of move closures - // There might be an index projection or raw ptr ahead, so we don't stop here. - place.projections.truncate(i); - return place; - } - _ => {} - } + if let Some(i) = place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref) { + // We only drop Derefs in case of move closures + // There might be an index projection or raw ptr ahead, so we don't stop here. + place.projections.truncate(i); } place From f688bee4ece7b16e8e77b2f4217a8345da742733 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Thu, 31 Dec 2020 18:17:25 -0800 Subject: [PATCH 18/22] Add a `Result::ok_or_err` method to extract a `T` from `Result` --- library/core/src/result.rs | 35 +++++++++++++++++++++++++++++++++++ library/core/tests/lib.rs | 1 + library/core/tests/result.rs | 9 +++++++++ 3 files changed, 45 insertions(+) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index a43ba5882edcd..32074c41b8b12 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1276,6 +1276,41 @@ impl Result, E> { } } +impl Result { + /// Returns the [`Ok`] value if `self` is `Ok`, and the [`Err`] value if + /// `self` is `Err`. + /// + /// In other words, this function returns the value (the `T`) of a + /// `Result`, regardless of whether or not that result is `Ok` or + /// `Err`. + /// + /// This can be useful in conjunction with APIs such as + /// [`Atomic*::compare_exchange`], or [`slice::binary_search`][binary_search], but only in + /// cases where you don't care if the result was `Ok` or not. + /// + /// [`Atomic*::compare_exchange`]: crate::sync::atomic::AtomicBool::compare_exchange + /// [binary_search]: ../primitive.slice.html#method.binary_search + /// + /// # Examples + /// + /// ``` + /// #![feature(ok_or_err)] + /// let ok: Result = Ok(3); + /// let err: Result = Err(4); + /// + /// assert_eq!(ok.ok_or_err(), 3); + /// assert_eq!(err.ok_or_err(), 4); + /// ``` + #[inline] + #[unstable(feature = "ok_or_err", reason = "newly added", issue = "none")] + pub const fn ok_or_err(self) -> T { + match self { + Ok(v) => v, + Err(v) => v, + } + } +} + // This is a separate function to reduce the code size of the methods #[inline(never)] #[cold] diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 40dc6473b7d40..5cd6e9efd9eed 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -65,6 +65,7 @@ #![feature(never_type)] #![feature(unwrap_infallible)] #![feature(option_result_unwrap_unchecked)] +#![feature(ok_or_err)] #![feature(option_unwrap_none)] #![feature(peekable_peek_mut)] #![feature(once_cell)] diff --git a/library/core/tests/result.rs b/library/core/tests/result.rs index 7aa44c6e593b3..53087eec499df 100644 --- a/library/core/tests/result.rs +++ b/library/core/tests/result.rs @@ -95,6 +95,15 @@ fn test_unwrap_or() { assert_eq!(ok_err.unwrap_or(50), 50); } +#[test] +fn test_ok_or_err() { + let ok: Result = Ok(100); + let err: Result = Err(200); + + assert_eq!(ok.ok_or_err(), 100); + assert_eq!(err.ok_or_err(), 200); +} + #[test] fn test_unwrap_or_else() { fn handler(msg: &'static str) -> isize { From 7d303661cdfbf96768b341f500acde83dc052342 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Thu, 31 Dec 2020 19:50:24 -0800 Subject: [PATCH 19/22] Fix doc link for slice::binary_search --- library/core/src/result.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 32074c41b8b12..bbbf1b412a332 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1289,7 +1289,7 @@ impl Result { /// cases where you don't care if the result was `Ok` or not. /// /// [`Atomic*::compare_exchange`]: crate::sync::atomic::AtomicBool::compare_exchange - /// [binary_search]: ../primitive.slice.html#method.binary_search + /// [binary_search]: ../../std/primitive.slice.html#method.binary_search /// /// # Examples /// From 2711b011e6b2c06c6f289a8a2362c3af31dbc32c Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 17 Feb 2021 08:54:52 -0800 Subject: [PATCH 20/22] Rename Result::ok_or_err to Result::into_ok_or_err --- library/core/src/result.rs | 10 +++++----- library/core/tests/lib.rs | 2 +- library/core/tests/result.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index bbbf1b412a332..9cfa7b6211a19 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1294,16 +1294,16 @@ impl Result { /// # Examples /// /// ``` - /// #![feature(ok_or_err)] + /// #![feature(result_into_ok_or_err)] /// let ok: Result = Ok(3); /// let err: Result = Err(4); /// - /// assert_eq!(ok.ok_or_err(), 3); - /// assert_eq!(err.ok_or_err(), 4); + /// assert_eq!(ok.into_ok_or_err(), 3); + /// assert_eq!(err.into_ok_or_err(), 4); /// ``` #[inline] - #[unstable(feature = "ok_or_err", reason = "newly added", issue = "none")] - pub const fn ok_or_err(self) -> T { + #[unstable(feature = "result_into_ok_or_err", reason = "newly added", issue = "none")] + pub const fn into_ok_or_err(self) -> T { match self { Ok(v) => v, Err(v) => v, diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index 5cd6e9efd9eed..34e05760db259 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -65,7 +65,7 @@ #![feature(never_type)] #![feature(unwrap_infallible)] #![feature(option_result_unwrap_unchecked)] -#![feature(ok_or_err)] +#![feature(result_into_ok_or_err)] #![feature(option_unwrap_none)] #![feature(peekable_peek_mut)] #![feature(once_cell)] diff --git a/library/core/tests/result.rs b/library/core/tests/result.rs index 53087eec499df..5fcd7b4d3a327 100644 --- a/library/core/tests/result.rs +++ b/library/core/tests/result.rs @@ -100,8 +100,8 @@ fn test_ok_or_err() { let ok: Result = Ok(100); let err: Result = Err(200); - assert_eq!(ok.ok_or_err(), 100); - assert_eq!(err.ok_or_err(), 200); + assert_eq!(ok.into_ok_or_err(), 100); + assert_eq!(err.into_ok_or_err(), 200); } #[test] From 404da0bc901b92c2bf74a3c84fb0bd52cbf7f934 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 17 Feb 2021 09:04:03 -0800 Subject: [PATCH 21/22] Add link to tracking issue #82223 --- library/core/src/result.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/result.rs b/library/core/src/result.rs index 9cfa7b6211a19..d8747f8b8d6dc 100644 --- a/library/core/src/result.rs +++ b/library/core/src/result.rs @@ -1302,7 +1302,7 @@ impl Result { /// assert_eq!(err.into_ok_or_err(), 4); /// ``` #[inline] - #[unstable(feature = "result_into_ok_or_err", reason = "newly added", issue = "none")] + #[unstable(feature = "result_into_ok_or_err", reason = "newly added", issue = "82223")] pub const fn into_ok_or_err(self) -> T { match self { Ok(v) => v, From fa23ddf6e6932b0caa6ee14bd479264398a3a2a7 Mon Sep 17 00:00:00 2001 From: Yonggang Luo Date: Thu, 8 Oct 2020 22:26:31 +0000 Subject: [PATCH 22/22] Expose force_quotes on Windows. Quotes the arg and not quotes the arg have different effect on Windows when the program called are msys2/cygwin program. Refer to https://github.com/msys2/MSYS2-packages/issues/2176 Signed-off-by: Yonggang Luo --- library/std/src/sys/windows/ext/process.rs | 21 ++++++++++++++++ library/std/src/sys/windows/process.rs | 12 ++++++--- library/std/src/sys/windows/process/tests.rs | 26 ++++++++++++++------ 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/library/std/src/sys/windows/ext/process.rs b/library/std/src/sys/windows/ext/process.rs index 3d680a7f2d94f..67412e1677937 100644 --- a/library/std/src/sys/windows/ext/process.rs +++ b/library/std/src/sys/windows/ext/process.rs @@ -105,6 +105,22 @@ pub trait CommandExt: Sealed { /// [1]: https://docs.microsoft.com/en-us/windows/win32/procthread/process-creation-flags #[stable(feature = "windows_process_extensions", since = "1.16.0")] fn creation_flags(&mut self, flags: u32) -> &mut process::Command; + + /// Forces all arguments to be wrapped in quote (`"`) characters. + /// + /// This is useful for passing arguments to [MSYS2/Cygwin][1] based + /// executables: these programs will expand unquoted arguments containing + /// wildcard characters (`?` and `*`) by searching for any file paths + /// matching the wildcard pattern. + /// + /// Adding quotes has no effect when passing arguments to programs + /// that use [msvcrt][2]. This includes programs built with both + /// MinGW and MSVC. + /// + /// [1]: + /// [2]: + #[unstable(feature = "windows_process_extensions_force_quotes", issue = "82227")] + fn force_quotes(&mut self, enabled: bool) -> &mut process::Command; } #[stable(feature = "windows_process_extensions", since = "1.16.0")] @@ -113,4 +129,9 @@ impl CommandExt for process::Command { self.as_inner_mut().creation_flags(flags); self } + + fn force_quotes(&mut self, enabled: bool) -> &mut process::Command { + self.as_inner_mut().force_quotes(enabled); + self + } } diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index 243065b94b125..83d37795ee5c1 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -78,6 +78,7 @@ pub struct Command { stdin: Option, stdout: Option, stderr: Option, + force_quotes_enabled: bool, } pub enum Stdio { @@ -109,6 +110,7 @@ impl Command { stdin: None, stdout: None, stderr: None, + force_quotes_enabled: false, } } @@ -134,6 +136,10 @@ impl Command { self.flags = flags; } + pub fn force_quotes(&mut self, enabled: bool) { + self.force_quotes_enabled = enabled; + } + pub fn get_program(&self) -> &OsStr { &self.program } @@ -181,7 +187,7 @@ impl Command { si.dwFlags = c::STARTF_USESTDHANDLES; let program = program.as_ref().unwrap_or(&self.program); - let mut cmd_str = make_command_line(program, &self.args)?; + let mut cmd_str = make_command_line(program, &self.args, self.force_quotes_enabled)?; cmd_str.push(0); // add null terminator // stolen from the libuv code. @@ -467,7 +473,7 @@ fn zeroed_process_information() -> c::PROCESS_INFORMATION { // Produces a wide string *without terminating null*; returns an error if // `prog` or any of the `args` contain a nul. -fn make_command_line(prog: &OsStr, args: &[OsString]) -> io::Result> { +fn make_command_line(prog: &OsStr, args: &[OsString], force_quotes: bool) -> io::Result> { // Encode the command and arguments in a command line string such // that the spawned process may recover them using CommandLineToArgvW. let mut cmd: Vec = Vec::new(); @@ -476,7 +482,7 @@ fn make_command_line(prog: &OsStr, args: &[OsString]) -> io::Result> { append_arg(&mut cmd, prog, true)?; for arg in args { cmd.push(' ' as u16); - append_arg(&mut cmd, arg, false)?; + append_arg(&mut cmd, arg, force_quotes)?; } return Ok(cmd); diff --git a/library/std/src/sys/windows/process/tests.rs b/library/std/src/sys/windows/process/tests.rs index 81627ad139bb9..8830ae049c65d 100644 --- a/library/std/src/sys/windows/process/tests.rs +++ b/library/std/src/sys/windows/process/tests.rs @@ -3,29 +3,41 @@ use crate::ffi::{OsStr, OsString}; #[test] fn test_make_command_line() { - fn test_wrapper(prog: &str, args: &[&str]) -> String { + fn test_wrapper(prog: &str, args: &[&str], force_quotes: bool) -> String { let command_line = &make_command_line( OsStr::new(prog), &args.iter().map(|a| OsString::from(a)).collect::>(), + force_quotes, ) .unwrap(); String::from_utf16(command_line).unwrap() } - assert_eq!(test_wrapper("prog", &["aaa", "bbb", "ccc"]), "\"prog\" aaa bbb ccc"); + assert_eq!(test_wrapper("prog", &["aaa", "bbb", "ccc"], false), "\"prog\" aaa bbb ccc"); assert_eq!( - test_wrapper("C:\\Program Files\\blah\\blah.exe", &["aaa"]), + test_wrapper("C:\\Program Files\\blah\\blah.exe", &["aaa"], false), "\"C:\\Program Files\\blah\\blah.exe\" aaa" ); assert_eq!( - test_wrapper("C:\\Program Files\\test", &["aa\"bb"]), + test_wrapper("C:\\Program Files\\blah\\blah.exe", &["aaa", "v*"], false), + "\"C:\\Program Files\\blah\\blah.exe\" aaa v*" + ); + assert_eq!( + test_wrapper("C:\\Program Files\\blah\\blah.exe", &["aaa", "v*"], true), + "\"C:\\Program Files\\blah\\blah.exe\" \"aaa\" \"v*\"" + ); + assert_eq!( + test_wrapper("C:\\Program Files\\test", &["aa\"bb"], false), "\"C:\\Program Files\\test\" aa\\\"bb" ); - assert_eq!(test_wrapper("echo", &["a b c"]), "\"echo\" \"a b c\""); - assert_eq!(test_wrapper("echo", &["\" \\\" \\", "\\"]), "\"echo\" \"\\\" \\\\\\\" \\\\\" \\"); + assert_eq!(test_wrapper("echo", &["a b c"], false), "\"echo\" \"a b c\""); + assert_eq!( + test_wrapper("echo", &["\" \\\" \\", "\\"], false), + "\"echo\" \"\\\" \\\\\\\" \\\\\" \\" + ); assert_eq!( - test_wrapper("\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}", &[]), + test_wrapper("\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}", &[], false), "\"\u{03c0}\u{042f}\u{97f3}\u{00e6}\u{221e}\"" ); }