Skip to content

Commit

Permalink
Rollup merge of #113332 - petrochenkov:bindintern, r=cjgillot
Browse files Browse the repository at this point in the history 
resolve: Use `Interned` for some interned structures

Enough to get rid of all existing `ptr::eq`s and "partial" uses of `Interned`.
  • Loading branch information
@fee1-dead
fee1-dead authored Jul 6, 2023
2 parents a105aa2 + 9f3fba8 commit 0162726
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 255 deletions.
33 changes: 16 additions & 17 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
//! Imports are also considered items and placed into modules here, but not resolved yet.

use crate::def_collector::collect_definitions;
use crate::imports::{Import, ImportKind};
use crate::imports::{ImportData, ImportKind};
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
use crate::{errors, BindingKey, MacroData};
use crate::{errors, BindingKey, MacroData, NameBindingData};
use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};
Expand All @@ -31,15 +31,14 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::Span;

use std::cell::Cell;
use std::ptr;

type Res = def::Res<NodeId>;

impl<'a, Id: Into<DefId>> ToNameBinding<'a>
for (Module<'a>, ty::Visibility<Id>, Span, LocalExpnId)
{
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
arenas.alloc_name_binding(NameBinding {
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Module(self.0),
ambiguity: None,
vis: self.1.to_def_id(),
Expand All @@ -50,8 +49,8 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a>
}

impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span, LocalExpnId) {
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
arenas.alloc_name_binding(NameBinding {
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> NameBinding<'a> {
arenas.alloc_name_binding(NameBindingData {
kind: NameBindingKind::Res(self.0),
ambiguity: None,
vis: self.1.to_def_id(),
Expand All @@ -71,7 +70,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let binding = def.to_name_binding(self.arenas);
let key = self.new_disambiguated_key(ident, ns);
if let Err(old_binding) = self.try_define(parent, key, binding) {
self.report_conflict(parent, ident, ns, old_binding, &binding);
self.report_conflict(parent, ident, ns, old_binding, binding);
}
}

Expand Down Expand Up @@ -142,8 +141,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
Some(def_id) => self.macro_def_scope(def_id),
None => expn_id
.as_local()
.and_then(|expn_id| self.ast_transform_scopes.get(&expn_id))
.unwrap_or(&self.graph_root),
.and_then(|expn_id| self.ast_transform_scopes.get(&expn_id).copied())
.unwrap_or(self.graph_root),
}
}

Expand Down Expand Up @@ -354,7 +353,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
vis: ty::Visibility,
) {
let current_module = self.parent_scope.module;
let import = self.r.arenas.alloc_import(Import {
let import = self.r.arenas.alloc_import(ImportData {
kind,
parent_scope: self.parent_scope,
module_path,
Expand All @@ -378,7 +377,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
if !type_ns_only || ns == TypeNS {
let key = BindingKey::new(target, ns);
let mut resolution = this.resolution(current_module, key).borrow_mut();
resolution.add_single_import(import);
resolution.single_imports.insert(import);
}
});
}
Expand Down Expand Up @@ -848,7 +847,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
(used, Some(ModuleOrUniformRoot::Module(module)), binding)
})
.unwrap_or((true, None, self.r.dummy_binding));
let import = self.r.arenas.alloc_import(Import {
let import = self.r.arenas.alloc_import(ImportData {
kind: ImportKind::ExternCrate { source: orig_name, target: ident, id: item.id },
root_id: item.id,
parent_scope: self.parent_scope,
Expand All @@ -864,7 +863,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
});
self.r.potentially_unused_imports.push(import);
let imported_binding = self.r.import(binding, import);
if ptr::eq(parent, self.r.graph_root) {
if parent == self.r.graph_root {
if let Some(entry) = self.r.extern_prelude.get(&ident.normalize_to_macros_2_0()) {
if expansion != LocalExpnId::ROOT
&& orig_name.is_some()
Expand Down Expand Up @@ -996,7 +995,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
fn add_macro_use_binding(
&mut self,
name: Symbol,
binding: &'a NameBinding<'a>,
binding: NameBinding<'a>,
span: Span,
allow_shadowing: bool,
) {
Expand Down Expand Up @@ -1058,7 +1057,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
}

let macro_use_import = |this: &Self, span| {
this.r.arenas.alloc_import(Import {
this.r.arenas.alloc_import(ImportData {
kind: ImportKind::MacroUse,
root_id: item.id,
parent_scope: this.parent_scope,
Expand Down Expand Up @@ -1228,7 +1227,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
self.r.set_binding_parent_module(binding, parent_scope.module);
self.r.all_macro_rules.insert(ident.name, res);
if is_macro_export {
let import = self.r.arenas.alloc_import(Import {
let import = self.r.arenas.alloc_import(ImportData {
kind: ImportKind::MacroExport,
root_id: item.id,
parent_scope: self.parent_scope,
Expand Down
59 changes: 28 additions & 31 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::ptr;

use rustc_ast::expand::StrippedCfgItem;
use rustc_ast::ptr::P;
use rustc_ast::visit::{self, Visitor};
Expand Down Expand Up @@ -182,13 +180,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

pub(crate) fn report_conflict<'b>(
pub(crate) fn report_conflict(
&mut self,
parent: Module<'_>,
ident: Ident,
ns: Namespace,
new_binding: &NameBinding<'b>,
old_binding: &NameBinding<'b>,
new_binding: NameBinding<'a>,
old_binding: NameBinding<'a>,
) {
// Error on the second of two conflicting names
if old_binding.span.lo() > new_binding.span.lo() {
Expand Down Expand Up @@ -262,7 +260,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

// See https://github.com/rust-lang/rust/issues/32354
use NameBindingKind::Import;
let can_suggest = |binding: &NameBinding<'_>, import: &self::Import<'_>| {
let can_suggest = |binding: NameBinding<'_>, import: self::Import<'_>| {
!binding.span.is_dummy()
&& !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport)
};
Expand All @@ -272,22 +270,22 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
(Import { import: new, .. }, Import { import: old, .. })
if {
(new.has_attributes || old.has_attributes)
&& can_suggest(old_binding, old)
&& can_suggest(new_binding, new)
&& can_suggest(old_binding, *old)
&& can_suggest(new_binding, *new)
} =>
{
if old.has_attributes {
Some((new, new_binding.span, true))
Some((*new, new_binding.span, true))
} else {
Some((old, old_binding.span, true))
Some((*old, old_binding.span, true))
}
}
// Otherwise prioritize the new binding.
(Import { import, .. }, other) if can_suggest(new_binding, import) => {
Some((import, new_binding.span, other.is_import()))
(Import { import, .. }, other) if can_suggest(new_binding, *import) => {
Some((*import, new_binding.span, other.is_import()))
}
(other, Import { import, .. }) if can_suggest(old_binding, import) => {
Some((import, old_binding.span, other.is_import()))
(other, Import { import, .. }) if can_suggest(old_binding, *import) => {
Some((*import, old_binding.span, other.is_import()))
}
_ => None,
};
Expand Down Expand Up @@ -341,7 +339,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&self,
err: &mut Diagnostic,
name: Symbol,
import: &Import<'_>,
import: Import<'_>,
binding_span: Span,
) {
let suggested_name = if name.as_str().chars().next().unwrap().is_uppercase() {
Expand Down Expand Up @@ -413,7 +411,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
fn add_suggestion_for_duplicate_nested_use(
&self,
err: &mut Diagnostic,
import: &Import<'_>,
import: Import<'_>,
binding_span: Span,
) {
assert!(import.is_nested());
Expand Down Expand Up @@ -455,7 +453,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&mut self,
finalize: Option<Finalize>,
path: &[Segment],
second_binding: Option<&NameBinding<'_>>,
second_binding: Option<NameBinding<'_>>,
) {
let Some(Finalize { node_id, root_span, .. }) = finalize else {
return;
Expand Down Expand Up @@ -1198,7 +1196,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// avoid suggesting anything with a hygienic name
if ident.name == lookup_ident.name
&& ns == namespace
&& !ptr::eq(in_module, parent_scope.module)
&& in_module != parent_scope.module
&& !ident.span.normalize_to_macros_2_0().from_expansion()
{
let res = name_binding.res();
Expand Down Expand Up @@ -1515,7 +1513,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
true
}

fn binding_description(&self, b: &NameBinding<'_>, ident: Ident, from_prelude: bool) -> String {
fn binding_description(&self, b: NameBinding<'_>, ident: Ident, from_prelude: bool) -> String {
let res = b.res();
if b.span.is_dummy() || !self.tcx.sess.source_map().is_span_accessible(b.span) {
// These already contain the "built-in" prefix or look bad with it.
Expand Down Expand Up @@ -1555,7 +1553,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
err.span_label(ident.span, "ambiguous name");
err.note(format!("ambiguous because of {}", kind.descr()));

let mut could_refer_to = |b: &NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
let mut could_refer_to = |b: NameBinding<'_>, misc: AmbiguityErrorMisc, also: &str| {
let what = self.binding_description(b, ident, misc == AmbiguityErrorMisc::FromPrelude);
let note_msg = format!("`{ident}` could{also} refer to {what}");

Expand Down Expand Up @@ -1595,7 +1593,7 @@ impl<'a, 'tcx> Resolver<'a, '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> {
fn ctor_fields_span(&self, binding: NameBinding<'_>) -> Option<Span> {
if let NameBindingKind::Res(Res::Def(
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn),
ctor_def_id,
Expand All @@ -1622,7 +1620,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
if ctor_fields_span.is_some() { plain_descr + " constructor" } else { plain_descr };
let import_descr = nonimport_descr.clone() + " import";
let get_descr =
|b: &NameBinding<'_>| if b.is_import() { &import_descr } else { &nonimport_descr };
|b: NameBinding<'_>| if b.is_import() { &import_descr } else { &nonimport_descr };

// Print the primary message.
let descr = get_descr(binding);
Expand Down Expand Up @@ -1702,7 +1700,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
_ => None,
};

let first = ptr::eq(binding, first_binding);
let first = binding == first_binding;
let msg = format!(
"{and_refers_to}the {item} `{name}`{which} is defined here{dots}",
and_refers_to = if first { "" } else { "...and refers to " },
Expand Down Expand Up @@ -1732,7 +1730,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
pub(crate) fn find_similarly_named_module_or_crate(
&mut self,
ident: Symbol,
current_module: &Module<'a>,
current_module: Module<'a>,
) -> Option<Symbol> {
let mut candidates = self
.extern_prelude
Expand All @@ -1742,7 +1740,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
self.module_map
.iter()
.filter(|(_, module)| {
current_module.is_ancestor_of(module) && !ptr::eq(current_module, *module)
current_module.is_ancestor_of(**module) && current_module != **module
})
.flat_map(|(_, module)| module.kind.name()),
)
Expand All @@ -1762,7 +1760,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
opt_ns: Option<Namespace>, // `None` indicates a module path in import
parent_scope: &ParentScope<'a>,
ribs: Option<&PerNS<Vec<Rib<'a>>>>,
ignore_binding: Option<&'a NameBinding<'a>>,
ignore_binding: Option<NameBinding<'a>>,
module: Option<ModuleOrUniformRoot<'a>>,
failed_segment_idx: usize,
ident: Ident,
Expand Down Expand Up @@ -1945,7 +1943,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}

suggestion = suggestion.or_else(|| {
self.find_similarly_named_module_or_crate(ident.name, &parent_scope.module).map(
self.find_similarly_named_module_or_crate(ident.name, parent_scope.module).map(
|sugg| {
(
vec![(ident.span, sugg.to_string())],
Expand Down Expand Up @@ -2114,7 +2112,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
/// ```
pub(crate) fn check_for_module_export_macro(
&mut self,
import: &'a Import<'a>,
import: Import<'a>,
module: ModuleOrUniformRoot<'a>,
ident: Ident,
) -> Option<(Option<Suggestion>, Option<String>)> {
Expand All @@ -2126,9 +2124,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
crate_module = parent;
}

if ModuleOrUniformRoot::same_def(ModuleOrUniformRoot::Module(crate_module), module) {
// Don't make a suggestion if the import was already from the root of the
// crate.
if module == ModuleOrUniformRoot::Module(crate_module) {
// Don't make a suggestion if the import was already from the root of the crate.
return None;
}

Expand Down
Loading

0 comments on commit 0162726

Please sign in to comment.