Skip to content

Introduce ModernIdent type to unify macro 2.0 hygiene handling #144439

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+108 −40
Open

Introduce ModernIdent type to unify macro 2.0 hygiene handling #144439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 2 additions & 2 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 @@ -27,7 +27,7 @@ use rustc_middle::metadata::ModChild;
use rustc_middle::ty::{Feed, Visibility};
use rustc_middle::{bug, span_bug};
use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind};
use rustc_span::{Ident, Span, Symbol, kw, sym};
use rustc_span::{Ident, Macros20NormalizedIdent, Span, Symbol, kw, sym};
use thin_vec::ThinVec;
use tracing::debug;

Expand Down Expand Up @@ -969,7 +969,7 @@ impl<'a, 'ra, 'tcx> BuildReducedGraphVisitor<'a, 'ra, 'tcx> {
self.r.potentially_unused_imports.push(import);
let imported_binding = self.r.import(binding, import);
if parent == self.r.graph_root {
let ident = ident.normalize_to_macros_2_0();
let ident = Macros20NormalizedIdent::new(ident);
if let Some(entry) = self.r.extern_prelude.get(&ident)
&& expansion != LocalExpnId::ROOT
&& orig_name.is_some()
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/check_unused.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use rustc_session::lint::BuiltinLintDiag;
use rustc_session::lint::builtin::{
MACRO_USE_EXTERN_CRATE, UNUSED_EXTERN_CRATES, UNUSED_IMPORTS, UNUSED_QUALIFICATIONS,
};
use rustc_span::{DUMMY_SP, Ident, Span, kw};
use rustc_span::{DUMMY_SP, Ident, Macros20NormalizedIdent, Span, kw};

use crate::imports::{Import, ImportKind};
use crate::{LexicalScopeBinding, NameBindingKind, Resolver, module_to_string};
Expand Down Expand Up @@ -203,7 +203,7 @@ impl<'a, 'ra, 'tcx> UnusedImportCheckVisitor<'a, 'ra, 'tcx> {
if self
.r
.extern_prelude
.get(&extern_crate.ident)
.get(&unsafe { Macros20NormalizedIdent::new_without_normalize(extern_crate.ident) })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here use new_without_normalize for indexing

.is_none_or(|entry| entry.introduced_by_item)
{
continue;
Expand Down
18 changes: 10 additions & 8 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::edition::Edition;
use rustc_span::hygiene::MacroKind;
use rustc_span::source_map::SourceMap;
use rustc_span::{BytePos, Ident, Span, Symbol, SyntaxContext, kw, sym};
use rustc_span::{BytePos, Ident, Macros20NormalizedIdent, Span, Symbol, SyntaxContext, kw, sym};
use thin_vec::{ThinVec, thin_vec};
use tracing::{debug, instrument};

Expand Down Expand Up @@ -322,8 +322,10 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// Check if the target of the use for both bindings is the same.
let duplicate = new_binding.res().opt_def_id() == old_binding.res().opt_def_id();
let has_dummy_span = new_binding.span.is_dummy() || old_binding.span.is_dummy();
let from_item =
self.extern_prelude.get(&ident).is_none_or(|entry| entry.introduced_by_item);
let from_item = self
.extern_prelude
.get(&unsafe { Macros20NormalizedIdent::new_without_normalize(ident) })
.is_none_or(|entry| entry.introduced_by_item);
Comment on lines +325 to +328
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here also for Indexing

// Only suggest removing an import if both bindings are to the same def, if both spans
// aren't dummy spans. Further, if both bindings are imports, then the ident must have
// been introduced by an item.
Expand Down Expand Up @@ -532,7 +534,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
module.for_each_child(self, |_this, ident, _ns, binding| {
let res = binding.res();
if filter_fn(res) && ctxt.is_none_or(|ctxt| ctxt == ident.span.ctxt()) {
names.push(TypoSuggestion::typo_from_ident(ident, res));
names.push(TypoSuggestion::typo_from_ident(ident.0, res));
}
});
}
Expand Down Expand Up @@ -1100,7 +1102,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
Scope::ExternPrelude => {
suggestions.extend(this.extern_prelude.iter().filter_map(|(ident, _)| {
let res = Res::Def(DefKind::Mod, CRATE_DEF_ID.to_def_id());
filter_fn(res).then_some(TypoSuggestion::typo_from_ident(*ident, res))
filter_fn(res).then_some(TypoSuggestion::typo_from_ident(ident.0, res))
}));
}
Scope::ToolPrelude => {
Expand Down Expand Up @@ -1246,7 +1248,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
};
segms.append(&mut path_segments.clone());

segms.push(ast::PathSegment::from_ident(ident));
segms.push(ast::PathSegment::from_ident(ident.0));
let path = Path { span: name_binding.span, segments: segms, tokens: None };

if child_accessible
Expand Down Expand Up @@ -1319,7 +1321,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if let Some(def_id) = name_binding.res().module_like_def_id() {
// form the path
let mut path_segments = path_segments.clone();
path_segments.push(ast::PathSegment::from_ident(ident));
path_segments.push(ast::PathSegment::from_ident(ident.0));

let alias_import = if let NameBindingKind::Import { import, .. } =
name_binding.kind
Expand Down Expand Up @@ -1455,7 +1457,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
if needs_disambiguation {
crate_path.push(ast::PathSegment::path_root(rustc_span::DUMMY_SP));
}
crate_path.push(ast::PathSegment::from_ident(ident));
crate_path.push(ast::PathSegment::from_ident(ident.0));

suggestions.extend(self.lookup_import_candidates_from_module(
lookup_ident,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_resolve/src/imports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let imported_binding = self.import(binding, *import);
let _ = self.try_define_local(
import.parent_scope.module,
ident,
ident.0,
key.ns,
imported_binding,
warn_ambiguity,
Expand Down Expand Up @@ -1517,7 +1517,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
.is_some_and(|binding| binding.warn_ambiguity_recursive());
let _ = self.try_define_local(
import.parent_scope.module,
key.ident,
key.ident.0,
key.ns,
imported_binding,
warn_ambiguity,
Expand Down Expand Up @@ -1550,7 +1550,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
next_binding = binding;
}

children.push(ModChild { ident, res, vis: binding.vis, reexport_chain });
children.push(ModChild { ident: ident.0, res, vis: binding.vis, reexport_chain });
}
});

Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_resolve/src/late/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1475,7 +1475,10 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
})
.collect();
if let [target] = targets.as_slice() {
return Some(TypoSuggestion::single_item_from_ident(target.0.ident, target.1));
return Some(TypoSuggestion::single_item_from_ident(
target.0.ident.0,
target.1,
));
}
}
}
Expand Down Expand Up @@ -2495,7 +2498,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
Res::Def(DefKind::Mod, crate_id.as_def_id());

filter_fn(crate_mod).then(|| {
TypoSuggestion::typo_from_ident(*ident, crate_mod)
TypoSuggestion::typo_from_ident(ident.0, crate_mod)
})
})
}));
Expand Down Expand Up @@ -2657,7 +2660,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
if let Some(module_def_id) = name_binding.res().module_like_def_id() {
// form the path
let mut path_segments = path_segments.clone();
path_segments.push(ast::PathSegment::from_ident(ident));
path_segments.push(ast::PathSegment::from_ident(ident.0));
let doc_visible = doc_visible
&& (module_def_id.is_local() || !r.tcx.is_doc_hidden(module_def_id));
if module_def_id == def_id {
Expand Down Expand Up @@ -2696,7 +2699,7 @@ impl<'ast, 'ra, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
enum_module.for_each_child(self.r, |_, ident, _, name_binding| {
if let Res::Def(DefKind::Ctor(CtorOf::Variant, kind), def_id) = name_binding.res() {
let mut segms = enum_import_suggestion.path.segments.clone();
segms.push(ast::PathSegment::from_ident(ident));
segms.push(ast::PathSegment::from_ident(ident.0));
let path = Path { span: name_binding.span, segments: segms, tokens: None };
variants.push((path, def_id, kind));
}
Expand Down
37 changes: 23 additions & 14 deletions compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ use rustc_query_system::ich::StableHashingContext;
use rustc_session::lint::builtin::PRIVATE_MACRO_USE;
use rustc_session::lint::{BuiltinLintDiag, LintBuffer};
use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind, SyntaxContext, Transparency};
use rustc_span::{DUMMY_SP, Ident, Span, Symbol, kw, sym};
use rustc_span::{DUMMY_SP, Ident, Macros20NormalizedIdent, Span, Symbol, kw, sym};
use smallvec::{SmallVec, smallvec};
use tracing::debug;

Expand Down Expand Up @@ -531,7 +531,7 @@ impl ModuleKind {
struct BindingKey {
/// The identifier for the binding, always the `normalize_to_macros_2_0` version of the
/// identifier.
ident: Ident,
ident: Macros20NormalizedIdent,
Comment on lines 531 to +534
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main Change(1/2)

ns: Namespace,
/// When we add an underscore binding (with ident `_`) to some module, this field has
/// a non-zero value that uniquely identifies this binding in that module.
Expand All @@ -543,7 +543,7 @@ struct BindingKey {

impl BindingKey {
fn new(ident: Ident, ns: Namespace) -> Self {
BindingKey { ident: ident.normalize_to_macros_2_0(), ns, disambiguator: 0 }
BindingKey { ident: Macros20NormalizedIdent::new(ident), ns, disambiguator: 0 }
}

fn new_disambiguated(
Expand All @@ -552,7 +552,7 @@ impl BindingKey {
disambiguator: impl FnOnce() -> u32,
) -> BindingKey {
let disambiguator = if ident.name == kw::Underscore { disambiguator() } else { 0 };
BindingKey { ident: ident.normalize_to_macros_2_0(), ns, disambiguator }
BindingKey { ident: Macros20NormalizedIdent::new(ident), ns, disambiguator }
}
}

Expand Down Expand Up @@ -659,7 +659,7 @@ impl<'ra> Module<'ra> {
fn for_each_child<'tcx, R: AsRef<Resolver<'ra, 'tcx>>>(
self,
resolver: &R,
mut f: impl FnMut(&R, Ident, Namespace, NameBinding<'ra>),
mut f: impl FnMut(&R, Macros20NormalizedIdent, Namespace, NameBinding<'ra>),
) {
for (key, name_resolution) in resolver.as_ref().resolutions(self).borrow().iter() {
if let Some(binding) = name_resolution.borrow().best_binding() {
Expand All @@ -671,7 +671,7 @@ impl<'ra> Module<'ra> {
fn for_each_child_mut<'tcx, R: AsMut<Resolver<'ra, 'tcx>>>(
self,
resolver: &mut R,
mut f: impl FnMut(&mut R, Ident, Namespace, NameBinding<'ra>),
mut f: impl FnMut(&mut R, Macros20NormalizedIdent, Namespace, NameBinding<'ra>),
) {
for (key, name_resolution) in resolver.as_mut().resolutions(self).borrow().iter() {
if let Some(binding) = name_resolution.borrow().best_binding() {
Expand All @@ -690,7 +690,7 @@ impl<'ra> Module<'ra> {
return;
}
if let Res::Def(DefKind::Trait | DefKind::TraitAlias, def_id) = binding.res() {
collected_traits.push((name, binding, r.as_ref().get_module(def_id)))
collected_traits.push((name.0, binding, r.as_ref().get_module(def_id)))
}
});
*traits = Some(collected_traits.into_boxed_slice());
Expand Down Expand Up @@ -1049,7 +1049,7 @@ pub struct Resolver<'ra, 'tcx> {
graph_root: Module<'ra>,

prelude: Option<Module<'ra>>,
extern_prelude: FxIndexMap<Ident, ExternPreludeEntry<'ra>>,
extern_prelude: FxIndexMap<Macros20NormalizedIdent, ExternPreludeEntry<'ra>>,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main change (2/2)


/// N.B., this is used only for better diagnostics, not name resolution itself.
field_names: LocalDefIdMap<Vec<Ident>>,
Expand Down Expand Up @@ -1482,19 +1482,28 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
let mut invocation_parents = FxHashMap::default();
invocation_parents.insert(LocalExpnId::ROOT, InvocationParent::ROOT);

let mut extern_prelude: FxIndexMap<Ident, ExternPreludeEntry<'_>> = tcx
let mut extern_prelude: FxIndexMap<Macros20NormalizedIdent, ExternPreludeEntry<'_>> = tcx
.sess
.opts
.externs
.iter()
.filter(|(_, entry)| entry.add_prelude)
.map(|(name, _)| (Ident::from_str(name), Default::default()))
.map(|(name, _)| {
(
unsafe {
Macros20NormalizedIdent::new_without_normalize(Ident::from_str(name))
},
Default::default(),
Comment on lines -1491 to +1496
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here will return extern_prelude which petrochenkov suggest to convert to Macro20NormalizedIdent.

)
})
.collect();

if !attr::contains_name(attrs, sym::no_core) {
extern_prelude.insert(Ident::with_dummy_span(sym::core), Default::default());
extern_prelude
.insert(Macros20NormalizedIdent::with_dummy_span(sym::core), Default::default());
if !attr::contains_name(attrs, sym::no_std) {
extern_prelude.insert(Ident::with_dummy_span(sym::std), Default::default());
extern_prelude
.insert(Macros20NormalizedIdent::with_dummy_span(sym::std), Default::default());
}
}

Expand Down Expand Up @@ -2005,7 +2014,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
// Avoid marking `extern crate` items that refer to a name from extern prelude,
// but not introduce it, as used if they are accessed from lexical scope.
if used == Used::Scope {
if let Some(entry) = self.extern_prelude.get(&ident.normalize_to_macros_2_0()) {
if let Some(entry) = self.extern_prelude.get(&Macros20NormalizedIdent::new(ident)) {
if !entry.introduced_by_item && entry.binding == Some(used_binding) {
return;
}
Expand Down Expand Up @@ -2168,7 +2177,7 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
return None;
}

let norm_ident = ident.normalize_to_macros_2_0();
let norm_ident = Macros20NormalizedIdent::new(ident);
let binding = self.extern_prelude.get(&norm_ident).cloned().and_then(|entry| {
Some(if let Some(binding) = entry.binding {
if finalize {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,11 +535,11 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
target_trait.for_each_child(self, |this, ident, ns, _binding| {
// FIXME: Adjust hygiene for idents from globs, like for glob imports.
if let Some(overriding_keys) = this.impl_binding_keys.get(&impl_def_id)
&& overriding_keys.contains(&BindingKey::new(ident, ns))
&& overriding_keys.contains(&BindingKey::new(ident.0, ns))
{
// The name is overridden, do not produce it from the glob delegation.
} else {
idents.push((ident, None));
idents.push((ident.0, None));
}
});
Ok(idents)
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_span/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ pub use span_encoding::{DUMMY_SP, Span};

pub mod symbol;
pub use symbol::{
ByteSymbol, Ident, MacroRulesNormalizedIdent, STDLIB_STABLE_CRATES, Symbol, kw, sym,
ByteSymbol, Ident, MacroRulesNormalizedIdent, Macros20NormalizedIdent, STDLIB_STABLE_CRATES,
Symbol, kw, sym,
};

mod analyze_source_file;
Expand Down
Loading
Loading