From e433f5585296ba8892893e4c78f51d2d42ac7ea4 Mon Sep 17 00:00:00 2001 From: Kevin Jiang Date: Thu, 18 Mar 2021 00:15:39 -0400 Subject: [PATCH 1/2] Fixed diagnostic and added test for issue 81508 --- compiler/rustc_resolve/src/late.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 69 +++++++++++++++++++++----- src/test/ui/resolve/issue-81508.rs | 22 ++++++++ src/test/ui/resolve/issue-81508.stderr | 21 ++++++++ 4 files changed, 100 insertions(+), 14 deletions(-) create mode 100644 src/test/ui/resolve/issue-81508.rs create mode 100644 src/test/ui/resolve/issue-81508.stderr diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 1377bb781d008..987f1e6cc747a 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1031,7 +1031,6 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } ItemKind::Static(ref ty, _, ref expr) | ItemKind::Const(_, ref ty, ref expr) => { - debug!("resolve_item ItemKind::Const"); self.with_item_rib(HasGenericParams::No, |this| { this.visit_ty(ty); if let Some(expr) = expr { @@ -1597,6 +1596,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { .try_resolve_as_non_binding(pat_src, pat, bmode, ident, has_sub) .unwrap_or_else(|| self.fresh_binding(ident, pat.id, pat_src, bindings)); self.r.record_partial_res(pat.id, PartialRes::new(res)); + self.r.record_local_span(pat.id, pat.span); } PatKind::TupleStruct(ref path, ref sub_patterns) => { self.smart_resolve_path( diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index fe5078f26bd98..3a8cb0fd09ad2 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -884,6 +884,10 @@ pub struct Resolver<'a> { /// "self-confirming" import resolutions during import validation. unusable_binding: Option<&'a NameBinding<'a>>, + // Spans for local variables found during resolution + // Used for suggestions during error reporting + local_span_map: NodeMap, + /// Resolutions for nodes that have a single resolution. partial_res_map: NodeMap, /// Resolutions for import nodes, which have multiple resolutions in different namespaces. @@ -1262,6 +1266,7 @@ impl<'a> Resolver<'a> { last_import_segment: false, unusable_binding: None, + local_span_map: Default::default(), partial_res_map: Default::default(), import_res_map: Default::default(), label_res_map: Default::default(), @@ -1879,7 +1884,6 @@ impl<'a> Resolver<'a> { ribs, ))); } - module = match ribs[i].kind { ModuleRibKind(module) => module, MacroDefinition(def) if def == self.macro_def(ident.span.ctxt()) => { @@ -1890,7 +1894,6 @@ impl<'a> Resolver<'a> { } _ => continue, }; - match module.kind { ModuleKind::Block(..) => {} // We can see through blocks _ => break, @@ -1909,17 +1912,19 @@ impl<'a> Resolver<'a> { return Some(LexicalScopeBinding::Item(binding)); } } + let returned_item = self + .early_resolve_ident_in_lexical_scope( + orig_ident, + ScopeSet::Late(ns, module, record_used_id), + parent_scope, + record_used, + record_used, + path_span, + ) + .ok() + .map(LexicalScopeBinding::Item); - self.early_resolve_ident_in_lexical_scope( - orig_ident, - ScopeSet::Late(ns, module, record_used_id), - parent_scope, - record_used, - record_used, - path_span, - ) - .ok() - .map(LexicalScopeBinding::Item) + returned_item } fn hygienic_lexical_parent( @@ -2386,7 +2391,40 @@ impl<'a> Resolver<'a> { .next() .map_or(false, |c| c.is_ascii_uppercase()) { - (format!("use of undeclared type `{}`", ident), None) + // Add check case for similarly named item in alternative namespace + let mut suggestion = None; + + if ribs.is_some() { + if let Some(res) = self.resolve_ident_in_lexical_scope( + ident, + ValueNS, + parent_scope, + None, + path_span, + &ribs.unwrap()[ValueNS], + ) { + let mut match_span: Option = None; + match res { + LexicalScopeBinding::Res(Res::Local(id)) => { + match_span = + Some(*self.local_span_map.get(&id).unwrap()); + } + LexicalScopeBinding::Item(name_binding) => { + match_span = Some(name_binding.span); + } + _ => (), + }; + if let Some(span) = match_span { + suggestion = Some(( + vec![(span, String::from(""))], + format!("{} is defined here, but is not a type", ident), + Applicability::MaybeIncorrect, + )); + } + } + } + + (format!("use of undeclared type `{}`", ident), suggestion) } else { (format!("use of undeclared crate or module `{}`", ident), None) } @@ -2797,6 +2835,11 @@ impl<'a> Resolver<'a> { } } + fn record_local_span(&mut self, node: NodeId, span: Span) { + debug!("(recording local) recording {:?} for {:?}", node, span); + self.local_span_map.insert(node, span); + } + fn is_accessible_from(&self, vis: ty::Visibility, module: Module<'a>) -> bool { vis.is_accessible_from(module.nearest_parent_mod, self) } diff --git a/src/test/ui/resolve/issue-81508.rs b/src/test/ui/resolve/issue-81508.rs new file mode 100644 index 0000000000000..23605cd2fd91d --- /dev/null +++ b/src/test/ui/resolve/issue-81508.rs @@ -0,0 +1,22 @@ +// Confusing diagnostic when using variable as a type: +// +// Previous warnings indicate Foo is not used, when in fact it is +// used improperly as a variable or constant. New warning points +// out user may be trying to use variable as a type. Test demonstrates +// cases for both local variable and const. + +fn main() { + let Baz: &str = ""; + + println!("{}", Baz::Bar); //~ ERROR: failed to resolve: use of undeclared type `Baz` +} + +#[allow(non_upper_case_globals)] +pub const Foo: &str = ""; + +mod submod { + use super::Foo; + fn function() { + println!("{}", Foo::Bar); //~ ERROR: failed to resolve: use of undeclared type `Foo` + } +} diff --git a/src/test/ui/resolve/issue-81508.stderr b/src/test/ui/resolve/issue-81508.stderr new file mode 100644 index 0000000000000..b0d5e1a269231 --- /dev/null +++ b/src/test/ui/resolve/issue-81508.stderr @@ -0,0 +1,21 @@ +error[E0433]: failed to resolve: use of undeclared type `Baz` + --> $DIR/issue-81508.rs:11:20 + | +LL | let Baz: &str = ""; + | --- help: Baz is defined here, but is not a type +LL | +LL | println!("{}", Baz::Bar); + | ^^^ use of undeclared type `Baz` + +error[E0433]: failed to resolve: use of undeclared type `Foo` + --> $DIR/issue-81508.rs:20:24 + | +LL | use super::Foo; + | ---------- help: Foo is defined here, but is not a type +LL | fn function() { +LL | println!("{}", Foo::Bar); + | ^^^ use of undeclared type `Foo` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0433`. From f51f25ab7de7c737ab67d79e23691b8f8f642a8c Mon Sep 17 00:00:00 2001 From: K Date: Wed, 7 Apr 2021 12:35:39 -0400 Subject: [PATCH 2/2] Added additional comments and minor edits --- compiler/rustc_resolve/src/late.rs | 2 +- compiler/rustc_resolve/src/lib.rs | 104 +++++++++++++++---------- src/test/ui/resolve/issue-81508.stderr | 4 +- 3 files changed, 64 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 987f1e6cc747a..9321f11f65933 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -1596,7 +1596,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { .try_resolve_as_non_binding(pat_src, pat, bmode, ident, has_sub) .unwrap_or_else(|| self.fresh_binding(ident, pat.id, pat_src, bindings)); self.r.record_partial_res(pat.id, PartialRes::new(res)); - self.r.record_local_span(pat.id, pat.span); + self.r.record_pat_span(pat.id, pat.span); } PatKind::TupleStruct(ref path, ref sub_patterns) => { self.smart_resolve_path( diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 3a8cb0fd09ad2..665c32ddf069c 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -884,9 +884,9 @@ pub struct Resolver<'a> { /// "self-confirming" import resolutions during import validation. unusable_binding: Option<&'a NameBinding<'a>>, - // Spans for local variables found during resolution - // Used for suggestions during error reporting - local_span_map: NodeMap, + // Spans for local variables found during pattern resolution. + // Used for suggestions during error reporting. + pat_span_map: NodeMap, /// Resolutions for nodes that have a single resolution. partial_res_map: NodeMap, @@ -1266,7 +1266,7 @@ impl<'a> Resolver<'a> { last_import_segment: false, unusable_binding: None, - local_span_map: Default::default(), + pat_span_map: Default::default(), partial_res_map: Default::default(), import_res_map: Default::default(), label_res_map: Default::default(), @@ -1884,6 +1884,7 @@ impl<'a> Resolver<'a> { ribs, ))); } + module = match ribs[i].kind { ModuleRibKind(module) => module, MacroDefinition(def) if def == self.macro_def(ident.span.ctxt()) => { @@ -1894,6 +1895,7 @@ impl<'a> Resolver<'a> { } _ => continue, }; + match module.kind { ModuleKind::Block(..) => {} // We can see through blocks _ => break, @@ -1912,19 +1914,16 @@ impl<'a> Resolver<'a> { return Some(LexicalScopeBinding::Item(binding)); } } - let returned_item = self - .early_resolve_ident_in_lexical_scope( - orig_ident, - ScopeSet::Late(ns, module, record_used_id), - parent_scope, - record_used, - record_used, - path_span, - ) - .ok() - .map(LexicalScopeBinding::Item); - - returned_item + self.early_resolve_ident_in_lexical_scope( + orig_ident, + ScopeSet::Late(ns, module, record_used_id), + parent_scope, + record_used, + record_used, + path_span, + ) + .ok() + .map(LexicalScopeBinding::Item) } fn hygienic_lexical_parent( @@ -2391,11 +2390,9 @@ impl<'a> Resolver<'a> { .next() .map_or(false, |c| c.is_ascii_uppercase()) { - // Add check case for similarly named item in alternative namespace - let mut suggestion = None; - - if ribs.is_some() { - if let Some(res) = self.resolve_ident_in_lexical_scope( + // Check whether the name refers to an item in the value namespace. + let suggestion = if ribs.is_some() { + let match_span = match self.resolve_ident_in_lexical_scope( ident, ValueNS, parent_scope, @@ -2403,26 +2400,47 @@ impl<'a> Resolver<'a> { path_span, &ribs.unwrap()[ValueNS], ) { - let mut match_span: Option = None; - match res { - LexicalScopeBinding::Res(Res::Local(id)) => { - match_span = - Some(*self.local_span_map.get(&id).unwrap()); - } - LexicalScopeBinding::Item(name_binding) => { - match_span = Some(name_binding.span); - } - _ => (), - }; - if let Some(span) = match_span { - suggestion = Some(( - vec![(span, String::from(""))], - format!("{} is defined here, but is not a type", ident), - Applicability::MaybeIncorrect, - )); + // Name matches a local variable. For example: + // ``` + // fn f() { + // let Foo: &str = ""; + // println!("{}", Foo::Bar); // Name refers to local + // // variable `Foo`. + // } + // ``` + Some(LexicalScopeBinding::Res(Res::Local(id))) => { + Some(*self.pat_span_map.get(&id).unwrap()) } + + // Name matches item from a local name binding + // created by `use` declaration. For example: + // ``` + // pub Foo: &str = ""; + // + // mod submod { + // use super::Foo; + // println!("{}", Foo::Bar); // Name refers to local + // // binding `Foo`. + // } + // ``` + Some(LexicalScopeBinding::Item(name_binding)) => { + Some(name_binding.span) + } + _ => None, + }; + + if let Some(span) = match_span { + Some(( + vec![(span, String::from(""))], + format!("`{}` is defined here, but is not a type", ident), + Applicability::MaybeIncorrect, + )) + } else { + None } - } + } else { + None + }; (format!("use of undeclared type `{}`", ident), suggestion) } else { @@ -2835,9 +2853,9 @@ impl<'a> Resolver<'a> { } } - fn record_local_span(&mut self, node: NodeId, span: Span) { - debug!("(recording local) recording {:?} for {:?}", node, span); - self.local_span_map.insert(node, span); + fn record_pat_span(&mut self, node: NodeId, span: Span) { + debug!("(recording pat) recording {:?} for {:?}", node, span); + self.pat_span_map.insert(node, span); } fn is_accessible_from(&self, vis: ty::Visibility, module: Module<'a>) -> bool { diff --git a/src/test/ui/resolve/issue-81508.stderr b/src/test/ui/resolve/issue-81508.stderr index b0d5e1a269231..15555631b9047 100644 --- a/src/test/ui/resolve/issue-81508.stderr +++ b/src/test/ui/resolve/issue-81508.stderr @@ -2,7 +2,7 @@ error[E0433]: failed to resolve: use of undeclared type `Baz` --> $DIR/issue-81508.rs:11:20 | LL | let Baz: &str = ""; - | --- help: Baz is defined here, but is not a type + | --- help: `Baz` is defined here, but is not a type LL | LL | println!("{}", Baz::Bar); | ^^^ use of undeclared type `Baz` @@ -11,7 +11,7 @@ error[E0433]: failed to resolve: use of undeclared type `Foo` --> $DIR/issue-81508.rs:20:24 | LL | use super::Foo; - | ---------- help: Foo is defined here, but is not a type + | ---------- help: `Foo` is defined here, but is not a type LL | fn function() { LL | println!("{}", Foo::Bar); | ^^^ use of undeclared type `Foo`