From bc18eb471772403de20cd9bc0a836ce1f5e09e98 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 16:12:11 +0300 Subject: [PATCH 1/9] expand: Remove obsolete `DirectoryOwnership::UnownedViaMod` This ownership kind is only constructed in the case of path attributes like `#[path = ".."]` without a file name segment, which always represent some kind of directories and will produce and error on attempt to parse them as a module file. --- compiler/rustc_expand/src/module.rs | 61 +++----------------- src/test/ui/modules/path-no-file-name.rs | 7 +++ src/test/ui/modules/path-no-file-name.stderr | 8 +++ 3 files changed, 24 insertions(+), 52 deletions(-) create mode 100644 src/test/ui/modules/path-no-file-name.rs create mode 100644 src/test/ui/modules/path-no-file-name.stderr diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 076d3b02be93f..99c1591d40a81 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -22,7 +22,6 @@ pub enum DirectoryOwnership { relative: Option, }, UnownedViaBlock, - UnownedViaMod, } /// Information about the path to a module. @@ -134,23 +133,20 @@ fn submod_path<'a>( dir_path: &Path, ) -> PResult<'a, ModulePathSuccess> { if let Some(path) = submod_path_from_attr(sess, attrs, dir_path) { - let ownership = match path.file_name().and_then(|s| s.to_str()) { - // All `#[path]` files are treated as though they are a `mod.rs` file. - // This means that `mod foo;` declarations inside `#[path]`-included - // files are siblings, - // - // 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 }, - _ => DirectoryOwnership::UnownedViaMod, - }; + // All `#[path]` files are treated as though they are a `mod.rs` file. + // This means that `mod foo;` declarations inside `#[path]`-included + // files are siblings, + // + // 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 + let ownership = DirectoryOwnership::Owned { relative: None }; return Ok(ModulePathSuccess { ownership, path }); } let relative = match ownership { DirectoryOwnership::Owned { relative } => relative, - DirectoryOwnership::UnownedViaBlock | DirectoryOwnership::UnownedViaMod => None, + DirectoryOwnership::UnownedViaBlock => None, }; let ModulePath { path_exists, name, result } = default_submod_path(&sess.parse_sess, id, span, relative, dir_path); @@ -160,10 +156,6 @@ fn submod_path<'a>( let _ = result.map_err(|mut err| err.cancel()); error_decl_mod_in_block(&sess.parse_sess, span, path_exists, &name) } - DirectoryOwnership::UnownedViaMod => { - let _ = result.map_err(|mut err| err.cancel()); - error_cannot_declare_mod_here(&sess.parse_sess, span, path_exists, &name) - } } } @@ -182,41 +174,6 @@ fn error_decl_mod_in_block<'a, T>( Err(err) } -fn error_cannot_declare_mod_here<'a, T>( - sess: &'a ParseSess, - span: Span, - path_exists: bool, - name: &str, -) -> PResult<'a, T> { - let mut err = - sess.span_diagnostic.struct_span_err(span, "cannot declare a new module at this location"); - if !span.is_dummy() { - if let FileName::Real(src_name) = sess.source_map().span_to_filename(span) { - let src_path = src_name.into_local_path(); - if let Some(stem) = src_path.file_stem() { - let mut dest_path = src_path.clone(); - dest_path.set_file_name(stem); - dest_path.push("mod.rs"); - err.span_note( - span, - &format!( - "maybe move this module `{}` to its own directory via `{}`", - src_path.display(), - dest_path.display() - ), - ); - } - } - } - if path_exists { - err.span_note( - span, - &format!("... or maybe `use` the module `{}` instead of possibly redeclaring it", name), - ); - } - Err(err) -} - /// Derive a submodule path from the first found `#[path = "path_string"]`. /// The provided `dir_path` is joined with the `path_string`. pub(super) fn submod_path_from_attr( diff --git a/src/test/ui/modules/path-no-file-name.rs b/src/test/ui/modules/path-no-file-name.rs new file mode 100644 index 0000000000000..f62cd2a9eb4e4 --- /dev/null +++ b/src/test/ui/modules/path-no-file-name.rs @@ -0,0 +1,7 @@ +// normalize-stderr-test: "\.:.*\(" -> ".: $$ACCESS_DENIED_MSG (" +// normalize-stderr-test: "os error \d+" -> "os error $$ACCESS_DENIED_CODE" + +#[path = "."] +mod m; //~ ERROR couldn't read + +fn main() {} diff --git a/src/test/ui/modules/path-no-file-name.stderr b/src/test/ui/modules/path-no-file-name.stderr new file mode 100644 index 0000000000000..32a213c68f654 --- /dev/null +++ b/src/test/ui/modules/path-no-file-name.stderr @@ -0,0 +1,8 @@ +error: couldn't read $DIR/.: $ACCESS_DENIED_MSG (os error $ACCESS_DENIED_CODE) + --> $DIR/path-no-file-name.rs:5:1 + | +LL | mod m; + | ^^^^^^ + +error: aborting due to previous error + From 39052c55bb0259930dd9e6b0fc27a715793b07c8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 21 Feb 2021 19:15:43 +0300 Subject: [PATCH 2/9] 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. --- .../rustc_builtin_macros/src/source_util.rs | 7 +- compiler/rustc_expand/src/base.rs | 22 +++++- compiler/rustc_expand/src/expand.rs | 71 ++++++++++--------- compiler/rustc_expand/src/module.rs | 22 +++--- compiler/rustc_session/src/parse.rs | 4 -- src/test/ui/parser/circular_modules_main.rs | 6 +- .../ui/parser/circular_modules_main.stderr | 18 ++--- 7 files changed, 80 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index 28efd483c8670..98659f15a7e2f 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/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>, diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index ce8103c0f850d..a0632522ca0e5 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/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; } -#[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, - 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, + /// 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, }, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index b474cad1242e8..8d633afacf478 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/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 } _ => { diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 99c1591d40a81..55b48e4f6b254 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -42,10 +42,10 @@ 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, - pop_mod_stack: &mut bool, -) -> (Vec>, Span, Directory) { +) -> (Vec>, 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. @@ -53,20 +53,16 @@ crate fn parse_external_mod( 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(" -> "); } diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index 81b38347414e8..592773bfe1b44 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/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>, /// Places where raw identifiers were used. This is used for feature-gating raw identifiers. pub raw_identifier_spans: Lock>, - /// Used to determine and report recursive module inclusions. - pub included_mod_stack: Lock>, source_map: Lrc, pub buffered_lints: Lock>, /// 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()), diff --git a/src/test/ui/parser/circular_modules_main.rs b/src/test/ui/parser/circular_modules_main.rs index 1ae36a1f7605e..d4b47efe68158 100644 --- a/src/test/ui/parser/circular_modules_main.rs +++ b/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(); } diff --git a/src/test/ui/parser/circular_modules_main.stderr b/src/test/ui/parser/circular_modules_main.stderr index 5d4db8c31a2c3..ee45f65a3bd5a 100644 --- a/src/test/ui/parser/circular_modules_main.stderr +++ b/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 From 5bdf81d5fa7ada2274617ebfa97e6bd157ae5a52 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 18:42:57 +0300 Subject: [PATCH 3/9] expand: Determine module directory path directly instead of relying on span --- compiler/rustc_expand/src/module.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 55b48e4f6b254..e744ab27e8f56 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -4,8 +4,8 @@ use rustc_errors::{struct_span_err, PResult}; use rustc_parse::new_parser_from_file; use rustc_session::parse::ParseSess; use rustc_session::Session; -use rustc_span::source_map::{FileName, Span}; use rustc_span::symbol::{sym, Ident}; +use rustc_span::Span; use std::path::{self, Path, PathBuf}; @@ -64,13 +64,8 @@ crate fn parse_external_mod( // (1) ...instead, we return a dummy module. 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); - let mut path = match path { - FileName::Real(name) => name.into_local_path(), - other => PathBuf::from(other.to_string()), - }; - path.pop(); + // Extract the directory path for submodules of the module. + let path = file_path.parent().unwrap_or(&file_path).to_owned(); (items, inner_span, file_path, Directory { ownership, path }) } From 3d0b622ab7ae3f803d757f73f2a9a7c857d771bb Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 18:58:45 +0300 Subject: [PATCH 4/9] expand: Less path cloning during module loading --- compiler/rustc_expand/src/expand.rs | 36 ++++++++++++-------- compiler/rustc_expand/src/module.rs | 52 ++++++++++++++++------------- 2 files changed, 51 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 8d633afacf478..a81bc381c508a 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -3,7 +3,7 @@ use crate::config::StripUnconfigured; use crate::configure; use crate::hygiene::SyntaxContext; use crate::mbe::macro_rules::annotate_err_with_kind; -use crate::module::{parse_external_mod, push_directory, Directory, DirectoryOwnership}; +use crate::module::{parse_external_mod, push_directory, DirectoryOwnership, ParsedExternalMod}; use crate::placeholders::{placeholder, PlaceholderExpander}; use rustc_ast as ast; @@ -1277,28 +1277,36 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { }) } ast::ItemKind::Mod(_, ref mut mod_kind) if ident != Ident::invalid() => { - 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 { + let (file_path, dir_path, dir_ownership) = 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); + let (dir_path, dir_ownership) = push_directory( + &self.cx.sess, + ident, + &attrs, + &self.cx.current_expansion.module, + self.cx.current_expansion.directory_ownership, + ); item.attrs = attrs; - (None, dir_path) + (None, dir_path, dir_ownership) } 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, file_path, dir_path) = parse_external_mod( + let ParsedExternalMod { + items, + inner_span, + file_path, + dir_path, + dir_ownership, + } = parse_external_mod( &self.cx.sess, ident, span, - &self.cx.current_expansion.module.file_path_stack, - dir, + &self.cx.current_expansion.module, + self.cx.current_expansion.directory_ownership, &mut attrs, ); @@ -1312,12 +1320,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { item.attrs = krate.attrs; // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure. item = configure!(self, item); - (Some(file_path), dir_path) + (Some(file_path), dir_path, dir_ownership) } }; // Set the module info before we flat map. - let mut module = self.cx.current_expansion.module.with_dir_path(path); + let mut module = self.cx.current_expansion.module.with_dir_path(dir_path); module.mod_path.push(ident); if let Some(file_path) = file_path { module.file_path_stack.push(file_path); @@ -1326,7 +1334,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { 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); + mem::replace(&mut self.cx.current_expansion.directory_ownership, dir_ownership); let result = noop_flat_map_item(item, self); diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index e744ab27e8f56..4ba24ace8eb8c 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -1,3 +1,4 @@ +use crate::base::ModuleData; use rustc_ast::ptr::P; use rustc_ast::{token, Attribute, Item}; use rustc_errors::{struct_span_err, PResult}; @@ -9,12 +10,6 @@ use rustc_span::Span; use std::path::{self, Path, PathBuf}; -#[derive(Clone)] -pub struct Directory { - pub path: PathBuf, - pub ownership: DirectoryOwnership, -} - #[derive(Copy, Clone)] pub enum DirectoryOwnership { Owned { @@ -38,22 +33,30 @@ pub struct ModulePathSuccess { pub ownership: DirectoryOwnership, } +crate struct ParsedExternalMod { + pub items: Vec>, + pub inner_span: Span, + pub file_path: PathBuf, + pub dir_path: PathBuf, + pub dir_ownership: DirectoryOwnership, +} + 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, + module: &ModuleData, + mut dir_ownership: DirectoryOwnership, attrs: &mut Vec, -) -> (Vec>, Span, PathBuf, Directory) { +) -> ParsedExternalMod { // 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; + let mp = submod_path(sess, id, span, &attrs, dir_ownership, &module.dir_path)?; + dir_ownership = mp.ownership; // Ensure file paths are acyclic. - error_on_circular_module(&sess.parse_sess, span, &mp.path, file_path_stack)?; + error_on_circular_module(&sess.parse_sess, span, &mp.path, &module.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)); @@ -65,9 +68,9 @@ crate fn parse_external_mod( 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 = file_path.parent().unwrap_or(&file_path).to_owned(); + let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); - (items, inner_span, file_path, Directory { ownership, path }) + ParsedExternalMod { items, inner_span, file_path, dir_path, dir_ownership } } fn error_on_circular_module<'a>( @@ -92,11 +95,13 @@ crate fn push_directory( sess: &Session, id: Ident, attrs: &[Attribute], - Directory { mut ownership, mut path }: Directory, -) -> Directory { - if let Some(filename) = sess.first_attr_value_str_by_name(attrs, sym::path) { - path.push(&*filename.as_str()); - ownership = DirectoryOwnership::Owned { relative: None }; + module: &ModuleData, + mut dir_ownership: DirectoryOwnership, +) -> (PathBuf, DirectoryOwnership) { + let mut dir_path = module.dir_path.clone(); + if let Some(file_path) = sess.first_attr_value_str_by_name(attrs, sym::path) { + dir_path.push(&*file_path.as_str()); + dir_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 @@ -104,15 +109,16 @@ crate fn push_directory( // // 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 ownership { + if let DirectoryOwnership::Owned { relative } = &mut dir_ownership { if let Some(ident) = relative.take() { // Remove the relative offset. - path.push(&*ident.as_str()); + dir_path.push(&*ident.as_str()); } } - path.push(&*id.as_str()); + dir_path.push(&*id.as_str()); } - Directory { ownership, path } + + (dir_path, dir_ownership) } fn submod_path<'a>( From 46b67aa74d8ee7d9c41983e15f8cd0f17ee27ae7 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 19:06:36 +0300 Subject: [PATCH 5/9] expand: Some more consistent naming in module loading --- .../rustc_builtin_macros/src/source_util.rs | 4 +- compiler/rustc_expand/src/base.rs | 6 +- compiler/rustc_expand/src/expand.rs | 20 +++-- compiler/rustc_expand/src/module.rs | 86 +++++++++---------- 4 files changed, 59 insertions(+), 57 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/source_util.rs b/compiler/rustc_builtin_macros/src/source_util.rs index 98659f15a7e2f..4aafcb2fb6dfe 100644 --- a/compiler/rustc_builtin_macros/src/source_util.rs +++ b/compiler/rustc_builtin_macros/src/source_util.rs @@ -4,7 +4,7 @@ use rustc_ast::token; use rustc_ast::tokenstream::TokenStream; use rustc_ast_pretty::pprust; use rustc_expand::base::{self, *}; -use rustc_expand::module::DirectoryOwnership; +use rustc_expand::module::DirOwnership; use rustc_parse::parser::{ForceCollect, Parser}; use rustc_parse::{self, new_parser_from_file}; use rustc_session::lint::builtin::INCOMPLETE_INCLUDE; @@ -116,7 +116,7 @@ pub fn expand_include<'cx>( // `MacroExpander::fully_expand_fragment` later restores, so "stack discipline" is maintained. 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 }; + cx.current_expansion.dir_ownership = DirOwnership::Owned { relative: None }; struct ExpandResult<'a> { p: Parser<'a>, diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index a0632522ca0e5..f88110cf106a7 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -1,5 +1,5 @@ use crate::expand::{self, AstFragment, Invocation}; -use crate::module::DirectoryOwnership; +use crate::module::DirOwnership; use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal}; @@ -921,7 +921,7 @@ pub struct ExpansionData { pub id: ExpnId, pub depth: usize, pub module: Rc, - pub directory_ownership: DirectoryOwnership, + pub dir_ownership: DirOwnership, pub prior_type_ascription: Option<(Span, bool)>, } @@ -963,7 +963,7 @@ impl<'a> ExtCtxt<'a> { id: ExpnId::root(), depth: 0, module: Default::default(), - directory_ownership: DirectoryOwnership::Owned { relative: None }, + dir_ownership: DirOwnership::Owned { relative: None }, prior_type_ascription: None, }, force_mode: false, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index a81bc381c508a..eee2c6ff80869 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -3,7 +3,7 @@ use crate::config::StripUnconfigured; use crate::configure; use crate::hygiene::SyntaxContext; use crate::mbe::macro_rules::annotate_err_with_kind; -use crate::module::{parse_external_mod, push_directory, DirectoryOwnership, ParsedExternalMod}; +use crate::module::{mod_dir_path, parse_external_mod, DirOwnership, ParsedExternalMod}; use crate::placeholders::{placeholder, PlaceholderExpander}; use rustc_ast as ast; @@ -1246,10 +1246,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { } fn visit_block(&mut self, block: &mut P) { - let old_directory_ownership = self.cx.current_expansion.directory_ownership; - self.cx.current_expansion.directory_ownership = DirectoryOwnership::UnownedViaBlock; + let orig_dir_ownership = mem::replace( + &mut self.cx.current_expansion.dir_ownership, + DirOwnership::UnownedViaBlock, + ); noop_visit_block(block, self); - self.cx.current_expansion.directory_ownership = old_directory_ownership; + self.cx.current_expansion.dir_ownership = orig_dir_ownership; } fn flat_map_item(&mut self, item: P) -> SmallVec<[P; 1]> { @@ -1280,12 +1282,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { let (file_path, dir_path, dir_ownership) = match mod_kind { ModKind::Loaded(_, Inline::Yes, _) => { // Inline `mod foo { ... }`, but we still need to push directories. - let (dir_path, dir_ownership) = push_directory( + let (dir_path, dir_ownership) = mod_dir_path( &self.cx.sess, ident, &attrs, &self.cx.current_expansion.module, - self.cx.current_expansion.directory_ownership, + self.cx.current_expansion.dir_ownership, ); item.attrs = attrs; (None, dir_path, dir_ownership) @@ -1306,7 +1308,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { ident, span, &self.cx.current_expansion.module, - self.cx.current_expansion.directory_ownership, + self.cx.current_expansion.dir_ownership, &mut attrs, ); @@ -1334,12 +1336,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { 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, dir_ownership); + mem::replace(&mut self.cx.current_expansion.dir_ownership, dir_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.dir_ownership = orig_dir_ownership; self.cx.current_expansion.module = orig_module; result diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 4ba24ace8eb8c..607c68f82dfa9 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -11,7 +11,7 @@ use rustc_span::Span; use std::path::{self, Path, PathBuf}; #[derive(Copy, Clone)] -pub enum DirectoryOwnership { +pub enum DirOwnership { Owned { // None if `mod.rs`, `Some("foo")` if we're in `foo.rs`. relative: Option, @@ -29,8 +29,8 @@ pub struct ModulePath<'a> { // Public for rustfmt usage. pub struct ModulePathSuccess { - pub path: PathBuf, - pub ownership: DirectoryOwnership, + pub file_path: PathBuf, + pub dir_ownership: DirOwnership, } crate struct ParsedExternalMod { @@ -38,31 +38,31 @@ crate struct ParsedExternalMod { pub inner_span: Span, pub file_path: PathBuf, pub dir_path: PathBuf, - pub dir_ownership: DirectoryOwnership, + pub dir_ownership: DirOwnership, } crate fn parse_external_mod( sess: &Session, - id: Ident, + ident: Ident, span: Span, // The span to blame on errors. module: &ModuleData, - mut dir_ownership: DirectoryOwnership, + mut dir_ownership: DirOwnership, attrs: &mut Vec, ) -> ParsedExternalMod { // 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, dir_ownership, &module.dir_path)?; - dir_ownership = mp.ownership; + let mp = mod_file_path(sess, ident, span, &attrs, &module.dir_path, dir_ownership)?; + dir_ownership = mp.dir_ownership; // Ensure file paths are acyclic. - error_on_circular_module(&sess.parse_sess, span, &mp.path, &module.file_path_stack)?; + error_on_circular_module(&sess.parse_sess, span, &mp.file_path, &module.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 parser = new_parser_from_file(&sess.parse_sess, &mp.file_path, Some(span)); let (mut inner_attrs, items, inner_span) = parser.parse_mod(&token::Eof)?; attrs.append(&mut inner_attrs); - (items, inner_span, mp.path) + (items, inner_span, mp.file_path) }; // (1) ...instead, we return a dummy module. let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default(); @@ -76,32 +76,32 @@ crate fn parse_external_mod( fn error_on_circular_module<'a>( sess: &'a ParseSess, span: Span, - path: &Path, + file_path: &Path, file_path_stack: &[PathBuf], ) -> PResult<'a, ()> { - if let Some(i) = file_path_stack.iter().position(|p| *p == path) { + if let Some(i) = file_path_stack.iter().position(|p| *p == file_path) { let mut err = String::from("circular modules: "); for p in &file_path_stack[i..] { err.push_str(&p.to_string_lossy()); err.push_str(" -> "); } - err.push_str(&path.to_string_lossy()); + err.push_str(&file_path.to_string_lossy()); return Err(sess.span_diagnostic.struct_span_err(span, &err[..])); } Ok(()) } -crate fn push_directory( +crate fn mod_dir_path( sess: &Session, - id: Ident, + ident: Ident, attrs: &[Attribute], module: &ModuleData, - mut dir_ownership: DirectoryOwnership, -) -> (PathBuf, DirectoryOwnership) { + mut dir_ownership: DirOwnership, +) -> (PathBuf, DirOwnership) { let mut dir_path = module.dir_path.clone(); if let Some(file_path) = sess.first_attr_value_str_by_name(attrs, sym::path) { dir_path.push(&*file_path.as_str()); - dir_ownership = DirectoryOwnership::Owned { relative: None }; + dir_ownership = DirOwnership::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 @@ -109,27 +109,27 @@ crate fn push_directory( // // 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 dir_ownership { + if let DirOwnership::Owned { relative } = &mut dir_ownership { if let Some(ident) = relative.take() { // Remove the relative offset. dir_path.push(&*ident.as_str()); } } - dir_path.push(&*id.as_str()); + dir_path.push(&*ident.as_str()); } (dir_path, dir_ownership) } -fn submod_path<'a>( +fn mod_file_path<'a>( sess: &'a Session, - id: Ident, + ident: Ident, span: Span, attrs: &[Attribute], - ownership: DirectoryOwnership, dir_path: &Path, + dir_ownership: DirOwnership, ) -> PResult<'a, ModulePathSuccess> { - if let Some(path) = submod_path_from_attr(sess, attrs, dir_path) { + if let Some(file_path) = mod_file_path_from_attr(sess, attrs, dir_path) { // All `#[path]` files are treated as though they are a `mod.rs` file. // This means that `mod foo;` declarations inside `#[path]`-included // files are siblings, @@ -137,19 +137,19 @@ fn submod_path<'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 - let ownership = DirectoryOwnership::Owned { relative: None }; - return Ok(ModulePathSuccess { ownership, path }); + let dir_ownership = DirOwnership::Owned { relative: None }; + return Ok(ModulePathSuccess { file_path, dir_ownership }); } - let relative = match ownership { - DirectoryOwnership::Owned { relative } => relative, - DirectoryOwnership::UnownedViaBlock => None, + let relative = match dir_ownership { + DirOwnership::Owned { relative } => relative, + DirOwnership::UnownedViaBlock => None, }; let ModulePath { path_exists, name, result } = - default_submod_path(&sess.parse_sess, id, span, relative, dir_path); - match ownership { - DirectoryOwnership::Owned { .. } => Ok(result?), - DirectoryOwnership::UnownedViaBlock => { + default_submod_path(&sess.parse_sess, ident, span, relative, dir_path); + match dir_ownership { + DirOwnership::Owned { .. } => Ok(result?), + DirOwnership::UnownedViaBlock => { let _ = result.map_err(|mut err| err.cancel()); error_decl_mod_in_block(&sess.parse_sess, span, path_exists, &name) } @@ -173,7 +173,7 @@ fn error_decl_mod_in_block<'a, T>( /// Derive a submodule path from the first found `#[path = "path_string"]`. /// The provided `dir_path` is joined with the `path_string`. -pub(super) fn submod_path_from_attr( +fn mod_file_path_from_attr( sess: &Session, attrs: &[Attribute], dir_path: &Path, @@ -196,15 +196,15 @@ pub(super) fn submod_path_from_attr( // Public for rustfmt usage. pub fn default_submod_path<'a>( sess: &'a ParseSess, - id: Ident, + ident: Ident, span: Span, relative: Option, dir_path: &Path, ) -> ModulePath<'a> { // If we're in a foo.rs file instead of a mod.rs file, // we need to look for submodules in - // `./foo/.rs` and `./foo//mod.rs` rather than - // `./.rs` and `.//mod.rs`. + // `./foo/.rs` and `./foo//mod.rs` rather than + // `./.rs` and `.//mod.rs`. let relative_prefix_string; let relative_prefix = if let Some(ident) = relative { relative_prefix_string = format!("{}{}", ident.name, path::MAIN_SEPARATOR); @@ -213,7 +213,7 @@ pub fn default_submod_path<'a>( "" }; - let mod_name = id.name.to_string(); + let mod_name = ident.name.to_string(); let default_path_str = format!("{}{}.rs", relative_prefix, mod_name); let secondary_path_str = format!("{}{}{}mod.rs", relative_prefix, mod_name, path::MAIN_SEPARATOR); @@ -224,12 +224,12 @@ pub fn default_submod_path<'a>( let result = match (default_exists, secondary_exists) { (true, false) => Ok(ModulePathSuccess { - path: default_path, - ownership: DirectoryOwnership::Owned { relative: Some(id) }, + file_path: default_path, + dir_ownership: DirOwnership::Owned { relative: Some(ident) }, }), (false, true) => Ok(ModulePathSuccess { - path: secondary_path, - ownership: DirectoryOwnership::Owned { relative: None }, + file_path: secondary_path, + dir_ownership: DirOwnership::Owned { relative: None }, }), (false, false) => { let mut err = struct_span_err!( From da3419e18c68f4eb8beb3765fc9fde4c2e689d43 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 19:49:09 +0300 Subject: [PATCH 6/9] rustc_interface: Hide some hacky details of early linting from expand --- compiler/rustc_expand/src/base.rs | 9 ++++++--- compiler/rustc_expand/src/expand.rs | 10 ++++------ compiler/rustc_expand/src/lib.rs | 1 + compiler/rustc_interface/src/passes.rs | 6 ++++-- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index f88110cf106a7..05c01deef5e1e 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -5,7 +5,7 @@ use rustc_ast::ptr::P; use rustc_ast::token::{self, Nonterminal}; use rustc_ast::tokenstream::{CanSynthesizeMissingTokens, LazyTokenStream, TokenStream}; use rustc_ast::visit::{AssocCtxt, Visitor}; -use rustc_ast::{self as ast, AstLike, Attribute, NodeId, PatKind}; +use rustc_ast::{self as ast, AstLike, Attribute, Item, NodeId, PatKind}; use rustc_attr::{self as attr, Deprecation, Stability}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::{self, Lrc}; @@ -925,6 +925,9 @@ pub struct ExpansionData { pub prior_type_ascription: Option<(Span, bool)>, } +type OnExternModLoaded<'a> = + Option<&'a dyn Fn(Ident, Vec, Vec>, Span) -> (Vec, Vec>)>; + /// One of these is made during expansion and incrementally updated as we go; /// when a macro expansion occurs, the resulting nodes have the `backtrace() /// -> expn_data` of their expansion context stored into their span. @@ -942,7 +945,7 @@ pub struct ExtCtxt<'a> { /// Called directly after having parsed an external `mod foo;` in expansion. /// /// `Ident` is the module name. - pub(super) extern_mod_loaded: Option<&'a dyn Fn(&ast::Crate, Ident)>, + pub(super) extern_mod_loaded: OnExternModLoaded<'a>, } impl<'a> ExtCtxt<'a> { @@ -950,7 +953,7 @@ impl<'a> ExtCtxt<'a> { sess: &'a Session, ecfg: expand::ExpansionConfig<'a>, resolver: &'a mut dyn ResolverExpand, - extern_mod_loaded: Option<&'a dyn Fn(&ast::Crate, Ident)>, + extern_mod_loaded: OnExternModLoaded<'a>, ) -> ExtCtxt<'a> { ExtCtxt { sess, diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index eee2c6ff80869..a42a60fe9a52b 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1298,7 +1298,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { ModKind::Unloaded => { // We have an outline `mod foo;` so we need to parse the file. let ParsedExternalMod { - items, + mut items, inner_span, file_path, dir_path, @@ -1312,14 +1312,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { &mut attrs, ); - let krate = - ast::Crate { attrs, items, span: inner_span, proc_macros: vec![] }; if let Some(extern_mod_loaded) = self.cx.extern_mod_loaded { - extern_mod_loaded(&krate, ident); + (attrs, items) = extern_mod_loaded(ident, attrs, items, inner_span); } - *mod_kind = ModKind::Loaded(krate.items, Inline::No, inner_span); - item.attrs = krate.attrs; + *mod_kind = ModKind::Loaded(items, Inline::No, inner_span); + item.attrs = attrs; // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure. item = configure!(self, item); (Some(file_path), dir_path, dir_ownership) diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index c5d8ff25ea94b..bcccb04c9efd3 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -1,5 +1,6 @@ #![feature(crate_visibility_modifier)] #![feature(decl_macro)] +#![feature(destructuring_assignment)] #![feature(or_patterns)] #![feature(proc_macro_diagnostic)] #![feature(proc_macro_internals)] diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 5217066bbefde..8328c07fbb730 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -302,8 +302,10 @@ fn configure_and_expand_inner<'a>( ..rustc_expand::expand::ExpansionConfig::default(crate_name.to_string()) }; - let extern_mod_loaded = |k: &ast::Crate, ident: Ident| { - pre_expansion_lint(sess, lint_store, k, &*ident.name.as_str()) + let extern_mod_loaded = |ident: Ident, attrs, items, span| { + let krate = ast::Crate { attrs, items, span, proc_macros: vec![] }; + pre_expansion_lint(sess, lint_store, &krate, &ident.name.as_str()); + (krate.attrs, krate.items) }; let mut ecx = ExtCtxt::new(&sess, cfg, &mut resolver, Some(&extern_mod_loaded)); From 29a9ef28188e2ebdd3c56177672cd437dd11e25d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 20:11:30 +0300 Subject: [PATCH 7/9] expand: Align some code with the PR fixing inner attributes on out-of-line modules --- compiler/rustc_expand/src/expand.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index a42a60fe9a52b..dd2eaa0f3d53c 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -1280,8 +1280,12 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { } ast::ItemKind::Mod(_, ref mut mod_kind) if ident != Ident::invalid() => { let (file_path, dir_path, dir_ownership) = match mod_kind { - ModKind::Loaded(_, Inline::Yes, _) => { + ModKind::Loaded(_, inline, _) => { // Inline `mod foo { ... }`, but we still need to push directories. + assert!( + *inline == Inline::Yes, + "`mod` item is loaded from a file for the second time" + ); let (dir_path, dir_ownership) = mod_dir_path( &self.cx.sess, ident, @@ -1292,11 +1296,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { item.attrs = attrs; (None, dir_path, dir_ownership) } - 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 old_attrs_len = attrs.len(); let ParsedExternalMod { mut items, inner_span, @@ -1318,8 +1320,13 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> { *mod_kind = ModKind::Loaded(items, Inline::No, inner_span); item.attrs = attrs; - // File can have inline attributes, e.g., `#![cfg(...)]` & co. => Reconfigure. - item = configure!(self, item); + if item.attrs.len() > old_attrs_len { + // If we loaded an out-of-line module and added some inner attributes, + // then we need to re-configure it. + // FIXME: Attributes also need to be recollected + // for resolution and expansion. + item = configure!(self, item); + } (Some(file_path), dir_path, dir_ownership) } }; From 1e1d574aeac3fae44dac430ecfe8086abcdf9a5f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 20:56:49 +0300 Subject: [PATCH 8/9] expand: Share some code between inline and out-of-line module treatment --- compiler/rustc_expand/src/module.rs | 36 ++++++++++++++--------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 607c68f82dfa9..8f5200804e0dc 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -98,25 +98,26 @@ crate fn mod_dir_path( module: &ModuleData, mut dir_ownership: DirOwnership, ) -> (PathBuf, DirOwnership) { + if let Some(file_path) = mod_file_path_from_attr(sess, attrs, &module.dir_path) { + // For inline modules file path from `#[path]` is actually the directory path + // for historical reasons, so we don't pop the last segment here. + return (file_path, DirOwnership::Owned { relative: None }); + } + + // 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`. let mut dir_path = module.dir_path.clone(); - if let Some(file_path) = sess.first_attr_value_str_by_name(attrs, sym::path) { - dir_path.push(&*file_path.as_str()); - dir_ownership = DirOwnership::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 DirOwnership::Owned { relative } = &mut dir_ownership { - if let Some(ident) = relative.take() { - // Remove the relative offset. - dir_path.push(&*ident.as_str()); - } + if let DirOwnership::Owned { relative } = &mut dir_ownership { + if let Some(ident) = relative.take() { + // Remove the relative offset. + dir_path.push(&*ident.as_str()); } - dir_path.push(&*ident.as_str()); } + dir_path.push(&*ident.as_str()); (dir_path, dir_ownership) } @@ -179,8 +180,7 @@ fn mod_file_path_from_attr( dir_path: &Path, ) -> Option { // Extract path string from first `#[path = "path_string"]` attribute. - let path_string = sess.first_attr_value_str_by_name(attrs, sym::path)?; - let path_string = path_string.as_str(); + let path_string = sess.first_attr_value_str_by_name(attrs, sym::path)?.as_str(); // On windows, the base path might have the form // `\\?\foo\bar` in which case it does not tolerate From 1fe2eb83ecfed43874b4cdf39d0be8910995dd1d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 22 Feb 2021 21:22:40 +0300 Subject: [PATCH 9/9] expand: Introduce enum for module loading errors and make module loading speculative --- compiler/rustc_expand/src/lib.rs | 1 + compiler/rustc_expand/src/module.rs | 172 +++++++++--------- .../directory_ownership/macro-expanded-mod.rs | 2 +- .../macro-expanded-mod.stderr | 2 +- .../non-inline-mod-restriction.rs | 2 +- .../non-inline-mod-restriction.stderr | 2 +- 6 files changed, 90 insertions(+), 91 deletions(-) diff --git a/compiler/rustc_expand/src/lib.rs b/compiler/rustc_expand/src/lib.rs index bcccb04c9efd3..1a93975533de0 100644 --- a/compiler/rustc_expand/src/lib.rs +++ b/compiler/rustc_expand/src/lib.rs @@ -1,3 +1,4 @@ +#![feature(bool_to_option)] #![feature(crate_visibility_modifier)] #![feature(decl_macro)] #![feature(destructuring_assignment)] diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index 8f5200804e0dc..2ec656d4895e7 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -1,7 +1,7 @@ use crate::base::ModuleData; use rustc_ast::ptr::P; use rustc_ast::{token, Attribute, Item}; -use rustc_errors::{struct_span_err, PResult}; +use rustc_errors::{struct_span_err, DiagnosticBuilder}; use rustc_parse::new_parser_from_file; use rustc_session::parse::ParseSess; use rustc_session::Session; @@ -19,14 +19,6 @@ pub enum DirOwnership { UnownedViaBlock, } -/// Information about the path to a module. -// Public for rustfmt usage. -pub struct ModulePath<'a> { - name: String, - path_exists: bool, - pub result: PResult<'a, ModulePathSuccess>, -} - // Public for rustfmt usage. pub struct ModulePathSuccess { pub file_path: PathBuf, @@ -41,6 +33,14 @@ crate struct ParsedExternalMod { pub dir_ownership: DirOwnership, } +pub enum ModError<'a> { + CircularInclusion(Vec), + ModInBlock(Option), + FileNotFound(Ident, PathBuf), + MultipleCandidates(Ident, String, String), + ParserError(DiagnosticBuilder<'a>), +} + crate fn parse_external_mod( sess: &Session, ident: Ident, @@ -50,22 +50,26 @@ crate fn parse_external_mod( attrs: &mut Vec, ) -> ParsedExternalMod { // We bail on the first error, but that error does not cause a fatal error... (1) - let result: PResult<'_, _> = try { + let result: Result<_, ModError<'_>> = try { // Extract the file path and the new ownership. - let mp = mod_file_path(sess, ident, span, &attrs, &module.dir_path, dir_ownership)?; + let mp = mod_file_path(sess, ident, &attrs, &module.dir_path, dir_ownership)?; dir_ownership = mp.dir_ownership; // Ensure file paths are acyclic. - error_on_circular_module(&sess.parse_sess, span, &mp.file_path, &module.file_path_stack)?; + if let Some(pos) = module.file_path_stack.iter().position(|p| p == &mp.file_path) { + Err(ModError::CircularInclusion(module.file_path_stack[pos..].to_vec()))?; + } // Actually parse the external file as a module. let mut parser = new_parser_from_file(&sess.parse_sess, &mp.file_path, Some(span)); - let (mut inner_attrs, items, inner_span) = parser.parse_mod(&token::Eof)?; + let (mut inner_attrs, items, inner_span) = + parser.parse_mod(&token::Eof).map_err(|err| ModError::ParserError(err))?; attrs.append(&mut inner_attrs); (items, inner_span, mp.file_path) }; // (1) ...instead, we return a dummy module. - let (items, inner_span, file_path) = result.map_err(|mut err| err.emit()).unwrap_or_default(); + let (items, inner_span, file_path) = + result.map_err(|err| err.report(sess, span)).unwrap_or_default(); // Extract the directory path for submodules of the module. let dir_path = file_path.parent().unwrap_or(&file_path).to_owned(); @@ -73,24 +77,6 @@ crate fn parse_external_mod( ParsedExternalMod { items, inner_span, file_path, dir_path, dir_ownership } } -fn error_on_circular_module<'a>( - sess: &'a ParseSess, - span: Span, - file_path: &Path, - file_path_stack: &[PathBuf], -) -> PResult<'a, ()> { - if let Some(i) = file_path_stack.iter().position(|p| *p == file_path) { - let mut err = String::from("circular modules: "); - for p in &file_path_stack[i..] { - err.push_str(&p.to_string_lossy()); - err.push_str(" -> "); - } - err.push_str(&file_path.to_string_lossy()); - return Err(sess.span_diagnostic.struct_span_err(span, &err[..])); - } - Ok(()) -} - crate fn mod_dir_path( sess: &Session, ident: Ident, @@ -125,11 +111,10 @@ crate fn mod_dir_path( fn mod_file_path<'a>( sess: &'a Session, ident: Ident, - span: Span, attrs: &[Attribute], dir_path: &Path, dir_ownership: DirOwnership, -) -> PResult<'a, ModulePathSuccess> { +) -> Result> { if let Some(file_path) = mod_file_path_from_attr(sess, attrs, dir_path) { // All `#[path]` files are treated as though they are a `mod.rs` file. // This means that `mod foo;` declarations inside `#[path]`-included @@ -146,30 +131,14 @@ fn mod_file_path<'a>( DirOwnership::Owned { relative } => relative, DirOwnership::UnownedViaBlock => None, }; - let ModulePath { path_exists, name, result } = - default_submod_path(&sess.parse_sess, ident, span, relative, dir_path); + let result = default_submod_path(&sess.parse_sess, ident, relative, dir_path); match dir_ownership { - DirOwnership::Owned { .. } => Ok(result?), - DirOwnership::UnownedViaBlock => { - let _ = result.map_err(|mut err| err.cancel()); - error_decl_mod_in_block(&sess.parse_sess, span, path_exists, &name) - } - } -} - -fn error_decl_mod_in_block<'a, T>( - sess: &'a ParseSess, - span: Span, - path_exists: bool, - name: &str, -) -> PResult<'a, T> { - let msg = "Cannot declare a non-inline module inside a block unless it has a path attribute"; - let mut err = sess.span_diagnostic.struct_span_err(span, msg); - if path_exists { - let msg = format!("Maybe `use` the module `{}` instead of redeclaring it", name); - err.span_note(span, &msg); + DirOwnership::Owned { .. } => result, + DirOwnership::UnownedViaBlock => Err(ModError::ModInBlock(match result { + Ok(_) | Err(ModError::MultipleCandidates(..)) => Some(ident), + _ => None, + })), } - Err(err) } /// Derive a submodule path from the first found `#[path = "path_string"]`. @@ -197,10 +166,9 @@ fn mod_file_path_from_attr( pub fn default_submod_path<'a>( sess: &'a ParseSess, ident: Ident, - span: Span, relative: Option, dir_path: &Path, -) -> ModulePath<'a> { +) -> Result> { // If we're in a foo.rs file instead of a mod.rs file, // we need to look for submodules in // `./foo/.rs` and `./foo//mod.rs` rather than @@ -222,7 +190,7 @@ pub fn default_submod_path<'a>( let default_exists = sess.source_map().file_exists(&default_path); let secondary_exists = sess.source_map().file_exists(&secondary_path); - let result = match (default_exists, secondary_exists) { + match (default_exists, secondary_exists) { (true, false) => Ok(ModulePathSuccess { file_path: default_path, dir_ownership: DirOwnership::Owned { relative: Some(ident) }, @@ -231,35 +199,65 @@ pub fn default_submod_path<'a>( file_path: secondary_path, dir_ownership: DirOwnership::Owned { relative: None }, }), - (false, false) => { - let mut err = struct_span_err!( - sess.span_diagnostic, - span, - E0583, - "file not found for module `{}`", - mod_name, - ); - err.help(&format!( - "to create the module `{}`, create file \"{}\"", - mod_name, - default_path.display(), - )); - Err(err) - } + (false, false) => Err(ModError::FileNotFound(ident, default_path)), (true, true) => { - let mut err = struct_span_err!( - sess.span_diagnostic, - span, - E0761, - "file for module `{}` found at both {} and {}", - mod_name, - default_path_str, - secondary_path_str, - ); - err.help("delete or rename one of them to remove the ambiguity"); - Err(err) + Err(ModError::MultipleCandidates(ident, default_path_str, secondary_path_str)) } - }; + } +} - ModulePath { name: mod_name, path_exists: default_exists || secondary_exists, result } +impl ModError<'_> { + fn report(self, sess: &Session, span: Span) { + let diag = &sess.parse_sess.span_diagnostic; + match self { + ModError::CircularInclusion(file_paths) => { + let mut msg = String::from("circular modules: "); + for file_path in &file_paths { + msg.push_str(&file_path.display().to_string()); + msg.push_str(" -> "); + } + msg.push_str(&file_paths[0].display().to_string()); + diag.struct_span_err(span, &msg) + } + ModError::ModInBlock(ident) => { + let msg = "cannot declare a non-inline module inside a block unless it has a path attribute"; + let mut err = diag.struct_span_err(span, msg); + if let Some(ident) = ident { + let note = + format!("maybe `use` the module `{}` instead of redeclaring it", ident); + err.span_note(span, ¬e); + } + err + } + ModError::FileNotFound(ident, default_path) => { + let mut err = struct_span_err!( + diag, + span, + E0583, + "file not found for module `{}`", + ident, + ); + err.help(&format!( + "to create the module `{}`, create file \"{}\"", + ident, + default_path.display(), + )); + err + } + ModError::MultipleCandidates(ident, default_path_short, secondary_path_short) => { + let mut err = struct_span_err!( + diag, + span, + E0761, + "file for module `{}` found at both {} and {}", + ident, + default_path_short, + secondary_path_short, + ); + err.help("delete or rename one of them to remove the ambiguity"); + err + } + ModError::ParserError(err) => err, + }.emit() + } } diff --git a/src/test/ui/directory_ownership/macro-expanded-mod.rs b/src/test/ui/directory_ownership/macro-expanded-mod.rs index 9cb159603a8c5..fa81769e5a800 100644 --- a/src/test/ui/directory_ownership/macro-expanded-mod.rs +++ b/src/test/ui/directory_ownership/macro-expanded-mod.rs @@ -2,7 +2,7 @@ macro_rules! mod_decl { ($i:ident) => { - mod $i; //~ ERROR Cannot declare a non-inline module inside a block + mod $i; //~ ERROR cannot declare a non-inline module inside a block }; } diff --git a/src/test/ui/directory_ownership/macro-expanded-mod.stderr b/src/test/ui/directory_ownership/macro-expanded-mod.stderr index f90419247c92b..4039728e18ac3 100644 --- a/src/test/ui/directory_ownership/macro-expanded-mod.stderr +++ b/src/test/ui/directory_ownership/macro-expanded-mod.stderr @@ -1,4 +1,4 @@ -error: Cannot declare a non-inline module inside a block unless it has a path attribute +error: cannot declare a non-inline module inside a block unless it has a path attribute --> $DIR/macro-expanded-mod.rs:5:9 | LL | mod $i; diff --git a/src/test/ui/directory_ownership/non-inline-mod-restriction.rs b/src/test/ui/directory_ownership/non-inline-mod-restriction.rs index af31b8a49281f..de4f816656cc4 100644 --- a/src/test/ui/directory_ownership/non-inline-mod-restriction.rs +++ b/src/test/ui/directory_ownership/non-inline-mod-restriction.rs @@ -1,5 +1,5 @@ // Test that non-inline modules are not allowed inside blocks. fn main() { - mod foo; //~ ERROR Cannot declare a non-inline module inside a block + mod foo; //~ ERROR cannot declare a non-inline module inside a block } diff --git a/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr b/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr index d034942ca5d4c..64189bee43f6e 100644 --- a/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr +++ b/src/test/ui/directory_ownership/non-inline-mod-restriction.stderr @@ -1,4 +1,4 @@ -error: Cannot declare a non-inline module inside a block unless it has a path attribute +error: cannot declare a non-inline module inside a block unless it has a path attribute --> $DIR/non-inline-mod-restriction.rs:4:5 | LL | mod foo;