From 95aac4487d1cb89913539619c201079d0fc2b463 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 11 Oct 2020 23:59:32 +0200 Subject: [PATCH 01/24] transmute_copy: explain that alignment is handled correctly --- library/core/src/mem/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/core/src/mem/mod.rs b/library/core/src/mem/mod.rs index a2c7da6e6958e..e84014c68a676 100644 --- a/library/core/src/mem/mod.rs +++ b/library/core/src/mem/mod.rs @@ -884,10 +884,10 @@ pub fn drop(_x: T) {} /// Interprets `src` as having type `&U`, and then reads `src` without moving /// the contained value. /// -/// This function will unsafely assume the pointer `src` is valid for -/// [`size_of::`][size_of] bytes by transmuting `&T` to `&U` and then reading -/// the `&U`. It will also unsafely create a copy of the contained value instead of -/// moving out of `src`. +/// This function will unsafely assume the pointer `src` is valid for [`size_of::`][size_of] +/// bytes by transmuting `&T` to `&U` and then reading the `&U` (except that this is done in a way +/// that is correct even when `&U` makes stricter alignment requirements than `&T`). It will also +/// unsafely create a copy of the contained value instead of moving out of `src`. /// /// It is not a compile-time error if `T` and `U` have different sizes, but it /// is highly encouraged to only invoke this function where `T` and `U` have the From 08d5e9673677d5874484b2f9837a42db9fea9331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 20 Oct 2020 00:00:00 +0000 Subject: [PATCH 02/24] Initialize tracing subscriber in compiletest tool The logging in compiletest was migrated from log crate to a tracing, but the initialization code was never changed, so logging is non-functional. Initialize tracing subscriber using default settings. --- Cargo.lock | 2 +- src/tools/compiletest/Cargo.toml | 2 +- src/tools/compiletest/src/main.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0d2170a992747..722632e53c75d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -639,7 +639,6 @@ name = "compiletest" version = "0.0.0" dependencies = [ "diff", - "env_logger 0.7.1", "getopts", "glob", "lazy_static", @@ -650,6 +649,7 @@ dependencies = [ "serde", "serde_json", "tracing", + "tracing-subscriber", "walkdir", "winapi 0.3.9", ] diff --git a/src/tools/compiletest/Cargo.toml b/src/tools/compiletest/Cargo.toml index c601084d11917..209254a5d5e2f 100644 --- a/src/tools/compiletest/Cargo.toml +++ b/src/tools/compiletest/Cargo.toml @@ -6,9 +6,9 @@ edition = "2018" [dependencies] diff = "0.1.10" -env_logger = { version = "0.7", default-features = false } getopts = "0.2" tracing = "0.1" +tracing-subscriber = { version = "0.2.13", default-features = false, features = ["fmt", "env-filter", "smallvec", "parking_lot", "ansi"] } regex = "1.0" serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 190a9c6221060..2b2a6cfa8bd6f 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -35,7 +35,7 @@ pub mod runtest; pub mod util; fn main() { - env_logger::init(); + tracing_subscriber::fmt::init(); let config = parse_config(env::args().collect()); From 2ec4d82e159c7d8437721ee53645c3620aaa69b3 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 21:26:02 +0200 Subject: [PATCH 03/24] Add issue template link to IRLO --- .github/ISSUE_TEMPLATE/config.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index bd7dc0ac95c1f..898b983ba3790 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -3,3 +3,6 @@ contact_links: - name: Rust Programming Language Forum url: https://users.rust-lang.org about: Please ask and answer questions about Rust here. + - name: Rust Internals Forum + url: https://internals.rust-lang.org/ + about: Please discuss language feature requests here. From 95cbfb1aa458d336a0425144117b2e5262a51380 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 20 Oct 2020 21:36:39 +0200 Subject: [PATCH 04/24] Retitle forum links --- .github/ISSUE_TEMPLATE/config.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml index 898b983ba3790..7d4cfaece4481 100644 --- a/.github/ISSUE_TEMPLATE/config.yml +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -1,8 +1,8 @@ blank_issues_enabled: true contact_links: - - name: Rust Programming Language Forum + - name: Question url: https://users.rust-lang.org - about: Please ask and answer questions about Rust here. - - name: Rust Internals Forum + about: Please ask and answer questions about Rust on the user forum. + - name: Feature Request url: https://internals.rust-lang.org/ - about: Please discuss language feature requests here. + about: Please discuss language feature requests on the internals forum. From 57d01a9aeea505b2f4b0c420e1b9975d51690ff8 Mon Sep 17 00:00:00 2001 From: Simon Vandel Sillesen Date: Thu, 22 Oct 2020 22:22:17 +0200 Subject: [PATCH 05/24] Check which places are dead Fixes #78192 --- .../rustc_mir/src/transform/instcombine.rs | 20 +++++++++---- .../const_prop/ref_deref.main.ConstProp.diff | 2 +- .../ref_deref_project.main.ConstProp.diff | 2 +- src/test/mir-opt/inst_combine_deref.rs | 2 +- src/test/mir-opt/issue-78192.rs | 9 ++++++ .../mir-opt/issue_78192.f.InstCombine.diff | 29 +++++++++++++++++++ 6 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 src/test/mir-opt/issue-78192.rs create mode 100644 src/test/mir-opt/issue_78192.f.InstCombine.diff diff --git a/compiler/rustc_mir/src/transform/instcombine.rs b/compiler/rustc_mir/src/transform/instcombine.rs index c0b3d5ff18b5c..c5c14ca7caeb5 100644 --- a/compiler/rustc_mir/src/transform/instcombine.rs +++ b/compiler/rustc_mir/src/transform/instcombine.rs @@ -119,11 +119,6 @@ impl OptimizationFinder<'b, 'tcx> { } fn find_deref_of_address(&mut self, rvalue: &Rvalue<'tcx>, location: Location) -> Option<()> { - // FIXME(#78192): This optimization can result in unsoundness. - if !self.tcx.sess.opts.debugging_opts.unsound_mir_opts { - return None; - } - // Look for the sequence // // _2 = &_1; @@ -137,6 +132,8 @@ impl OptimizationFinder<'b, 'tcx> { _ => None, }?; + let mut dead_locals_seen = vec![]; + let stmt_index = location.statement_index; // Look behind for statement that assigns the local from a address of operator. // 6 is chosen as a heuristic determined by seeing the number of times @@ -160,6 +157,11 @@ impl OptimizationFinder<'b, 'tcx> { BorrowKind::Shared, place_taken_address_of, ) => { + // Make sure that the place has not been marked dead + if dead_locals_seen.contains(&place_taken_address_of.local) { + return None; + } + self.optimizations .unneeded_deref .insert(location, *place_taken_address_of); @@ -178,13 +180,19 @@ impl OptimizationFinder<'b, 'tcx> { // Inline asm can do anything, so bail out of the optimization. rustc_middle::mir::StatementKind::LlvmInlineAsm(_) => return None, + // Remember `StorageDead`s, as the local being marked dead could be the + // place RHS we are looking for, in which case we need to abort to avoid UB + // using an uninitialized place + rustc_middle::mir::StatementKind::StorageDead(dead) => { + dead_locals_seen.push(*dead) + } + // Check that `local_being_deref` is not being used in a mutating way which can cause misoptimization. rustc_middle::mir::StatementKind::Assign(box (_, _)) | rustc_middle::mir::StatementKind::Coverage(_) | rustc_middle::mir::StatementKind::Nop | rustc_middle::mir::StatementKind::FakeRead(_, _) | rustc_middle::mir::StatementKind::StorageLive(_) - | rustc_middle::mir::StatementKind::StorageDead(_) | rustc_middle::mir::StatementKind::Retag(_, _) | rustc_middle::mir::StatementKind::AscribeUserType(_, _) | rustc_middle::mir::StatementKind::SetDiscriminant { .. } => { diff --git a/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff b/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff index 4fd1b8b227649..feef65f52ebe0 100644 --- a/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff @@ -19,7 +19,7 @@ // + span: $DIR/ref_deref.rs:5:6: 5:10 // + literal: Const { ty: &i32, val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref[317d]::main), const_param_did: None }, [], Some(promoted[0])) } _2 = _4; // scope 0 at $DIR/ref_deref.rs:5:6: 5:10 -- _1 = (*_2); // scope 0 at $DIR/ref_deref.rs:5:5: 5:10 +- _1 = (*_4); // scope 0 at $DIR/ref_deref.rs:5:5: 5:10 + _1 = const 4_i32; // scope 0 at $DIR/ref_deref.rs:5:5: 5:10 StorageDead(_2); // scope 0 at $DIR/ref_deref.rs:5:10: 5:11 StorageDead(_1); // scope 0 at $DIR/ref_deref.rs:5:10: 5:11 diff --git a/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff b/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff index 812c7c9771801..7ec0751263fb1 100644 --- a/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff +++ b/src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff @@ -19,7 +19,7 @@ // + span: $DIR/ref_deref_project.rs:5:6: 5:17 // + literal: Const { ty: &(i32, i32), val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref_project[317d]::main), const_param_did: None }, [], Some(promoted[0])) } _2 = &((*_4).1: i32); // scope 0 at $DIR/ref_deref_project.rs:5:6: 5:17 - _1 = (*_2); // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17 + _1 = ((*_4).1: i32); // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17 StorageDead(_2); // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18 StorageDead(_1); // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18 _0 = const (); // scope 0 at $DIR/ref_deref_project.rs:4:11: 6:2 diff --git a/src/test/mir-opt/inst_combine_deref.rs b/src/test/mir-opt/inst_combine_deref.rs index 78361c336607c..3be8c2f3ac732 100644 --- a/src/test/mir-opt/inst_combine_deref.rs +++ b/src/test/mir-opt/inst_combine_deref.rs @@ -1,4 +1,4 @@ -// compile-flags: -O -Zunsound-mir-opts +// compile-flags: -O // EMIT_MIR inst_combine_deref.simple_opt.InstCombine.diff fn simple_opt() -> u64 { let x = 5; diff --git a/src/test/mir-opt/issue-78192.rs b/src/test/mir-opt/issue-78192.rs new file mode 100644 index 0000000000000..906d094f72b4a --- /dev/null +++ b/src/test/mir-opt/issue-78192.rs @@ -0,0 +1,9 @@ +// EMIT_MIR issue_78192.f.InstCombine.diff +pub fn f(a: &T) -> *const T { + let b: &*const T = &(a as *const T); + *b +} + +fn main() { + f(&2); +} diff --git a/src/test/mir-opt/issue_78192.f.InstCombine.diff b/src/test/mir-opt/issue_78192.f.InstCombine.diff new file mode 100644 index 0000000000000..ec3be78525802 --- /dev/null +++ b/src/test/mir-opt/issue_78192.f.InstCombine.diff @@ -0,0 +1,29 @@ +- // MIR for `f` before InstCombine ++ // MIR for `f` after InstCombine + + fn f(_1: &T) -> *const T { + debug a => _1; // in scope 0 at $DIR/issue-78192.rs:2:13: 2:14 + let mut _0: *const T; // return place in scope 0 at $DIR/issue-78192.rs:2:23: 2:31 + let _2: &*const T; // in scope 0 at $DIR/issue-78192.rs:3:9: 3:10 + let _3: &*const T; // in scope 0 at $DIR/issue-78192.rs:3:24: 3:40 + let _4: *const T; // in scope 0 at $DIR/issue-78192.rs:3:25: 3:40 + scope 1 { + debug b => _2; // in scope 1 at $DIR/issue-78192.rs:3:9: 3:10 + } + + bb0: { + StorageLive(_2); // scope 0 at $DIR/issue-78192.rs:3:9: 3:10 + StorageLive(_3); // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 + StorageLive(_4); // scope 0 at $DIR/issue-78192.rs:3:25: 3:40 + _4 = &raw const (*_1); // scope 0 at $DIR/issue-78192.rs:3:26: 3:27 + _3 = &_4; // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 +- _2 = &(*_3); // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 ++ _2 = _3; // scope 0 at $DIR/issue-78192.rs:3:24: 3:40 + StorageDead(_3); // scope 0 at $DIR/issue-78192.rs:3:40: 3:41 + _0 = (*_2); // scope 1 at $DIR/issue-78192.rs:4:5: 4:7 + StorageDead(_4); // scope 0 at $DIR/issue-78192.rs:5:1: 5:2 + StorageDead(_2); // scope 0 at $DIR/issue-78192.rs:5:1: 5:2 + return; // scope 0 at $DIR/issue-78192.rs:5:2: 5:2 + } + } + From 3815c6abbaf077fbfe798578953dbe332090917e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 23 Oct 2020 00:12:20 +0200 Subject: [PATCH 06/24] Add codegen test for #45964 --- .../issue-45964-bounds-check-slice-pos.rs | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/test/codegen/issue-45964-bounds-check-slice-pos.rs diff --git a/src/test/codegen/issue-45964-bounds-check-slice-pos.rs b/src/test/codegen/issue-45964-bounds-check-slice-pos.rs new file mode 100644 index 0000000000000..aa59c713b7846 --- /dev/null +++ b/src/test/codegen/issue-45964-bounds-check-slice-pos.rs @@ -0,0 +1,38 @@ +// This test case checks that slice::{r}position functions do not +// prevent optimizing away bounds checks + +// compile-flags: -O + +#![crate_type="rlib"] + +// CHECK-LABEL: @test +#[no_mangle] +pub fn test(y: &[u32], x: &u32, z: &u32) -> bool { + let result = match y.iter().position(|a| a == x) { + Some(p) => Ok(p), + None => Err(()), + }; + + if let Ok(p) = result { + // CHECK-NOT: panic + y[p] == *z + } else { + false + } +} + +// CHECK-LABEL: @rtest +#[no_mangle] +pub fn rtest(y: &[u32], x: &u32, z: &u32) -> bool { + let result = match y.iter().rposition(|a| a == x) { + Some(p) => Ok(p), + None => Err(()), + }; + + if let Ok(p) = result { + // CHECK-NOT: panic + y[p] == *z + } else { + false + } +} From 86df9039b2a49dde3a2453d75c4c58540a568a1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 21 Oct 2020 14:25:09 -0700 Subject: [PATCH 07/24] Tweak "use `.await`" suggestion --- .../src/traits/error_reporting/suggestions.rs | 10 +++++----- .../src/check/fn_ctxt/suggestions.rs | 16 ++++++---------- .../incorrect-syntax-suggestions.stderr | 9 +++++---- src/test/ui/async-await/issue-61076.stderr | 18 ++++++++++-------- .../suggest-missing-await-closure.fixed | 4 ++-- .../suggest-missing-await-closure.rs | 4 ++-- .../suggest-missing-await-closure.stderr | 9 +++++---- .../ui/async-await/suggest-missing-await.fixed | 8 ++++---- .../ui/async-await/suggest-missing-await.rs | 8 ++++---- .../async-await/suggest-missing-await.stderr | 13 +++++++------ src/test/ui/suggestions/issue-72766.stderr | 9 +++++---- 11 files changed, 55 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index efa9bd633ba8c..fa837e04db35e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -21,7 +21,7 @@ use rustc_middle::ty::{ }; use rustc_middle::ty::{TypeAndMut, TypeckResults}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{MultiSpan, Span, DUMMY_SP}; +use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP}; use rustc_target::spec::abi; use std::fmt; @@ -2114,10 +2114,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { if self.predicate_may_hold(&try_obligation) && impls_future { if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span) { if snippet.ends_with('?') { - err.span_suggestion( - span, - "consider using `.await` here", - format!("{}.await?", snippet.trim_end_matches('?')), + err.span_suggestion_verbose( + span.with_hi(span.hi() - BytePos(1)).shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), Applicability::MaybeIncorrect, ); } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 9bad02c41b4b1..18db8d63850b2 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -497,16 +497,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if self.infcx.predicate_may_hold(&obligation) { debug!("suggest_missing_await: obligation held: {:?}", obligation); - if let Ok(code) = self.sess().source_map().span_to_snippet(sp) { - err.span_suggestion( - sp, - "consider using `.await` here", - format!("{}.await", code), - Applicability::MaybeIncorrect, - ); - } else { - debug!("suggest_missing_await: no snippet for {:?}", sp); - } + err.span_suggestion_verbose( + sp.shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), + Applicability::MaybeIncorrect, + ); } else { debug!("suggest_missing_await: obligation did not hold: {:?}", obligation) } diff --git a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr index 6a653fc060b1d..d90ce0f1bb482 100644 --- a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr +++ b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr @@ -237,13 +237,14 @@ error[E0277]: the `?` operator can only be applied to values that implement `Try --> $DIR/incorrect-syntax-suggestions.rs:16:19 | LL | let _ = await bar()?; - | ^^^^^^ - | | - | the `?` operator cannot be applied to type `impl Future` - | help: consider using `.await` here: `bar().await?` + | ^^^^^^ the `?` operator cannot be applied to type `impl Future` | = help: the trait `Try` is not implemented for `impl Future` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | let _ = await bar().await?; + | ^^^^^^ error[E0277]: the `?` operator can only be applied to values that implement `Try` --> $DIR/incorrect-syntax-suggestions.rs:63:19 diff --git a/src/test/ui/async-await/issue-61076.stderr b/src/test/ui/async-await/issue-61076.stderr index 88ea7251eaf1f..afba889f014fe 100644 --- a/src/test/ui/async-await/issue-61076.stderr +++ b/src/test/ui/async-await/issue-61076.stderr @@ -2,25 +2,27 @@ error[E0277]: the `?` operator can only be applied to values that implement `Try --> $DIR/issue-61076.rs:42:5 | LL | foo()?; - | ^^^^^^ - | | - | the `?` operator cannot be applied to type `impl Future` - | help: consider using `.await` here: `foo().await?` + | ^^^^^^ the `?` operator cannot be applied to type `impl Future` | = help: the trait `Try` is not implemented for `impl Future` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | foo().await?; + | ^^^^^^ error[E0277]: the `?` operator can only be applied to values that implement `Try` --> $DIR/issue-61076.rs:56:5 | LL | t?; - | ^^ - | | - | the `?` operator cannot be applied to type `T` - | help: consider using `.await` here: `t.await?` + | ^^ the `?` operator cannot be applied to type `T` | = help: the trait `Try` is not implemented for `T` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | t.await?; + | ^^^^^^ error[E0609]: no field `0` on type `impl Future` --> $DIR/issue-61076.rs:58:26 diff --git a/src/test/ui/async-await/suggest-missing-await-closure.fixed b/src/test/ui/async-await/suggest-missing-await-closure.fixed index 37b30ffe6800f..febcd02184261 100644 --- a/src/test/ui/async-await/suggest-missing-await-closure.fixed +++ b/src/test/ui/async-await/suggest-missing-await-closure.fixed @@ -15,8 +15,8 @@ async fn suggest_await_in_async_closure() { let x = make_u32(); take_u32(x.await) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await }; } diff --git a/src/test/ui/async-await/suggest-missing-await-closure.rs b/src/test/ui/async-await/suggest-missing-await-closure.rs index 18076a1516171..faabf6ee3f16f 100644 --- a/src/test/ui/async-await/suggest-missing-await-closure.rs +++ b/src/test/ui/async-await/suggest-missing-await-closure.rs @@ -15,8 +15,8 @@ async fn suggest_await_in_async_closure() { let x = make_u32(); take_u32(x) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await }; } diff --git a/src/test/ui/async-await/suggest-missing-await-closure.stderr b/src/test/ui/async-await/suggest-missing-await-closure.stderr index ed2c4cbfccc98..2151057aa7fc0 100644 --- a/src/test/ui/async-await/suggest-missing-await-closure.stderr +++ b/src/test/ui/async-await/suggest-missing-await-closure.stderr @@ -5,13 +5,14 @@ LL | async fn make_u32() -> u32 { | --- the `Output` of this `async fn`'s found opaque type ... LL | take_u32(x) - | ^ - | | - | expected `u32`, found opaque type - | help: consider using `.await` here: `x.await` + | ^ expected `u32`, found opaque type | = note: expected type `u32` found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | take_u32(x.await) + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/async-await/suggest-missing-await.fixed b/src/test/ui/async-await/suggest-missing-await.fixed index 1ec59d906206e..e548fda7cb4f5 100644 --- a/src/test/ui/async-await/suggest-missing-await.fixed +++ b/src/test/ui/async-await/suggest-missing-await.fixed @@ -12,8 +12,8 @@ async fn suggest_await_in_async_fn() { let x = make_u32(); take_u32(x.await) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } async fn dummy() {} @@ -23,8 +23,8 @@ async fn suggest_await_in_async_fn_return() { dummy().await; //~^ ERROR mismatched types [E0308] //~| HELP try adding a semicolon - //~| HELP consider using `.await` here - //~| SUGGESTION dummy().await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } fn main() {} diff --git a/src/test/ui/async-await/suggest-missing-await.rs b/src/test/ui/async-await/suggest-missing-await.rs index 70cc1f1d5a2c6..464a9602119e1 100644 --- a/src/test/ui/async-await/suggest-missing-await.rs +++ b/src/test/ui/async-await/suggest-missing-await.rs @@ -12,8 +12,8 @@ async fn suggest_await_in_async_fn() { let x = make_u32(); take_u32(x) //~^ ERROR mismatched types [E0308] - //~| HELP consider using `.await` here - //~| SUGGESTION x.await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } async fn dummy() {} @@ -23,8 +23,8 @@ async fn suggest_await_in_async_fn_return() { dummy() //~^ ERROR mismatched types [E0308] //~| HELP try adding a semicolon - //~| HELP consider using `.await` here - //~| SUGGESTION dummy().await + //~| HELP consider `await`ing on the `Future` + //~| SUGGESTION .await } fn main() {} diff --git a/src/test/ui/async-await/suggest-missing-await.stderr b/src/test/ui/async-await/suggest-missing-await.stderr index c6355680e253b..d729420e93019 100644 --- a/src/test/ui/async-await/suggest-missing-await.stderr +++ b/src/test/ui/async-await/suggest-missing-await.stderr @@ -5,13 +5,14 @@ LL | async fn make_u32() -> u32 { | --- the `Output` of this `async fn`'s found opaque type ... LL | take_u32(x) - | ^ - | | - | expected `u32`, found opaque type - | help: consider using `.await` here: `x.await` + | ^ expected `u32`, found opaque type | = note: expected type `u32` found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | take_u32(x.await) + | ^^^^^^ error[E0308]: mismatched types --> $DIR/suggest-missing-await.rs:23:5 @@ -28,10 +29,10 @@ help: try adding a semicolon | LL | dummy(); | ^ -help: consider using `.await` here +help: consider `await`ing on the `Future` | LL | dummy().await - | + | ^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/suggestions/issue-72766.stderr b/src/test/ui/suggestions/issue-72766.stderr index a1a5949b19660..5c9c549fa0779 100644 --- a/src/test/ui/suggestions/issue-72766.stderr +++ b/src/test/ui/suggestions/issue-72766.stderr @@ -2,13 +2,14 @@ error[E0277]: the `?` operator can only be applied to values that implement `Try --> $DIR/issue-72766.rs:14:5 | LL | SadGirl {}.call()?; - | ^^^^^^^^^^^^^^^^^^ - | | - | the `?` operator cannot be applied to type `impl Future` - | help: consider using `.await` here: `SadGirl {}.call().await?` + | ^^^^^^^^^^^^^^^^^^ the `?` operator cannot be applied to type `impl Future` | = help: the trait `Try` is not implemented for `impl Future` = note: required by `into_result` +help: consider `await`ing on the `Future` + | +LL | SadGirl {}.call().await?; + | ^^^^^^ error: aborting due to previous error From a4ee3ca1e4f1177516139a4704456958c7d08c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 21 Oct 2020 19:08:28 -0700 Subject: [PATCH 08/24] Suggest semicolon removal on prior match arm --- compiler/rustc_typeck/src/check/_match.rs | 9 +++- .../match-prev-arm-needing-semi.rs | 32 +++++++++++ .../match-prev-arm-needing-semi.stderr | 53 +++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/suggestions/match-prev-arm-needing-semi.rs create mode 100644 src/test/ui/suggestions/match-prev-arm-needing-semi.stderr diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 398e013e62fb5..94e886be54dcf 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -188,11 +188,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } else { - let (arm_span, semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind { + let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind + { self.find_block_span(blk, prior_arm_ty) } else { (arm.body.span, None) }; + if semi_span.is_none() && i > 0 { + if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind { + let (_, semi_span_prev) = self.find_block_span(blk, Some(arm_ty)); + semi_span = semi_span_prev; + } + } let (span, code) = match i { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs new file mode 100644 index 0000000000000..d8d6de4bf5591 --- /dev/null +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -0,0 +1,32 @@ +// edition:2018 + +fn dummy() -> i32 { 42 } + +fn extra_semicolon() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => { + dummy(); //~ NOTE this is found to be + //~^ HELP consider removing this semicolon + } + false => dummy(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected `()`, found `i32` + }; +} + +async fn async_dummy() {} //~ NOTE the `Output` of this `async fn`'s found opaque type + +async fn async_extra_semicolon_same() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => { + async_dummy(); //~ NOTE this is found to be + //~^ HELP consider removing this semicolon + } + false => async_dummy(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected `()`, found opaque type + //~| NOTE expected type `()` + //~| HELP consider `await`ing on the `Future` + }; +} + +fn main() {} + diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr new file mode 100644 index 0000000000000..e242a018843a2 --- /dev/null +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -0,0 +1,53 @@ +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:24:18 + | +LL | async fn async_dummy() {} + | - the `Output` of this `async fn`'s found opaque type +... +LL | let _ = match true { + | _____________- +LL | | true => { +LL | | async_dummy(); + | | -------------- this is found to be of type `()` +LL | | +LL | | } +LL | | false => async_dummy(), + | | ^^^^^^^^^^^^^ expected `()`, found opaque type +... | +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `()` + found opaque type `impl Future` +help: consider removing this semicolon + | +LL | async_dummy() + | -- +help: consider `await`ing on the `Future` + | +LL | false => async_dummy().await, + | ^^^^^^ + +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:11:18 + | +LL | let _ = match true { + | _____________- +LL | | true => { +LL | | dummy(); + | | -------- + | | | | + | | | help: consider removing this semicolon + | | this is found to be of type `()` +LL | | +LL | | } +LL | | false => dummy(), + | | ^^^^^^^ expected `()`, found `i32` +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. From 671d7c4afb36a7dcedf9ec8a6f3ef00c19bfc260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 21 Oct 2020 19:43:15 -0700 Subject: [PATCH 09/24] Account for possible boxable `impl Future` in semicolon removal suggestions --- .../src/infer/error_reporting/mod.rs | 46 ++++++++++----- compiler/rustc_middle/src/traits/mod.rs | 4 +- compiler/rustc_typeck/src/check/_match.rs | 2 +- .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 57 +++++++++++++++++-- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 23 +++++--- .../match-prev-arm-needing-semi.rs | 15 ++++- .../match-prev-arm-needing-semi.stderr | 37 +++++++++++- 7 files changed, 152 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 3a0ec6327c186..f6aa3c3d5ba6b 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -688,13 +688,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { }; let msg = "`match` arms have incompatible types"; err.span_label(outer_error_span, msg); - if let Some(sp) = semi_span { - err.span_suggestion_short( - sp, - "consider removing this semicolon", - String::new(), - Applicability::MachineApplicable, - ); + if let Some((sp, boxed)) = semi_span { + if boxed { + err.span_suggestion_verbose( + sp, + "consider removing this semicolon and boxing the expression", + String::new(), + Applicability::HasPlaceholders, + ); + } else { + err.span_suggestion_short( + sp, + "consider removing this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } } if let Some(ret_sp) = opt_suggest_box_span { // Get return type span and point to it. @@ -717,13 +726,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { if let Some(sp) = outer { err.span_label(sp, "`if` and `else` have incompatible types"); } - if let Some(sp) = semicolon { - err.span_suggestion_short( - sp, - "consider removing this semicolon", - String::new(), - Applicability::MachineApplicable, - ); + if let Some((sp, boxed)) = semicolon { + if boxed { + err.span_suggestion_verbose( + sp, + "consider removing this semicolon and boxing the expression", + String::new(), + Applicability::HasPlaceholders, + ); + } else { + err.span_suggestion_short( + sp, + "consider removing this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } } if let Some(ret_sp) = opt_suggest_box_span { self.suggest_boxing_for_return_impl_trait( diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index bbc46b8d60835..daa3010abb544 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -344,7 +344,7 @@ static_assert_size!(ObligationCauseCode<'_>, 32); pub struct MatchExpressionArmCause<'tcx> { pub arm_span: Span, pub scrut_span: Span, - pub semi_span: Option, + pub semi_span: Option<(Span, bool)>, pub source: hir::MatchSource, pub prior_arms: Vec, pub last_ty: Ty<'tcx>, @@ -357,7 +357,7 @@ pub struct IfExpressionCause { pub then: Span, pub else_sp: Span, pub outer: Option, - pub semicolon: Option, + pub semicolon: Option<(Span, bool)>, pub opt_suggest_box_span: Option, } diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 94e886be54dcf..27c85317e82f9 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -521,7 +521,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, block: &'tcx hir::Block<'tcx>, expected_ty: Option>, - ) -> (Span, Option) { + ) -> (Span, Option<(Span, bool)>) { if let Some(expr) = &block.expr { (expr.span, None) } else if let Some(stmt) = block.stmts.last() { diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index 017b0abd1d607..f26a168bcb288 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -1061,7 +1061,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, blk: &'tcx hir::Block<'tcx>, expected_ty: Ty<'tcx>, - ) -> Option { + ) -> Option<(Span, bool)> { // Be helpful when the user wrote `{... expr;}` and // taking the `;` off is enough to fix the error. let last_stmt = blk.stmts.last()?; @@ -1070,13 +1070,62 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => return None, }; let last_expr_ty = self.node_ty(last_expr.hir_id); - if matches!(last_expr_ty.kind(), ty::Error(_)) - || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err() + let needs_box = match (last_expr_ty.kind(), expected_ty.kind()) { + (ty::Opaque(last_def_id, last_bounds), ty::Opaque(exp_def_id, exp_bounds)) => { + debug!( + "both opaque, likely future {:?} {:?} {:?} {:?}", + last_def_id, last_bounds, exp_def_id, exp_bounds + ); + let last_hir_id = self.tcx.hir().local_def_id_to_hir_id(last_def_id.expect_local()); + let exp_hir_id = self.tcx.hir().local_def_id_to_hir_id(exp_def_id.expect_local()); + if let ( + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }), + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }), + ) = ( + &self.tcx.hir().expect_item(last_hir_id).kind, + &self.tcx.hir().expect_item(exp_hir_id).kind, + ) { + debug!("{:?} {:?}", last_bounds, exp_bounds); + last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| { + match (left, right) { + ( + hir::GenericBound::Trait(tl, ml), + hir::GenericBound::Trait(tr, mr), + ) => { + tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id() + && ml == mr + } + ( + hir::GenericBound::LangItemTrait(langl, _, _, argsl), + hir::GenericBound::LangItemTrait(langr, _, _, argsr), + ) => { + // FIXME: consider the bounds! + debug!("{:?} {:?}", argsl, argsr); + langl == langr + } + _ => false, + } + }) + } else { + false + } + } + _ => false, + }; + debug!( + "needs_box {:?} {:?} {:?}", + needs_box, + last_expr_ty.kind(), + self.can_sub(self.param_env, last_expr_ty, expected_ty) + ); + if (matches!(last_expr_ty.kind(), ty::Error(_)) + || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err()) + && !needs_box { return None; } let original_span = original_sp(last_stmt.span, blk.span); - Some(original_span.with_lo(original_span.hi() - BytePos(1))) + Some((original_span.with_lo(original_span.hi() - BytePos(1)), needs_box)) } // Instantiates the given path, which must refer to an item with the given diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index fd2f5eb5018d4..a124ad16612b1 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -758,13 +758,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected_ty: Ty<'tcx>, err: &mut DiagnosticBuilder<'_>, ) { - if let Some(span_semi) = self.could_remove_semicolon(blk, expected_ty) { - err.span_suggestion( - span_semi, - "consider removing this semicolon", - String::new(), - Applicability::MachineApplicable, - ); + if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) { + if boxed { + err.span_suggestion_verbose( + span_semi, + "consider removing this semicolon and boxing the expression", + String::new(), + Applicability::HasPlaceholders, + ); + } else { + err.span_suggestion_short( + span_semi, + "consider removing this semicolon", + String::new(), + Applicability::MachineApplicable, + ); + } } } diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs index d8d6de4bf5591..9704242e10541 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -14,6 +14,7 @@ fn extra_semicolon() { } async fn async_dummy() {} //~ NOTE the `Output` of this `async fn`'s found opaque type +async fn async_dummy2() {} //~ NOTE the `Output` of this `async fn`'s found opaque type async fn async_extra_semicolon_same() { let _ = match true { //~ NOTE `match` arms have incompatible types @@ -28,5 +29,17 @@ async fn async_extra_semicolon_same() { }; } -fn main() {} +async fn async_extra_semicolon_different() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => { + async_dummy(); //~ NOTE this is found to be + //~^ HELP consider removing this semicolon + } + false => async_dummy2(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected `()`, found opaque type + //~| NOTE expected type `()` + //~| HELP consider `await`ing on the `Future` + }; +} +fn main() {} diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index e242a018843a2..00b02fbc0072f 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -1,5 +1,5 @@ error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:24:18 + --> $DIR/match-prev-arm-needing-semi.rs:25:18 | LL | async fn async_dummy() {} | - the `Output` of this `async fn`'s found opaque type @@ -20,7 +20,7 @@ LL | | }; | = note: expected type `()` found opaque type `impl Future` -help: consider removing this semicolon +help: consider removing this semicolon and boxing the expression | LL | async_dummy() | -- @@ -29,6 +29,37 @@ help: consider `await`ing on the `Future` LL | false => async_dummy().await, | ^^^^^^ +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:38:18 + | +LL | async fn async_dummy2() {} + | - the `Output` of this `async fn`'s found opaque type +... +LL | let _ = match true { + | _____________- +LL | | true => { +LL | | async_dummy(); + | | -------------- this is found to be of type `()` +LL | | +LL | | } +LL | | false => async_dummy2(), + | | ^^^^^^^^^^^^^^ expected `()`, found opaque type +... | +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `()` + found opaque type `impl Future` +help: consider removing this semicolon and boxing the expression + | +LL | async_dummy() + | -- +help: consider `await`ing on the `Future` + | +LL | false => async_dummy2().await, + | ^^^^^^ + error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:11:18 | @@ -48,6 +79,6 @@ LL | | LL | | }; | |_____- `match` arms have incompatible types -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0308`. From 62ba365195e37b0508dc35f73b55243cb1aef7f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 10:27:40 -0700 Subject: [PATCH 10/24] Review comments: use newtype instead of `bool` --- .../src/infer/error_reporting/mod.rs | 5 +- compiler/rustc_middle/src/traits/mod.rs | 17 ++++++- compiler/rustc_typeck/src/check/_match.rs | 39 ++++++++++------ .../rustc_typeck/src/check/fn_ctxt/_impl.rs | 46 +++++++++---------- .../rustc_typeck/src/check/fn_ctxt/checks.rs | 4 +- 5 files changed, 68 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index f6aa3c3d5ba6b..9e93cfc27d0dd 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -54,6 +54,7 @@ use crate::infer::OriginalQueryValues; use crate::traits::error_reporting::report_object_safety_error; use crate::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, + StatementAsExpression, }; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -689,7 +690,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let msg = "`match` arms have incompatible types"; err.span_label(outer_error_span, msg); if let Some((sp, boxed)) = semi_span { - if boxed { + if matches!(boxed, StatementAsExpression::NeedsBoxing) { err.span_suggestion_verbose( sp, "consider removing this semicolon and boxing the expression", @@ -727,7 +728,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { err.span_label(sp, "`if` and `else` have incompatible types"); } if let Some((sp, boxed)) = semicolon { - if boxed { + if matches!(boxed, StatementAsExpression::NeedsBoxing) { err.span_suggestion_verbose( sp, "consider removing this semicolon and boxing the expression", diff --git a/compiler/rustc_middle/src/traits/mod.rs b/compiler/rustc_middle/src/traits/mod.rs index daa3010abb544..4deb7225dcb61 100644 --- a/compiler/rustc_middle/src/traits/mod.rs +++ b/compiler/rustc_middle/src/traits/mod.rs @@ -340,11 +340,24 @@ impl ObligationCauseCode<'_> { #[cfg(target_arch = "x86_64")] static_assert_size!(ObligationCauseCode<'_>, 32); +#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] +pub enum StatementAsExpression { + CorrectType, + NeedsBoxing, +} + +impl<'tcx> ty::Lift<'tcx> for StatementAsExpression { + type Lifted = StatementAsExpression; + fn lift_to_tcx(self, _tcx: TyCtxt<'tcx>) -> Option { + Some(self) + } +} + #[derive(Clone, Debug, PartialEq, Eq, Hash, Lift)] pub struct MatchExpressionArmCause<'tcx> { pub arm_span: Span, pub scrut_span: Span, - pub semi_span: Option<(Span, bool)>, + pub semi_span: Option<(Span, StatementAsExpression)>, pub source: hir::MatchSource, pub prior_arms: Vec, pub last_ty: Ty<'tcx>, @@ -357,7 +370,7 @@ pub struct IfExpressionCause { pub then: Span, pub else_sp: Span, pub outer: Option, - pub semicolon: Option<(Span, bool)>, + pub semicolon: Option<(Span, StatementAsExpression)>, pub opt_suggest_box_span: Option, } diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 27c85317e82f9..e8eea65137ff7 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -9,6 +9,7 @@ use rustc_trait_selection::opaque_types::InferCtxtExt as _; use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt; use rustc_trait_selection::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, + StatementAsExpression, }; impl<'a, 'tcx> FnCtxt<'a, 'tcx> { @@ -188,18 +189,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } } else { - let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind - { - self.find_block_span(blk, prior_arm_ty) - } else { - (arm.body.span, None) - }; - if semi_span.is_none() && i > 0 { - if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind { - let (_, semi_span_prev) = self.find_block_span(blk, Some(arm_ty)); - semi_span = semi_span_prev; - } - } + let (arm_span, semi_span) = + self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty); let (span, code) = match i { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. @@ -249,6 +240,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { coercion.complete(self) } + fn get_appropriate_arm_semicolon_removal_span( + &self, + arms: &'tcx [hir::Arm<'tcx>], + i: usize, + prior_arm_ty: Option>, + arm_ty: Ty<'tcx>, + ) -> (Span, Option<(Span, StatementAsExpression)>) { + let arm = &arms[i]; + let (arm_span, mut semi_span) = if let hir::ExprKind::Block(blk, _) = &arm.body.kind { + self.find_block_span(blk, prior_arm_ty) + } else { + (arm.body.span, None) + }; + if semi_span.is_none() && i > 0 { + if let hir::ExprKind::Block(blk, _) = &arms[i - 1].body.kind { + let (_, semi_span_prev) = self.find_block_span(blk, Some(arm_ty)); + semi_span = semi_span_prev; + } + } + (arm_span, semi_span) + } + /// When the previously checked expression (the scrutinee) diverges, /// warn the user about the match arms being unreachable. fn warn_arms_when_scrutinee_diverges( @@ -521,7 +534,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, block: &'tcx hir::Block<'tcx>, expected_ty: Option>, - ) -> (Span, Option<(Span, bool)>) { + ) -> (Span, Option<(Span, StatementAsExpression)>) { if let Some(expr) = &block.expr { (expr.span, None) } else if let Some(stmt) = block.stmts.last() { diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs index f26a168bcb288..f87e6b607d46e 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/_impl.rs @@ -33,7 +33,9 @@ use rustc_span::{self, BytePos, MultiSpan, Span}; use rustc_trait_selection::infer::InferCtxtExt as _; use rustc_trait_selection::opaque_types::InferCtxtExt as _; use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _; -use rustc_trait_selection::traits::{self, ObligationCauseCode, TraitEngine, TraitEngineExt}; +use rustc_trait_selection::traits::{ + self, ObligationCauseCode, StatementAsExpression, TraitEngine, TraitEngineExt, +}; use std::collections::hash_map::Entry; use std::slice; @@ -1061,7 +1063,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { &self, blk: &'tcx hir::Block<'tcx>, expected_ty: Ty<'tcx>, - ) -> Option<(Span, bool)> { + ) -> Option<(Span, StatementAsExpression)> { // Be helpful when the user wrote `{... expr;}` and // taking the `;` off is enough to fix the error. let last_stmt = blk.stmts.last()?; @@ -1078,49 +1080,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); let last_hir_id = self.tcx.hir().local_def_id_to_hir_id(last_def_id.expect_local()); let exp_hir_id = self.tcx.hir().local_def_id_to_hir_id(exp_def_id.expect_local()); - if let ( - hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }), - hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }), - ) = ( + match ( &self.tcx.hir().expect_item(last_hir_id).kind, &self.tcx.hir().expect_item(exp_hir_id).kind, ) { - debug!("{:?} {:?}", last_bounds, exp_bounds); - last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| { + ( + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: last_bounds, .. }), + hir::ItemKind::OpaqueTy(hir::OpaqueTy { bounds: exp_bounds, .. }), + ) if last_bounds.iter().zip(exp_bounds.iter()).all(|(left, right)| { match (left, right) { ( hir::GenericBound::Trait(tl, ml), hir::GenericBound::Trait(tr, mr), - ) => { - tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id() - && ml == mr + ) if tl.trait_ref.trait_def_id() == tr.trait_ref.trait_def_id() + && ml == mr => + { + true } ( hir::GenericBound::LangItemTrait(langl, _, _, argsl), hir::GenericBound::LangItemTrait(langr, _, _, argsr), - ) => { + ) if langl == langr => { // FIXME: consider the bounds! debug!("{:?} {:?}", argsl, argsr); - langl == langr + true } _ => false, } - }) - } else { - false + }) => + { + StatementAsExpression::NeedsBoxing + } + _ => StatementAsExpression::CorrectType, } } - _ => false, + _ => StatementAsExpression::CorrectType, }; - debug!( - "needs_box {:?} {:?} {:?}", - needs_box, - last_expr_ty.kind(), - self.can_sub(self.param_env, last_expr_ty, expected_ty) - ); if (matches!(last_expr_ty.kind(), ty::Error(_)) || self.can_sub(self.param_env, last_expr_ty, expected_ty).is_err()) - && !needs_box + && matches!(needs_box, StatementAsExpression::CorrectType) { return None; } diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs index a124ad16612b1..a820661d8432a 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/checks.rs @@ -20,7 +20,7 @@ use rustc_middle::ty::{self, Ty}; use rustc_session::Session; use rustc_span::symbol::{sym, Ident}; use rustc_span::{self, MultiSpan, Span}; -use rustc_trait_selection::traits::{self, ObligationCauseCode}; +use rustc_trait_selection::traits::{self, ObligationCauseCode, StatementAsExpression}; use std::mem::replace; use std::slice; @@ -759,7 +759,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err: &mut DiagnosticBuilder<'_>, ) { if let Some((span_semi, boxed)) = self.could_remove_semicolon(blk, expected_ty) { - if boxed { + if let StatementAsExpression::NeedsBoxing = boxed { err.span_suggestion_verbose( span_semi, "consider removing this semicolon and boxing the expression", From 3a0227bc49397879b240373d84b2a80e05569724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 10:51:49 -0700 Subject: [PATCH 11/24] Silence unnecessary `await foo?` knock-down error --- .../rustc_parse/src/parser/diagnostics.rs | 8 +- .../incorrect-syntax-suggestions.rs | 5 +- .../incorrect-syntax-suggestions.stderr | 95 +++++++------------ 3 files changed, 40 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 1ea01d95a134e..39e1256a57835 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1207,7 +1207,13 @@ impl<'a> Parser<'a> { self.recover_await_prefix(await_sp)? }; let sp = self.error_on_incorrect_await(lo, hi, &expr, is_question); - let expr = self.mk_expr(lo.to(sp), ExprKind::Await(expr), attrs); + let kind = match expr.kind { + // Avoid knock-down errors as we don't know whether to interpret this as `foo().await?` + // or `foo()?.await` (the very reason we went with postfix syntax 😅). + ExprKind::Try(_) => ExprKind::Err, + _ => ExprKind::Await(expr), + }; + let expr = self.mk_expr(lo.to(sp), kind, attrs); self.maybe_recover_from_bad_qpath(expr, true) } diff --git a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs index 337487fc80b0e..554ac673d5155 100644 --- a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs +++ b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.rs @@ -14,7 +14,6 @@ async fn foo2() -> Result<(), ()> { } async fn foo3() -> Result<(), ()> { let _ = await bar()?; //~ ERROR incorrect use of `await` - //~^ ERROR the `?` operator can only be applied to values that implement `Try` Ok(()) } async fn foo21() -> Result<(), ()> { @@ -60,9 +59,7 @@ fn foo10() -> Result<(), ()> { Ok(()) } fn foo11() -> Result<(), ()> { - let _ = await bar()?; //~ ERROR `await` is only allowed inside `async` functions and blocks - //~^ ERROR incorrect use of `await` - //~| ERROR the `?` operator can only be applied to values that implement `Try` + let _ = await bar()?; //~ ERROR incorrect use of `await` Ok(()) } fn foo12() -> Result<(), ()> { diff --git a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr index d90ce0f1bb482..52615df6008ff 100644 --- a/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr +++ b/src/test/ui/async-await/await-keyword/incorrect-syntax-suggestions.stderr @@ -17,103 +17,103 @@ LL | let _ = await bar()?; | ^^^^^^^^^^^^ help: `await` is a postfix operation: `bar()?.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:21:13 + --> $DIR/incorrect-syntax-suggestions.rs:20:13 | LL | let _ = await { bar() }; | ^^^^^^^^^^^^^^^ help: `await` is a postfix operation: `{ bar() }.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:25:13 + --> $DIR/incorrect-syntax-suggestions.rs:24:13 | LL | let _ = await(bar()); | ^^^^^^^^^^^^ help: `await` is a postfix operation: `(bar()).await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:29:13 + --> $DIR/incorrect-syntax-suggestions.rs:28:13 | LL | let _ = await { bar() }?; | ^^^^^^^^^^^^^^^ help: `await` is a postfix operation: `{ bar() }.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:33:14 + --> $DIR/incorrect-syntax-suggestions.rs:32:14 | LL | let _ = (await bar())?; | ^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:37:24 + --> $DIR/incorrect-syntax-suggestions.rs:36:24 | LL | let _ = bar().await(); | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:41:24 + --> $DIR/incorrect-syntax-suggestions.rs:40:24 | LL | let _ = bar().await()?; | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:53:13 + --> $DIR/incorrect-syntax-suggestions.rs:52:13 | LL | let _ = await bar(); | ^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:58:13 + --> $DIR/incorrect-syntax-suggestions.rs:57:13 | LL | let _ = await? bar(); | ^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await?` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:63:13 + --> $DIR/incorrect-syntax-suggestions.rs:62:13 | LL | let _ = await bar()?; | ^^^^^^^^^^^^ help: `await` is a postfix operation: `bar()?.await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:69:14 + --> $DIR/incorrect-syntax-suggestions.rs:66:14 | LL | let _ = (await bar())?; | ^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:74:24 + --> $DIR/incorrect-syntax-suggestions.rs:71:24 | LL | let _ = bar().await(); | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:79:24 + --> $DIR/incorrect-syntax-suggestions.rs:76:24 | LL | let _ = bar().await()?; | ^^ help: `await` is not a method call, remove the parentheses error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:107:13 + --> $DIR/incorrect-syntax-suggestions.rs:104:13 | LL | let _ = await!(bar()); | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:111:13 + --> $DIR/incorrect-syntax-suggestions.rs:108:13 | LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:116:17 + --> $DIR/incorrect-syntax-suggestions.rs:113:17 | LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:124:17 + --> $DIR/incorrect-syntax-suggestions.rs:121:17 | LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ help: `await` is a postfix operation: `bar().await` error: expected expression, found `=>` - --> $DIR/incorrect-syntax-suggestions.rs:132:25 + --> $DIR/incorrect-syntax-suggestions.rs:129:25 | LL | match await { await => () } | ----- ^^ expected expression @@ -121,13 +121,13 @@ LL | match await { await => () } | while parsing this incorrect await expression error: incorrect use of `await` - --> $DIR/incorrect-syntax-suggestions.rs:132:11 + --> $DIR/incorrect-syntax-suggestions.rs:129:11 | LL | match await { await => () } | ^^^^^^^^^^^^^^^^^^^^^ help: `await` is a postfix operation: `{ await => () }.await` error: expected one of `.`, `?`, `{`, or an operator, found `}` - --> $DIR/incorrect-syntax-suggestions.rs:135:1 + --> $DIR/incorrect-syntax-suggestions.rs:132:1 | LL | match await { await => () } | ----- - expected one of `.`, `?`, `{`, or an operator @@ -138,7 +138,7 @@ LL | } | ^ unexpected token error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:53:13 + --> $DIR/incorrect-syntax-suggestions.rs:52:13 | LL | fn foo9() -> Result<(), ()> { | ---- this is not `async` @@ -146,7 +146,7 @@ LL | let _ = await bar(); | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:58:13 + --> $DIR/incorrect-syntax-suggestions.rs:57:13 | LL | fn foo10() -> Result<(), ()> { | ----- this is not `async` @@ -154,15 +154,7 @@ LL | let _ = await? bar(); | ^^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:63:13 - | -LL | fn foo11() -> Result<(), ()> { - | ----- this is not `async` -LL | let _ = await bar()?; - | ^^^^^^^^^^^^ only allowed inside `async` functions and blocks - -error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:69:14 + --> $DIR/incorrect-syntax-suggestions.rs:66:14 | LL | fn foo12() -> Result<(), ()> { | ----- this is not `async` @@ -170,7 +162,7 @@ LL | let _ = (await bar())?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:74:13 + --> $DIR/incorrect-syntax-suggestions.rs:71:13 | LL | fn foo13() -> Result<(), ()> { | ----- this is not `async` @@ -178,7 +170,7 @@ LL | let _ = bar().await(); | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:79:13 + --> $DIR/incorrect-syntax-suggestions.rs:76:13 | LL | fn foo14() -> Result<(), ()> { | ----- this is not `async` @@ -186,7 +178,7 @@ LL | let _ = bar().await()?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:84:13 + --> $DIR/incorrect-syntax-suggestions.rs:81:13 | LL | fn foo15() -> Result<(), ()> { | ----- this is not `async` @@ -194,7 +186,7 @@ LL | let _ = bar().await; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:88:13 + --> $DIR/incorrect-syntax-suggestions.rs:85:13 | LL | fn foo16() -> Result<(), ()> { | ----- this is not `async` @@ -202,7 +194,7 @@ LL | let _ = bar().await?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:93:17 + --> $DIR/incorrect-syntax-suggestions.rs:90:17 | LL | fn foo() -> Result<(), ()> { | --- this is not `async` @@ -210,7 +202,7 @@ LL | let _ = bar().await?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:100:17 + --> $DIR/incorrect-syntax-suggestions.rs:97:17 | LL | let foo = || { | -- this is not `async` @@ -218,7 +210,7 @@ LL | let _ = bar().await?; | ^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:116:17 + --> $DIR/incorrect-syntax-suggestions.rs:113:17 | LL | fn foo() -> Result<(), ()> { | --- this is not `async` @@ -226,36 +218,13 @@ LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ only allowed inside `async` functions and blocks error[E0728]: `await` is only allowed inside `async` functions and blocks - --> $DIR/incorrect-syntax-suggestions.rs:124:17 + --> $DIR/incorrect-syntax-suggestions.rs:121:17 | LL | let foo = || { | -- this is not `async` LL | let _ = await!(bar())?; | ^^^^^^^^^^^^^ only allowed inside `async` functions and blocks -error[E0277]: the `?` operator can only be applied to values that implement `Try` - --> $DIR/incorrect-syntax-suggestions.rs:16:19 - | -LL | let _ = await bar()?; - | ^^^^^^ the `?` operator cannot be applied to type `impl Future` - | - = help: the trait `Try` is not implemented for `impl Future` - = note: required by `into_result` -help: consider `await`ing on the `Future` - | -LL | let _ = await bar().await?; - | ^^^^^^ - -error[E0277]: the `?` operator can only be applied to values that implement `Try` - --> $DIR/incorrect-syntax-suggestions.rs:63:19 - | -LL | let _ = await bar()?; - | ^^^^^^ the `?` operator cannot be applied to type `impl Future` - | - = help: the trait `Try` is not implemented for `impl Future` - = note: required by `into_result` - -error: aborting due to 36 previous errors +error: aborting due to 33 previous errors -Some errors have detailed explanations: E0277, E0728. -For more information about an error, try `rustc --explain E0277`. +For more information about this error, try `rustc --explain E0728`. From 1829b4a887553797d6fa018ae2518ca0d45b67ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 11:34:46 -0700 Subject: [PATCH 12/24] Add test case for different `impl Future`s --- .../match-prev-arm-needing-semi.rs | 11 ++++++++ .../match-prev-arm-needing-semi.stderr | 28 +++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs index 9704242e10541..81e6abf7d7ba4 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -15,6 +15,7 @@ fn extra_semicolon() { async fn async_dummy() {} //~ NOTE the `Output` of this `async fn`'s found opaque type async fn async_dummy2() {} //~ NOTE the `Output` of this `async fn`'s found opaque type +//~^ NOTE the `Output` of this `async fn`'s found opaque type async fn async_extra_semicolon_same() { let _ = match true { //~ NOTE `match` arms have incompatible types @@ -42,4 +43,14 @@ async fn async_extra_semicolon_different() { }; } +async fn async_different_futures() { + let _ = match true { //~ NOTE `match` arms have incompatible types + true => async_dummy(), //~ NOTE this is found to be + false => async_dummy2(), //~ ERROR `match` arms have incompatible types + //~^ NOTE expected opaque type, found a different opaque type + //~| NOTE expected type `impl Future` + //~| NOTE distinct uses of `impl Trait` result in different opaque types + }; +} + fn main() {} diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index 00b02fbc0072f..8f4b860589efd 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -1,5 +1,5 @@ error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:25:18 + --> $DIR/match-prev-arm-needing-semi.rs:26:18 | LL | async fn async_dummy() {} | - the `Output` of this `async fn`'s found opaque type @@ -30,7 +30,7 @@ LL | false => async_dummy().await, | ^^^^^^ error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:38:18 + --> $DIR/match-prev-arm-needing-semi.rs:39:18 | LL | async fn async_dummy2() {} | - the `Output` of this `async fn`'s found opaque type @@ -60,6 +60,28 @@ help: consider `await`ing on the `Future` LL | false => async_dummy2().await, | ^^^^^^ +error[E0308]: `match` arms have incompatible types + --> $DIR/match-prev-arm-needing-semi.rs:49:18 + | +LL | async fn async_dummy2() {} + | - the `Output` of this `async fn`'s found opaque type +... +LL | let _ = match true { + | _____________- +LL | | true => async_dummy(), + | | ------------- this is found to be of type `impl Future` +LL | | false => async_dummy2(), + | | ^^^^^^^^^^^^^^ expected opaque type, found a different opaque type +LL | | +LL | | +LL | | +LL | | }; + | |_____- `match` arms have incompatible types + | + = note: expected type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:16:24>) + found opaque type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:17:25>) + = note: distinct uses of `impl Trait` result in different opaque types + error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:11:18 | @@ -79,6 +101,6 @@ LL | | LL | | }; | |_____- `match` arms have incompatible types -error: aborting due to 3 previous errors +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0308`. From c5485115dcbbb5a0837c2ac8cabd5ead8a3b8a66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 19:03:36 -0700 Subject: [PATCH 13/24] Add more `.await` suggestions on E0308 --- .../src/infer/error_reporting/mod.rs | 157 +++++++++++++----- compiler/rustc_middle/src/ty/error.rs | 37 ++--- compiler/rustc_typeck/src/check/demand.rs | 1 - .../src/check/fn_ctxt/suggestions.rs | 79 --------- .../dont-suggest-missing-await.stderr | 4 + src/test/ui/async-await/issue-61076.stderr | 4 + .../async-await/suggest-missing-await.fixed | 30 ---- .../ui/async-await/suggest-missing-await.rs | 1 - .../async-await/suggest-missing-await.stderr | 12 +- .../match-prev-arm-needing-semi.rs | 1 + .../match-prev-arm-needing-semi.stderr | 23 ++- .../ui/suggestions/opaque-type-error.stderr | 6 + 12 files changed, 167 insertions(+), 188 deletions(-) delete mode 100644 src/test/ui/async-await/suggest-missing-await.fixed diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 9e93cfc27d0dd..fcf21c61d8fd9 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -50,7 +50,6 @@ use super::region_constraints::GenericKind; use super::{InferCtxt, RegionVariableOrigin, SubregionOrigin, TypeTrace, ValuePairs}; use crate::infer; -use crate::infer::OriginalQueryValues; use crate::traits::error_reporting::report_object_safety_error; use crate::traits::{ IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode, @@ -65,7 +64,6 @@ use rustc_hir::def_id::DefId; use rustc_hir::lang_items::LangItem; use rustc_hir::{Item, ItemKind, Node}; use rustc_middle::ty::error::TypeError; -use rustc_middle::ty::ParamEnvAnd; use rustc_middle::ty::{ self, subst::{Subst, SubstsRef}, @@ -1621,6 +1619,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { Mismatch::Variable(exp_found) => Some(exp_found), Mismatch::Fixed(_) => None, }; + let exp_found = match terr { + // `terr` has more accurate type information than `exp_found` in match expressions. + ty::error::TypeError::Sorts(terr) + if exp_found.map_or(false, |ef| terr.found == ef.found) => + { + Some(*terr) + } + _ => exp_found, + }; + debug!("exp_found {:?} terr {:?}", exp_found, terr); if let Some(exp_found) = exp_found { self.suggest_as_ref_where_appropriate(span, &exp_found, diag); self.suggest_await_on_expect_found(cause, span, &exp_found, diag); @@ -1642,6 +1650,53 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { self.note_error_origin(diag, cause, exp_found); } + fn get_impl_future_output_ty(&self, ty: Ty<'tcx>) -> Option> { + if let ty::Opaque(def_id, substs) = ty.kind() { + let future_trait = self.tcx.require_lang_item(LangItem::Future, None); + // Future::Output + let item_def_id = self + .tcx + .associated_items(future_trait) + .in_definition_order() + .next() + .unwrap() + .def_id; + + let bounds = self.tcx.explicit_item_bounds(*def_id); + + for (predicate, _) in bounds { + let predicate = predicate.subst(self.tcx, substs); + if let ty::PredicateAtom::Projection(projection_predicate) = + predicate.skip_binders() + { + if projection_predicate.projection_ty.item_def_id == item_def_id { + // We don't account for multiple `Future::Output = Ty` contraints. + return Some(projection_predicate.ty); + } + } + } + } + None + } + + /// A possible error is to forget to add `.await` when using futures: + /// + /// ``` + /// async fn make_u32() -> u32 { + /// 22 + /// } + /// + /// fn take_u32(x: u32) {} + /// + /// async fn foo() { + /// let x = make_u32(); + /// take_u32(x); + /// } + /// ``` + /// + /// This routine checks if the found type `T` implements `Future` where `U` is the + /// expected type. If this is the case, and we are inside of an async body, it suggests adding + /// `.await` to the tail of the expression. fn suggest_await_on_expect_found( &self, cause: &ObligationCause<'tcx>, @@ -1651,50 +1706,76 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { ) { debug!( "suggest_await_on_expect_found: exp_span={:?}, expected_ty={:?}, found_ty={:?}", - exp_span, exp_found.expected, exp_found.found + exp_span, exp_found.expected, exp_found.found, ); - if let ty::Opaque(def_id, _) = *exp_found.expected.kind() { - let future_trait = self.tcx.require_lang_item(LangItem::Future, None); - // Future::Output - let item_def_id = self - .tcx - .associated_items(future_trait) - .in_definition_order() - .next() - .unwrap() - .def_id; + if let ObligationCauseCode::CompareImplMethodObligation { .. } = &cause.code { + return; + } - let projection_ty = self.tcx.projection_ty_from_predicates((def_id, item_def_id)); - if let Some(projection_ty) = projection_ty { - let projection_query = self.canonicalize_query( - &ParamEnvAnd { param_env: self.tcx.param_env(def_id), value: projection_ty }, - &mut OriginalQueryValues::default(), - ); - if let Ok(resp) = self.tcx.normalize_projection_ty(projection_query) { - let normalized_ty = resp.value.value.normalized_ty; - debug!("suggest_await_on_expect_found: normalized={:?}", normalized_ty); - if ty::TyS::same_type(normalized_ty, exp_found.found) { - let span = if let ObligationCauseCode::Pattern { - span, - origin_expr: _, - root_ty: _, - } = cause.code - { - // scrutinee's span - span.unwrap_or(exp_span) - } else { - exp_span - }; - diag.span_suggestion_verbose( - span.shrink_to_hi(), - "consider awaiting on the future", - ".await".to_string(), + match ( + self.get_impl_future_output_ty(exp_found.expected), + self.get_impl_future_output_ty(exp_found.found), + ) { + (Some(exp), Some(found)) if ty::TyS::same_type(exp, found) => match &cause.code { + ObligationCauseCode::IfExpression(box IfExpressionCause { then, .. }) => { + diag.multipart_suggestion( + "consider `await`ing on both `Future`s", + vec![ + (then.shrink_to_hi(), ".await".to_string()), + (exp_span.shrink_to_hi(), ".await".to_string()), + ], + Applicability::MaybeIncorrect, + ); + } + ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { + prior_arms, + .. + }) => { + if let [.., arm_span] = &prior_arms[..] { + diag.multipart_suggestion( + "consider `await`ing on both `Future`s", + vec![ + (arm_span.shrink_to_hi(), ".await".to_string()), + (exp_span.shrink_to_hi(), ".await".to_string()), + ], Applicability::MaybeIncorrect, ); + } else { + diag.help("consider `await`ing on both `Future`s"); } } + _ => { + diag.help("consider `await`ing on both `Future`s"); + } + }, + (_, Some(ty)) if ty::TyS::same_type(exp_found.expected, ty) => { + let span = match cause.code { + // scrutinee's span + ObligationCauseCode::Pattern { span: Some(span), .. } => span, + _ => exp_span, + }; + diag.span_suggestion_verbose( + span.shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), + Applicability::MaybeIncorrect, + ); + } + (Some(ty), _) if ty::TyS::same_type(ty, exp_found.found) => { + let span = match cause.code { + // scrutinee's span + ObligationCauseCode::Pattern { span: Some(span), .. } => span, + _ => exp_span, + }; + diag.span_suggestion_verbose( + span.shrink_to_hi(), + "consider `await`ing on the `Future`", + ".await".to_string(), + Applicability::MaybeIncorrect, + ); } + _ => {} } } diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index 235f8749cf917..5ec0ec0c56ad6 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -334,26 +334,15 @@ impl<'tcx> TyCtxt<'tcx> { debug!("note_and_explain_type_err err={:?} cause={:?}", err, cause); match err { Sorts(values) => { - let expected_str = values.expected.sort_string(self); - let found_str = values.found.sort_string(self); - if expected_str == found_str && expected_str == "closure" { - db.note("no two closures, even if identical, have the same type"); - db.help("consider boxing your closure and/or using it as a trait object"); - } - if expected_str == found_str && expected_str == "opaque type" { - // Issue #63167 - db.note("distinct uses of `impl Trait` result in different opaque types"); - let e_str = values.expected.to_string(); - let f_str = values.found.to_string(); - if e_str == f_str && &e_str == "impl std::future::Future" { - // FIXME: use non-string based check. - db.help( - "if both `Future`s have the same `Output` type, consider \ - `.await`ing on both of them", - ); - } - } match (values.expected.kind(), values.found.kind()) { + (ty::Closure(..), ty::Closure(..)) => { + db.note("no two closures, even if identical, have the same type"); + db.help("consider boxing your closure and/or using it as a trait object"); + } + (ty::Opaque(..), ty::Opaque(..)) => { + // Issue #63167 + db.note("distinct uses of `impl Trait` result in different opaque types"); + } (ty::Float(_), ty::Infer(ty::IntVar(_))) => { if let Ok( // Issue #53280 @@ -382,12 +371,12 @@ impl<'tcx> TyCtxt<'tcx> { } db.note( "a type parameter was expected, but a different one was found; \ - you might be missing a type parameter or trait bound", + you might be missing a type parameter or trait bound", ); db.note( "for more information, visit \ - https://doc.rust-lang.org/book/ch10-02-traits.html\ - #traits-as-parameters", + https://doc.rust-lang.org/book/ch10-02-traits.html\ + #traits-as-parameters", ); } (ty::Projection(_), ty::Projection(_)) => { @@ -471,8 +460,8 @@ impl Trait for X { } db.note( "for more information, visit \ - https://doc.rust-lang.org/book/ch10-02-traits.html\ - #traits-as-parameters", + https://doc.rust-lang.org/book/ch10-02-traits.html\ + #traits-as-parameters", ); } (ty::Param(p), ty::Closure(..) | ty::Generator(..)) => { diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index b8143787a2ddf..241803fab1e68 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -33,7 +33,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return; } self.suggest_boxing_when_appropriate(err, expr, expected, expr_ty); - self.suggest_missing_await(err, expr, expected, expr_ty); self.suggest_missing_parentheses(err, expr); self.note_need_for_fn_pointer(err, expected, expr_ty); self.note_internal_mutation_in_method(err, expr, expected, expr_ty); diff --git a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs index 18db8d63850b2..a8ad9f4fdf8af 100644 --- a/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs +++ b/compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs @@ -3,7 +3,6 @@ use crate::astconv::AstConv; use rustc_ast::util::parser::ExprPrecedence; use rustc_span::{self, Span}; -use rustc_trait_selection::traits; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; @@ -13,7 +12,6 @@ use rustc_hir::{ExprKind, ItemKind, Node}; use rustc_infer::infer; use rustc_middle::ty::{self, Ty}; use rustc_span::symbol::kw; -use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; use std::iter; @@ -433,83 +431,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - /// A possible error is to forget to add `.await` when using futures: - /// - /// ``` - /// async fn make_u32() -> u32 { - /// 22 - /// } - /// - /// fn take_u32(x: u32) {} - /// - /// async fn foo() { - /// let x = make_u32(); - /// take_u32(x); - /// } - /// ``` - /// - /// This routine checks if the found type `T` implements `Future` where `U` is the - /// expected type. If this is the case, and we are inside of an async body, it suggests adding - /// `.await` to the tail of the expression. - pub(in super::super) fn suggest_missing_await( - &self, - err: &mut DiagnosticBuilder<'_>, - expr: &hir::Expr<'_>, - expected: Ty<'tcx>, - found: Ty<'tcx>, - ) { - debug!("suggest_missing_await: expr={:?} expected={:?}, found={:?}", expr, expected, found); - // `.await` is not permitted outside of `async` bodies, so don't bother to suggest if the - // body isn't `async`. - let item_id = self.tcx().hir().get_parent_node(self.body_id); - if let Some(body_id) = self.tcx().hir().maybe_body_owned_by(item_id) { - let body = self.tcx().hir().body(body_id); - if let Some(hir::GeneratorKind::Async(_)) = body.generator_kind { - let sp = expr.span; - // Check for `Future` implementations by constructing a predicate to - // prove: `::Output == U` - let future_trait = self.tcx.require_lang_item(LangItem::Future, Some(sp)); - let item_def_id = self - .tcx - .associated_items(future_trait) - .in_definition_order() - .next() - .unwrap() - .def_id; - // `::Output` - let projection_ty = ty::ProjectionTy { - // `T` - substs: self - .tcx - .mk_substs_trait(found, self.fresh_substs_for_item(sp, item_def_id)), - // `Future::Output` - item_def_id, - }; - - let predicate = ty::PredicateAtom::Projection(ty::ProjectionPredicate { - projection_ty, - ty: expected, - }) - .potentially_quantified(self.tcx, ty::PredicateKind::ForAll); - let obligation = traits::Obligation::new(self.misc(sp), self.param_env, predicate); - - debug!("suggest_missing_await: trying obligation {:?}", obligation); - - if self.infcx.predicate_may_hold(&obligation) { - debug!("suggest_missing_await: obligation held: {:?}", obligation); - err.span_suggestion_verbose( - sp.shrink_to_hi(), - "consider `await`ing on the `Future`", - ".await".to_string(), - Applicability::MaybeIncorrect, - ); - } else { - debug!("suggest_missing_await: obligation did not hold: {:?}", obligation) - } - } - } - } - pub(in super::super) fn suggest_missing_parentheses( &self, err: &mut DiagnosticBuilder<'_>, diff --git a/src/test/ui/async-await/dont-suggest-missing-await.stderr b/src/test/ui/async-await/dont-suggest-missing-await.stderr index e70ed9badbd33..14e72c2b1e7e2 100644 --- a/src/test/ui/async-await/dont-suggest-missing-await.stderr +++ b/src/test/ui/async-await/dont-suggest-missing-await.stderr @@ -9,6 +9,10 @@ LL | take_u32(x) | = note: expected type `u32` found opaque type `impl Future` +help: consider `await`ing on the `Future` + | +LL | take_u32(x.await) + | ^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/async-await/issue-61076.stderr b/src/test/ui/async-await/issue-61076.stderr index afba889f014fe..df54ac88acee1 100644 --- a/src/test/ui/async-await/issue-61076.stderr +++ b/src/test/ui/async-await/issue-61076.stderr @@ -53,6 +53,10 @@ LL | Tuple(_) => {} | = note: expected opaque type `impl Future` found struct `Tuple` +help: consider `await`ing on the `Future` + | +LL | match tuple().await { + | ^^^^^^ error: aborting due to 6 previous errors diff --git a/src/test/ui/async-await/suggest-missing-await.fixed b/src/test/ui/async-await/suggest-missing-await.fixed deleted file mode 100644 index e548fda7cb4f5..0000000000000 --- a/src/test/ui/async-await/suggest-missing-await.fixed +++ /dev/null @@ -1,30 +0,0 @@ -// edition:2018 -// run-rustfix - -fn take_u32(_x: u32) {} - -async fn make_u32() -> u32 { - 22 -} - -#[allow(unused)] -async fn suggest_await_in_async_fn() { - let x = make_u32(); - take_u32(x.await) - //~^ ERROR mismatched types [E0308] - //~| HELP consider `await`ing on the `Future` - //~| SUGGESTION .await -} - -async fn dummy() {} - -#[allow(unused)] -async fn suggest_await_in_async_fn_return() { - dummy().await; - //~^ ERROR mismatched types [E0308] - //~| HELP try adding a semicolon - //~| HELP consider `await`ing on the `Future` - //~| SUGGESTION .await -} - -fn main() {} diff --git a/src/test/ui/async-await/suggest-missing-await.rs b/src/test/ui/async-await/suggest-missing-await.rs index 464a9602119e1..d629054911dac 100644 --- a/src/test/ui/async-await/suggest-missing-await.rs +++ b/src/test/ui/async-await/suggest-missing-await.rs @@ -1,5 +1,4 @@ // edition:2018 -// run-rustfix fn take_u32(_x: u32) {} diff --git a/src/test/ui/async-await/suggest-missing-await.stderr b/src/test/ui/async-await/suggest-missing-await.stderr index d729420e93019..46615dae7e2ba 100644 --- a/src/test/ui/async-await/suggest-missing-await.stderr +++ b/src/test/ui/async-await/suggest-missing-await.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/suggest-missing-await.rs:13:14 + --> $DIR/suggest-missing-await.rs:12:14 | LL | async fn make_u32() -> u32 { | --- the `Output` of this `async fn`'s found opaque type @@ -15,7 +15,7 @@ LL | take_u32(x.await) | ^^^^^^ error[E0308]: mismatched types - --> $DIR/suggest-missing-await.rs:23:5 + --> $DIR/suggest-missing-await.rs:22:5 | LL | async fn dummy() {} | - the `Output` of this `async fn`'s found opaque type @@ -25,14 +25,14 @@ LL | dummy() | = note: expected unit type `()` found opaque type `impl Future` -help: try adding a semicolon - | -LL | dummy(); - | ^ help: consider `await`ing on the `Future` | LL | dummy().await | ^^^^^^ +help: try adding a semicolon + | +LL | dummy(); + | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs index 81e6abf7d7ba4..b8ac030b0bbbe 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.rs +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.rs @@ -46,6 +46,7 @@ async fn async_extra_semicolon_different() { async fn async_different_futures() { let _ = match true { //~ NOTE `match` arms have incompatible types true => async_dummy(), //~ NOTE this is found to be + //~| HELP consider `await`ing on both `Future`s false => async_dummy2(), //~ ERROR `match` arms have incompatible types //~^ NOTE expected opaque type, found a different opaque type //~| NOTE expected type `impl Future` diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index 8f4b860589efd..7a4f74a1994ce 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -20,14 +20,14 @@ LL | | }; | = note: expected type `()` found opaque type `impl Future` -help: consider removing this semicolon and boxing the expression - | -LL | async_dummy() - | -- help: consider `await`ing on the `Future` | LL | false => async_dummy().await, | ^^^^^^ +help: consider removing this semicolon and boxing the expression + | +LL | async_dummy() + | -- error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:39:18 @@ -51,17 +51,17 @@ LL | | }; | = note: expected type `()` found opaque type `impl Future` -help: consider removing this semicolon and boxing the expression - | -LL | async_dummy() - | -- help: consider `await`ing on the `Future` | LL | false => async_dummy2().await, | ^^^^^^ +help: consider removing this semicolon and boxing the expression + | +LL | async_dummy() + | -- error[E0308]: `match` arms have incompatible types - --> $DIR/match-prev-arm-needing-semi.rs:49:18 + --> $DIR/match-prev-arm-needing-semi.rs:50:18 | LL | async fn async_dummy2() {} | - the `Output` of this `async fn`'s found opaque type @@ -81,6 +81,11 @@ LL | | }; = note: expected type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:16:24>) found opaque type `impl Future` (opaque type at <$DIR/match-prev-arm-needing-semi.rs:17:25>) = note: distinct uses of `impl Trait` result in different opaque types +help: consider `await`ing on both `Future`s + | +LL | true => async_dummy().await, +LL | false => async_dummy2().await, + | error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:11:18 diff --git a/src/test/ui/suggestions/opaque-type-error.stderr b/src/test/ui/suggestions/opaque-type-error.stderr index a7c2b82942f05..d74076cbc9b8e 100644 --- a/src/test/ui/suggestions/opaque-type-error.stderr +++ b/src/test/ui/suggestions/opaque-type-error.stderr @@ -16,6 +16,12 @@ LL | | }.await = note: expected type `impl Future` (opaque type at <$DIR/opaque-type-error.rs:8:19>) found opaque type `impl Future` (opaque type at <$DIR/opaque-type-error.rs:12:19>) = note: distinct uses of `impl Trait` result in different opaque types +help: consider `await`ing on both `Future`s + | +LL | thing_one().await +LL | } else { +LL | thing_two().await + | error: aborting due to previous error From f5d7443a6bd90b78e61b9c47e5032b5e1be1e49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 22 Oct 2020 20:02:55 -0700 Subject: [PATCH 14/24] Suggest semicolon removal and boxing when appropriate --- .../src/infer/error_reporting/mod.rs | 35 ++++++++++++++----- .../match-prev-arm-needing-semi.stderr | 23 +++++++----- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index fcf21c61d8fd9..f7e4ace8fc5fc 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -688,12 +688,26 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { let msg = "`match` arms have incompatible types"; err.span_label(outer_error_span, msg); if let Some((sp, boxed)) = semi_span { - if matches!(boxed, StatementAsExpression::NeedsBoxing) { - err.span_suggestion_verbose( + if let (StatementAsExpression::NeedsBoxing, [.., prior_arm]) = + (boxed, &prior_arms[..]) + { + err.multipart_suggestion( + "consider removing this semicolon and boxing the expressions", + vec![ + (prior_arm.shrink_to_lo(), "Box::new(".to_string()), + (prior_arm.shrink_to_hi(), ")".to_string()), + (arm_span.shrink_to_lo(), "Box::new(".to_string()), + (arm_span.shrink_to_hi(), ")".to_string()), + (sp, String::new()), + ], + Applicability::HasPlaceholders, + ); + } else if matches!(boxed, StatementAsExpression::NeedsBoxing) { + err.span_suggestion_short( sp, - "consider removing this semicolon and boxing the expression", + "consider removing this semicolon and boxing the expressions", String::new(), - Applicability::HasPlaceholders, + Applicability::MachineApplicable, ); } else { err.span_suggestion_short( @@ -727,11 +741,16 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } if let Some((sp, boxed)) = semicolon { if matches!(boxed, StatementAsExpression::NeedsBoxing) { - err.span_suggestion_verbose( - sp, + err.multipart_suggestion( "consider removing this semicolon and boxing the expression", - String::new(), - Applicability::HasPlaceholders, + vec![ + (then.shrink_to_lo(), "Box::new(".to_string()), + (then.shrink_to_hi(), ")".to_string()), + (else_sp.shrink_to_lo(), "Box::new(".to_string()), + (else_sp.shrink_to_hi(), ")".to_string()), + (sp, String::new()), + ], + Applicability::MachineApplicable, ); } else { err.span_suggestion_short( diff --git a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr index 7a4f74a1994ce..e9803a78f94b3 100644 --- a/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr +++ b/src/test/ui/suggestions/match-prev-arm-needing-semi.stderr @@ -24,10 +24,13 @@ help: consider `await`ing on the `Future` | LL | false => async_dummy().await, | ^^^^^^ -help: consider removing this semicolon and boxing the expression +help: consider removing this semicolon and boxing the expressions + | +LL | Box::new(async_dummy()) +LL | +LL | } +LL | false => Box::new(async_dummy()), | -LL | async_dummy() - | -- error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:39:18 @@ -55,10 +58,13 @@ help: consider `await`ing on the `Future` | LL | false => async_dummy2().await, | ^^^^^^ -help: consider removing this semicolon and boxing the expression +help: consider removing this semicolon and boxing the expressions + | +LL | Box::new(async_dummy()) +LL | +LL | } +LL | false => Box::new(async_dummy2()), | -LL | async_dummy() - | -- error[E0308]: `match` arms have incompatible types --> $DIR/match-prev-arm-needing-semi.rs:50:18 @@ -70,10 +76,10 @@ LL | let _ = match true { | _____________- LL | | true => async_dummy(), | | ------------- this is found to be of type `impl Future` +LL | | LL | | false => async_dummy2(), | | ^^^^^^^^^^^^^^ expected opaque type, found a different opaque type -LL | | -LL | | +... | LL | | LL | | }; | |_____- `match` arms have incompatible types @@ -84,6 +90,7 @@ LL | | }; help: consider `await`ing on both `Future`s | LL | true => async_dummy().await, +LL | LL | false => async_dummy2().await, | From d413bb6f5716261f2740eb67540df1da1469ce12 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 18 Jul 2020 21:59:53 +0900 Subject: [PATCH 15/24] `#[deny(unsafe_op_in_unsafe_fn)]` in sys/wasm --- library/std/src/sys/wasm/alloc.rs | 8 +++--- library/std/src/sys/wasm/condvar_atomics.rs | 11 +++++--- library/std/src/sys/wasm/mod.rs | 2 ++ library/std/src/sys/wasm/mutex_atomics.rs | 29 +++++++++++++-------- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/library/std/src/sys/wasm/alloc.rs b/library/std/src/sys/wasm/alloc.rs index 32b8b5bdaea7a..e12af19718a7c 100644 --- a/library/std/src/sys/wasm/alloc.rs +++ b/library/std/src/sys/wasm/alloc.rs @@ -25,25 +25,25 @@ unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { let _lock = lock::lock(); - DLMALLOC.malloc(layout.size(), layout.align()) + unsafe { DLMALLOC.malloc(layout.size(), layout.align()) } } #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { let _lock = lock::lock(); - DLMALLOC.calloc(layout.size(), layout.align()) + unsafe { DLMALLOC.calloc(layout.size(), layout.align()) } } #[inline] unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { let _lock = lock::lock(); - DLMALLOC.free(ptr, layout.size(), layout.align()) + unsafe { DLMALLOC.free(ptr, layout.size(), layout.align()) } } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { let _lock = lock::lock(); - DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) + unsafe { DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) } } } diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index a96bb18e6ef1a..b9133e9fb7dbc 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -44,13 +44,18 @@ impl Condvar { pub unsafe fn notify_one(&self) { self.cnt.fetch_add(1, SeqCst); - wasm32::memory_atomic_notify(self.ptr(), 1); + // SAFETY: ptr() is always valid + unsafe { + wasm32::memory_atomic_notify(self.ptr(), 1); + } } #[inline] pub unsafe fn notify_all(&self) { - self.cnt.fetch_add(1, SeqCst); - wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" + unsafe { + self.cnt.fetch_add(1, SeqCst); + wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" + } } pub unsafe fn wait(&self, mutex: &Mutex) { diff --git a/library/std/src/sys/wasm/mod.rs b/library/std/src/sys/wasm/mod.rs index 11c6896f050b2..82683c0f624cf 100644 --- a/library/std/src/sys/wasm/mod.rs +++ b/library/std/src/sys/wasm/mod.rs @@ -14,6 +14,8 @@ //! compiling for wasm. That way it's a compile time error for something that's //! guaranteed to be a runtime error! +#![deny(unsafe_op_in_unsafe_fn)] + pub mod alloc; pub mod args; #[path = "../unsupported/cmath.rs"] diff --git a/library/std/src/sys/wasm/mutex_atomics.rs b/library/std/src/sys/wasm/mutex_atomics.rs index 2970fcf806cbf..11d7f4c389dec 100644 --- a/library/std/src/sys/wasm/mutex_atomics.rs +++ b/library/std/src/sys/wasm/mutex_atomics.rs @@ -28,11 +28,14 @@ impl Mutex { pub unsafe fn lock(&self) { while !self.try_lock() { - let val = wasm32::memory_atomic_wait32( - self.ptr(), - 1, // we expect our mutex is locked - -1, // wait infinitely - ); + // SAFETY: the caller must uphold the safety contract for `memory_atomic_wait32`. + let val = unsafe { + wasm32::memory_atomic_wait32( + self.ptr(), + 1, // we expect our mutex is locked + -1, // wait infinitely + ) + }; // we should have either woke up (0) or got a not-equal due to a // race (1). We should never time out (2) debug_assert!(val == 0 || val == 1); @@ -47,7 +50,7 @@ impl Mutex { #[inline] pub unsafe fn try_lock(&self) -> bool { - self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() + unsafe { self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() } } #[inline] @@ -83,7 +86,7 @@ unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { pub const unsafe fn uninitialized() -> ReentrantMutex { - ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } + unsafe { ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } } } pub unsafe fn init(&self) { @@ -93,19 +96,20 @@ impl ReentrantMutex { pub unsafe fn lock(&self) { let me = thread::my_id(); while let Err(owner) = self._try_lock(me) { - let val = wasm32::memory_atomic_wait32(self.ptr(), owner as i32, -1); + // SAFETY: the caller must gurantee that `self.ptr()` and `owner` are valid i32. + let val = unsafe { wasm32::memory_atomic_wait32(self.ptr(), owner as i32, -1) }; debug_assert!(val == 0 || val == 1); } } #[inline] pub unsafe fn try_lock(&self) -> bool { - self._try_lock(thread::my_id()).is_ok() + unsafe { self._try_lock(thread::my_id()).is_ok() } } #[inline] unsafe fn _try_lock(&self, id: u32) -> Result<(), u32> { - let id = id.checked_add(1).unwrap(); // make sure `id` isn't 0 + let id = id.checked_add(1).unwrap(); match self.owner.compare_exchange(0, id, SeqCst, SeqCst) { // we transitioned from unlocked to locked Ok(_) => { @@ -132,7 +136,10 @@ impl ReentrantMutex { match *self.recursions.get() { 0 => { self.owner.swap(0, SeqCst); - wasm32::memory_atomic_notify(self.ptr() as *mut i32, 1); // wake up one waiter, if any + // SAFETY: the caller must gurantee that `self.ptr()` is valid i32. + unsafe { + wasm32::atomic_notify(self.ptr() as *mut i32, 1); + } // wake up one waiter, if any } ref mut n => *n -= 1, } From eed45107da8da6293d9cecc302ad3b9870848b51 Mon Sep 17 00:00:00 2001 From: chansuke Date: Wed, 21 Oct 2020 08:15:28 +0900 Subject: [PATCH 16/24] Add some description for (malloc/calloc/free/realloc) --- library/std/src/sys/wasm/alloc.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/sys/wasm/alloc.rs b/library/std/src/sys/wasm/alloc.rs index e12af19718a7c..b85f4d50021e3 100644 --- a/library/std/src/sys/wasm/alloc.rs +++ b/library/std/src/sys/wasm/alloc.rs @@ -24,24 +24,28 @@ static mut DLMALLOC: dlmalloc::Dlmalloc = dlmalloc::DLMALLOC_INIT; unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + // SAFETY: DLMALLOC.malloc() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.malloc(layout.size(), layout.align()) } } #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + // SAFETY: DLMALLOC.calloc() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.calloc(layout.size(), layout.align()) } } #[inline] unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { + // SAFETY: DLMALLOC.free() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.free(ptr, layout.size(), layout.align()) } } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { + // SAFETY: DLMALLOC.realloc() is guranteed to be safe since lock::lock() aqcuire a globl lock let _lock = lock::lock(); unsafe { DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) } } From de87ae79610925502f45ec07cf24bac51e037ed1 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 24 Oct 2020 17:59:58 +0900 Subject: [PATCH 17/24] Add documents for DLMALLOC --- library/std/src/sys/wasm/alloc.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/wasm/alloc.rs b/library/std/src/sys/wasm/alloc.rs index b85f4d50021e3..b61a7872265f3 100644 --- a/library/std/src/sys/wasm/alloc.rs +++ b/library/std/src/sys/wasm/alloc.rs @@ -24,28 +24,32 @@ static mut DLMALLOC: dlmalloc::Dlmalloc = dlmalloc::DLMALLOC_INIT; unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { - // SAFETY: DLMALLOC.malloc() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling malloc() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.malloc(layout.size(), layout.align()) } } #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { - // SAFETY: DLMALLOC.calloc() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling calloc() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.calloc(layout.size(), layout.align()) } } #[inline] unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) { - // SAFETY: DLMALLOC.free() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling free() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.free(ptr, layout.size(), layout.align()) } } #[inline] unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 { - // SAFETY: DLMALLOC.realloc() is guranteed to be safe since lock::lock() aqcuire a globl lock + // SAFETY: DLMALLOC access is guranteed to be safe because the lock gives us unique and non-reentrant access. + // Calling realloc() is safe because preconditions on this function match the trait method preconditions. let _lock = lock::lock(); unsafe { DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) } } From d147f78e367386bf63ccb03d453e151e37cfdd81 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 24 Oct 2020 18:14:17 +0900 Subject: [PATCH 18/24] Fix unsafe operation of wasm32::memory_atomic_notify --- library/std/src/sys/wasm/condvar_atomics.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index b9133e9fb7dbc..c2c47910582a0 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -52,8 +52,9 @@ impl Condvar { #[inline] pub unsafe fn notify_all(&self) { + self.cnt.fetch_add(1, SeqCst); + // SAFETY: memory_atomic_notify()is always valid unsafe { - self.cnt.fetch_add(1, SeqCst); wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" } } From d37b8cf729187e5dcabb3650031eb806d1f79770 Mon Sep 17 00:00:00 2001 From: chansuke Date: Sat, 24 Oct 2020 18:22:18 +0900 Subject: [PATCH 19/24] Remove unnecessary unsafe block from condvar_atomics & mutex_atomics --- library/std/src/sys/wasm/condvar_atomics.rs | 2 +- library/std/src/sys/wasm/mutex_atomics.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/library/std/src/sys/wasm/condvar_atomics.rs b/library/std/src/sys/wasm/condvar_atomics.rs index c2c47910582a0..0c1c076cc9142 100644 --- a/library/std/src/sys/wasm/condvar_atomics.rs +++ b/library/std/src/sys/wasm/condvar_atomics.rs @@ -53,7 +53,7 @@ impl Condvar { #[inline] pub unsafe fn notify_all(&self) { self.cnt.fetch_add(1, SeqCst); - // SAFETY: memory_atomic_notify()is always valid + // SAFETY: ptr() is always valid unsafe { wasm32::memory_atomic_notify(self.ptr(), u32::MAX); // -1 == "wake everyone" } diff --git a/library/std/src/sys/wasm/mutex_atomics.rs b/library/std/src/sys/wasm/mutex_atomics.rs index 11d7f4c389dec..479182ffa44d5 100644 --- a/library/std/src/sys/wasm/mutex_atomics.rs +++ b/library/std/src/sys/wasm/mutex_atomics.rs @@ -50,7 +50,7 @@ impl Mutex { #[inline] pub unsafe fn try_lock(&self) -> bool { - unsafe { self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() } + self.locked.compare_exchange(0, 1, SeqCst, SeqCst).is_ok() } #[inline] @@ -86,7 +86,7 @@ unsafe impl Sync for ReentrantMutex {} impl ReentrantMutex { pub const unsafe fn uninitialized() -> ReentrantMutex { - unsafe { ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } } + ReentrantMutex { owner: AtomicU32::new(0), recursions: UnsafeCell::new(0) } } pub unsafe fn init(&self) { From 75bbd80c098c240ffe0f97f50670e4cb66dc59c3 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Fri, 23 Oct 2020 18:41:07 +0100 Subject: [PATCH 20/24] Add some regression tests Closes #56229 Closes #59494 Closes #70746 Closes #73229 --- src/test/ui/issues/issue-56229.rs | 35 +++++++++++++++++++++++++++ src/test/ui/issues/issue-59494.rs | 23 ++++++++++++++++++ src/test/ui/issues/issue-59494.stderr | 14 +++++++++++ src/test/ui/issues/issue-70746.rs | 29 ++++++++++++++++++++++ src/test/ui/issues/issue-73229.rs | 33 +++++++++++++++++++++++++ 5 files changed, 134 insertions(+) create mode 100644 src/test/ui/issues/issue-56229.rs create mode 100644 src/test/ui/issues/issue-59494.rs create mode 100644 src/test/ui/issues/issue-59494.stderr create mode 100644 src/test/ui/issues/issue-70746.rs create mode 100644 src/test/ui/issues/issue-73229.rs diff --git a/src/test/ui/issues/issue-56229.rs b/src/test/ui/issues/issue-56229.rs new file mode 100644 index 0000000000000..9e5897b98925a --- /dev/null +++ b/src/test/ui/issues/issue-56229.rs @@ -0,0 +1,35 @@ +// check-pass + +trait Mirror { + type Other; +} + +#[derive(Debug)] +struct Even(usize); +struct Odd; + +impl Mirror for Even { + type Other = Odd; +} + +impl Mirror for Odd { + type Other = Even; +} + +trait Dyn: AsRef<::Other> {} + +impl Dyn for Even {} + +impl AsRef for Even { + fn as_ref(&self) -> &Even { + self + } +} + +fn code(d: &dyn Dyn) -> &T::Other { + d.as_ref() +} + +fn main() { + println!("{:?}", code(&Even(22))); +} diff --git a/src/test/ui/issues/issue-59494.rs b/src/test/ui/issues/issue-59494.rs new file mode 100644 index 0000000000000..06b8eb777c035 --- /dev/null +++ b/src/test/ui/issues/issue-59494.rs @@ -0,0 +1,23 @@ +fn t7p(f: impl Fn(B) -> C, g: impl Fn(A) -> B) -> impl Fn(A) -> C { + move |a: A| -> C { f(g(a)) } +} + +fn t8n(f: impl Fn(A) -> B, g: impl Fn(A) -> C) -> impl Fn(A) -> (B, C) +where + A: Copy, +{ + move |a: A| -> (B, C) { + let b = a; + let fa = f(a); + let ga = g(b); + (fa, ga) + } +} + +fn main() { + let f = |(_, _)| {}; + let g = |(a, _)| a; + let t7 = |env| |a| |b| t7p(f, g)(((env, a), b)); + let t8 = t8n(t7, t7p(f, g)); + //~^ ERROR: expected a `Fn<(_,)>` closure, found `impl Fn<(((_, _), _),)> +} diff --git a/src/test/ui/issues/issue-59494.stderr b/src/test/ui/issues/issue-59494.stderr new file mode 100644 index 0000000000000..e2ac5d94da108 --- /dev/null +++ b/src/test/ui/issues/issue-59494.stderr @@ -0,0 +1,14 @@ +error[E0277]: expected a `Fn<(_,)>` closure, found `impl Fn<(((_, _), _),)>` + --> $DIR/issue-59494.rs:21:22 + | +LL | fn t8n(f: impl Fn(A) -> B, g: impl Fn(A) -> C) -> impl Fn(A) -> (B, C) + | ---------- required by this bound in `t8n` +... +LL | let t8 = t8n(t7, t7p(f, g)); + | ^^^^^^^^^ expected an `Fn<(_,)>` closure, found `impl Fn<(((_, _), _),)>` + | + = help: the trait `Fn<(_,)>` is not implemented for `impl Fn<(((_, _), _),)>` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/issues/issue-70746.rs b/src/test/ui/issues/issue-70746.rs new file mode 100644 index 0000000000000..8930c15f57edc --- /dev/null +++ b/src/test/ui/issues/issue-70746.rs @@ -0,0 +1,29 @@ +// check-pass + +pub trait Trait1 { + type C; +} + +struct T1; +impl Trait1 for T1 { + type C = usize; +} +pub trait Callback: FnMut(::C) {} +impl::C)> Callback for F {} + +pub struct State { + callback: Option>>, +} +impl State { + fn new() -> Self { + Self { callback: None } + } + fn test_cb(&mut self, d: ::C) { + (self.callback.as_mut().unwrap())(d) + } +} + +fn main() { + let mut s = State::::new(); + s.test_cb(1); +} diff --git a/src/test/ui/issues/issue-73229.rs b/src/test/ui/issues/issue-73229.rs new file mode 100644 index 0000000000000..35346199add92 --- /dev/null +++ b/src/test/ui/issues/issue-73229.rs @@ -0,0 +1,33 @@ +// check-pass + +fn any() -> T { + loop {} +} + +trait Foo { + type V; +} + +trait Callback: Fn(&T, &T::V) {} +impl Callback for F {} + +struct Bar { + callback: Box>, +} + +impl Bar { + fn event(&self) { + (self.callback)(any(), any()); + } +} + +struct A; +struct B; +impl Foo for A { + type V = B; +} + +fn main() { + let foo = Bar:: { callback: Box::new(|_: &A, _: &B| ()) }; + foo.event(); +} From c6ab758e54bd509f75df7a21fd72aa740ac5a4b0 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Sat, 24 Oct 2020 09:44:57 -0400 Subject: [PATCH 21/24] Switch from tuple matching to match guards --- compiler/rustc_codegen_llvm/src/va_arg.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/va_arg.rs b/compiler/rustc_codegen_llvm/src/va_arg.rs index 5f820f83a9438..0d4aa08a873b4 100644 --- a/compiler/rustc_codegen_llvm/src/va_arg.rs +++ b/compiler/rustc_codegen_llvm/src/va_arg.rs @@ -173,26 +173,24 @@ pub(super) fn emit_va_arg( // is lacking in some instances, so we should only use it as a fallback. let target = &bx.cx.tcx.sess.target; let arch = &bx.cx.tcx.sess.target.arch; - match (&**arch, target.options.is_like_windows) { + match &**arch { // Windows x86 - ("x86", true) => { + "x86" if target.options.is_like_windows => { emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(4).unwrap(), false) } // Generic x86 - ("x86", _) => { - emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(4).unwrap(), true) - } + "x86" => emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(4).unwrap(), true), // Windows AArch64 - ("aarch64", true) => { + "aarch64" if target.options.is_like_windows => { emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(8).unwrap(), false) } // iOS AArch64 - ("aarch64", _) if target.target_os == "ios" => { + "aarch64" if target.target_os == "ios" => { emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(8).unwrap(), true) } - ("aarch64", _) => emit_aapcs_va_arg(bx, addr, target_ty), + "aarch64" => emit_aapcs_va_arg(bx, addr, target_ty), // Windows x86_64 - ("x86_64", true) => { + "x86_64" if target.options.is_like_windows => { let target_ty_size = bx.cx.size_of(target_ty).bytes(); let indirect: bool = target_ty_size > 8 || !target_ty_size.is_power_of_two(); emit_ptr_va_arg(bx, addr, target_ty, indirect, Align::from_bytes(8).unwrap(), false) From 7b4c397b73912d3c604667933fb7e64f0c1a366a Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Fri, 23 Oct 2020 18:00:18 +0900 Subject: [PATCH 22/24] Do not try to report on closures to avoid ICE --- .../nice_region_error/static_impl_trait.rs | 8 ++++++++ src/test/ui/regions/issue-78262.rs | 9 +++++++++ src/test/ui/regions/issue-78262.stderr | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+) create mode 100644 src/test/ui/regions/issue-78262.rs create mode 100644 src/test/ui/regions/issue-78262.stderr diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs index 441cfeea20a48..e9d5ebad7de03 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -39,6 +39,14 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { ) if **sub_r == RegionKind::ReStatic => { // This is for an implicit `'static` requirement coming from `impl dyn Trait {}`. if let ObligationCauseCode::UnifyReceiver(ctxt) = &cause.code { + // This may have a closure and it would cause ICE + // through `find_param_with_region` (#78262). + let anon_reg_sup = tcx.is_suitable_region(sup_r)?; + let fn_returns = tcx.return_type_impl_or_dyn_traits(anon_reg_sup.def_id); + if fn_returns.is_empty() { + return None; + } + let param = self.find_param_with_region(sup_r, sub_r)?; let lifetime = if sup_r.has_name() { format!("lifetime `{}`", sup_r) diff --git a/src/test/ui/regions/issue-78262.rs b/src/test/ui/regions/issue-78262.rs new file mode 100644 index 0000000000000..2324152d2c081 --- /dev/null +++ b/src/test/ui/regions/issue-78262.rs @@ -0,0 +1,9 @@ +trait TT {} + +impl dyn TT { + fn func(&self) {} +} + +fn main() { + let f = |x: &dyn TT| x.func(); //~ ERROR: mismatched types +} diff --git a/src/test/ui/regions/issue-78262.stderr b/src/test/ui/regions/issue-78262.stderr new file mode 100644 index 0000000000000..580cea00ecd4f --- /dev/null +++ b/src/test/ui/regions/issue-78262.stderr @@ -0,0 +1,18 @@ +error[E0308]: mismatched types + --> $DIR/issue-78262.rs:8:28 + | +LL | let f = |x: &dyn TT| x.func(); + | ^^^^ lifetime mismatch + | + = note: expected reference `&(dyn TT + 'static)` + found reference `&dyn TT` +note: the anonymous lifetime #1 defined on the body at 8:13... + --> $DIR/issue-78262.rs:8:13 + | +LL | let f = |x: &dyn TT| x.func(); + | ^^^^^^^^^^^^^^^^^^^^^ + = note: ...does not necessarily outlive the static lifetime + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 4ec396ea5dd7bddfaa667766ab6cd8824c8028da Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Sun, 25 Oct 2020 11:43:26 +0900 Subject: [PATCH 23/24] Test with NLL explicitly --- .../{issue-78262.stderr => issue-78262.default.stderr} | 6 +++--- src/test/ui/regions/issue-78262.nll.stderr | 10 ++++++++++ src/test/ui/regions/issue-78262.rs | 7 ++++++- 3 files changed, 19 insertions(+), 4 deletions(-) rename src/test/ui/regions/{issue-78262.stderr => issue-78262.default.stderr} (79%) create mode 100644 src/test/ui/regions/issue-78262.nll.stderr diff --git a/src/test/ui/regions/issue-78262.stderr b/src/test/ui/regions/issue-78262.default.stderr similarity index 79% rename from src/test/ui/regions/issue-78262.stderr rename to src/test/ui/regions/issue-78262.default.stderr index 580cea00ecd4f..e97b8eca94892 100644 --- a/src/test/ui/regions/issue-78262.stderr +++ b/src/test/ui/regions/issue-78262.default.stderr @@ -1,13 +1,13 @@ error[E0308]: mismatched types - --> $DIR/issue-78262.rs:8:28 + --> $DIR/issue-78262.rs:12:28 | LL | let f = |x: &dyn TT| x.func(); | ^^^^ lifetime mismatch | = note: expected reference `&(dyn TT + 'static)` found reference `&dyn TT` -note: the anonymous lifetime #1 defined on the body at 8:13... - --> $DIR/issue-78262.rs:8:13 +note: the anonymous lifetime #1 defined on the body at 12:13... + --> $DIR/issue-78262.rs:12:13 | LL | let f = |x: &dyn TT| x.func(); | ^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/regions/issue-78262.nll.stderr b/src/test/ui/regions/issue-78262.nll.stderr new file mode 100644 index 0000000000000..4607dbad4220b --- /dev/null +++ b/src/test/ui/regions/issue-78262.nll.stderr @@ -0,0 +1,10 @@ +error[E0521]: borrowed data escapes outside of closure + --> $DIR/issue-78262.rs:12:26 + | +LL | let f = |x: &dyn TT| x.func(); + | - ^^^^^^^^ `x` escapes the closure body here + | | + | `x` is a reference that is only valid in the closure body + +error: aborting due to previous error + diff --git a/src/test/ui/regions/issue-78262.rs b/src/test/ui/regions/issue-78262.rs index 2324152d2c081..0bdb0abac307d 100644 --- a/src/test/ui/regions/issue-78262.rs +++ b/src/test/ui/regions/issue-78262.rs @@ -1,3 +1,7 @@ +// revisions: nll default +// ignore-compare-mode-nll +//[nll]compile-flags: -Z borrowck=mir + trait TT {} impl dyn TT { @@ -5,5 +9,6 @@ impl dyn TT { } fn main() { - let f = |x: &dyn TT| x.func(); //~ ERROR: mismatched types + let f = |x: &dyn TT| x.func(); //[default]~ ERROR: mismatched types + //[nll]~^ ERROR: borrowed data escapes outside of closure } From 0a91755ff4b6899e1c0675c48b4652e890ce63aa Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Mon, 19 Oct 2020 21:48:58 -0400 Subject: [PATCH 24/24] Properly define va_arg and va_list for aarch64-apple-darwin From [Apple][]: > Because of these changes, the type `va_list` is an alias for `char*`, > and not for the struct type in the generic procedure call standard. With this change `/x.py test --stage 1 src/test/ui/abi/variadic-ffi` passes. Fixes #78092 [Apple]: https://developer.apple.com/documentation/xcode/writing_arm64_code_for_apple_platforms --- compiler/rustc_codegen_llvm/src/va_arg.rs | 4 ++-- library/core/src/ffi.rs | 18 +++++++++++------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/va_arg.rs b/compiler/rustc_codegen_llvm/src/va_arg.rs index 0d4aa08a873b4..b6a0516b8bc9c 100644 --- a/compiler/rustc_codegen_llvm/src/va_arg.rs +++ b/compiler/rustc_codegen_llvm/src/va_arg.rs @@ -184,8 +184,8 @@ pub(super) fn emit_va_arg( "aarch64" if target.options.is_like_windows => { emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(8).unwrap(), false) } - // iOS AArch64 - "aarch64" if target.target_os == "ios" => { + // macOS / iOS AArch64 + "aarch64" if target.options.is_like_osx => { emit_ptr_va_arg(bx, addr, target_ty, false, Align::from_bytes(8).unwrap(), true) } "aarch64" => emit_aapcs_va_arg(bx, addr, target_ty), diff --git a/library/core/src/ffi.rs b/library/core/src/ffi.rs index e146a97ae94d1..4b303acfd3bfb 100644 --- a/library/core/src/ffi.rs +++ b/library/core/src/ffi.rs @@ -62,7 +62,7 @@ impl fmt::Debug for c_void { // The name is WIP, using `VaListImpl` for now. #[cfg(any( all(not(target_arch = "aarch64"), not(target_arch = "powerpc"), not(target_arch = "x86_64")), - all(target_arch = "aarch64", target_os = "ios"), + all(target_arch = "aarch64", any(target_os = "macos", target_os = "ios")), target_arch = "wasm32", target_arch = "asmjs", windows @@ -85,7 +85,7 @@ pub struct VaListImpl<'f> { #[cfg(any( all(not(target_arch = "aarch64"), not(target_arch = "powerpc"), not(target_arch = "x86_64")), - all(target_arch = "aarch64", target_os = "ios"), + all(target_arch = "aarch64", any(target_os = "macos", target_os = "ios")), target_arch = "wasm32", target_arch = "asmjs", windows @@ -107,7 +107,11 @@ impl<'f> fmt::Debug for VaListImpl<'f> { /// /// [AArch64 Procedure Call Standard]: /// http://infocenter.arm.com/help/topic/com.arm.doc.ihi0055b/IHI0055B_aapcs64.pdf -#[cfg(all(target_arch = "aarch64", not(target_os = "ios"), not(windows)))] +#[cfg(all( + target_arch = "aarch64", + not(any(target_os = "macos", target_os = "ios")), + not(windows) +))] #[repr(C)] #[derive(Debug)] #[unstable( @@ -181,7 +185,7 @@ pub struct VaList<'a, 'f: 'a> { not(target_arch = "powerpc"), not(target_arch = "x86_64") ), - all(target_arch = "aarch64", target_os = "ios"), + all(target_arch = "aarch64", any(target_os = "macos", target_os = "ios")), target_arch = "wasm32", target_arch = "asmjs", windows @@ -190,7 +194,7 @@ pub struct VaList<'a, 'f: 'a> { #[cfg(all( any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"), - any(not(target_arch = "aarch64"), not(target_os = "ios")), + any(not(target_arch = "aarch64"), not(any(target_os = "macos", target_os = "ios"))), not(target_arch = "wasm32"), not(target_arch = "asmjs"), not(windows) @@ -202,7 +206,7 @@ pub struct VaList<'a, 'f: 'a> { #[cfg(any( all(not(target_arch = "aarch64"), not(target_arch = "powerpc"), not(target_arch = "x86_64")), - all(target_arch = "aarch64", target_os = "ios"), + all(target_arch = "aarch64", any(target_os = "macos", target_os = "ios")), target_arch = "wasm32", target_arch = "asmjs", windows @@ -223,7 +227,7 @@ impl<'f> VaListImpl<'f> { #[cfg(all( any(target_arch = "aarch64", target_arch = "powerpc", target_arch = "x86_64"), - any(not(target_arch = "aarch64"), not(target_os = "ios")), + any(not(target_arch = "aarch64"), not(any(target_os = "macos", target_os = "ios"))), not(target_arch = "wasm32"), not(target_arch = "asmjs"), not(windows)