Auto merge of #55560 - cramertj:current-dir-path, r=<try>

Make #[path] always relative to the current directory

Previously, #[path] would be relative to the current
directory plus an offset for any inline modules.
However, it did not respect the module offset based
on a non-"mod.rs" filename unless there was *also*
a nested module.

After this commit, #[path] ignores inline modules
and is only ever relative to the current directory.
This means that the old #[path = ...] mod x { ... }
(#[path] applied to inline modules) no longer has
any effect.

This is a [breaking change]. Opening to see what a crater run says.

r? @petrochenkov

Author: bors

src/libsyntax/ext/base.rs

@@ -828,7 +828,7 @@ impl<'a> ExtCtxt<'a> {
                 mark: Mark::root(),
                 depth: 0,
                 module: Rc::new(ModuleData { mod_path: Vec::new(), directory: PathBuf::new() }),
-                directory_ownership: DirectoryOwnership::Owned { relative: None },
+                directory_ownership: DirectoryOwnership::Owned { relative: vec![] },
                 crate_span: None,
             },
             expansions: FxHashMap::default(),

src/libsyntax/ext/expand.rs

@@ -1311,8 +1311,9 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
     }
 
     fn fold_block(&mut self, block: P<Block>) -> P<Block> {
-        let old_directory_ownership = self.cx.current_expansion.directory_ownership;
-        self.cx.current_expansion.directory_ownership = DirectoryOwnership::UnownedViaBlock;
+        let old_directory_ownership =
+            mem::replace(&mut self.cx.current_expansion.directory_ownership,
+                         DirectoryOwnership::UnownedViaBlock);
         let result = noop_fold_block(block, self);
         self.cx.current_expansion.directory_ownership = old_directory_ownership;
         result
@@ -1346,7 +1347,13 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
                     return noop_fold_item(item, self);
                 }
 
-                let orig_directory_ownership = self.cx.current_expansion.directory_ownership;
+                // If the directory ownership is replaced, this var
+                // holds the original so that it can be swapped back in.
+                let mut orig_directory_ownership = None;
+                // If the directory ownership `relative` field was appended to,
+                // this bool is `true` so that it can be popped at the end.
+                let mut directory_ownership_needs_pop = false;
+
                 let mut module = (*self.cx.current_expansion.module).clone();
                 module.mod_path.push(item.ident);
 
@@ -1356,12 +1363,11 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
                 let inline_module = item.span.contains(inner) || inner.is_dummy();
 
                 if inline_module {
-                    if let Some(path) = attr::first_attr_value_str_by_name(&item.attrs, "path") {
-                        self.cx.current_expansion.directory_ownership =
-                            DirectoryOwnership::Owned { relative: None };
-                        module.directory.push(&*path.as_str());
-                    } else {
-                        module.directory.push(&*item.ident.as_str());
+                    if let DirectoryOwnership::Owned { relative } =
+                        &mut self.cx.current_expansion.directory_ownership
+                    {
+                        relative.push(item.ident);
+                        directory_ownership_needs_pop = true;
                     }
                 } else {
                     let path = self.cx.parse_sess.source_map().span_to_unmapped_path(inner);
@@ -1370,22 +1376,37 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
                         other => PathBuf::from(other.to_string()),
                     };
                     let directory_ownership = match path.file_name().unwrap().to_str() {
-                        Some("mod.rs") => DirectoryOwnership::Owned { relative: None },
+                        Some("mod.rs") => DirectoryOwnership::Owned { relative: vec![] },
                         Some(_) => DirectoryOwnership::Owned {
-                            relative: Some(item.ident),
+                            relative: vec![item.ident],
                         },
                         None => DirectoryOwnership::UnownedViaMod(false),
                     };
                     path.pop();
                     module.directory = path;
-                    self.cx.current_expansion.directory_ownership = directory_ownership;
+                    orig_directory_ownership =
+                        Some(mem::replace(
+                            &mut self.cx.current_expansion.directory_ownership,
+                            directory_ownership));
                 }
 
                 let orig_module =
                     mem::replace(&mut self.cx.current_expansion.module, Rc::new(module));
+
                 let result = noop_fold_item(item, self);
+
+                // Clean up, restoring all replaced or mutated expansion state.
                 self.cx.current_expansion.module = orig_module;
-                self.cx.current_expansion.directory_ownership = orig_directory_ownership;
+                if let Some(orig_directory_ownership) = orig_directory_ownership {
+                    self.cx.current_expansion.directory_ownership = orig_directory_ownership;
+                }
+                if directory_ownership_needs_pop {
+                    if let DirectoryOwnership::Owned { relative } =
+                        &mut self.cx.current_expansion.directory_ownership
+                    {
+                        relative.pop();
+                    }
+                }
                 result
             }
 

src/libsyntax/ext/source_util.rs

@@ -100,7 +100,7 @@ pub fn expand_include<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[tokenstream::T
     };
     // The file will be added to the code map by the parser
     let path = res_rel_file(cx, sp, file);
-    let directory_ownership = DirectoryOwnership::Owned { relative: None };
+    let directory_ownership = DirectoryOwnership::Owned { relative: vec![] };
     let p = parse::new_sub_parser_from_file(cx.parse_sess(), &path, directory_ownership, None, sp);
 
     struct ExpandResult<'a> {

src/libsyntax/ext/tt/macro_rules.rs

@@ -156,7 +156,7 @@ fn generic_extension<'cx>(cx: &'cx mut ExtCtxt,
 
                 let directory = Directory {
                     path: Cow::from(cx.current_expansion.module.directory.as_path()),
-                    ownership: cx.current_expansion.directory_ownership,
+                    ownership: cx.current_expansion.directory_ownership.clone(),
                 };
                 let mut p = Parser::new(cx.parse_sess(), tts, Some(directory), true, false);
                 p.root_module_name = cx.current_expansion.module.mod_path.last()

src/libsyntax/parse/lexer/mod.rs

@@ -1890,7 +1890,6 @@ mod tests {
             missing_fragment_specifiers: Lock::new(FxHashSet::default()),
             raw_identifier_spans: Lock::new(Vec::new()),
             registered_diagnostics: Lock::new(ErrorMap::new()),
-            non_modrs_mods: Lock::new(vec![]),
             buffered_lints: Lock::new(vec![]),
         }
     }

src/libsyntax/parse/mod.rs

@@ -52,9 +52,6 @@ pub struct ParseSess {
     pub raw_identifier_spans: Lock<Vec<Span>>,
     /// The registered diagnostics codes
     crate registered_diagnostics: Lock<ErrorMap>,
-    // Spans where a `mod foo;` statement was included in a non-mod.rs file.
-    // These are used to issue errors if the non_modrs_mods feature is not enabled.
-    pub non_modrs_mods: Lock<Vec<(ast::Ident, Span)>>,
     /// Used to determine and report recursive mod inclusions
     included_mod_stack: Lock<Vec<PathBuf>>,
     source_map: Lrc<SourceMap>,
@@ -81,7 +78,6 @@ impl ParseSess {
             registered_diagnostics: Lock::new(ErrorMap::new()),
             included_mod_stack: Lock::new(vec![]),
             source_map,
-            non_modrs_mods: Lock::new(vec![]),
             buffered_lints: Lock::new(vec![]),
         }
     }
@@ -113,11 +109,14 @@ pub struct Directory<'a> {
     pub ownership: DirectoryOwnership,
 }
 
-#[derive(Copy, Clone)]
+#[derive(Clone)]
 pub enum DirectoryOwnership {
     Owned {
-        // None if `mod.rs`, `Some("foo")` if we're in `foo.rs`
-        relative: Option<ast::Ident>,
+        // Module offset from the current directory.
+        // Starts out as empty in `mod.rs`, starts as `["foo"]` in `foo.rs`.
+        // Contains one additional element for every inline nested module we've entered.
+        // e.g. `mod y { mod z { ... } }` in `x.rs` would have `["x", "y", "z"]`
+        relative: Vec<ast::Ident>,
     },
     UnownedViaBlock,
     UnownedViaMod(bool /* legacy warnings? */),

src/libsyntax/parse/parser.rs

@@ -556,7 +556,7 @@ impl<'a> Parser<'a> {
             recurse_into_file_modules,
             directory: Directory {
                 path: Cow::from(PathBuf::new()),
-                ownership: DirectoryOwnership::Owned { relative: None }
+                ownership: DirectoryOwnership::Owned { relative: vec![] }
             },
             root_module_name: None,
             expected_tokens: Vec::new(),
@@ -6456,8 +6456,12 @@ impl<'a> Parser<'a> {
             }
         } else {
             let old_directory = self.directory.clone();
-            self.push_directory(id, &outer_attrs);
-
+            // Push inline `mod x { ... }`'s `x` onto the `relative` offset of the module
+            // from the current directory's location. This ensures that `mod x { mod y; }`
+            // corresponds to `x/y.rs`, not `y.rs`
+            if let DirectoryOwnership::Owned { relative } = &mut self.directory.ownership {
+                relative.push(id);
+            }
             self.expect(&token::OpenDelim(token::Brace))?;
             let mod_inner_lo = self.span;
             let attrs = self.parse_inner_attributes()?;
@@ -6468,26 +6472,6 @@ impl<'a> Parser<'a> {
         }
     }
 
-    fn push_directory(&mut self, id: Ident, attrs: &[Attribute]) {
-        if let Some(path) = attr::first_attr_value_str_by_name(attrs, "path") {
-            self.directory.path.to_mut().push(&path.as_str());
-            self.directory.ownership = DirectoryOwnership::Owned { relative: None };
-        } else {
-            // We have to push on the current module name in the case of relative
-            // paths in order to ensure that any additional module paths from inline
-            // `mod x { ... }` come after the relative extension.
-            //
-            // For example, a `mod z { ... }` inside `x/y.rs` should set the current
-            // directory path to `/x/y/z`, not `/x/z` with a relative offset of `y`.
-            if let DirectoryOwnership::Owned { relative } = &mut self.directory.ownership {
-                if let Some(ident) = relative.take() { // remove the relative offset
-                    self.directory.path.to_mut().push(ident.as_str());
-                }
-            }
-            self.directory.path.to_mut().push(&id.as_str());
-        }
-    }
-
     pub fn submod_path_from_attr(attrs: &[Attribute], dir_path: &Path) -> Option<PathBuf> {
         if let Some(s) = attr::first_attr_value_str_by_name(attrs, "path") {
             let s = s.as_str();
@@ -6507,21 +6491,20 @@ impl<'a> Parser<'a> {
     /// Returns either a path to a module, or .
     pub fn default_submod_path(
         id: ast::Ident,
-        relative: Option<ast::Ident>,
+        relative: &[ast::Ident],
         dir_path: &Path,
         source_map: &SourceMap) -> ModulePath
     {
-        // If we're in a foo.rs file instead of a mod.rs file,
-        // we need to look for submodules in
-        // `./foo/<id>.rs` and `./foo/<id>/mod.rs` rather than
-        // `./<id>.rs` and `./<id>/mod.rs`.
-        let relative_prefix_string;
-        let relative_prefix = if let Some(ident) = relative {
-            relative_prefix_string = format!("{}{}", ident.as_str(), path::MAIN_SEPARATOR);
-            &relative_prefix_string
-        } else {
-            ""
-        };
+        // Offset the current directory first by the name of
+        // the file if not `mod.rs`, then by any nested modules.
+        // e.g. `mod y { mod z; }` in `x.rs` should look for
+        // `./x/y/z.rs` and `./x/y/z/mod.rs` rather than
+        // `./z.rs` and `./z/mod.rs`.
+        let mut relative_prefix = String::new();
+        for ident in relative {
+            relative_prefix.push_str(&ident.as_str());
+            relative_prefix.push(path::MAIN_SEPARATOR);
+        }
 
         let mod_name = id.to_string();
         let default_path_str = format!("{}{}.rs", relative_prefix, mod_name);
@@ -6536,14 +6519,14 @@ impl<'a> Parser<'a> {
             (true, false) => Ok(ModulePathSuccess {
                 path: default_path,
                 directory_ownership: DirectoryOwnership::Owned {
-                    relative: Some(id),
+                    relative: vec![id],
                 },
                 warn: false,
             }),
             (false, true) => Ok(ModulePathSuccess {
                 path: secondary_path,
                 directory_ownership: DirectoryOwnership::Owned {
-                    relative: None,
+                    relative: vec![],
                 },
                 warn: false,
             }),
@@ -6582,27 +6565,18 @@ impl<'a> Parser<'a> {
                     // Note that this will produce weirdness when a file named `foo.rs` is
                     // `#[path]` included and contains a `mod foo;` declaration.
                     // If you encounter this, it's your own darn fault :P
-                    Some(_) => DirectoryOwnership::Owned { relative: None },
+                    Some(_) => DirectoryOwnership::Owned { relative: vec![] },
                     _ => DirectoryOwnership::UnownedViaMod(true),
                 },
                 path,
                 warn: false,
             });
         }
 
-        let relative = match self.directory.ownership {
-            DirectoryOwnership::Owned { relative } => {
-                // Push the usage onto the list of non-mod.rs mod uses.
-                // This is used later for feature-gate error reporting.
-                if let Some(cur_file_ident) = relative {
-                    self.sess
-                        .non_modrs_mods.borrow_mut()
-                        .push((cur_file_ident, id_sp));
-                }
-                relative
-            },
+        let relative = match &self.directory.ownership {
+            DirectoryOwnership::Owned { relative } => &**relative,
             DirectoryOwnership::UnownedViaBlock |
-            DirectoryOwnership::UnownedViaMod(_) => None,
+            DirectoryOwnership::UnownedViaMod(_) => &[],
         };
         let paths = Parser::default_submod_path(
                         id, relative, &self.directory.path, self.sess.source_map());

src/libsyntax/tokenstream.rs

@@ -107,7 +107,7 @@ impl TokenTree {
         // `None` is because we're not interpolating
         let directory = Directory {
             path: Cow::from(cx.current_expansion.module.directory.as_path()),
-            ownership: cx.current_expansion.directory_ownership,
+            ownership: cx.current_expansion.directory_ownership.clone(),
         };
         macro_parser::parse(cx.parse_sess(), tts, mtch, Some(directory), true)
     }

src/test/run-pass/modules/mod_dir_path.rs

@@ -13,21 +13,23 @@
 // ignore-pretty issue #37195
 
 mod mod_dir_simple {
-    #[path = "test.rs"]
+    #[path = "mod_dir_simple/test.rs"]
     pub mod syrup;
 }
 
 pub fn main() {
     assert_eq!(mod_dir_simple::syrup::foo(), 10);
 
-    #[path = "auxiliary"]
     mod foo {
+        #[path = "auxiliary/two_macros_2.rs"]
         mod two_macros_2;
     }
 
-    #[path = "auxiliary"]
     mod bar {
-        macro_rules! m { () => { mod two_macros_2; } }
+        macro_rules! m { () => {
+            #[path = "auxiliary/two_macros_2.rs"]
+            mod two_macros_2;
+        } }
         m!();
     }
 }

src/test/run-pass/modules/mod_dir_path2.rs

@@ -11,9 +11,8 @@
 // run-pass
 // ignore-pretty issue #37195
 
-#[path = "mod_dir_simple"]
 mod pancakes {
-    #[path = "test.rs"]
+    #[path = "mod_dir_simple/test.rs"]
     pub mod syrup;
 }