From c7899a027ee7b563664a2f6b2967e1f96e2e619a Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Fri, 17 Apr 2020 17:51:32 -0400 Subject: [PATCH 1/7] Remove unused abs_path method from rustc_span::source_map::FileLoader --- src/librustc_span/source_map.rs | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/librustc_span/source_map.rs b/src/librustc_span/source_map.rs index 49e2144b3e380..d27aae0d6ed9b 100644 --- a/src/librustc_span/source_map.rs +++ b/src/librustc_span/source_map.rs @@ -20,7 +20,6 @@ use std::path::{Path, PathBuf}; use std::sync::atomic::Ordering; use log::debug; -use std::env; use std::fs; use std::io; @@ -64,9 +63,6 @@ pub trait FileLoader { /// Query the existence of a file. fn file_exists(&self, path: &Path) -> bool; - /// Returns an absolute path to a file, if possible. - fn abs_path(&self, path: &Path) -> Option; - /// Read the contents of an UTF-8 file into memory. fn read_file(&self, path: &Path) -> io::Result; } @@ -79,14 +75,6 @@ impl FileLoader for RealFileLoader { fs::metadata(path).is_ok() } - fn abs_path(&self, path: &Path) -> Option { - if path.is_absolute() { - Some(path.to_path_buf()) - } else { - env::current_dir().ok().map(|cwd| cwd.join(path)) - } - } - fn read_file(&self, path: &Path) -> io::Result { fs::read_to_string(path) } From bb1eedb026931853e2f37e752c45b7f3b59c5fa6 Mon Sep 17 00:00:00 2001 From: YI Date: Wed, 22 Apr 2020 15:33:34 +0800 Subject: [PATCH 2/7] add message for resolution failure because wrong namespace --- src/librustc_resolve/lib.rs | 125 ++++++++++++++++---------- src/test/ui/issues/issue-71406.rs | 6 ++ src/test/ui/issues/issue-71406.stderr | 9 ++ 3 files changed, 95 insertions(+), 45 deletions(-) create mode 100644 src/test/ui/issues/issue-71406.rs create mode 100644 src/test/ui/issues/issue-71406.stderr diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e94d7d6a85fb4..71fb8147793a8 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2064,52 +2064,64 @@ impl<'a> Resolver<'a> { }; } - let binding = if let Some(module) = module { - self.resolve_ident_in_module( - module, - ident, - ns, - parent_scope, - record_used, - path_span, - ) - } else if ribs.is_none() || opt_ns.is_none() || opt_ns == Some(MacroNS) { - let scopes = ScopeSet::All(ns, opt_ns.is_none()); - self.early_resolve_ident_in_lexical_scope( - ident, - scopes, - parent_scope, - record_used, - record_used, - path_span, - ) - } else { - let record_used_id = - if record_used { crate_lint.node_id().or(Some(CRATE_NODE_ID)) } else { None }; - match self.resolve_ident_in_lexical_scope( - ident, - ns, - parent_scope, - record_used_id, - path_span, - &ribs.unwrap()[ns], - ) { - // we found a locally-imported or available item/module - Some(LexicalScopeBinding::Item(binding)) => Ok(binding), - // we found a local variable or type param - Some(LexicalScopeBinding::Res(res)) - if opt_ns == Some(TypeNS) || opt_ns == Some(ValueNS) => - { - record_segment_res(self, res); - return PathResult::NonModule(PartialRes::with_unresolved_segments( - res, - path.len() - 1, - )); + enum FindBindingResult<'a> { + Binding(Result<&'a NameBinding<'a>, Determinacy>), + PathResult(PathResult<'a>), + } + let find_binding_in_ns = |this: &mut Self, ns| { + let binding = if let Some(module) = module { + this.resolve_ident_in_module( + module, + ident, + ns, + parent_scope, + record_used, + path_span, + ) + } else if ribs.is_none() || opt_ns.is_none() || opt_ns == Some(MacroNS) { + let scopes = ScopeSet::All(ns, opt_ns.is_none()); + this.early_resolve_ident_in_lexical_scope( + ident, + scopes, + parent_scope, + record_used, + record_used, + path_span, + ) + } else { + let record_used_id = if record_used { + crate_lint.node_id().or(Some(CRATE_NODE_ID)) + } else { + None + }; + match this.resolve_ident_in_lexical_scope( + ident, + ns, + parent_scope, + record_used_id, + path_span, + &ribs.unwrap()[ns], + ) { + // we found a locally-imported or available item/module + Some(LexicalScopeBinding::Item(binding)) => Ok(binding), + // we found a local variable or type param + Some(LexicalScopeBinding::Res(res)) + if opt_ns == Some(TypeNS) || opt_ns == Some(ValueNS) => + { + record_segment_res(this, res); + return FindBindingResult::PathResult(PathResult::NonModule( + PartialRes::with_unresolved_segments(res, path.len() - 1), + )); + } + _ => Err(Determinacy::determined(record_used)), } - _ => Err(Determinacy::determined(record_used)), - } + }; + FindBindingResult::Binding(binding) + }; + let binding = match find_binding_in_ns(self, ns) { + FindBindingResult::PathResult(x) => return x, + FindBindingResult::Binding(binding) => binding, }; - match binding { Ok(binding) => { if i == 1 { @@ -2199,7 +2211,30 @@ impl<'a> Resolver<'a> { } else if i == 0 { (format!("use of undeclared type or module `{}`", ident), None) } else { - (format!("could not find `{}` in `{}`", ident, path[i - 1].ident), None) + let mut msg = + format!("could not find `{}` in `{}`", ident, path[i - 1].ident); + if ns == TypeNS { + if let FindBindingResult::Binding(Ok(_)) = + find_binding_in_ns(self, ValueNS) + { + msg = format!( + "`{}` in `{}` is a concrete value, not a module or Struct you specified", + ident, + path[i - 1].ident + ); + }; + } else if ns == ValueNS { + if let FindBindingResult::Binding(Ok(_)) = + find_binding_in_ns(self, TypeNS) + { + msg = format!( + "`{}` in `{}` is a type, not a concrete value you specified", + ident, + path[i - 1].ident + ); + }; + } + (msg, None) }; return PathResult::Failed { span: ident.span, diff --git a/src/test/ui/issues/issue-71406.rs b/src/test/ui/issues/issue-71406.rs new file mode 100644 index 0000000000000..e3de30f92896e --- /dev/null +++ b/src/test/ui/issues/issue-71406.rs @@ -0,0 +1,6 @@ +use std::sync::mpsc; + +fn main() { + let (tx, rx) = mpsc::channel::new(1); + //~^ ERROR `channel` in `mpsc` is a concrete value, not a module or Struct you specified +} diff --git a/src/test/ui/issues/issue-71406.stderr b/src/test/ui/issues/issue-71406.stderr new file mode 100644 index 0000000000000..22a2ca4f3e233 --- /dev/null +++ b/src/test/ui/issues/issue-71406.stderr @@ -0,0 +1,9 @@ +error[E0433]: failed to resolve: `channel` in `mpsc` is a concrete value, not a module or Struct you specified + --> $DIR/issue-71406.rs:4:26 + | +LL | let (tx, rx) = mpsc::channel::new(1); + | ^^^^^^^ `channel` in `mpsc` is a concrete value, not a module or Struct you specified + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0433`. From eb8a7031ef1ecd9b44a929f0b93f0e41c78fda25 Mon Sep 17 00:00:00 2001 From: YI Date: Sun, 26 Apr 2020 10:28:33 +0800 Subject: [PATCH 3/7] use defkind.descr in wrong namespace resolve failure --- src/librustc_resolve/lib.rs | 39 ++++++++++++++------------- src/test/ui/issues/issue-71406.rs | 2 +- src/test/ui/issues/issue-71406.stderr | 4 +-- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 71fb8147793a8..2d53b7553a156 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2213,25 +2213,28 @@ impl<'a> Resolver<'a> { } else { let mut msg = format!("could not find `{}` in `{}`", ident, path[i - 1].ident); - if ns == TypeNS { - if let FindBindingResult::Binding(Ok(_)) = - find_binding_in_ns(self, ValueNS) + if ns == TypeNS || ns == ValueNS { + let ns_to_try = if ns == TypeNS { ValueNS } else { TypeNS }; + if let FindBindingResult::Binding(Ok(binding)) = + find_binding_in_ns(self, ns_to_try) { - msg = format!( - "`{}` in `{}` is a concrete value, not a module or Struct you specified", - ident, - path[i - 1].ident - ); - }; - } else if ns == ValueNS { - if let FindBindingResult::Binding(Ok(_)) = - find_binding_in_ns(self, TypeNS) - { - msg = format!( - "`{}` in `{}` is a type, not a concrete value you specified", - ident, - path[i - 1].ident - ); + let mut found = |what| { + msg = format!( + "expected {}, found {} `{}` in `{}`", + ns.descr(), + what, + ident, + path[i - 1].ident + ) + }; + if binding.module().is_some() { + found("module") + } else { + match binding.res() { + def::Res::::Def(kind, id) => found(kind.descr(id)), + _ => found(ns_to_try.descr()), + } + } }; } (msg, None) diff --git a/src/test/ui/issues/issue-71406.rs b/src/test/ui/issues/issue-71406.rs index e3de30f92896e..6266112c3a86c 100644 --- a/src/test/ui/issues/issue-71406.rs +++ b/src/test/ui/issues/issue-71406.rs @@ -2,5 +2,5 @@ use std::sync::mpsc; fn main() { let (tx, rx) = mpsc::channel::new(1); - //~^ ERROR `channel` in `mpsc` is a concrete value, not a module or Struct you specified + //~^ ERROR expected type, found function `channel` in `mpsc` } diff --git a/src/test/ui/issues/issue-71406.stderr b/src/test/ui/issues/issue-71406.stderr index 22a2ca4f3e233..918163b609473 100644 --- a/src/test/ui/issues/issue-71406.stderr +++ b/src/test/ui/issues/issue-71406.stderr @@ -1,8 +1,8 @@ -error[E0433]: failed to resolve: `channel` in `mpsc` is a concrete value, not a module or Struct you specified +error[E0433]: failed to resolve: expected type, found function `channel` in `mpsc` --> $DIR/issue-71406.rs:4:26 | LL | let (tx, rx) = mpsc::channel::new(1); - | ^^^^^^^ `channel` in `mpsc` is a concrete value, not a module or Struct you specified + | ^^^^^^^ expected type, found function `channel` in `mpsc` error: aborting due to previous error From f5223a34354183f0f45e5c25c199b05e3422a045 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 1 Feb 2020 00:18:14 +0300 Subject: [PATCH 4/7] Stabilize `Span::mixed_site` --- src/libproc_macro/lib.rs | 2 +- src/test/ui/proc-macro/auxiliary/mixed-site-span.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libproc_macro/lib.rs b/src/libproc_macro/lib.rs index 5995f64dc7806..ad86028fb76bc 100644 --- a/src/libproc_macro/lib.rs +++ b/src/libproc_macro/lib.rs @@ -303,7 +303,7 @@ impl Span { /// definition site (local variables, labels, `$crate`) and sometimes at the macro /// call site (everything else). /// The span location is taken from the call-site. - #[unstable(feature = "proc_macro_mixed_site", issue = "65049")] + #[stable(feature = "proc_macro_mixed_site", since = "1.45.0")] pub fn mixed_site() -> Span { Span(bridge::client::Span::mixed_site()) } diff --git a/src/test/ui/proc-macro/auxiliary/mixed-site-span.rs b/src/test/ui/proc-macro/auxiliary/mixed-site-span.rs index dea5ea04aa850..c63006e7a407d 100644 --- a/src/test/ui/proc-macro/auxiliary/mixed-site-span.rs +++ b/src/test/ui/proc-macro/auxiliary/mixed-site-span.rs @@ -2,7 +2,6 @@ // no-prefer-dynamic #![feature(proc_macro_hygiene)] -#![feature(proc_macro_mixed_site)] #![feature(proc_macro_quote)] #![crate_type = "proc-macro"] From 7aebdb639ade703a2a9f55a83df21a79fbbd9f69 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 26 Apr 2020 19:00:57 +0200 Subject: [PATCH 5/7] remove Unique::from for shared pointer types --- src/liballoc/collections/btree/node.rs | 2 +- src/liballoc/raw_vec.rs | 4 ++-- src/libcore/ptr/unique.rs | 17 ----------------- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/src/liballoc/collections/btree/node.rs b/src/liballoc/collections/btree/node.rs index 84d34db2d458a..5569c293e2f66 100644 --- a/src/liballoc/collections/btree/node.rs +++ b/src/liballoc/collections/btree/node.rs @@ -131,7 +131,7 @@ impl BoxedNode { } unsafe fn from_ptr(ptr: NonNull>) -> Self { - BoxedNode { ptr: Unique::from(ptr) } + BoxedNode { ptr: Unique::new_unchecked(ptr.as_ptr()) } } fn as_ptr(&self) -> NonNull> { diff --git a/src/liballoc/raw_vec.rs b/src/liballoc/raw_vec.rs index 0780b33e53ae6..ca165b61e26a7 100644 --- a/src/liballoc/raw_vec.rs +++ b/src/liballoc/raw_vec.rs @@ -151,7 +151,7 @@ impl RawVec { let memory = alloc.alloc(layout, init).unwrap_or_else(|_| handle_alloc_error(layout)); Self { - ptr: memory.ptr.cast().into(), + ptr: unsafe { Unique::new_unchecked(memory.ptr.cast().as_ptr()) }, cap: Self::capacity_from_bytes(memory.size), alloc, } @@ -469,7 +469,7 @@ impl RawVec { } fn set_memory(&mut self, memory: MemoryBlock) { - self.ptr = memory.ptr.cast().into(); + self.ptr = unsafe { Unique::new_unchecked(memory.ptr.cast().as_ptr()) }; self.cap = Self::capacity_from_bytes(memory.size); } diff --git a/src/libcore/ptr/unique.rs b/src/libcore/ptr/unique.rs index 87b56d951c6ce..d93dc1f326231 100644 --- a/src/libcore/ptr/unique.rs +++ b/src/libcore/ptr/unique.rs @@ -3,7 +3,6 @@ use crate::fmt; use crate::marker::{PhantomData, Unsize}; use crate::mem; use crate::ops::{CoerceUnsized, DispatchFromDyn}; -use crate::ptr::NonNull; // ignore-tidy-undocumented-unsafe @@ -171,19 +170,3 @@ impl From<&mut T> for Unique { unsafe { Unique { pointer: reference as *mut T, _marker: PhantomData } } } } - -#[unstable(feature = "ptr_internals", issue = "none")] -impl From<&T> for Unique { - #[inline] - fn from(reference: &T) -> Self { - unsafe { Unique { pointer: reference as *const T, _marker: PhantomData } } - } -} - -#[unstable(feature = "ptr_internals", issue = "none")] -impl From> for Unique { - #[inline] - fn from(p: NonNull) -> Self { - unsafe { Unique::new_unchecked(p.as_ptr()) } - } -} From 6e3ba6f40fb30dcdbb553b7044ef45ec49745ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 22 Apr 2020 11:58:21 -0700 Subject: [PATCH 6/7] Tweak some suggestions in `rustc_resolve` --- src/librustc_resolve/late/diagnostics.rs | 39 ++++++++++++------- src/test/ui/codemap_tests/two_files.stderr | 6 ++- src/test/ui/error-codes/E0423.stderr | 9 +++-- src/test/ui/resolve/issue-3907.stderr | 6 ++- src/test/ui/resolve/issue-5035.stderr | 6 ++- ...xed-closure-sugar-nonexistent-trait.stderr | 6 ++- .../ui/struct-literal-variant-in-if.stderr | 9 +++-- 7 files changed, 56 insertions(+), 25 deletions(-) diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs index e7fa88bff97d5..8727e280bcb9d 100644 --- a/src/librustc_resolve/late/diagnostics.rs +++ b/src/librustc_resolve/late/diagnostics.rs @@ -383,7 +383,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { has_self_arg } - fn followed_by_brace(&self, span: Span) -> (bool, Option<(Span, String)>) { + fn followed_by_brace(&self, span: Span) -> (bool, Option) { // HACK(estebank): find a better way to figure out that this was a // parser issue where a struct literal is being used on an expression // where a brace being opened means a block is being started. Look @@ -406,7 +406,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { _ => false, }; // In case this could be a struct literal that needs to be surrounded - // by parenthesis, find the appropriate span. + // by parentheses, find the appropriate span. let mut i = 0; let mut closing_brace = None; loop { @@ -414,10 +414,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { match sm.span_to_snippet(sp) { Ok(ref snippet) => { if snippet == "}" { - let sp = span.to(sp); - if let Ok(snippet) = sm.span_to_snippet(sp) { - closing_brace = Some((sp, snippet)); - } + closing_brace = Some(span.to(sp)); break; } } @@ -479,17 +476,23 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { suggested = path_sep(err, &parent); } PathSource::Expr(None) if followed_by_brace => { - if let Some((sp, snippet)) = closing_brace { - err.span_suggestion( - sp, - "surround the struct literal with parenthesis", - format!("({})", snippet), + if let Some(sp) = closing_brace { + err.multipart_suggestion( + "surround the struct literal with parentheses", + vec![ + (sp.shrink_to_lo(), "(".to_string()), + (sp.shrink_to_hi(), ")".to_string()), + ], Applicability::MaybeIncorrect, ); } else { err.span_label( - span, // Note the parenthesis surrounding the suggestion below - format!("did you mean `({} {{ /* fields */ }})`?", path_str), + span, // Note the parentheses surrounding the suggestion below + format!( + "you might want to surround a struct literal with parentheses: \ + `({} {{ /* fields */ }})`?", + path_str + ), ); } suggested = true; @@ -516,10 +519,16 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> { err.note("if you want the `try` keyword, you need to be in the 2018 edition"); } } - (Res::Def(DefKind::TyAlias, _), PathSource::Trait(_)) => { + (Res::Def(DefKind::TyAlias, def_id), PathSource::Trait(_)) => { err.span_label(span, "type aliases cannot be used as traits"); if nightly_options::is_nightly_build() { - err.note("did you mean to use a trait alias?"); + let msg = "you might have meant to use `#![feature(trait_alias)]` instead of a \ + `type` alias"; + if let Some(span) = self.r.definitions.opt_span(def_id) { + err.span_help(span, msg); + } else { + err.help(msg); + } } } (Res::Def(DefKind::Mod, _), PathSource::Expr(Some(parent))) => { diff --git a/src/test/ui/codemap_tests/two_files.stderr b/src/test/ui/codemap_tests/two_files.stderr index 5027b78b38e34..de2ffc2e5dc1d 100644 --- a/src/test/ui/codemap_tests/two_files.stderr +++ b/src/test/ui/codemap_tests/two_files.stderr @@ -4,7 +4,11 @@ error[E0404]: expected trait, found type alias `Bar` LL | impl Bar for Baz { } | ^^^ type aliases cannot be used as traits | - = note: did you mean to use a trait alias? +help: you might have meant to use `#![feature(trait_alias)]` instead of a `type` alias + --> $DIR/two_files_data.rs:5:1 + | +LL | type Bar = dyn Foo; + | ^^^^^^^^^^^^^^^^^^^ error: aborting due to previous error diff --git a/src/test/ui/error-codes/E0423.stderr b/src/test/ui/error-codes/E0423.stderr index a985e963e5726..d4860394259b7 100644 --- a/src/test/ui/error-codes/E0423.stderr +++ b/src/test/ui/error-codes/E0423.stderr @@ -45,9 +45,12 @@ error[E0423]: expected value, found struct `T` --> $DIR/E0423.rs:14:8 | LL | if T {} == T {} { println!("Ok"); } - | ^--- - | | - | help: surround the struct literal with parenthesis: `(T {})` + | ^ + | +help: surround the struct literal with parentheses + | +LL | if (T {}) == T {} { println!("Ok"); } + | ^ ^ error: aborting due to 5 previous errors diff --git a/src/test/ui/resolve/issue-3907.stderr b/src/test/ui/resolve/issue-3907.stderr index 384df571e2a80..16436a9accc85 100644 --- a/src/test/ui/resolve/issue-3907.stderr +++ b/src/test/ui/resolve/issue-3907.stderr @@ -4,7 +4,11 @@ error[E0404]: expected trait, found type alias `Foo` LL | impl Foo for S { | ^^^ type aliases cannot be used as traits | - = note: did you mean to use a trait alias? +help: you might have meant to use `#![feature(trait_alias)]` instead of a `type` alias + --> $DIR/issue-3907.rs:5:1 + | +LL | type Foo = dyn issue_3907::Foo; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: possible better candidate is found in another module, you can import it into scope | LL | use issue_3907::Foo; diff --git a/src/test/ui/resolve/issue-5035.stderr b/src/test/ui/resolve/issue-5035.stderr index 622f0dfcda4cb..41dff2fe54205 100644 --- a/src/test/ui/resolve/issue-5035.stderr +++ b/src/test/ui/resolve/issue-5035.stderr @@ -16,7 +16,11 @@ LL | impl K for isize {} | type aliases cannot be used as traits | help: a trait with a similar name exists: `I` | - = note: did you mean to use a trait alias? +help: you might have meant to use `#![feature(trait_alias)]` instead of a `type` alias + --> $DIR/issue-5035.rs:2:1 + | +LL | type K = dyn I; + | ^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/resolve/unboxed-closure-sugar-nonexistent-trait.stderr b/src/test/ui/resolve/unboxed-closure-sugar-nonexistent-trait.stderr index c86a6d70344cc..2974d08eb23b1 100644 --- a/src/test/ui/resolve/unboxed-closure-sugar-nonexistent-trait.stderr +++ b/src/test/ui/resolve/unboxed-closure-sugar-nonexistent-trait.stderr @@ -10,7 +10,11 @@ error[E0404]: expected trait, found type alias `Typedef` LL | fn g isize>(x: F) {} | ^^^^^^^^^^^^^^^^^^^^^^^ type aliases cannot be used as traits | - = note: did you mean to use a trait alias? +help: you might have meant to use `#![feature(trait_alias)]` instead of a `type` alias + --> $DIR/unboxed-closure-sugar-nonexistent-trait.rs:4:1 + | +LL | type Typedef = isize; + | ^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/src/test/ui/struct-literal-variant-in-if.stderr b/src/test/ui/struct-literal-variant-in-if.stderr index d232a46f8ec29..4cd1169cc1bb8 100644 --- a/src/test/ui/struct-literal-variant-in-if.stderr +++ b/src/test/ui/struct-literal-variant-in-if.stderr @@ -46,9 +46,12 @@ error[E0423]: expected value, found struct variant `E::V` --> $DIR/struct-literal-variant-in-if.rs:10:13 | LL | if x == E::V { field } {} - | ^^^^---------- - | | - | help: surround the struct literal with parenthesis: `(E::V { field })` + | ^^^^ + | +help: surround the struct literal with parentheses + | +LL | if x == (E::V { field }) {} + | ^ ^ error[E0308]: mismatched types --> $DIR/struct-literal-variant-in-if.rs:10:20 From be90f90810438eed6f0090acfb4d29a787a43c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 21 Apr 2020 17:20:12 -0700 Subject: [PATCH 7/7] Point at the return type on `.into()` failure caused by `?` Fix #35946. --- .../traits/error_reporting/mod.rs | 38 ++++++++++++------- .../traits/error_reporting/suggestions.rs | 13 +++++++ src/test/ui/issues/issue-32709.stderr | 2 + src/test/ui/option-to-result.stderr | 6 +++ src/test/ui/try-on-option.stderr | 3 ++ 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 8a9017960fb23..094168d5d2e40 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -317,20 +317,30 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .starts_with("std::convert::From` into a `Result` using `Option::ok_or` or `Option::ok_or_else`", - ".ok_or_else(|| /* error value */)".to_string(), - Applicability::HasPlaceholders, - ); - } else if is_try && is_from && should_convert_result_to_option { - err.span_suggestion_verbose( - span.shrink_to_lo(), - "consider converting the `Result` into an `Option` using `Result::ok`", - ".ok()".to_string(), - Applicability::MachineApplicable, - ); + if is_try && is_from { + if should_convert_option_to_result { + err.span_suggestion_verbose( + span.shrink_to_lo(), + "consider converting the `Option` into a `Result` \ + using `Option::ok_or` or `Option::ok_or_else`", + ".ok_or_else(|| /* error value */)".to_string(), + Applicability::HasPlaceholders, + ); + } else if should_convert_result_to_option { + err.span_suggestion_verbose( + span.shrink_to_lo(), + "consider converting the `Result` into an `Option` \ + using `Result::ok`", + ".ok()".to_string(), + Applicability::MachineApplicable, + ); + } + if let Some(ret_span) = self.return_type_span(obligation) { + err.span_label( + ret_span, + &format!("expected `{}` because of this", trait_ref.self_ty()), + ); + } } let explanation = diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 2b19699d6ec75..ce34d5868a8ae 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -84,6 +84,8 @@ pub trait InferCtxtExt<'tcx> { trait_ref: &ty::Binder>, ); + fn return_type_span(&self, obligation: &PredicateObligation<'tcx>) -> Option; + fn suggest_impl_trait( &self, err: &mut DiagnosticBuilder<'tcx>, @@ -760,6 +762,17 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { } } + fn return_type_span(&self, obligation: &PredicateObligation<'tcx>) -> Option { + let hir = self.tcx.hir(); + let parent_node = hir.get_parent_node(obligation.cause.body_id); + let sig = match hir.find(parent_node) { + Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, ..), .. })) => sig, + _ => return None, + }; + + if let hir::FnRetTy::Return(ret_ty) = sig.decl.output { Some(ret_ty.span) } else { None } + } + /// If all conditions are met to identify a returned `dyn Trait`, suggest using `impl Trait` if /// applicable and signal that the error has been expanded appropriately and needs to be /// emitted. diff --git a/src/test/ui/issues/issue-32709.stderr b/src/test/ui/issues/issue-32709.stderr index 04b8c3aa35396..af272633f210d 100644 --- a/src/test/ui/issues/issue-32709.stderr +++ b/src/test/ui/issues/issue-32709.stderr @@ -1,6 +1,8 @@ error[E0277]: `?` couldn't convert the error to `()` --> $DIR/issue-32709.rs:4:11 | +LL | fn a() -> Result { + | --------------- expected `()` because of this LL | Err(5)?; | ^ the trait `std::convert::From<{integer}>` is not implemented for `()` | diff --git a/src/test/ui/option-to-result.stderr b/src/test/ui/option-to-result.stderr index f673ef7fc1e69..5fa06778389d9 100644 --- a/src/test/ui/option-to-result.stderr +++ b/src/test/ui/option-to-result.stderr @@ -1,6 +1,9 @@ error[E0277]: `?` couldn't convert the error to `()` --> $DIR/option-to-result.rs:5:6 | +LL | fn test_result() -> Result<(),()> { + | ------------- expected `()` because of this +LL | let a:Option<()> = Some(()); LL | a?; | ^ the trait `std::convert::From` is not implemented for `()` | @@ -14,6 +17,9 @@ LL | a.ok_or_else(|| /* error value */)?; error[E0277]: `?` couldn't convert the error to `std::option::NoneError` --> $DIR/option-to-result.rs:11:6 | +LL | fn test_option() -> Option{ + | ----------- expected `std::option::NoneError` because of this +LL | let a:Result = Ok(5); LL | a?; | ^ the trait `std::convert::From` is not implemented for `std::option::NoneError` | diff --git a/src/test/ui/try-on-option.stderr b/src/test/ui/try-on-option.stderr index 7a4bb75967b1f..33ca58bf7feb1 100644 --- a/src/test/ui/try-on-option.stderr +++ b/src/test/ui/try-on-option.stderr @@ -1,6 +1,9 @@ error[E0277]: `?` couldn't convert the error to `()` --> $DIR/try-on-option.rs:7:6 | +LL | fn foo() -> Result { + | --------------- expected `()` because of this +LL | let x: Option = None; LL | x?; | ^ the trait `std::convert::From` is not implemented for `()` |