Skip to content

syntax: Tighten search paths for inner modules #14251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 19, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
File renamed without changes.
2 changes: 2 additions & 0 deletions src/libsyntax/ext/source_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ pub fn expand_include(cx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
&res_rel_file(cx,
sp,
&Path::new(file)),
true,
None,
sp);
base::MacExpr::new(p.parse_expr())
}
Expand Down
7 changes: 6 additions & 1 deletion src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,13 @@ pub fn new_parser_from_file<'a>(sess: &'a ParseSess,
pub fn new_sub_parser_from_file<'a>(sess: &'a ParseSess,
cfg: ast::CrateConfig,
path: &Path,
owns_directory: bool,
module_name: Option<StrBuf>,
sp: Span) -> Parser<'a> {
filemap_to_parser(sess, file_to_filemap(sess, path, Some(sp)), cfg)
let mut p = filemap_to_parser(sess, file_to_filemap(sess, path, Some(sp)), cfg);
p.owns_directory = owns_directory;
p.root_module_name = module_name;
p
}

/// Given a filemap and config, return a parser
Expand Down
50 changes: 44 additions & 6 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,8 @@ pub fn Parser<'a>(
obsolete_set: HashSet::new(),
mod_path_stack: Vec::new(),
open_braces: Vec::new(),
owns_directory: true,
root_module_name: None,
}
}

Expand Down Expand Up @@ -342,6 +344,13 @@ pub struct Parser<'a> {
pub mod_path_stack: Vec<InternedString>,
/// Stack of spans of open delimiters. Used for error message.
pub open_braces: Vec<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.
pub root_module_name: Option<StrBuf>,
}

fn is_plain_ident_or_underscore(t: &token::Token) -> bool {
Expand Down Expand Up @@ -4179,9 +4188,12 @@ impl<'a> Parser<'a> {
self.push_mod_path(id, outer_attrs);
self.expect(&token::LBRACE);
let mod_inner_lo = self.span.lo;
let old_owns_directory = self.owns_directory;
self.owns_directory = true;
let (inner, next) = self.parse_inner_attrs_and_next();
let m = self.parse_mod_items(token::RBRACE, next, mod_inner_lo);
self.expect(&token::RBRACE);
self.owns_directory = old_owns_directory;
self.pop_mod_path();
(id, ItemMod(m), Some(inner))
}
Expand Down Expand Up @@ -4211,21 +4223,42 @@ impl<'a> Parser<'a> {
prefix.pop();
let mod_path = Path::new(".").join_many(self.mod_path_stack.as_slice());
let dir_path = prefix.join(&mod_path);
let file_path = match ::attr::first_attr_value_str_by_name(
let mod_string = token::get_ident(id);
let (file_path, owns_directory) = match ::attr::first_attr_value_str_by_name(
outer_attrs, "path") {
Some(d) => dir_path.join(d),
Some(d) => (dir_path.join(d), true),
None => {
let mod_string = token::get_ident(id);
let mod_name = mod_string.get().to_owned();
let default_path_str = mod_name + ".rs";
let secondary_path_str = mod_name + "/mod.rs";
let default_path = dir_path.join(default_path_str.as_slice());
let secondary_path = dir_path.join(secondary_path_str.as_slice());
let default_exists = default_path.exists();
let secondary_exists = secondary_path.exists();

if !self.owns_directory {
self.span_err(id_sp,
"cannot declare a new module at this location");
let this_module = match self.mod_path_stack.last() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When does the self.module_name branch get taken? I.e. shouldn't mod_path_stack always have the current module at the end?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see, it's just used for the recursive parse thing. Shouldn't the recursive parser just push to mod_path_stack? (If not, could module_name be changed to be called something slightly less general?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some comments to the field and renamed to root_module_name. The recursive parser doesn't share the parent's mod_path_stack, and pushing onto the stack would mess with further mod foo; declarations (appending an extra component on the path).

Some(name) => name.get().to_strbuf(),
None => self.root_module_name.get_ref().clone(),
};
self.span_note(id_sp,
format!("maybe move this module `{0}` \
to its own directory via \
`{0}/mod.rs`", this_module));
if default_exists || secondary_exists {
self.span_note(id_sp,
format!("... or maybe `use` the module \
`{}` instead of possibly \
redeclaring it", mod_name));
}
self.abort_if_errors();
}

match (default_exists, secondary_exists) {
(true, false) => default_path,
(false, true) => secondary_path,
(true, false) => (default_path, false),
(false, true) => (secondary_path, true),
(false, false) => {
self.span_fatal(id_sp, format!("file not found for module `{}`", mod_name));
}
Expand All @@ -4238,11 +4271,14 @@ impl<'a> Parser<'a> {
}
};

self.eval_src_mod_from_path(file_path, id_sp)
self.eval_src_mod_from_path(file_path, owns_directory,
mod_string.get().to_strbuf(), id_sp)
}

fn eval_src_mod_from_path(&mut self,
path: Path,
owns_directory: bool,
name: StrBuf,
id_sp: Span) -> (ast::Item_, Vec<ast::Attribute> ) {
let mut included_mod_stack = self.sess.included_mod_stack.borrow_mut();
match included_mod_stack.iter().position(|p| *p == path) {
Expand All @@ -4265,6 +4301,8 @@ impl<'a> Parser<'a> {
new_sub_parser_from_file(self.sess,
self.cfg.clone(),
&path,
owns_directory,
Some(name),
id_sp);
let mod_inner_lo = p0.span.lo;
let (mod_attrs, next) = p0.parse_inner_attrs_and_next();
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/circular_modules_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.


#[path = "circular_modules_hello.rs"]
mod circular_modules_hello; //~ERROR: circular modules

pub fn hi_str() -> StrBuf {
Expand Down
15 changes: 15 additions & 0 deletions src/test/compile-fail/mod_file_not_owning.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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

mod mod_file_not_owning_aux1;

fn main() {}
13 changes: 13 additions & 0 deletions src/test/compile-fail/mod_file_not_owning_aux1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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;
11 changes: 11 additions & 0 deletions src/test/compile-fail/mod_file_not_owning_aux2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright 2014 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// ignore-test this is not a test
1 change: 1 addition & 0 deletions src/test/run-pass/mod_dir_simple/load_another_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#[path = "test.rs"]
pub mod test;