From 5675d9d28007691c4f396abcd4b29a228150eeb4 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 5 Nov 2016 04:16:26 +0000 Subject: [PATCH 1/4] Clean up directory ownership semantics. --- src/libsyntax/ext/base.rs | 8 +-- src/libsyntax/ext/expand.rs | 24 ++++--- src/libsyntax/ext/source_util.rs | 5 +- src/libsyntax/ext/tt/macro_rules.rs | 13 ++-- src/libsyntax/parse/mod.rs | 17 ++++- src/libsyntax/parse/parser.rs | 106 +++++++++++++--------------- 6 files changed, 94 insertions(+), 79 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index ddf4cf11f2048..ddbca47429d18 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -18,7 +18,7 @@ use errors::DiagnosticBuilder; use ext::expand::{self, Expansion}; use ext::hygiene::Mark; use fold::{self, Folder}; -use parse::{self, parser}; +use parse::{self, parser, DirectoryOwnership}; use parse::token; use ptr::P; use symbol::Symbol; @@ -568,9 +568,7 @@ pub struct ExpansionData { pub depth: usize, pub backtrace: ExpnId, pub module: Rc, - - // True if non-inline modules without a `#[path]` are forbidden at the root of this expansion. - pub no_noninline_mod: bool, + pub directory_ownership: DirectoryOwnership, } /// One of these is made during expansion and incrementally updated as we go; @@ -601,7 +599,7 @@ impl<'a> ExtCtxt<'a> { depth: 0, backtrace: NO_EXPANSION, module: Rc::new(ModuleData { mod_path: Vec::new(), directory: PathBuf::new() }), - no_noninline_mod: false, + directory_ownership: DirectoryOwnership::Owned, }, } } diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 844fb77e29d79..3e8f118ce62de 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -21,7 +21,7 @@ use ext::base::*; use feature_gate::{self, Features}; use fold; use fold::*; -use parse::{ParseSess, PResult, lexer}; +use parse::{ParseSess, DirectoryOwnership, PResult, lexer}; use parse::parser::Parser; use parse::token; use print::pprust; @@ -727,9 +727,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { } fn fold_block(&mut self, block: P) -> P { - let no_noninline_mod = mem::replace(&mut self.cx.current_expansion.no_noninline_mod, true); + let old_directory_ownership = self.cx.current_expansion.directory_ownership; + self.cx.current_expansion.directory_ownership = DirectoryOwnership::UnownedViaBlock; let result = noop_fold_block(block, self); - self.cx.current_expansion.no_noninline_mod = no_noninline_mod; + self.cx.current_expansion.directory_ownership = old_directory_ownership; result } @@ -768,7 +769,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { return noop_fold_item(item, self); } - let orig_no_noninline_mod = self.cx.current_expansion.no_noninline_mod; + let orig_directory_ownership = self.cx.current_expansion.directory_ownership; let mut module = (*self.cx.current_expansion.module).clone(); module.mod_path.push(item.ident); @@ -779,23 +780,28 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { if inline_module { if let Some(path) = attr::first_attr_value_str_by_name(&item.attrs, "path") { - self.cx.current_expansion.no_noninline_mod = false; + self.cx.current_expansion.directory_ownership = DirectoryOwnership::Owned; module.directory.push(&*path.as_str()); } else { module.directory.push(&*item.ident.name.as_str()); } } else { - self.cx.current_expansion.no_noninline_mod = false; - module.directory = + let mut path = PathBuf::from(self.cx.parse_sess.codemap().span_to_filename(inner)); - module.directory.pop(); + let directory_ownership = match path.file_name().unwrap().to_str() { + Some("mod.rs") => DirectoryOwnership::Owned, + _ => DirectoryOwnership::UnownedViaMod, + }; + path.pop(); + module.directory = path; + 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); self.cx.current_expansion.module = orig_module; - self.cx.current_expansion.no_noninline_mod = orig_no_noninline_mod; + self.cx.current_expansion.directory_ownership = orig_directory_ownership; return result; } // Ensure that test functions are accessible from the test harness. diff --git a/src/libsyntax/ext/source_util.rs b/src/libsyntax/ext/source_util.rs index 320d49b64634c..39b92c7d007de 100644 --- a/src/libsyntax/ext/source_util.rs +++ b/src/libsyntax/ext/source_util.rs @@ -13,7 +13,7 @@ use syntax_pos::{self, Pos, Span}; use ext::base::*; use ext::base; use ext::build::AstBuilder; -use parse::token; +use parse::{token, DirectoryOwnership}; use parse; use print::pprust; use ptr::P; @@ -90,7 +90,8 @@ 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, Path::new(&file)); - let p = parse::new_sub_parser_from_file(cx.parse_sess(), &path, true, None, sp); + let directory_ownership = DirectoryOwnership::Owned; + let p = parse::new_sub_parser_from_file(cx.parse_sess(), &path, directory_ownership, None, sp); struct ExpandResult<'a> { p: parse::parser::Parser<'a>, diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index 59b8b50e88cb6..4164b4a93ec91 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -19,7 +19,7 @@ use ext::tt::macro_parser::{MatchedSeq, MatchedNonterminal}; use ext::tt::macro_parser::{parse, parse_failure_msg}; use parse::ParseSess; use parse::lexer::new_tt_reader; -use parse::parser::{Parser, Restrictions}; +use parse::parser::Parser; use parse::token::{self, NtTT, Token}; use parse::token::Token::*; use print; @@ -117,11 +117,12 @@ fn generic_extension<'cx>(cx: &'cx ExtCtxt, let trncbr = new_tt_reader(&cx.parse_sess.span_diagnostic, Some(named_matches), rhs); let mut p = Parser::new(cx.parse_sess(), Box::new(trncbr)); - p.directory = cx.current_expansion.module.directory.clone(); - p.restrictions = match cx.current_expansion.no_noninline_mod { - true => Restrictions::NO_NONINLINE_MOD, - false => Restrictions::empty(), - }; + let module = &cx.current_expansion.module; + p.directory.path = module.directory.clone(); + p.directory.ownership = cx.current_expansion.directory_ownership; + p.root_module_name = + module.mod_path.last().map(|id| (*id.name.as_str()).to_owned()); + p.check_unknown_macro_variable(); // Let the context choose how to interpret the result. // Weird, but useful for X-macros. diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index be340a5b5aa93..f969e45b83a8e 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -76,6 +76,19 @@ impl ParseSess { } } +#[derive(Clone)] +pub struct Directory { + pub path: PathBuf, + pub ownership: DirectoryOwnership, +} + +#[derive(Copy, Clone)] +pub enum DirectoryOwnership { + Owned, + UnownedViaBlock, + UnownedViaMod, +} + // a bunch of utility functions of the form parse__from_ // where includes crate, expr, item, stmt, tts, and one that // uses a HOF to parse anything, and includes file and @@ -152,11 +165,11 @@ pub fn new_parser_from_file<'a>(sess: &'a ParseSess, path: &Path) -> Parser<'a> /// On an error, use the given span as the source of the problem. pub fn new_sub_parser_from_file<'a>(sess: &'a ParseSess, path: &Path, - owns_directory: bool, + directory_ownership: DirectoryOwnership, module_name: Option, sp: Span) -> Parser<'a> { let mut p = filemap_to_parser(sess, file_to_filemap(sess, path, Some(sp))); - p.owns_directory = owns_directory; + p.directory.ownership = directory_ownership; p.root_module_name = module_name; p } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 4997e464c2bf5..ee69125ffae83 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -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, /// 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,8 @@ pub struct ModulePath { } pub struct ModulePathSuccess { - pub path: ::std::path::PathBuf, - pub owns_directory: bool, + pub path: PathBuf, + pub directory_ownership: DirectoryOwnership, } pub struct ModulePathError { @@ -296,9 +292,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 +305,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 +3961,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,33 +5268,33 @@ 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 } = + self.submod_path(id, &outer_attrs, id_span)?; + let (module, attrs) = + self.eval_src_mod(path, directory_ownership, id.to_string(), id_span)?; + 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 { + fn push_directory(&mut self, id: Ident, attrs: &[Attribute]) { 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 + 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()); } } @@ -5317,8 +5314,14 @@ 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, true) => Ok(ModulePathSuccess { + path: secondary_path, + directory_ownership: DirectoryOwnership::Owned, + }), (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 +5349,19 @@ 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, + }, + path: path, + }); } - 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 +5371,10 @@ impl<'a> Parser<'a> { err.span_note(id_sp, &msg); } return Err(err); - } else if !self.owns_directory { + } else if let DirectoryOwnership::UnownedViaMod = self.directory.ownership { 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(), }; @@ -5390,25 +5399,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 )> { - 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 )> { 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 +5418,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)?; From 30ac06fd732bb53c0c4e6ddcc1dba9a54fc9e9ae Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 5 Nov 2016 05:31:35 +0000 Subject: [PATCH 2/4] Add a regression test and organize tests. --- .../macro-expanded-mod.rs | 0 .../macro_expanded_mod_helper/foo/bar.rs | 0 .../macro_expanded_mod_helper/foo/mod.rs | 0 .../mod_file_not_owning.rs | 2 -- .../mod_file_not_owning_aux1.rs | 5 ++++- .../mod_file_not_owning_aux2.rs | 0 .../non-inline-mod-restriction.rs | 0 .../directory_ownership/unowned_mod_with_path.rs | 15 +++++++++++++++ 8 files changed, 19 insertions(+), 3 deletions(-) rename src/test/compile-fail/{ => directory_ownership}/macro-expanded-mod.rs (100%) rename src/test/compile-fail/{ => directory_ownership}/macro_expanded_mod_helper/foo/bar.rs (100%) rename src/test/compile-fail/{ => directory_ownership}/macro_expanded_mod_helper/foo/mod.rs (100%) rename src/test/compile-fail/{ => directory_ownership}/mod_file_not_owning.rs (94%) rename src/test/compile-fail/{ => directory_ownership}/mod_file_not_owning_aux1.rs (87%) rename src/test/compile-fail/{ => directory_ownership}/mod_file_not_owning_aux2.rs (100%) rename src/test/compile-fail/{ => directory_ownership}/non-inline-mod-restriction.rs (100%) create mode 100644 src/test/compile-fail/directory_ownership/unowned_mod_with_path.rs diff --git a/src/test/compile-fail/macro-expanded-mod.rs b/src/test/compile-fail/directory_ownership/macro-expanded-mod.rs similarity index 100% rename from src/test/compile-fail/macro-expanded-mod.rs rename to src/test/compile-fail/directory_ownership/macro-expanded-mod.rs diff --git a/src/test/compile-fail/macro_expanded_mod_helper/foo/bar.rs b/src/test/compile-fail/directory_ownership/macro_expanded_mod_helper/foo/bar.rs similarity index 100% rename from src/test/compile-fail/macro_expanded_mod_helper/foo/bar.rs rename to src/test/compile-fail/directory_ownership/macro_expanded_mod_helper/foo/bar.rs diff --git a/src/test/compile-fail/macro_expanded_mod_helper/foo/mod.rs b/src/test/compile-fail/directory_ownership/macro_expanded_mod_helper/foo/mod.rs similarity index 100% rename from src/test/compile-fail/macro_expanded_mod_helper/foo/mod.rs rename to src/test/compile-fail/directory_ownership/macro_expanded_mod_helper/foo/mod.rs diff --git a/src/test/compile-fail/mod_file_not_owning.rs b/src/test/compile-fail/directory_ownership/mod_file_not_owning.rs similarity index 94% rename from src/test/compile-fail/mod_file_not_owning.rs rename to src/test/compile-fail/directory_ownership/mod_file_not_owning.rs index 7dcff6e6644f1..adbcedd91f205 100644 --- a/src/test/compile-fail/mod_file_not_owning.rs +++ b/src/test/compile-fail/directory_ownership/mod_file_not_owning.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// compile-flags: -Z parse-only - // error-pattern: cannot declare a new module at this location mod mod_file_not_owning_aux1; diff --git a/src/test/compile-fail/mod_file_not_owning_aux1.rs b/src/test/compile-fail/directory_ownership/mod_file_not_owning_aux1.rs similarity index 87% rename from src/test/compile-fail/mod_file_not_owning_aux1.rs rename to src/test/compile-fail/directory_ownership/mod_file_not_owning_aux1.rs index 2d522be6dc5dc..4ac94a92e376c 100644 --- a/src/test/compile-fail/mod_file_not_owning_aux1.rs +++ b/src/test/compile-fail/directory_ownership/mod_file_not_owning_aux1.rs @@ -10,4 +10,7 @@ // ignore-test this is not a test -mod mod_file_not_owning_aux2; +macro_rules! m { + () => { mod mod_file_not_owning_aux2; } +} +m!(); diff --git a/src/test/compile-fail/mod_file_not_owning_aux2.rs b/src/test/compile-fail/directory_ownership/mod_file_not_owning_aux2.rs similarity index 100% rename from src/test/compile-fail/mod_file_not_owning_aux2.rs rename to src/test/compile-fail/directory_ownership/mod_file_not_owning_aux2.rs diff --git a/src/test/compile-fail/non-inline-mod-restriction.rs b/src/test/compile-fail/directory_ownership/non-inline-mod-restriction.rs similarity index 100% rename from src/test/compile-fail/non-inline-mod-restriction.rs rename to src/test/compile-fail/directory_ownership/non-inline-mod-restriction.rs diff --git a/src/test/compile-fail/directory_ownership/unowned_mod_with_path.rs b/src/test/compile-fail/directory_ownership/unowned_mod_with_path.rs new file mode 100644 index 0000000000000..854f790befcf7 --- /dev/null +++ b/src/test/compile-fail/directory_ownership/unowned_mod_with_path.rs @@ -0,0 +1,15 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// error-pattern: cannot declare a new module at this location + +// This is not a directory owner since the file name is not "mod.rs". +#[path = "mod_file_not_owning_aux1.rs"] +mod foo; From 808a7ca805e25fd60bfbdfce5780a05e98b5f1b1 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Sat, 5 Nov 2016 10:46:44 +0000 Subject: [PATCH 3/4] Fix fallout in tests. --- src/test/parse-fail/circular_modules_hello.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/parse-fail/circular_modules_hello.rs b/src/test/parse-fail/circular_modules_hello.rs index 4de817dbd9ca7..94770aa875b42 100644 --- a/src/test/parse-fail/circular_modules_hello.rs +++ b/src/test/parse-fail/circular_modules_hello.rs @@ -12,6 +12,7 @@ // ignore-test: this is an auxiliary file for circular-modules-main.rs +#[path = "circular_modules_main.rs"] mod circular_modules_main; pub fn say_hello() { From fa8c53bae4592395565fc3c06cdcc093fc4b0dbe Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Mon, 14 Nov 2016 09:31:03 +0000 Subject: [PATCH 4/4] Start warning cycle. --- src/librustc/lint/builtin.rs | 10 +++- src/librustc_lint/lib.rs | 4 ++ src/librustc_passes/ast_validation.rs | 7 +++ src/libsyntax/ext/expand.rs | 2 +- src/libsyntax/parse/mod.rs | 2 +- src/libsyntax/parse/parser.rs | 46 +++++++++++++++---- .../backcompat-warnings.rs | 21 +++++++++ .../mod_file_not_owning_aux3.rs | 13 ++++++ 8 files changed, 92 insertions(+), 13 deletions(-) create mode 100644 src/test/compile-fail/directory_ownership/backcompat-warnings.rs create mode 100644 src/test/compile-fail/directory_ownership/mod_file_not_owning_aux3.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 82a46f76401d5..c369bc10e9482 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -204,6 +204,13 @@ declare_lint! { "detects extra requirements in impls that were erroneously allowed" } +declare_lint! { + pub LEGACY_DIRECTORY_OWNERSHIP, + Warn, + "non-inline, non-`#[path]` modules (e.g. `mod foo;`) were erroneously allowed in some files \ + not named `mod.rs`" +} + /// Does nothing as a lint pass, but registers some `Lint`s /// which are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -242,7 +249,8 @@ impl LintPass for HardwiredLints { LIFETIME_UNDERSCORE, SAFE_EXTERN_STATICS, PATTERNS_IN_FNS_WITHOUT_BODY, - EXTRA_REQUIREMENT_IN_IMPL + EXTRA_REQUIREMENT_IN_IMPL, + LEGACY_DIRECTORY_OWNERSHIP ) } } diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 114c0ea556ef5..1a3ea5db871eb 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -232,6 +232,10 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { id: LintId::of(EXTRA_REQUIREMENT_IN_IMPL), reference: "issue #37166 ", }, + FutureIncompatibleInfo { + id: LintId::of(LEGACY_DIRECTORY_OWNERSHIP), + reference: "issue #37872 ", + }, ]); // Register renamed and removed lints diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 89c3efaafcdcc..105155d75aba6 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -207,6 +207,13 @@ impl<'a> Visitor for AstValidator<'a> { ItemKind::Mod(_) => { // Ensure that `path` attributes on modules are recorded as used (c.f. #35584). attr::first_attr_value_str_by_name(&item.attrs, "path"); + if let Some(attr) = + item.attrs.iter().find(|attr| attr.name() == "warn_directory_ownership") { + let lint = lint::builtin::LEGACY_DIRECTORY_OWNERSHIP; + let msg = "cannot declare a new module at this location"; + self.session.add_lint(lint, item.id, item.span, msg.to_string()); + attr::mark_used(attr); + } } ItemKind::Union(ref vdata, _) => { if !vdata.is_struct() { diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 3e8f118ce62de..fd6cae1e1b668 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -790,7 +790,7 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> { PathBuf::from(self.cx.parse_sess.codemap().span_to_filename(inner)); let directory_ownership = match path.file_name().unwrap().to_str() { Some("mod.rs") => DirectoryOwnership::Owned, - _ => DirectoryOwnership::UnownedViaMod, + _ => DirectoryOwnership::UnownedViaMod(false), }; path.pop(); module.directory = path; diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index f969e45b83a8e..bfaf00a3d3f08 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -86,7 +86,7 @@ pub struct Directory { pub enum DirectoryOwnership { Owned, UnownedViaBlock, - UnownedViaMod, + UnownedViaMod(bool /* legacy warnings? */), } // a bunch of utility functions of the form parse__from_ diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index ee69125ffae83..b00e6b5d58f04 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}; @@ -243,6 +243,7 @@ pub struct ModulePath { pub struct ModulePathSuccess { pub path: PathBuf, pub directory_ownership: DirectoryOwnership, + warn: bool, } pub struct ModulePathError { @@ -5268,10 +5269,25 @@ impl<'a> Parser<'a> { self.bump(); if in_cfg { // This mod is in an external file. Let's go get it! - let ModulePathSuccess { path, directory_ownership } = + let ModulePathSuccess { path, directory_ownership, warn } = self.submod_path(id, &outer_attrs, id_span)?; - let (module, attrs) = + 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() }; @@ -5290,7 +5306,7 @@ 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") { + 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 { @@ -5299,7 +5315,7 @@ impl<'a> Parser<'a> { } pub fn submod_path_from_attr(attrs: &[ast::Attribute], dir_path: &Path) -> Option { - ::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 . @@ -5316,11 +5332,13 @@ impl<'a> Parser<'a> { let result = match (default_exists, secondary_exists) { (true, false) => Ok(ModulePathSuccess { path: default_path, - directory_ownership: DirectoryOwnership::UnownedViaMod, + 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), @@ -5353,9 +5371,10 @@ impl<'a> Parser<'a> { return Ok(ModulePathSuccess { directory_ownership: match path.file_name().and_then(|s| s.to_str()) { Some("mod.rs") => DirectoryOwnership::Owned, - _ => DirectoryOwnership::UnownedViaMod, + _ => DirectoryOwnership::UnownedViaMod(true), }, path: path, + warn: false, }); } @@ -5371,7 +5390,12 @@ impl<'a> Parser<'a> { err.span_note(id_sp, &msg); } return Err(err); - } else if let DirectoryOwnership::UnownedViaMod = self.directory.ownership { + } 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.path.file_name() { @@ -5387,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 { diff --git a/src/test/compile-fail/directory_ownership/backcompat-warnings.rs b/src/test/compile-fail/directory_ownership/backcompat-warnings.rs new file mode 100644 index 0000000000000..75e3426a39935 --- /dev/null +++ b/src/test/compile-fail/directory_ownership/backcompat-warnings.rs @@ -0,0 +1,21 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// error-pattern: cannot declare a new module at this location +// error-pattern: will become a hard error +// error-pattern: compilation successful + +#![feature(rustc_attrs)] + +#[path="mod_file_not_owning_aux3.rs"] +mod foo; + +#[rustc_error] +fn main() {} diff --git a/src/test/compile-fail/directory_ownership/mod_file_not_owning_aux3.rs b/src/test/compile-fail/directory_ownership/mod_file_not_owning_aux3.rs new file mode 100644 index 0000000000000..3a164fd55d927 --- /dev/null +++ b/src/test/compile-fail/directory_ownership/mod_file_not_owning_aux3.rs @@ -0,0 +1,13 @@ +// Copyright 2016 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// ignore-test this is not a test + +mod mod_file_not_owning_aux2;