expand: Move module file path stack from global session to expansion data

Also don't push the paths on the stack directly in `fn parse_external_mod`, return them instead.

Author: petrochenkov

compiler/rustc_builtin_macros/src/source_util.rs

@@ -101,7 +101,7 @@ pub fn expand_include<'cx>(
         None => return DummyResult::any(sp),
     };
     // The file will be added to the code map by the parser
-    let mut file = match cx.resolve_path(file, sp) {
+    let file = match cx.resolve_path(file, sp) {
         Ok(f) => f,
         Err(mut err) => {
             err.emit();
@@ -114,10 +114,9 @@ pub fn expand_include<'cx>(
     // then the path of `bar.rs` should be relative to the directory of `file`.
     // See https://github.com/rust-lang/rust/pull/69838/files#r395217057 for a discussion.
     // `MacroExpander::fully_expand_fragment` later restores, so "stack discipline" is maintained.
-    file.pop();
+    let dir_path = file.parent().unwrap_or(&file).to_owned();
+    cx.current_expansion.module = Rc::new(cx.current_expansion.module.with_dir_path(dir_path));
     cx.current_expansion.directory_ownership = DirectoryOwnership::Owned { relative: None };
-    let mod_path = cx.current_expansion.module.mod_path.clone();
-    cx.current_expansion.module = Rc::new(ModuleData { mod_path, directory: file });
 
     struct ExpandResult<'a> {
         p: Parser<'a>,

compiler/rustc_expand/src/base.rs

@@ -894,10 +894,26 @@ pub trait ResolverExpand {
     fn cfg_accessible(&mut self, expn_id: ExpnId, path: &ast::Path) -> Result<bool, Indeterminate>;
 }
 
-#[derive(Clone)]
+#[derive(Clone, Default)]
 pub struct ModuleData {
+    /// Path to the module starting from the crate name, like `my_crate::foo::bar`.
     pub mod_path: Vec<Ident>,
-    pub directory: PathBuf,
+    /// Stack of paths to files loaded by out-of-line module items,
+    /// used to detect and report recursive module inclusions.
+    pub file_path_stack: Vec<PathBuf>,
+    /// Directory to search child module files in,
+    /// often (but not necessarily) the parent of the top file path on the `file_path_stack`.
+    pub dir_path: PathBuf,
+}
+
+impl ModuleData {
+    pub fn with_dir_path(&self, dir_path: PathBuf) -> ModuleData {
+        ModuleData {
+            mod_path: self.mod_path.clone(),
+            file_path_stack: self.file_path_stack.clone(),
+            dir_path,
+        }
+    }
 }
 
 #[derive(Clone)]
@@ -946,7 +962,7 @@ impl<'a> ExtCtxt<'a> {
             current_expansion: ExpansionData {
                 id: ExpnId::root(),
                 depth: 0,
-                module: Rc::new(ModuleData { mod_path: Vec::new(), directory: PathBuf::new() }),
+                module: Default::default(),
                 directory_ownership: DirectoryOwnership::Owned { relative: None },
                 prior_type_ascription: None,
             },

compiler/rustc_expand/src/expand.rs

@@ -355,16 +355,17 @@ impl<'a, 'b> MacroExpander<'a, 'b> {
     // FIXME: Avoid visiting the crate as a `Mod` item,
     // make crate a first class expansion target instead.
     pub fn expand_crate(&mut self, mut krate: ast::Crate) -> ast::Crate {
-        let mut module = ModuleData {
-            mod_path: vec![Ident::from_str(&self.cx.ecfg.crate_name)],
-            directory: match self.cx.source_map().span_to_unmapped_path(krate.span) {
-                FileName::Real(name) => name.into_local_path(),
-                other => PathBuf::from(other.to_string()),
-            },
+        let file_path = match self.cx.source_map().span_to_unmapped_path(krate.span) {
+            FileName::Real(name) => name.into_local_path(),
+            other => PathBuf::from(other.to_string()),
         };
-        module.directory.pop();
-        self.cx.root_path = module.directory.clone();
-        self.cx.current_expansion.module = Rc::new(module);
+        let dir_path = file_path.parent().unwrap_or(&file_path).to_owned();
+        self.cx.root_path = dir_path.clone();
+        self.cx.current_expansion.module = Rc::new(ModuleData {
+            mod_path: vec![Ident::from_str(&self.cx.ecfg.crate_name)],
+            file_path_stack: vec![file_path],
+            dir_path,
+        });
 
         let krate_item = AstFragment::Items(smallvec![P(ast::Item {
             attrs: krate.attrs,
@@ -1276,25 +1277,30 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
                 })
             }
             ast::ItemKind::Mod(_, ref mut mod_kind) if ident != Ident::invalid() => {
-                let sess = &self.cx.sess.parse_sess;
-                let orig_ownership = self.cx.current_expansion.directory_ownership;
-                let mut module = (*self.cx.current_expansion.module).clone();
-
-                let pushed = &mut false; // Record `parse_external_mod` pushing so we can pop.
-                let dir = Directory { ownership: orig_ownership, path: module.directory };
-                let Directory { ownership, path } = match mod_kind {
+                let dir = Directory {
+                    ownership: self.cx.current_expansion.directory_ownership,
+                    path: self.cx.current_expansion.module.dir_path.clone(),
+                };
+                let (file_path, Directory { ownership, path }) = match mod_kind {
                     ModKind::Loaded(_, Inline::Yes, _) => {
                         // Inline `mod foo { ... }`, but we still need to push directories.
+                        let dir_path = push_directory(&self.cx.sess, ident, &attrs, dir);
                         item.attrs = attrs;
-                        push_directory(&self.cx.sess, ident, &item.attrs, dir)
+                        (None, dir_path)
                     }
                     ModKind::Loaded(_, Inline::No, _) => {
                         panic!("`mod` item is loaded from a file for the second time")
                     }
                     ModKind::Unloaded => {
                         // We have an outline `mod foo;` so we need to parse the file.
-                        let (items, inner_span, dir) =
-                            parse_external_mod(&self.cx.sess, ident, span, dir, &mut attrs, pushed);
+                        let (items, inner_span, file_path, dir_path) = parse_external_mod(
+                            &self.cx.sess,
+                            ident,
+                            span,
+                            &self.cx.current_expansion.module.file_path_stack,
+                            dir,
+                            &mut attrs,
+                        );
 
                         let krate =
                             ast::Crate { attrs, items, span: inner_span, proc_macros: vec![] };
@@ -1305,34 +1311,29 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
                         *mod_kind = ModKind::Loaded(krate.items, Inline::No, inner_span);
                         item.attrs = krate.attrs;
                         // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure.
-                        item = match self.configure(item) {
-                            Some(node) => node,
-                            None => {
-                                if *pushed {
-                                    sess.included_mod_stack.borrow_mut().pop();
-                                }
-                                return Default::default();
-                            }
-                        };
-                        dir
+                        item = configure!(self, item);
+                        (Some(file_path), dir_path)
                     }
                 };
 
                 // Set the module info before we flat map.
-                self.cx.current_expansion.directory_ownership = ownership;
-                module.directory = path;
+                let mut module = self.cx.current_expansion.module.with_dir_path(path);
                 module.mod_path.push(ident);
+                if let Some(file_path) = file_path {
+                    module.file_path_stack.push(file_path);
+                }
+
                 let orig_module =
                     mem::replace(&mut self.cx.current_expansion.module, Rc::new(module));
+                let orig_dir_ownership =
+                    mem::replace(&mut self.cx.current_expansion.directory_ownership, ownership);
 
                 let result = noop_flat_map_item(item, self);
 
                 // Restore the module info.
+                self.cx.current_expansion.directory_ownership = orig_dir_ownership;
                 self.cx.current_expansion.module = orig_module;
-                self.cx.current_expansion.directory_ownership = orig_ownership;
-                if *pushed {
-                    sess.included_mod_stack.borrow_mut().pop();
-                }
+
                 result
             }
             _ => {

compiler/rustc_expand/src/module.rs

@@ -42,31 +42,27 @@ crate fn parse_external_mod(
     sess: &Session,
     id: Ident,
     span: Span, // The span to blame on errors.
+    file_path_stack: &[PathBuf],
     Directory { mut ownership, path }: Directory,
     attrs: &mut Vec<Attribute>,
-    pop_mod_stack: &mut bool,
-) -> (Vec<P<Item>>, Span, Directory) {
+) -> (Vec<P<Item>>, Span, PathBuf, Directory) {
     // We bail on the first error, but that error does not cause a fatal error... (1)
     let result: PResult<'_, _> = try {
         // Extract the file path and the new ownership.
         let mp = submod_path(sess, id, span, &attrs, ownership, &path)?;
         ownership = mp.ownership;
 
         // Ensure file paths are acyclic.
-        let mut included_mod_stack = sess.parse_sess.included_mod_stack.borrow_mut();
-        error_on_circular_module(&sess.parse_sess, span, &mp.path, &included_mod_stack)?;
-        included_mod_stack.push(mp.path.clone());
-        *pop_mod_stack = true; // We have pushed, so notify caller.
-        drop(included_mod_stack);
+        error_on_circular_module(&sess.parse_sess, span, &mp.path, file_path_stack)?;
 
         // Actually parse the external file as a module.
         let mut parser = new_parser_from_file(&sess.parse_sess, &mp.path, Some(span));
         let (mut inner_attrs, items, inner_span) = parser.parse_mod(&token::Eof)?;
         attrs.append(&mut inner_attrs);
-        (items, inner_span)
+        (items, inner_span, mp.path)
     };
     // (1) ...instead, we return a dummy module.
-    let (items, inner_span) = result.map_err(|mut err| err.emit()).unwrap_or_default();
+    let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default();
 
     // Extract the directory path for submodules of  the module.
     let path = sess.source_map().span_to_unmapped_path(inner_span);
@@ -76,18 +72,18 @@ crate fn parse_external_mod(
     };
     path.pop();
 
-    (items, inner_span, Directory { ownership, path })
+    (items, inner_span, file_path, Directory { ownership, path })
 }
 
 fn error_on_circular_module<'a>(
     sess: &'a ParseSess,
     span: Span,
     path: &Path,
-    included_mod_stack: &[PathBuf],
+    file_path_stack: &[PathBuf],
 ) -> PResult<'a, ()> {
-    if let Some(i) = included_mod_stack.iter().position(|p| *p == path) {
+    if let Some(i) = file_path_stack.iter().position(|p| *p == path) {
         let mut err = String::from("circular modules: ");
-        for p in &included_mod_stack[i..] {
+        for p in &file_path_stack[i..] {
             err.push_str(&p.to_string_lossy());
             err.push_str(" -> ");
         }

compiler/rustc_session/src/parse.rs

@@ -13,7 +13,6 @@ use rustc_span::hygiene::ExpnId;
 use rustc_span::source_map::{FilePathMapping, SourceMap};
 use rustc_span::{MultiSpan, Span, Symbol};
 
-use std::path::PathBuf;
 use std::str;
 
 /// The set of keys (and, optionally, values) that define the compilation
@@ -122,8 +121,6 @@ pub struct ParseSess {
     pub missing_fragment_specifiers: Lock<FxHashMap<Span, NodeId>>,
     /// Places where raw identifiers were used. This is used for feature-gating raw identifiers.
     pub raw_identifier_spans: Lock<Vec<Span>>,
-    /// Used to determine and report recursive module inclusions.
-    pub included_mod_stack: Lock<Vec<PathBuf>>,
     source_map: Lrc<SourceMap>,
     pub buffered_lints: Lock<Vec<BufferedEarlyLint>>,
     /// Contains the spans of block expressions that could have been incomplete based on the
@@ -157,7 +154,6 @@ impl ParseSess {
             edition: ExpnId::root().expn_data().edition,
             missing_fragment_specifiers: Default::default(),
             raw_identifier_spans: Lock::new(Vec::new()),
-            included_mod_stack: Lock::new(vec![]),
             source_map,
             buffered_lints: Lock::new(vec![]),
             ambiguous_block_expr_parse: Lock::new(FxHashMap::default()),

src/test/ui/parser/circular_modules_main.rs

@@ -1,10 +1,12 @@
+// error-pattern: circular modules
+
 #[path = "circular_modules_hello.rs"]
-mod circular_modules_hello; //~ ERROR: circular modules
+mod circular_modules_hello;
 
 pub fn hi_str() -> String {
     "Hi!".to_string()
 }
 
 fn main() {
-    circular_modules_hello::say_hello(); //~ ERROR cannot find function `say_hello` in module
+    circular_modules_hello::say_hello();
 }

src/test/ui/parser/circular_modules_main.stderr

@@ -1,18 +1,18 @@
-error: circular modules: $DIR/circular_modules_hello.rs -> $DIR/circular_modules_main.rs -> $DIR/circular_modules_hello.rs
-  --> $DIR/circular_modules_main.rs:2:1
+error: circular modules: $DIR/circular_modules_main.rs -> $DIR/circular_modules_hello.rs -> $DIR/circular_modules_main.rs
+  --> $DIR/circular_modules_hello.rs:4:1
    |
-LL | mod circular_modules_hello;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL | mod circular_modules_main;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error[E0425]: cannot find function `say_hello` in module `circular_modules_hello`
-  --> $DIR/circular_modules_main.rs:9:29
+error[E0425]: cannot find function `hi_str` in module `circular_modules_main`
+  --> $DIR/circular_modules_hello.rs:7:43
    |
-LL |     circular_modules_hello::say_hello();
-   |                             ^^^^^^^^^ not found in `circular_modules_hello`
+LL |     println!("{}", circular_modules_main::hi_str());
+   |                                           ^^^^^^ not found in `circular_modules_main`
    |
 help: consider importing this function
    |
-LL | use circular_modules_hello::say_hello;
+LL | use hi_str;
    |
 
 error: aborting due to 2 previous errors