mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

remove find_use_placement

Browse Source 
A more robust solution to finding where to place use suggestions was added.
The algorithm uses the AST to find the span for the suggestion so we pass this span
down to the HIR during lowering and use it.

Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
This commit is contained in:
Fausto 2022-03-18 17:13:38 -04:00 committed by Miguel Guarniz
parent 0677edc86e
commit 8c2353b6c1
12 changed files with 57 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
compiler/rustc_ast_lowering/src/index.rs
View File

@ -52,7 +52,9 @@ pub(super) fn index_hir<'hir>(
};
match item {
OwnerNode::Crate(citem) => collector.visit_mod(&citem, citem.inner, hir::CRATE_HIR_ID),
OwnerNode::Crate(citem) => {
collector.visit_mod(&citem, citem.spans.inner_span, hir::CRATE_HIR_ID)
}
OwnerNode::Item(item) => collector.visit_item(item),
OwnerNode::TraitItem(item) => collector.visit_trait_item(item),
OwnerNode::ImplItem(item) => collector.visit_impl_item(item),

13
compiler/rustc_ast_lowering/src/item.rs
View File

@ -124,7 +124,7 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
debug_assert_eq!(self.resolver.local_def_id(CRATE_NODE_ID), CRATE_DEF_ID);
self.with_lctx(CRATE_NODE_ID, |lctx| {
let module = lctx.lower_mod(&c.items, c.spans.inner_span);
let module = lctx.lower_mod(&c.items, &c.spans);
lctx.lower_attrs(hir::CRATE_HIR_ID, &c.attrs);
hir::OwnerNode::Crate(lctx.arena.alloc(module))
})
@ -186,9 +186,12 @@ impl<'a, 'hir> ItemLowerer<'a, 'hir> {
}
impl<'hir> LoweringContext<'_, 'hir> {
pub(super) fn lower_mod(&mut self, items: &[P<Item>], inner: Span) -> hir::Mod<'hir> {
pub(super) fn lower_mod(&mut self, items: &[P<Item>], spans: &ModSpans) -> hir::Mod<'hir> {
hir::Mod {
inner: self.lower_span(inner),
spans: hir::ModSpans {
inner_span: self.lower_span(spans.inner_span),
inject_use_span: self.lower_span(spans.inject_use_span),
},
item_ids: self.arena.alloc_from_iter(items.iter().flat_map(|x| self.lower_item_ref(x))),
}
}
@ -308,8 +311,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
})
}
ItemKind::Mod(_, ref mod_kind) => match mod_kind {
ModKind::Loaded(items, _, ModSpans { inner_span, inject_use_span: _ }) => {
hir::ItemKind::Mod(self.lower_mod(items, *inner_span))
ModKind::Loaded(items, _, spans) => {
hir::ItemKind::Mod(self.lower_mod(items, spans))
}
ModKind::Unloaded => panic!("`mod` items should have been loaded by now"),
},

14
compiler/rustc_hir/src/hir.rs
View File

@ -2522,11 +2522,17 @@ impl FnRetTy<'_> {
#[derive(Encodable, Debug, HashStable_Generic)]
pub struct Mod<'hir> {
pub spans: ModSpans,
pub item_ids: &'hir [ItemId],
}
#[derive(Copy, Clone, Debug, HashStable_Generic, Encodable)]
pub struct ModSpans {
/// A span from the first token past `{` to the last token until `}`.
/// For `mod foo;`, the inner span ranges from the first token
/// to the last token in the external file.
pub inner: Span,
pub item_ids: &'hir [ItemId],
pub inner_span: Span,
pub inject_use_span: Span,
}
#[derive(Debug, HashStable_Generic)]
@ -3024,8 +3030,8 @@ impl<'hir> OwnerNode<'hir> {
OwnerNode::Item(Item { span, .. })
| OwnerNode::ForeignItem(ForeignItem { span, .. })
| OwnerNode::ImplItem(ImplItem { span, .. })
| OwnerNode::TraitItem(TraitItem { span, .. })
| OwnerNode::Crate(Mod { inner: span, .. }) => *span,
| OwnerNode::TraitItem(TraitItem { span, .. }) => *span,
OwnerNode::Crate(Mod { spans: ModSpans { inner_span, .. }, .. }) => *inner_span,
}
}

4
compiler/rustc_middle/src/hir/map/mod.rs
View File

@ -586,7 +586,7 @@ impl<'hir> Map<'hir> {
Some(OwnerNode::Item(&Item { span, kind: ItemKind::Mod(ref m), .. })) => {
(m, span, hir_id)
}
Some(OwnerNode::Crate(item)) => (item, item.inner, hir_id),
Some(OwnerNode::Crate(item)) => (item, item.spans.inner_span, hir_id),
node => panic!("not a module: {:?}", node),
}
}
@ -1014,7 +1014,7 @@ impl<'hir> Map<'hir> {
Node::Infer(i) => i.span,
Node::Visibility(v) => bug!("unexpected Visibility {:?}", v),
Node::Local(local) => local.span,
Node::Crate(item) => item.inner,
Node::Crate(item) => item.spans.inner_span,
};
Some(span)
}

4
compiler/rustc_save_analysis/src/dump_visitor.rs
View File

@ -1095,11 +1095,11 @@ impl<'tcx> DumpVisitor<'tcx> {
let sm = self.tcx.sess.source_map();
let krate_mod = self.tcx.hir().root_module();
let filename = sm.span_to_filename(krate_mod.inner);
let filename = sm.span_to_filename(krate_mod.spans.inner_span);
let data_id = id_from_hir_id(id, &self.save_ctxt);
let children =
krate_mod.item_ids.iter().map(|i| id_from_def_id(i.def_id.to_def_id())).collect();
let span = self.span_from_span(krate_mod.inner);
let span = self.span_from_span(krate_mod.spans.inner_span);
let attrs = self.tcx.hir().attrs(id);
self.dumper.dump_def(

2
compiler/rustc_save_analysis/src/lib.rs
View File

@ -282,7 +282,7 @@ impl<'tcx> SaveContext<'tcx> {
let qualname = format!("::{}", self.tcx.def_path_str(def_id));
let sm = self.tcx.sess.source_map();
let filename = sm.span_to_filename(m.inner);
let filename = sm.span_to_filename(m.spans.inner_span);
filter!(self.span_utils, item.ident.span);

118
compiler/rustc_typeck/src/check/method/suggest.rs
View File

@ -7,7 +7,7 @@ use rustc_errors::{
pluralize, struct_span_err, Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed,
};
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_hir::def_id::DefId;
use rustc_hir::lang_items::LangItem;
use rustc_hir::{ExprKind, Node, QPath};
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
@ -1473,12 +1473,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
fn suggest_use_candidates(
&self,
err: &mut Diagnostic,
mut msg: String,
candidates: Vec<DefId>,
) {
fn suggest_use_candidates(&self, err: &mut Diagnostic, msg: String, candidates: Vec<DefId>) {
let parent_map = self.tcx.visible_parent_map(());
// Separate out candidates that must be imported with a glob, because they are named `_`
@ -1502,30 +1497,19 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
});
let module_did = self.tcx.parent_module(self.body_id);
let (span, found_use) = find_use_placement(self.tcx, module_did);
if let Some(span) = span {
let (module, _, _) = self.tcx.hir().get_module(module_did);
let span = module.spans.inject_use_span;
let path_strings = candidates.iter().map(|trait_did| {
// Produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use { "" } else { "\n" };
format!(
"use {};\n{}",
with_crate_prefix!(self.tcx.def_path_str(*trait_did)),
additional_newline
)
format!("use {};\n", with_crate_prefix!(self.tcx.def_path_str(*trait_did)),)
});
let glob_path_strings = globs.iter().map(|trait_did| {
let parent_did = parent_map.get(trait_did).unwrap();
// Produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use { "" } else { "\n" };
format!(
"use {}::*; // trait {}\n{}",
"use {}::*; // trait {}\n",
with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
self.tcx.item_name(*trait_did),
additional_newline
)
});
@ -1535,47 +1519,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
path_strings.chain(glob_path_strings),
Applicability::MaybeIncorrect,
);
} else {
let limit = if candidates.len() + globs.len() == 5 { 5 } else { 4 };
for (i, trait_did) in candidates.iter().take(limit).enumerate() {
if candidates.len() + globs.len() > 1 {
msg.push_str(&format!(
"\ncandidate #{}: `use {};`",
i + 1,
with_crate_prefix!(self.tcx.def_path_str(*trait_did))
));
} else {
msg.push_str(&format!(
"\n`use {};`",
with_crate_prefix!(self.tcx.def_path_str(*trait_did))
));
}
}
for (i, trait_did) in
globs.iter().take(limit.saturating_sub(candidates.len())).enumerate()
{
let parent_did = parent_map.get(trait_did).unwrap();
if candidates.len() + globs.len() > 1 {
msg.push_str(&format!(
"\ncandidate #{}: `use {}::*; // trait {}`",
candidates.len() + i + 1,
with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
self.tcx.item_name(*trait_did),
));
} else {
msg.push_str(&format!(
"\n`use {}::*; // trait {}`",
with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
self.tcx.item_name(*trait_did),
));
}
}
if candidates.len() > limit {
msg.push_str(&format!("\nand {} others", candidates.len() + globs.len() - limit));
}
err.note(&msg);
}
}
fn suggest_valid_traits(
@ -2100,53 +2043,6 @@ pub fn all_traits(tcx: TyCtxt<'_>) -> Vec<TraitInfo> {
tcx.all_traits().map(|def_id| TraitInfo { def_id }).collect()
}
fn find_use_placement<'tcx>(tcx: TyCtxt<'tcx>, target_module: LocalDefId) -> (Option<Span>, bool) {
// FIXME(#94854): this code uses an out-of-date method for inferring a span
// to suggest. It would be better to thread the ModSpans from the AST into
// the HIR, and then use that to drive the suggestion here.
let mut span = None;
let mut found_use = false;
let (module, _, _) = tcx.hir().get_module(target_module);
// Find a `use` statement.
for &item_id in module.item_ids {
let item = tcx.hir().item(item_id);
match item.kind {
hir::ItemKind::Use(..) => {
// Don't suggest placing a `use` before the prelude
// import or other generated ones.
if !item.span.from_expansion() {
span = Some(item.span.shrink_to_lo());
found_use = true;
break;
}
}
// Don't place `use` before `extern crate`...
hir::ItemKind::ExternCrate(_) => {}
// ...but do place them before the first other item.
_ => {
if span.map_or(true, |span| item.span < span) {
if !item.span.from_expansion() {
span = Some(item.span.shrink_to_lo());
// Don't insert between attributes and an item.
let attrs = tcx.hir().attrs(item.hir_id());
// Find the first attribute on the item.
// FIXME: This is broken for active attributes.
for attr in attrs {
if !attr.span.is_dummy() && span.map_or(true, |span| attr.span < span) {
span = Some(attr.span.shrink_to_lo());
}
}
}
}
}
}
}
(span, found_use)
}
fn print_disambiguation_help<'tcx>(
item_name: Ident,
args: Option<&'tcx [hir::Expr<'tcx>]>,

7
src/librustdoc/html/render/span_map.rs
View File

@ -119,11 +119,14 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> {
fn visit_mod(&mut self, m: &'tcx Mod<'tcx>, span: Span, id: HirId) {
// To make the difference between "mod foo {}" and "mod foo;". In case we "import" another
// file, we want to link to it. Otherwise no need to create a link.
if !span.overlaps(m.inner) {
if !span.overlaps(m.spans.inner_span) {
// Now that we confirmed it's a file import, we want to get the span for the module
// name only and not all the "mod foo;".
if let Some(Node::Item(item)) = self.tcx.hir().find(id) {
self.matches.insert(item.ident.span, LinkFromSrc::Local(clean::Span::new(m.inner)));
self.matches.insert(
item.ident.span,
LinkFromSrc::Local(clean::Span::new(m.spans.inner_span)),
);
}
}
intravisit::walk_mod(self, m, id);

2
src/librustdoc/visit_ast.rs
View File

@ -154,7 +154,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
m: &'tcx hir::Mod<'tcx>,
name: Symbol,
) -> Module<'tcx> {
let mut om = Module::new(name, id, m.inner);
let mut om = Module::new(name, id, m.spans.inner_span);
let def_id = self.cx.tcx.hir().local_def_id(id).to_def_id();
// Keep track of if there were any private modules in the path.
let orig_inside_public_path = self.inside_public_path;

4
src/test/ui/imports/overlapping_pub_trait.rs
View File

@ -4,10 +4,10 @@
* This crate declares two public paths, `m::Tr` and `prelude::_`. Make sure we prefer the former.
*/
extern crate overlapping_pub_trait_source;
//~^ HELP the following trait is implemented but not in scope; perhaps add a `use` for it:
//~| SUGGESTION overlapping_pub_trait_source::m::Tr
fn main() {
//~^ HELP the following trait is implemented but not in scope; perhaps add a `use` for it:
//~| SUGGESTION overlapping_pub_trait_source::m::Tr
use overlapping_pub_trait_source::S;
S.method();
//~^ ERROR no method named `method` found for struct `S` in the current scope [E0599]

4
src/test/ui/imports/unnamed_pub_trait.rs
View File

@ -5,10 +5,10 @@
* importing it by name, and instead we suggest importing it by glob.
*/
extern crate unnamed_pub_trait_source;
//~^ HELP the following trait is implemented but not in scope; perhaps add a `use` for it:
//~| SUGGESTION unnamed_pub_trait_source::prelude::*; // trait Tr
fn main() {
//~^ HELP the following trait is implemented but not in scope; perhaps add a `use` for it:
//~| SUGGESTION unnamed_pub_trait_source::prelude::*; // trait Tr
use unnamed_pub_trait_source::S;
S.method();
//~^ ERROR no method named `method` found for struct `S` in the current scope [E0599]

1
src/test/ui/suggestions/use-placement-typeck.fixed
View File

@ -7,7 +7,6 @@
#![allow(unused)]
use m::Foo;
fn main() {
let s = m::S;
s.abc(); //~ ERROR no method named `abc`