Auto merge of #37602 - jseyfried:directory_ownership, r=nikomatsakis

parser: simplify directory ownership semantics

This PR simplifies the semantics of "directory ownership". After this PR,
 - a non-inline module without a `#[path]` attribute (e.g. `mod foo;`) is allowed iff its parent module/block (whichever is nearer) is a directory owner,
 - an non-inline module is a directory owner iff its corresponding file is named `mod.rs` (c.f. [comment](https://github.com/rust-lang/rust/issues/32401#issuecomment-201021902)),
 - a block is never a directory owner (c.f. #31534), and
 - an inline module is a directory owner iff either
   - its parent module/block is a directory owner (again, c.f. #31534), or
   - it has a `#[path]` attribute (c.f. #36789).

These semantics differ from today's in three orthogonal ways:
 - `#[path = "foo.rs"] mod foo;` is no longer a directory owner. This is a [breaking-change].
 - #36789 is generalized to apply to modules that are not directory owners in addition to blocks.
 - A macro-expanded non-inline module is only allowed where an ordinary non-inline module would be allowed. Today, we incorrectly allow macro-expanded non-inline modules in modules that are not directory owners (but not in blocks). This is a [breaking-change].

Fixes #32401.
r? @nikomatsakis

1 files changed, 82 insertions, 60 deletions

diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 4997e464c2b..b00e6b5d58f 100644
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -38,7 +38,7 @@ use ast::{Ty, TyKind, TypeBinding, TyParam, TyParamBounds};
 use ast::{ViewPath, ViewPathGlob, ViewPathList, ViewPathSimple};
 use ast::{Visibility, WhereClause};
 use ast::{BinOpKind, UnOp};
-use ast;
+use {ast, attr};
 use codemap::{self, CodeMap, Spanned, spanned, respan};
 use syntax_pos::{self, Span, BytePos, mk_sp};
 use errors::{self, DiagnosticBuilder};
@@ -49,7 +49,7 @@ use parse::common::SeqSep;
 use parse::lexer::{Reader, TokenAndSpan};
 use parse::obsolete::ObsoleteSyntax;
 use parse::token::{self, MatchNt, SubstNt};
-use parse::{new_sub_parser_from_file, ParseSess};
+use parse::{new_sub_parser_from_file, ParseSess, Directory, DirectoryOwnership};
 use util::parser::{AssocOp, Fixity};
 use print::pprust;
 use ptr::P;
@@ -68,7 +68,6 @@ bitflags! {
     flags Restrictions: u8 {
         const RESTRICTION_STMT_EXPR         = 1 << 0,
         const RESTRICTION_NO_STRUCT_LITERAL = 1 << 1,
-        const NO_NONINLINE_MOD  = 1 << 2,
     }
 }
 
@@ -200,12 +199,9 @@ pub struct Parser<'a> {
     /// extra detail when the same error is seen twice
     pub obsolete_set: HashSet<ObsoleteSyntax>,
     /// Used to determine the path to externally loaded source files
-    pub directory: PathBuf,
+    pub directory: Directory,
     /// Stack of open delimiters and their spans. Used for error message.
     pub open_braces: Vec<(token::DelimToken, Span)>,
-    /// Flag if this parser "owns" the directory that it is currently parsing
-    /// in. This will affect how nested files are looked up.
-    pub owns_directory: bool,
     /// Name of the root module this parser originated from. If `None`, then the
     /// name is not known. This does not change while the parser is descending
     /// into modules, and sub-parsers have new values for this name.
@@ -245,8 +241,9 @@ pub struct ModulePath {
 }
 
 pub struct ModulePathSuccess {
-    pub path: ::std::path::PathBuf,
-    pub owns_directory: bool,
+    pub path: PathBuf,
+    pub directory_ownership: DirectoryOwnership,
+    warn: bool,
 }
 
 pub struct ModulePathError {
@@ -296,9 +293,8 @@ impl<'a> Parser<'a> {
             quote_depth: 0,
             parsing_token_tree: false,
             obsolete_set: HashSet::new(),
-            directory: PathBuf::new(),
+            directory: Directory { path: PathBuf::new(), ownership: DirectoryOwnership::Owned },
             open_braces: Vec::new(),
-            owns_directory: true,
             root_module_name: None,
             expected_tokens: Vec::new(),
             tts: Vec::new(),
@@ -310,8 +306,8 @@ impl<'a> Parser<'a> {
         parser.token = tok.tok;
         parser.span = tok.sp;
         if parser.span != syntax_pos::DUMMY_SP {
-            parser.directory = PathBuf::from(sess.codemap().span_to_filename(parser.span));
-            parser.directory.pop();
+            parser.directory.path = PathBuf::from(sess.codemap().span_to_filename(parser.span));
+            parser.directory.path.pop();
         }
         parser
     }
@@ -3966,9 +3962,11 @@ impl<'a> Parser<'a> {
             }
         } else {
             // FIXME: Bad copy of attrs
-            let restrictions = self.restrictions | Restrictions::NO_NONINLINE_MOD;
-            match self.with_res(restrictions,
-                                |this| this.parse_item_(attrs.clone(), false, true))? {
+            let old_directory_ownership =
+                mem::replace(&mut self.directory.ownership, DirectoryOwnership::UnownedViaBlock);
+            let item = self.parse_item_(attrs.clone(), false, true)?;
+            self.directory.ownership = old_directory_ownership;
+            match item {
                 Some(i) => Stmt {
                     id: ast::DUMMY_NODE_ID,
                     span: mk_sp(lo, i.span.hi),
@@ -5271,38 +5269,53 @@ impl<'a> Parser<'a> {
             self.bump();
             if in_cfg {
                 // This mod is in an external file. Let's go get it!
-                let (m, attrs) = self.eval_src_mod(id, &outer_attrs, id_span)?;
-                Ok((id, m, Some(attrs)))
+                let ModulePathSuccess { path, directory_ownership, warn } =
+                    self.submod_path(id, &outer_attrs, id_span)?;
+                let (module, mut attrs) =
+                    self.eval_src_mod(path, directory_ownership, id.to_string(), id_span)?;
+                if warn {
+                    let attr = ast::Attribute {
+                        id: attr::mk_attr_id(),
+                        style: ast::AttrStyle::Outer,
+                        value: ast::MetaItem {
+                            name: Symbol::intern("warn_directory_ownership"),
+                            node: ast::MetaItemKind::Word,
+                            span: syntax_pos::DUMMY_SP,
+                        },
+                        is_sugared_doc: false,
+                        span: syntax_pos::DUMMY_SP,
+                    };
+                    attr::mark_known(&attr);
+                    attrs.push(attr);
+                }
+                Ok((id, module, Some(attrs)))
             } else {
                 let placeholder = ast::Mod { inner: syntax_pos::DUMMY_SP, items: Vec::new() };
                 Ok((id, ItemKind::Mod(placeholder), None))
             }
         } else {
-            let directory = self.directory.clone();
-            let restrictions = self.push_directory(id, &outer_attrs);
+            let old_directory = self.directory.clone();
+            self.push_directory(id, &outer_attrs);
             self.expect(&token::OpenDelim(token::Brace))?;
             let mod_inner_lo = self.span.lo;
             let attrs = self.parse_inner_attributes()?;
-            let m = self.with_res(restrictions, |this| {
-                this.parse_mod_items(&token::CloseDelim(token::Brace), mod_inner_lo)
-            })?;
-            self.directory = directory;
-            Ok((id, ItemKind::Mod(m), Some(attrs)))
+            let module = self.parse_mod_items(&token::CloseDelim(token::Brace), mod_inner_lo)?;
+            self.directory = old_directory;
+            Ok((id, ItemKind::Mod(module), Some(attrs)))
         }
     }
 
-    fn push_directory(&mut self, id: Ident, attrs: &[Attribute]) -> Restrictions {
-        if let Some(path) = ::attr::first_attr_value_str_by_name(attrs, "path") {
-            self.directory.push(&*path.as_str());
-            self.restrictions - Restrictions::NO_NONINLINE_MOD
+    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.push(&*path.as_str());
+            self.directory.ownership = DirectoryOwnership::Owned;
         } else {
-            self.directory.push(&*id.name.as_str());
-            self.restrictions
+            self.directory.path.push(&*id.name.as_str());
         }
     }
 
     pub fn submod_path_from_attr(attrs: &[ast::Attribute], dir_path: &Path) -> Option<PathBuf> {
-        ::attr::first_attr_value_str_by_name(attrs, "path").map(|d| dir_path.join(&*d.as_str()))
+        attr::first_attr_value_str_by_name(attrs, "path").map(|d| dir_path.join(&*d.as_str()))
     }
 
     /// Returns either a path to a module, or .
@@ -5317,8 +5330,16 @@ impl<'a> Parser<'a> {
         let secondary_exists = codemap.file_exists(&secondary_path);
 
         let result = match (default_exists, secondary_exists) {
-            (true, false) => Ok(ModulePathSuccess { path: default_path, owns_directory: false }),
-            (false, true) => Ok(ModulePathSuccess { path: secondary_path, owns_directory: true }),
+            (true, false) => Ok(ModulePathSuccess {
+                path: default_path,
+                directory_ownership: DirectoryOwnership::UnownedViaMod(false),
+                warn: false,
+            }),
+            (false, true) => Ok(ModulePathSuccess {
+                path: secondary_path,
+                directory_ownership: DirectoryOwnership::Owned,
+                warn: false,
+            }),
             (false, false) => Err(ModulePathError {
                 err_msg: format!("file not found for module `{}`", mod_name),
                 help_msg: format!("name the file either {} or {} inside the directory {:?}",
@@ -5346,13 +5367,20 @@ impl<'a> Parser<'a> {
                    id: ast::Ident,
                    outer_attrs: &[ast::Attribute],
                    id_sp: Span) -> PResult<'a, ModulePathSuccess> {
-        if let Some(p) = Parser::submod_path_from_attr(outer_attrs, &self.directory) {
-            return Ok(ModulePathSuccess { path: p, owns_directory: true });
+        if let Some(path) = Parser::submod_path_from_attr(outer_attrs, &self.directory.path) {
+            return Ok(ModulePathSuccess {
+                directory_ownership: match path.file_name().and_then(|s| s.to_str()) {
+                    Some("mod.rs") => DirectoryOwnership::Owned,
+                    _ => DirectoryOwnership::UnownedViaMod(true),
+                },
+                path: path,
+                warn: false,
+            });
         }
 
-        let paths = Parser::default_submod_path(id, &self.directory, self.sess.codemap());
+        let paths = Parser::default_submod_path(id, &self.directory.path, self.sess.codemap());
 
-        if self.restrictions.contains(Restrictions::NO_NONINLINE_MOD) {
+        if let DirectoryOwnership::UnownedViaBlock = self.directory.ownership {
             let msg =
                 "Cannot declare a non-inline module inside a block unless it has a path attribute";
             let mut err = self.diagnostic().struct_span_err(id_sp, msg);
@@ -5362,10 +5390,15 @@ impl<'a> Parser<'a> {
                 err.span_note(id_sp, &msg);
             }
             return Err(err);
-        } else if !self.owns_directory {
+        } else if let DirectoryOwnership::UnownedViaMod(warn) = self.directory.ownership {
+            if warn {
+                if let Ok(result) = paths.result {
+                    return Ok(ModulePathSuccess { warn: true, ..result });
+                }
+            }
             let mut err = self.diagnostic().struct_span_err(id_sp,
                 "cannot declare a new module at this location");
-            let this_module = match self.directory.file_name() {
+            let this_module = match self.directory.path.file_name() {
                 Some(file_name) => file_name.to_str().unwrap().to_owned(),
                 None => self.root_module_name.as_ref().unwrap().clone(),
             };
@@ -5378,8 +5411,10 @@ impl<'a> Parser<'a> {
                               &format!("... or maybe `use` the module `{}` instead \
                                         of possibly redeclaring it",
                                        paths.name));
-            }
-            return Err(err);
+                return Err(err);
+            } else {
+                return Err(err);
+            };
         }
 
         match paths.result {
@@ -5390,25 +5425,11 @@ impl<'a> Parser<'a> {
 
     /// Read a module from a source file.
     fn eval_src_mod(&mut self,
-                    id: ast::Ident,
-                    outer_attrs: &[ast::Attribute],
+                    path: PathBuf,
+                    directory_ownership: DirectoryOwnership,
+                    name: String,
                     id_sp: Span)
                     -> PResult<'a, (ast::ItemKind, Vec<ast::Attribute> )> {
-        let ModulePathSuccess { path, owns_directory } = self.submod_path(id,
-                                                                          outer_attrs,
-                                                                          id_sp)?;
-
-        self.eval_src_mod_from_path(path,
-                                    owns_directory,
-                                    id.to_string(),
-                                    id_sp)
-    }
-
-    fn eval_src_mod_from_path(&mut self,
-                              path: PathBuf,
-                              owns_directory: bool,
-                              name: String,
-                              id_sp: Span) -> PResult<'a, (ast::ItemKind, Vec<ast::Attribute> )> {
         let mut included_mod_stack = self.sess.included_mod_stack.borrow_mut();
         if let Some(i) = included_mod_stack.iter().position(|p| *p == path) {
             let mut err = String::from("circular modules: ");
@@ -5423,7 +5444,8 @@ impl<'a> Parser<'a> {
         included_mod_stack.push(path.clone());
         drop(included_mod_stack);
 
-        let mut p0 = new_sub_parser_from_file(self.sess, &path, owns_directory, Some(name), id_sp);
+        let mut p0 =
+            new_sub_parser_from_file(self.sess, &path, directory_ownership, Some(name), id_sp);
         let mod_inner_lo = p0.span.lo;
         let mod_attrs = p0.parse_inner_attributes()?;
         let m0 = p0.parse_mod_items(&token::Eof, mod_inner_lo)?;