Skip to content
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

ast: Keep expansion status for out-of-line module items #82238

Merged
merged 2 commits into from
Feb 19, 2021
Merged

ast: Keep expansion status for out-of-line module items #82238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb65f15
ast: Stop using `Mod` in `Crate`
petrochenkov Feb 14, 2021
4a88165
ast: Keep expansion status for out-of-line module items
petrochenkov Feb 16, 2021
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
35 changes: 19 additions & 16 deletions compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,8 @@ pub struct WhereEqPredicate {

#[derive(Clone, Encodable, Decodable, Debug)]
pub struct Crate {
pub module: Mod,
pub attrs: Vec<Attribute>,
pub items: Vec<P<Item>>,
pub span: Span,
/// The order of items in the HIR is unrelated to the order of
/// items in the AST. However, we generate proc macro harnesses
Expand Down Expand Up @@ -2299,21 +2299,22 @@ impl FnRetTy {
}
}

/// Module declaration.
///
/// E.g., `mod foo;` or `mod foo { .. }`.
#[derive(Clone, PartialEq, Encodable, Decodable, Debug)]
pub enum Inline {
Yes,
No,
}

/// Module item kind.
#[derive(Clone, Encodable, Decodable, Debug)]
pub struct Mod {
/// A span from the first token past `{` to the last token until `}`.
/// For `mod foo;`, the inner span ranges from the first token
/// to the last token in the external file.
pub inner: Span,
/// `unsafe` keyword accepted syntactically for macro DSLs, but not
/// semantically by Rust.
pub unsafety: Unsafe,
pub items: Vec<P<Item>>,
/// `true` for `mod foo { .. }`; `false` for `mod foo;`.
pub inline: bool,
pub enum ModKind {
/// Module with inlined definition `mod foo { ... }`,
/// or with definition outlined to a separate file `mod foo;` and already loaded from it.
/// The inner span is from the first token past `{` to the last token until `}`,
/// or from the first to the last token in the loaded file.
Loaded(Vec<P<Item>>, Inline, Span),
/// Module with definition outlined to a separate file `mod foo;` but not yet loaded from it.
Unloaded,
}

/// Foreign module declaration.
Expand Down Expand Up @@ -2710,7 +2711,9 @@ pub enum ItemKind {
/// A module declaration (`mod`).
///
/// E.g., `mod foo;` or `mod foo { .. }`.
Mod(Mod),
/// `unsafe` keyword on modules is accepted syntactically for macro DSLs, but not
/// semantically by Rust.
Mod(Unsafe, ModKind),
/// An external module (`extern`).
///
/// E.g., `extern {}` or `extern "C" {}`.
Expand Down
31 changes: 15 additions & 16 deletions compiler/rustc_ast/src/mut_visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,6 @@ pub trait MutVisitor: Sized {
noop_visit_ty_constraint(t, self);
}

fn visit_mod(&mut self, m: &mut Mod) {
noop_visit_mod(m, self);
}

fn visit_foreign_mod(&mut self, nm: &mut ForeignMod) {
noop_visit_foreign_mod(nm, self);
}
Expand Down Expand Up @@ -917,7 +913,13 @@ pub fn noop_visit_item_kind<T: MutVisitor>(kind: &mut ItemKind, vis: &mut T) {
vis.visit_generics(generics);
visit_opt(body, |body| vis.visit_block(body));
}
ItemKind::Mod(m) => vis.visit_mod(m),
ItemKind::Mod(_unsafety, mod_kind) => match mod_kind {
ModKind::Loaded(items, _inline, inner_span) => {
vis.visit_span(inner_span);
items.flat_map_in_place(|item| vis.flat_map_item(item));
}
ModKind::Unloaded => {}
},
ItemKind::ForeignMod(nm) => vis.visit_foreign_mod(nm),
ItemKind::GlobalAsm(_ga) => {}
ItemKind::TyAlias(box TyAliasKind(_, generics, bounds, ty)) => {
Expand Down Expand Up @@ -998,14 +1000,10 @@ pub fn noop_visit_fn_header<T: MutVisitor>(header: &mut FnHeader, vis: &mut T) {
vis.visit_asyncness(asyncness);
}

pub fn noop_visit_mod<T: MutVisitor>(module: &mut Mod, vis: &mut T) {
let Mod { inner, unsafety: _, items, inline: _ } = module;
vis.visit_span(inner);
items.flat_map_in_place(|item| vis.flat_map_item(item));
}

// FIXME: Avoid visiting the crate as a `Mod` item, flat map only the inner items if possible,
// or make crate visiting first class if necessary.
pub fn noop_visit_crate<T: MutVisitor>(krate: &mut Crate, vis: &mut T) {
visit_clobber(krate, |Crate { module, attrs, span, proc_macros }| {
visit_clobber(krate, |Crate { attrs, items, span, proc_macros }| {
let item_vis =
Visibility { kind: VisibilityKind::Public, span: span.shrink_to_lo(), tokens: None };
let item = P(Item {
Expand All @@ -1014,19 +1012,20 @@ pub fn noop_visit_crate<T: MutVisitor>(krate: &mut Crate, vis: &mut T) {
id: DUMMY_NODE_ID,
vis: item_vis,
span,
kind: ItemKind::Mod(module),
kind: ItemKind::Mod(Unsafe::No, ModKind::Loaded(items, Inline::Yes, span)),
tokens: None,
});
let items = vis.flat_map_item(item);

let len = items.len();
if len == 0 {
let module = Mod { inner: span, unsafety: Unsafe::No, items: vec![], inline: true };
Crate { module, attrs: vec![], span, proc_macros }
Crate { attrs: vec![], items: vec![], span, proc_macros }
} else if len == 1 {
let Item { attrs, span, kind, .. } = items.into_iter().next().unwrap().into_inner();
match kind {
ItemKind::Mod(module) => Crate { module, attrs, span, proc_macros },
ItemKind::Mod(_, ModKind::Loaded(items, ..)) => {
Crate { attrs, items, span, proc_macros }
}
_ => panic!("visitor converted a module to not a module"),
}
} else {
Expand Down
18 changes: 8 additions & 10 deletions compiler/rustc_ast/src/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<'a> FnKind<'a> {
/// Each method of the `Visitor` trait is a hook to be potentially
/// overridden. Each method's default implementation recursively visits
/// the substructure of the input via the corresponding `walk` method;
/// e.g., the `visit_mod` method by default calls `visit::walk_mod`.
/// e.g., the `visit_item` method by default calls `visit::walk_item`.
///
/// If you want to ensure that your code handles every variant
/// explicitly, you need to override each method. (And you also need
Expand All @@ -87,9 +87,6 @@ pub trait Visitor<'ast>: Sized {
fn visit_ident(&mut self, ident: Ident) {
walk_ident(self, ident);
}
fn visit_mod(&mut self, m: &'ast Mod, _s: Span, _attrs: &[Attribute], _n: NodeId) {
walk_mod(self, m);
}
fn visit_foreign_item(&mut self, i: &'ast ForeignItem) {
walk_foreign_item(self, i)
}
Expand Down Expand Up @@ -238,14 +235,10 @@ pub fn walk_ident<'a, V: Visitor<'a>>(visitor: &mut V, ident: Ident) {
}

pub fn walk_crate<'a, V: Visitor<'a>>(visitor: &mut V, krate: &'a Crate) {
visitor.visit_mod(&krate.module, krate.span, &krate.attrs, CRATE_NODE_ID);
walk_list!(visitor, visit_item, &krate.items);
walk_list!(visitor, visit_attribute, &krate.attrs);
}

pub fn walk_mod<'a, V: Visitor<'a>>(visitor: &mut V, module: &'a Mod) {
walk_list!(visitor, visit_item, &module.items);
}

pub fn walk_local<'a, V: Visitor<'a>>(visitor: &mut V, local: &'a Local) {
for attr in local.attrs.iter() {
visitor.visit_attribute(attr);
Expand Down Expand Up @@ -297,7 +290,12 @@ pub fn walk_item<'a, V: Visitor<'a>>(visitor: &mut V, item: &'a Item) {
let kind = FnKind::Fn(FnCtxt::Free, item.ident, sig, &item.vis, body.as_deref());
visitor.visit_fn(kind, item.span, item.id)
}
ItemKind::Mod(ref module) => visitor.visit_mod(module, item.span, &item.attrs, item.id),
ItemKind::Mod(_unsafety, ref mod_kind) => match mod_kind {
ModKind::Loaded(items, _inline, _inner_span) => {
walk_list!(visitor, visit_item, items)
}
ModKind::Unloaded => {}
},
ItemKind::ForeignMod(ref foreign_module) => {
walk_list!(visitor, visit_foreign_item, &foreign_module.items);
}
Expand Down
62 changes: 27 additions & 35 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ use rustc_span::source_map::{respan, DesugaringKind};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::Span;
use rustc_target::spec::abi;

use smallvec::{smallvec, SmallVec};
use std::collections::BTreeSet;
use tracing::debug;

use std::mem;

pub(super) struct ItemLowerer<'a, 'lowering, 'hir> {
pub(super) lctx: &'a mut LoweringContext<'lowering, 'hir>,
}
Expand All @@ -34,25 +34,6 @@ impl ItemLowerer<'_, '_, '_> {
}

impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
fn visit_mod(&mut self, m: &'a Mod, _s: Span, _attrs: &[Attribute], n: NodeId) {
let def_id = self.lctx.lower_node_id(n).expect_owner();

self.lctx.modules.insert(
Copy link
Member

Choose a reason for hiding this comment

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

Could this be moved to the ItemKind::Mod(..) => match arm below? That would allow you to avoid using lctx.modules.entry in some of the other code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think entry is preferable, why pre-inserting empty values explicitly if entry can do everything for you automatically.

Copy link
Member

Choose a reason for hiding this comment

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

I thought that the get_mut().unwrap() might have been a check to ensure that we had already processed the module in some way. If that's not the case, then I think your change is fine.

def_id,
hir::ModuleItems {
items: BTreeSet::new(),
trait_items: BTreeSet::new(),
impl_items: BTreeSet::new(),
foreign_items: BTreeSet::new(),
},
);

let old = self.lctx.current_module;
self.lctx.current_module = def_id;
visit::walk_mod(self, m);
self.lctx.current_module = old;
}

fn visit_item(&mut self, item: &'a Item) {
let mut item_hir_id = None;
self.lctx.with_hir_id_owner(item.id, |lctx| {
Expand All @@ -67,10 +48,18 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
if let Some(hir_id) = item_hir_id {
self.lctx.with_parent_item_lifetime_defs(hir_id, |this| {
let this = &mut ItemLowerer { lctx: this };
if let ItemKind::Impl(box ImplKind { ref of_trait, .. }) = item.kind {
this.with_trait_impl_ref(of_trait, |this| visit::walk_item(this, item));
} else {
visit::walk_item(this, item);
match item.kind {
ItemKind::Mod(..) => {
let def_id = this.lctx.lower_node_id(item.id).expect_owner();
let old_current_module =
mem::replace(&mut this.lctx.current_module, def_id);
visit::walk_item(this, item);
this.lctx.current_module = old_current_module;
}
ItemKind::Impl(box ImplKind { ref of_trait, .. }) => {
this.with_trait_impl_ref(of_trait, |this| visit::walk_item(this, item));
}
_ => visit::walk_item(this, item),
}
});
}
Expand All @@ -94,13 +83,13 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
let hir_item = lctx.lower_trait_item(item);
let id = hir_item.trait_item_id();
lctx.trait_items.insert(id, hir_item);
lctx.modules.get_mut(&lctx.current_module).unwrap().trait_items.insert(id);
lctx.modules.entry(lctx.current_module).or_default().trait_items.insert(id);
}
AssocCtxt::Impl => {
let hir_item = lctx.lower_impl_item(item);
let id = hir_item.impl_item_id();
lctx.impl_items.insert(id, hir_item);
lctx.modules.get_mut(&lctx.current_module).unwrap().impl_items.insert(id);
lctx.modules.entry(lctx.current_module).or_default().impl_items.insert(id);
}
});

Expand All @@ -113,7 +102,7 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
let hir_item = lctx.lower_foreign_item(item);
let id = hir_item.foreign_item_id();
lctx.foreign_items.insert(id, hir_item);
lctx.modules.get_mut(&lctx.current_module).unwrap().foreign_items.insert(id);
lctx.modules.entry(lctx.current_module).or_default().foreign_items.insert(id);
});

visit::walk_foreign_item(self, item);
Expand Down Expand Up @@ -157,7 +146,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
&mut self,
f: impl FnOnce(&mut LoweringContext<'_, '_>) -> T,
) -> T {
let old_in_scope_lifetimes = std::mem::replace(&mut self.in_scope_lifetimes, vec![]);
let old_in_scope_lifetimes = mem::replace(&mut self.in_scope_lifetimes, vec![]);

// this vector is only used when walking over impl headers,
// input types, and the like, and should not be non-empty in
Expand All @@ -172,12 +161,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
res
}

pub(super) fn lower_mod(&mut self, m: &Mod) -> hir::Mod<'hir> {
pub(super) fn lower_mod(&mut self, items: &[P<Item>], inner: Span) -> hir::Mod<'hir> {
hir::Mod {
inner: m.inner,
item_ids: self
.arena
.alloc_from_iter(m.items.iter().flat_map(|x| self.lower_item_id(x))),
inner,
item_ids: self.arena.alloc_from_iter(items.iter().flat_map(|x| self.lower_item_id(x))),
}
}

Expand Down Expand Up @@ -327,7 +314,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ItemKind::Fn(sig, generics, body_id)
})
}
ItemKind::Mod(ref m) => hir::ItemKind::Mod(self.lower_mod(m)),
ItemKind::Mod(_, ref mod_kind) => match mod_kind {
ModKind::Loaded(items, _, inner_span) => {
hir::ItemKind::Mod(self.lower_mod(items, *inner_span))
}
ModKind::Unloaded => panic!("`mod` items should have been loaded by now"),
},
ItemKind::ForeignMod(ref fm) => {
if fm.abi.is_none() {
self.maybe_lint_missing_abi(span, id, abi::Abi::C);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
visit::walk_crate(&mut MiscCollector { lctx: &mut self }, c);
visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c);

let module = self.lower_mod(&c.module);
let module = self.lower_mod(&c.items, c.span);
let attrs = self.lower_attrs(&c.attrs);
let body_ids = body_ids(&self.bodies);
let proc_macros =
Expand Down Expand Up @@ -608,7 +608,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
fn insert_item(&mut self, item: hir::Item<'hir>) -> hir::ItemId {
let id = hir::ItemId { def_id: item.def_id };
self.items.insert(id, item);
self.modules.get_mut(&self.current_module).unwrap().items.insert(id);
self.modules.entry(self.current_module).or_default().items.insert(id);
id
}

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_ast_passes/src/ast_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,12 +1054,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
walk_list!(self, visit_attribute, &item.attrs);
return;
}
ItemKind::Mod(Mod { inline, unsafety, .. }) => {
ItemKind::Mod(unsafety, ref mod_kind) => {
if let Unsafe::Yes(span) = unsafety {
self.err_handler().span_err(span, "module cannot be declared unsafe");
}
// Ensure that `path` attributes on modules are recorded as used (cf. issue #35584).
if !inline && !self.session.contains_name(&item.attrs, sym::path) {
if !matches!(mod_kind, ModKind::Loaded(_, Inline::Yes, _))
&& !self.session.contains_name(&item.attrs, sym::path)
{
self.check_mod_file_item_asciionly(item.ident);
}
}
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_ast_passes/src/node_count.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ impl<'ast> Visitor<'ast> for NodeCounter {
self.count += 1;
walk_ident(self, ident);
}
fn visit_mod(&mut self, m: &Mod, _s: Span, _a: &[Attribute], _n: NodeId) {
self.count += 1;
walk_mod(self, m)
}
fn visit_foreign_item(&mut self, i: &ForeignItem) {
self.count += 1;
walk_foreign_item(self, i)
Expand Down
Loading