Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove find_use_placement #95194

Merged
merged 1 commit into from
Apr 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
})
Expand Down Expand Up @@ -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))),
}
}
Expand Down Expand Up @@ -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"),
},
Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_save_analysis/src/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_save_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
146 changes: 21 additions & 125 deletions compiler/rustc_typeck/src/check/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 `_`
Expand All @@ -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(
Expand Down Expand Up @@ -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>]>,
Expand Down
7 changes: 5 additions & 2 deletions src/librustdoc/html/render/span_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/imports/overlapping_pub_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/imports/unnamed_pub_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/suggestions/use-placement-typeck.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#![allow(unused)]

use m::Foo;

fn main() {
let s = m::S;
s.abc(); //~ ERROR no method named `abc`
Expand Down