bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #95194 - kckeiks:update-algo-in-find-use-placement, r=pnkfelix

Browse source 
remove find_use_placement

A more robust solution to finding where to place use suggestions was added in #94584.
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 instead of calling `find_use_placement`

Fixes #94941
This commit is contained in:
Dylan DPC 2022-04-15 20:50:45 +02:00 • committed by GitHub
commit bdbf0998f3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 57 additions and 148 deletions
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

@ -2557,11 +2557,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)]
@ -3059,8 +3065,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

@ -584,7 +584,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),
}
}
@ -1012,7 +1012,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);

146
compiler/rustc_typeck/src/check/method/suggest.rs
View file

@ -8,7 +8,7 @@ use rustc_errors::{
MultiSpan,
};
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,80 +1497,28 @@ 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 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
)
});
let (module, _, _) = self.tcx.hir().get_module(module_did);
let span = module.spans.inject_use_span;
let glob_path_strings = globs.iter().map(|trait_did| {
let parent_did = parent_map.get(trait_did).unwrap();
let path_strings = candidates.iter().map(|trait_did| {
format!("use {};\n", with_crate_prefix!(self.tcx.def_path_str(*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 {}::*; // trait {}\n{}",
with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
self.tcx.item_name(*trait_did),
additional_newline
)
});
let glob_path_strings = globs.iter().map(|trait_did| {
let parent_did = parent_map.get(trait_did).unwrap();
format!(
"use {}::*; // trait {}\n",
with_crate_prefix!(self.tcx.def_path_str(*parent_did)),
self.tcx.item_name(*trait_did),
)
});
err.span_suggestions(
span,
&msg,
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);
}
err.span_suggestions(
span,
&msg,
path_strings.chain(glob_path_strings),
Applicability::MaybeIncorrect,
);
}
fn suggest_valid_traits(
@ -2106,53 +2049,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`