bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Move lev_distance to rustc_ast, make non-generic

Browse source 
rustc_ast currently has a few dependencies on rustc_lexer. Ideally, an AST
would not have any dependency its lexer, for minimizing unnecessarily
design-time dependencies. Breaking this dependency would also have practical
benefits, since modifying rustc_lexer would not trigger a rebuild of rustc_ast.

This commit does not remove the rustc_ast --> rustc_lexer dependency,
but it does remove one of the sources of this dependency, which is the
code that handles fuzzy matching between symbol names for making suggestions
in diagnostics. Since that code depends only on Symbol, it is easy to move
it to rustc_span. It might even be best to move it to a separate crate,
since other tools such as Cargo use the same algorithm, and have simply
contain a duplicate of the code.

This changes the signature of find_best_match_for_name so that it is no
longer generic over its input. I checked the optimized binaries, and this
function was duplicated at nearly every call site, because most call sites
used short-lived iterator chains, generic over Map and such. But there's
no good reason for a function like this to be generic, since all it does
is immediately convert the generic input (the Iterator impl) to a concrete
Vec<Symbol>. This has all of the costs of generics (duplicated method bodies)
with no benefit.

Changing find_best_match_for_name to be non-generic removed about 10KB of
code from the optimized binary. I know it's a drop in the bucket, but we have
to start reducing binary size, and beginning to tame over-use of generics
is part of that.
This commit is contained in:
Arlie Davis 2020-11-12 11:24:10 -08:00
parent 1c389ffeff
commit 5481c1bd6d
16 changed files with 96 additions and 84 deletions
Download patch file Download diff file Expand all files Collapse all files

4
compiler/rustc_resolve/src/diagnostics.rs
View file

@ -1,7 +1,6 @@
use std::cmp::Reverse;
use std::ptr;
use rustc_ast::util::lev_distance::find_best_match_for_name;
use rustc_ast::{self as ast, Path};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
@ -14,6 +13,7 @@ use rustc_middle::bug;
use rustc_middle::ty::{self, DefIdTree};
use rustc_session::Session;
use rustc_span::hygiene::MacroKind;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, MultiSpan, Span};
@ -716,7 +716,7 @@ impl<'a> Resolver<'a> {
suggestions.sort_by_cached_key(|suggestion| suggestion.candidate.as_str());
match find_best_match_for_name(
suggestions.iter().map(|suggestion| &suggestion.candidate),
&suggestions.iter().map(|suggestion| suggestion.candidate).collect::<Vec<Symbol>>(),
ident.name,
None,
) {

48
compiler/rustc_resolve/src/imports.rs
View file

@ -10,7 +10,6 @@ use crate::{CrateLint, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet
use crate::{NameBinding, NameBindingKind, PathResult, PrivacyError, ToNameBinding};
use rustc_ast::unwrap_or;
use rustc_ast::util::lev_distance::find_best_match_for_name;
use rustc_ast::NodeId;
use rustc_ast_lowering::ResolverAstLowering;
use rustc_data_structures::fx::FxHashSet;
@ -25,6 +24,7 @@ use rustc_session::lint::builtin::{PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPOR
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_session::DiagnosticMessageId;
use rustc_span::hygiene::ExpnId;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::symbol::{kw, Ident, Symbol};
use rustc_span::{MultiSpan, Span};
@ -1096,33 +1096,37 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
_ => None,
};
let resolutions = resolutions.as_ref().into_iter().flat_map(|r| r.iter());
let names = resolutions.filter_map(|(BindingKey { ident: i, .. }, resolution)| {
if *i == ident {
return None;
} // Never suggest the same name
match *resolution.borrow() {
NameResolution { binding: Some(name_binding), .. } => {
match name_binding.kind {
NameBindingKind::Import { binding, .. } => {
match binding.kind {
// Never suggest the name that has binding error
// i.e., the name that cannot be previously resolved
NameBindingKind::Res(Res::Err, _) => None,
_ => Some(&i.name),
let names = resolutions
.filter_map(|(BindingKey { ident: i, .. }, resolution)| {
if *i == ident {
return None;
} // Never suggest the same name
match *resolution.borrow() {
NameResolution { binding: Some(name_binding), .. } => {
match name_binding.kind {
NameBindingKind::Import { binding, .. } => {
match binding.kind {
// Never suggest the name that has binding error
// i.e., the name that cannot be previously resolved
NameBindingKind::Res(Res::Err, _) => None,
_ => Some(i.name),
}
}
_ => Some(i.name),
}
_ => Some(&i.name),
}
NameResolution { ref single_imports, .. }
if single_imports.is_empty() =>
{
None
}
_ => Some(i.name),
}
NameResolution { ref single_imports, .. } if single_imports.is_empty() => {
None
}
_ => Some(&i.name),
}
});
})
.collect::<Vec<Symbol>>();
let lev_suggestion =
find_best_match_for_name(names, ident.name, None).map(|suggestion| {
find_best_match_for_name(&names, ident.name, None).map(|suggestion| {
(
vec![(ident.span, suggestion.to_string())],
String::from("a similar name exists in the module"),

9
compiler/rustc_resolve/src/late/diagnostics.rs
View file

@ -5,7 +5,6 @@ use crate::path_names_to_string;
use crate::{CrateLint, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{PathResult, PathSource, Segment};
use rustc_ast::util::lev_distance::find_best_match_for_name;
use rustc_ast::visit::FnKind;
use rustc_ast::{self as ast, Expr, ExprKind, Item, ItemKind, NodeId, Path, Ty, TyKind};
use rustc_ast_pretty::pprust::path_segment_to_string;
@ -18,6 +17,7 @@ use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::PrimTy;
use rustc_session::parse::feature_err;
use rustc_span::hygiene::MacroKind;
use rustc_span::lev_distance::find_best_match_for_name;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{BytePos, MultiSpan, Span, DUMMY_SP};
@ -1206,7 +1206,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
names.sort_by_cached_key(|suggestion| suggestion.candidate.as_str());
match find_best_match_for_name(
names.iter().map(|suggestion| &suggestion.candidate),
&names.iter().map(|suggestion| suggestion.candidate).collect::<Vec<Symbol>>(),
name,
None,
) {
@ -1592,9 +1592,10 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> {
.bindings
.iter()
.filter(|(id, _)| id.span.ctxt() == label.span.ctxt())
.map(|(id, _)| &id.name);
.map(|(id, _)| id.name)
.collect::<Vec<Symbol>>();
find_best_match_for_name(names, label.name, None).map(|symbol| {
find_best_match_for_name(&names, label.name, None).map(|symbol| {
// Upon finding a similar name, get the ident that it was from - the span
// contained within helps make a useful diagnostic. In addition, determine
// whether this candidate is within scope.