Skip to content

Commit

Permalink
Auto merge of #95255 - petrochenkov:suggresolve, r=michaelwoerister
Browse files Browse the repository at this point in the history 
resolve: Do not build expensive suggestions if they are not actually used

And remove a bunch of (conditionally) unused parameters from path resolution functions.

This helps with performance issues in #94857, and should be helpful in general even without that.
  • Loading branch information
@bors
bors committed Mar 25, 2022
2 parents e70e211 + 1ad64a2 commit 903427b
Show file tree
Hide file tree
Showing 25 changed files with 457 additions and 824 deletions.
25 changes: 11 additions & 14 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::def_collector::collect_definitions;
use crate::imports::{Import, ImportKind};
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
use crate::{CrateLint, Determinacy, ExternPreludeEntry, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};

Expand Down Expand Up @@ -235,16 +235,16 @@ impl<'a> AsMut<Resolver<'a>> for BuildReducedGraphVisitor<'a, '_> {

impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
fn resolve_visibility(&mut self, vis: &ast::Visibility) -> ty::Visibility {
self.resolve_visibility_speculative(vis, false).unwrap_or_else(|err| {
self.try_resolve_visibility(vis, true).unwrap_or_else(|err| {
self.r.report_vis_error(err);
ty::Visibility::Public
})
}

fn resolve_visibility_speculative<'ast>(
fn try_resolve_visibility<'ast>(
&mut self,
vis: &'ast ast::Visibility,
speculative: bool,
finalize: bool,
) -> Result<ty::Visibility, VisResolutionError<'ast>> {
let parent_scope = &self.parent_scope;
match vis.kind {
Expand Down Expand Up @@ -296,13 +296,11 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
&segments,
Some(TypeNS),
parent_scope,
!speculative,
path.span,
CrateLint::SimplePath(id),
if finalize { Finalize::SimplePath(id, path.span) } else { Finalize::No },
) {
PathResult::Module(ModuleOrUniformRoot::Module(module)) => {
let res = module.res().expect("visibility resolved to unnamed block");
if !speculative {
if finalize {
self.r.record_partial_res(id, PartialRes::new(res));
}
if module.is_normal() {
Expand Down Expand Up @@ -772,7 +770,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
// correct visibilities for unnamed field placeholders specifically, so the
// constructor visibility should still be determined correctly.
let field_vis = self
.resolve_visibility_speculative(&field.vis, true)
.try_resolve_visibility(&field.vis, false)
.unwrap_or(ty::Visibility::Public);
if ctor_vis.is_at_least(field_vis, &*self.r) {
ctor_vis = field_vis;
Expand Down Expand Up @@ -1131,8 +1129,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
ident,
MacroNS,
&self.parent_scope,
false,
ident.span,
None,
);
if let Ok(binding) = result {
let import = macro_use_import(self, ident.span);
Expand Down Expand Up @@ -1272,9 +1269,9 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
let vis = match item.kind {
// Visibilities must not be resolved non-speculatively twice
// and we already resolved this one as a `fn` item visibility.
ItemKind::Fn(..) => self
.resolve_visibility_speculative(&item.vis, true)
.unwrap_or(ty::Visibility::Public),
ItemKind::Fn(..) => {
self.try_resolve_visibility(&item.vis, false).unwrap_or(ty::Visibility::Public)
}
_ => self.resolve_visibility(&item.vis),
};
if vis != ty::Visibility::Public {
Expand Down
29 changes: 11 additions & 18 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ use tracing::debug;
use crate::imports::{Import, ImportKind, ImportResolver};
use crate::path_names_to_string;
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind};
use crate::{
BindingError, CrateLint, HasGenericParams, MacroRulesScope, Module, ModuleOrUniformRoot,
};
use crate::{NameBinding, NameBindingKind, PrivacyError, VisResolutionError};
use crate::{BindingError, HasGenericParams, MacroRulesScope, Module, ModuleOrUniformRoot};
use crate::{Finalize, NameBinding, NameBindingKind, PrivacyError, VisResolutionError};
use crate::{ParentScope, PathResult, ResolutionError, Resolver, Scope, ScopeSet, Segment};

type Res = def::Res<ast::NodeId>;
Expand Down Expand Up @@ -1076,9 +1074,8 @@ impl<'a> Resolver<'a> {
ident,
ScopeSet::All(ns, false),
&parent_scope,
None,
false,
false,
ident.span,
) {
let desc = match binding.res() {
Res::Def(DefKind::Macro(MacroKind::Bang), _) => {
Expand Down Expand Up @@ -1405,10 +1402,10 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
_ => return None,
}

self.make_missing_self_suggestion(span, path.clone(), parent_scope)
.or_else(|| self.make_missing_crate_suggestion(span, path.clone(), parent_scope))
.or_else(|| self.make_missing_super_suggestion(span, path.clone(), parent_scope))
.or_else(|| self.make_external_crate_suggestion(span, path, parent_scope))
self.make_missing_self_suggestion(path.clone(), parent_scope)
.or_else(|| self.make_missing_crate_suggestion(path.clone(), parent_scope))
.or_else(|| self.make_missing_super_suggestion(path.clone(), parent_scope))
.or_else(|| self.make_external_crate_suggestion(path, parent_scope))
}

/// Suggest a missing `self::` if that resolves to an correct module.
Expand All @@ -1420,13 +1417,12 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
/// ```
fn make_missing_self_suggestion(
&mut self,
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Vec<String>)> {
// Replace first ident with `self` and check if that is valid.
path[0].ident.name = kw::SelfLower;
let result = self.r.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
let result = self.r.resolve_path(&path, None, parent_scope, Finalize::No);
debug!("make_missing_self_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result { Some((path, Vec::new())) } else { None }
}
Expand All @@ -1440,13 +1436,12 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
/// ```
fn make_missing_crate_suggestion(
&mut self,
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Vec<String>)> {
// Replace first ident with `crate` and check if that is valid.
path[0].ident.name = kw::Crate;
let result = self.r.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
let result = self.r.resolve_path(&path, None, parent_scope, Finalize::No);
debug!("make_missing_crate_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result {
Some((
Expand All @@ -1472,13 +1467,12 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
/// ```
fn make_missing_super_suggestion(
&mut self,
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Vec<String>)> {
// Replace first ident with `crate` and check if that is valid.
path[0].ident.name = kw::Super;
let result = self.r.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
let result = self.r.resolve_path(&path, None, parent_scope, Finalize::No);
debug!("make_missing_super_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result { Some((path, Vec::new())) } else { None }
}
Expand All @@ -1495,7 +1489,6 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
/// name as the first part of path.
fn make_external_crate_suggestion(
&mut self,
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Vec<String>)> {
Expand All @@ -1513,7 +1506,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
for name in extern_crate_names.into_iter() {
// Replace first ident with a crate name and check if that is valid.
path[0].ident.name = name;
let result = self.r.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
let result = self.r.resolve_path(&path, None, parent_scope, Finalize::No);
debug!(
"make_external_crate_suggestion: name={:?} path={:?} result={:?}",
name, path, result
Expand Down
Loading

0 comments on commit 903427b

Please sign in to comment.