Skip to content

Commit

Permalink
Auto merge of #37602 - jseyfried:directory_ownership, r=nikomatsakis
Browse files Browse the repository at this point in the history
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](#32401 (comment))),
 - 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
  • Loading branch information
bors authored Nov 22, 2016
2 parents 82d8833 + fa8c53b commit 1c11ea3
Show file tree
Hide file tree
Showing 20 changed files with 199 additions and 88 deletions.
10 changes: 9 additions & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
)
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/rust-lang/rust/issues/37166>",
},
FutureIncompatibleInfo {
id: LintId::of(LEGACY_DIRECTORY_OWNERSHIP),
reference: "issue #37872 <https://github.com/rust-lang/rust/issues/37872>",
},
]);

// Register renamed and removed lints
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
8 changes: 3 additions & 5 deletions src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -568,9 +568,7 @@ pub struct ExpansionData {
pub depth: usize,
pub backtrace: ExpnId,
pub module: Rc<ModuleData>,

// 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;
Expand Down Expand Up @@ -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,
},
}
}
Expand Down
24 changes: 15 additions & 9 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -727,9 +727,10 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
}

fn fold_block(&mut self, block: P<Block>) -> P<Block> {
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
}

Expand Down Expand Up @@ -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);

Expand All @@ -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(false),
};
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.
Expand Down
5 changes: 3 additions & 2 deletions src/libsyntax/ext/source_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
Expand Down
13 changes: 7 additions & 6 deletions src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
17 changes: 15 additions & 2 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(bool /* legacy warnings? */),
}

// a bunch of utility functions of the form parse_<thing>_from_<source>
// where <thing> includes crate, expr, item, stmt, tts, and one that
// uses a HOF to parse anything, and <source> includes file and
Expand Down Expand Up @@ -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<String>,
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
}
Expand Down
Loading

0 comments on commit 1c11ea3

Please sign in to comment.