Skip to content

Misc. rustc_resolve cleanups #135826

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jan 23, 2025
Merged

Misc. rustc_resolve cleanups #135826

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a93616a
rustc_resolve: remove unneeded `return`s
yotamofek Jan 18, 2025
cf91a93
rustc_resolve: flatten nested `if`s
yotamofek Jan 20, 2025
ae87d00
rustc_resolve: reduce rightwards drift with `let..else` 👉💨
yotamofek Jan 20, 2025
6f22dfe
rustc_resolve: use `Iterator` combinators instead of `for` loops wher…
yotamofek Jan 20, 2025
efabee2
rustc_resolve: don't open-code `Option::filter`
yotamofek Jan 21, 2025
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
32 changes: 15 additions & 17 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,11 +645,11 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
let self_spans = items
.iter()
.filter_map(|(use_tree, _)| {
if let ast::UseTreeKind::Simple(..) = use_tree.kind {
if use_tree.ident().name == kw::SelfLower {
if let ast::UseTreeKind::Simple(..) = use_tree.kind
&& use_tree.ident().name == kw::SelfLower
{
return Some(use_tree.span);
}
}

None
})
Expand Down Expand Up @@ -947,20 +947,20 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
let imported_binding = self.r.import(binding, import);
if parent == self.r.graph_root {
let ident = ident.normalize_to_macros_2_0();
if let Some(entry) = self.r.extern_prelude.get(&ident) {
if expansion != LocalExpnId::ROOT && orig_name.is_some() && !entry.is_import() {
if let Some(entry) = self.r.extern_prelude.get(&ident)
&& expansion != LocalExpnId::ROOT
&& orig_name.is_some()
&& !entry.is_import()
{
self.r.dcx().emit_err(
errors::MacroExpandedExternCrateCannotShadowExternArguments {
span: item.span,
},
errors::MacroExpandedExternCrateCannotShadowExternArguments { span: item.span },
);
// `return` is intended to discard this binding because it's an
// unregistered ambiguity error which would result in a panic
// caused by inconsistency `path_res`
// more details: https://github.com/rust-lang/rust/pull/111761
return;
}
}
let entry = self
.r
.extern_prelude
Expand Down Expand Up @@ -1040,11 +1040,11 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
span: item.span,
});
}
if let ItemKind::ExternCrate(Some(orig_name)) = item.kind {
if orig_name == kw::SelfLower {
if let ItemKind::ExternCrate(Some(orig_name)) = item.kind
&& orig_name == kw::SelfLower
{
self.r.dcx().emit_err(errors::MacroUseExternCrateSelf { span: attr.span });
}
}
let ill_formed = |span| {
self.r.dcx().emit_err(errors::BadMacroImport { span });
};
Expand Down Expand Up @@ -1179,15 +1179,13 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
return Some((MacroKind::Bang, item.ident, item.span));
} else if ast::attr::contains_name(&item.attrs, sym::proc_macro_attribute) {
return Some((MacroKind::Attr, item.ident, item.span));
} else if let Some(attr) = ast::attr::find_by_name(&item.attrs, sym::proc_macro_derive) {
if let Some(meta_item_inner) =
} else if let Some(attr) = ast::attr::find_by_name(&item.attrs, sym::proc_macro_derive)
&& let Some(meta_item_inner) =
attr.meta_item_list().and_then(|list| list.get(0).cloned())
&& let Some(ident) = meta_item_inner.ident()
{
if let Some(ident) = meta_item_inner.ident() {
return Some((MacroKind::Derive, ident, ident.span));
}
}
}
None
}

Expand Down
121 changes: 57 additions & 64 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,11 +225,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let (name, span) =
(ident.name, self.tcx.sess.source_map().guess_head_span(new_binding.span));

if let Some(s) = self.name_already_seen.get(&name) {
if s == &span {
if self.name_already_seen.get(&name) == Some(&span) {
return;
}
}

let old_kind = match (ns, old_binding.module()) {
(ValueNS, _) => "value",
Expand Down Expand Up @@ -380,22 +378,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
suggestion = Some(format!("self as {suggested_name}"))
}
ImportKind::Single { source, .. } => {
if let Some(pos) =
source.span.hi().0.checked_sub(binding_span.lo().0).map(|pos| pos as usize)
if let Some(pos) = source.span.hi().0.checked_sub(binding_span.lo().0)
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(binding_span)
&& pos as usize <= snippet.len()
{
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(binding_span) {
if pos <= snippet.len() {
span = binding_span
.with_lo(binding_span.lo() + BytePos(pos as u32))
.with_hi(
binding_span.hi()
- BytePos(if snippet.ends_with(';') { 1 } else { 0 }),
span = binding_span.with_lo(binding_span.lo() + BytePos(pos)).with_hi(
binding_span.hi() - BytePos(if snippet.ends_with(';') { 1 } else { 0 }),
);
suggestion = Some(format!(" as {suggested_name}"));
}
}
}
}
ImportKind::ExternCrate { source, target, .. } => {
suggestion = Some(format!(
"extern crate {} as {};",
Expand Down Expand Up @@ -510,14 +502,13 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// If the first element of our path was actually resolved to an
// `ExternCrate` (also used for `crate::...`) then no need to issue a
// warning, this looks all good!
if let Some(binding) = second_binding {
if let NameBindingKind::Import { import, .. } = binding.kind {
if let Some(binding) = second_binding
&& let NameBindingKind::Import { import, .. } = binding.kind
// Careful: we still want to rewrite paths from renamed extern crates.
if let ImportKind::ExternCrate { source: None, .. } = import.kind {
&& let ImportKind::ExternCrate { source: None, .. } = import.kind
{
return;
}
}
}

let diag = BuiltinLintDiag::AbsPathWithModule(root_span);
self.lint_buffer.buffer_lint(
Expand Down Expand Up @@ -1047,14 +1038,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if filter_fn(res) {
for derive in parent_scope.derives {
let parent_scope = &ParentScope { derives: &[], ..*parent_scope };
if let Ok((Some(ext), _)) = this.resolve_macro_path(
let Ok((Some(ext), _)) = this.resolve_macro_path(
derive,
Some(MacroKind::Derive),
parent_scope,
false,
false,
None,
) {
) else {
continue;
};
suggestions.extend(
ext.helper_attrs
.iter()
Expand All @@ -1063,7 +1056,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}
}
}
}
Scope::MacroRules(macro_rules_scope) => {
if let MacroRulesScope::Binding(macro_rules_binding) = macro_rules_scope.get() {
let res = macro_rules_binding.binding.res();
Expand Down Expand Up @@ -1215,13 +1207,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
}

// #90113: Do not count an inaccessible reexported item as a candidate.
if let NameBindingKind::Import { binding, .. } = name_binding.kind {
if this.is_accessible_from(binding.vis, parent_scope.module)
if let NameBindingKind::Import { binding, .. } = name_binding.kind
&& this.is_accessible_from(binding.vis, parent_scope.module)
&& !this.is_accessible_from(name_binding.vis, parent_scope.module)
{
return;
}
}

let res = name_binding.res();
let did = match res {
Expand Down Expand Up @@ -1253,15 +1244,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
segms.push(ast::PathSegment::from_ident(ident));
let path = Path { span: name_binding.span, segments: segms, tokens: None };

if child_accessible {
if child_accessible
// Remove invisible match if exists
if let Some(idx) = candidates
&& let Some(idx) = candidates
.iter()
.position(|v: &ImportSuggestion| v.did == did && !v.accessible)
{
candidates.remove(idx);
}
}

if candidates.iter().all(|v: &ImportSuggestion| v.did != did) {
// See if we're recommending TryFrom, TryInto, or FromIterator and add
Expand Down Expand Up @@ -1373,8 +1363,11 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// otherwise cause duplicate suggestions.
continue;
}
let crate_id = self.crate_loader(|c| c.maybe_process_path_extern(ident.name));
if let Some(crate_id) = crate_id {
let Some(crate_id) = self.crate_loader(|c| c.maybe_process_path_extern(ident.name))
else {
continue;
};

let crate_def_id = crate_id.as_def_id();
let crate_root = self.expect_module(crate_def_id);

Expand Down Expand Up @@ -1416,7 +1409,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
));
}
}
}

suggestions
}
Expand Down Expand Up @@ -1511,19 +1503,20 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
});
}
for ns in [Namespace::MacroNS, Namespace::TypeNS, Namespace::ValueNS] {
if let Ok(binding) = self.early_resolve_ident_in_lexical_scope(
let Ok(binding) = self.early_resolve_ident_in_lexical_scope(
ident,
ScopeSet::All(ns),
parent_scope,
None,
false,
None,
None,
) {
) else {
continue;
};

let desc = match binding.res() {
Res::Def(DefKind::Macro(MacroKind::Bang), _) => {
"a function-like macro".to_string()
}
Res::Def(DefKind::Macro(MacroKind::Bang), _) => "a function-like macro".to_string(),
Res::Def(DefKind::Macro(MacroKind::Attr), _) | Res::NonMacroAttr(..) => {
format!("an attribute: `#[{ident}]`")
}
Expand All @@ -1545,8 +1538,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
macro_kind.descr_expected(),
),
};
if let crate::NameBindingKind::Import { import, .. } = binding.kind {
if !import.span.is_dummy() {
if let crate::NameBindingKind::Import { import, .. } = binding.kind
&& !import.span.is_dummy()
{
let note = errors::IdentImporterHereButItIsDesc {
span: import.span,
imported_ident: ident,
Expand All @@ -1558,7 +1552,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
self.record_use(ident, binding, Used::Other);
return;
}
}
let note = errors::IdentInScopeButItIsDesc {
imported_ident: ident,
imported_ident_desc: &desc,
Expand All @@ -1567,7 +1560,6 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
return;
}
}
}

pub(crate) fn add_typo_suggestion(
&self,
Expand Down Expand Up @@ -1749,15 +1741,16 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
/// If the binding refers to a tuple struct constructor with fields,
/// returns the span of its fields.
fn ctor_fields_span(&self, binding: NameBinding<'_>) -> Option<Span> {
if let NameBindingKind::Res(Res::Def(
let NameBindingKind::Res(Res::Def(
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn),
ctor_def_id,
)) = binding.kind
{
else {
return None;
};

let def_id = self.tcx.parent(ctor_def_id);
return self.field_idents(def_id)?.iter().map(|&f| f.span).reduce(Span::to); // None for `struct Foo()`
}
None
self.field_idents(def_id)?.iter().map(|&f| f.span).reduce(Span::to) // None for `struct Foo()`
}

fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'ra>) {
Expand Down Expand Up @@ -2410,7 +2403,9 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let binding_key = BindingKey::new(ident, MacroNS);
let resolution = resolutions.get(&binding_key)?;
let binding = resolution.borrow().binding()?;
if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() {
let Res::Def(DefKind::Macro(MacroKind::Bang), _) = binding.res() else {
return None;
};
let module_name = crate_module.kind.name().unwrap();
let import_snippet = match import.kind {
ImportKind::Single { source, target, .. } if source != target => {
Expand All @@ -2436,21 +2431,21 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
debug!(found_closing_brace, ?binding_span);

let mut removal_span = binding_span;
if found_closing_brace {

// If the binding span ended with a closing brace, as in the below example:
// ie. `use a::b::{c, d};`
// ^
// Then expand the span of characters to remove to include the previous
// binding's trailing comma.
// ie. `use a::b::{c, d};`
// ^^^
if let Some(previous_span) =
if found_closing_brace
&& let Some(previous_span) =
extend_span_to_previous_binding(self.tcx.sess, binding_span)
{
debug!(?previous_span);
removal_span = removal_span.with_lo(previous_span.lo());
}
}
debug!(?removal_span);

// Remove the `removal_span`.
Expand All @@ -2462,11 +2457,8 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// ^^^^^^^^^
// or `use a::{b, c, d}};`
// ^^^^^^^^^^^
let (has_nested, after_crate_name) = find_span_immediately_after_crate_name(
self.tcx.sess,
module_name,
import.use_span,
);
let (has_nested, after_crate_name) =
find_span_immediately_after_crate_name(self.tcx.sess, module_name, import.use_span);
debug!(has_nested, ?after_crate_name);

let source_map = self.tcx.sess.source_map();
Expand Down Expand Up @@ -2496,8 +2488,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {

// Add a `};` to the end if nested, matching the `{` added at the start.
if !has_nested {
corrections
.push((source_map.end_point(after_crate_name), "};".to_string()));
corrections.push((source_map.end_point(after_crate_name), "};".to_string()));
}
} else {
// If the root import is module-relative, add the import separately
Expand All @@ -2513,12 +2504,14 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
String::from("a macro with this name exists at the root of the crate"),
Applicability::MaybeIncorrect,
));
Some((suggestion, Some("this could be because a macro annotated with `#[macro_export]` will be exported \
Some((
suggestion,
Some(
"this could be because a macro annotated with `#[macro_export]` will be exported \
at the root of the crate instead of the module where it is defined"
.to_string())))
} else {
None
}
.to_string(),
),
))
}

/// Finds a cfg-ed out item inside `module` with the matching name.
Expand Down Expand Up @@ -3064,8 +3057,9 @@ impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {

fn search_for_any_use_in_items(items: &[P<ast::Item>]) -> Option<Span> {
for item in items {
if let ItemKind::Use(..) = item.kind {
if is_span_suitable_for_use_injection(item.span) {
if let ItemKind::Use(..) = item.kind
&& is_span_suitable_for_use_injection(item.span)
{
let mut lo = item.span.lo();
for attr in &item.attrs {
if attr.span.eq_ctxt(item.span) {
Expand All @@ -3075,7 +3069,6 @@ fn search_for_any_use_in_items(items: &[P<ast::Item>]) -> Option<Span> {
return Some(Span::new(lo, lo, item.span.ctxt(), item.span.parent()));
}
}
}
None
}

Expand Down
Loading