Skip to content

Commit

Permalink
Auto merge of #55560 - cramertj:current-dir-path, r=<try>
Browse files Browse the repository at this point in the history
Make #[path] always relative to the current directory

Previously, #[path] would be relative to the current
directory plus an offset for any inline modules.
However, it did not respect the module offset based
on a non-"mod.rs" filename unless there was *also*
a nested module.

After this commit, #[path] ignores inline modules
and is only ever relative to the current directory.
This means that the old #[path = ...] mod x { ... }
(#[path] applied to inline modules) no longer has
any effect.

This is a [breaking change]. Opening to see what a crater run says.

r? @petrochenkov
  • Loading branch information
bors committed Nov 6, 2018
2 parents bdfeace + 561642c commit 3af441b
Show file tree
Hide file tree
Showing 13 changed files with 80 additions and 86 deletions.
2 changes: 1 addition & 1 deletion src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ impl<'a> ExtCtxt<'a> {
mark: Mark::root(),
depth: 0,
module: Rc::new(ModuleData { mod_path: Vec::new(), directory: PathBuf::new() }),
directory_ownership: DirectoryOwnership::Owned { relative: None },
directory_ownership: DirectoryOwnership::Owned { relative: vec![] },
crate_span: None,
},
expansions: FxHashMap::default(),
Expand Down
47 changes: 34 additions & 13 deletions src/libsyntax/ext/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1311,8 +1311,9 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
}

fn fold_block(&mut self, block: P<Block>) -> P<Block> {
let old_directory_ownership = self.cx.current_expansion.directory_ownership;
self.cx.current_expansion.directory_ownership = DirectoryOwnership::UnownedViaBlock;
let old_directory_ownership =
mem::replace(&mut self.cx.current_expansion.directory_ownership,
DirectoryOwnership::UnownedViaBlock);
let result = noop_fold_block(block, self);
self.cx.current_expansion.directory_ownership = old_directory_ownership;
result
Expand Down Expand Up @@ -1346,7 +1347,13 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
return noop_fold_item(item, self);
}

let orig_directory_ownership = self.cx.current_expansion.directory_ownership;
// If the directory ownership is replaced, this var
// holds the original so that it can be swapped back in.
let mut orig_directory_ownership = None;
// If the directory ownership `relative` field was appended to,
// this bool is `true` so that it can be popped at the end.
let mut directory_ownership_needs_pop = false;

let mut module = (*self.cx.current_expansion.module).clone();
module.mod_path.push(item.ident);

Expand All @@ -1356,12 +1363,11 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
let inline_module = item.span.contains(inner) || inner.is_dummy();

if inline_module {
if let Some(path) = attr::first_attr_value_str_by_name(&item.attrs, "path") {
self.cx.current_expansion.directory_ownership =
DirectoryOwnership::Owned { relative: None };
module.directory.push(&*path.as_str());
} else {
module.directory.push(&*item.ident.as_str());
if let DirectoryOwnership::Owned { relative } =
&mut self.cx.current_expansion.directory_ownership
{
relative.push(item.ident);
directory_ownership_needs_pop = true;
}
} else {
let path = self.cx.parse_sess.source_map().span_to_unmapped_path(inner);
Expand All @@ -1370,22 +1376,37 @@ impl<'a, 'b> Folder for InvocationCollector<'a, 'b> {
other => PathBuf::from(other.to_string()),
};
let directory_ownership = match path.file_name().unwrap().to_str() {
Some("mod.rs") => DirectoryOwnership::Owned { relative: None },
Some("mod.rs") => DirectoryOwnership::Owned { relative: vec![] },
Some(_) => DirectoryOwnership::Owned {
relative: Some(item.ident),
relative: vec![item.ident],
},
None => DirectoryOwnership::UnownedViaMod(false),
};
path.pop();
module.directory = path;
self.cx.current_expansion.directory_ownership = directory_ownership;
orig_directory_ownership =
Some(mem::replace(
&mut 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);

// Clean up, restoring all replaced or mutated expansion state.
self.cx.current_expansion.module = orig_module;
self.cx.current_expansion.directory_ownership = orig_directory_ownership;
if let Some(orig_directory_ownership) = orig_directory_ownership {
self.cx.current_expansion.directory_ownership = orig_directory_ownership;
}
if directory_ownership_needs_pop {
if let DirectoryOwnership::Owned { relative } =
&mut self.cx.current_expansion.directory_ownership
{
relative.pop();
}
}
result
}

Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/source_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ 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, file);
let directory_ownership = DirectoryOwnership::Owned { relative: None };
let directory_ownership = DirectoryOwnership::Owned { relative: vec![] };
let p = parse::new_sub_parser_from_file(cx.parse_sess(), &path, directory_ownership, None, sp);

struct ExpandResult<'a> {
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ fn generic_extension<'cx>(cx: &'cx mut ExtCtxt,

let directory = Directory {
path: Cow::from(cx.current_expansion.module.directory.as_path()),
ownership: cx.current_expansion.directory_ownership,
ownership: cx.current_expansion.directory_ownership.clone(),
};
let mut p = Parser::new(cx.parse_sess(), tts, Some(directory), true, false);
p.root_module_name = cx.current_expansion.module.mod_path.last()
Expand Down
1 change: 0 additions & 1 deletion src/libsyntax/parse/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1890,7 +1890,6 @@ mod tests {
missing_fragment_specifiers: Lock::new(FxHashSet::default()),
raw_identifier_spans: Lock::new(Vec::new()),
registered_diagnostics: Lock::new(ErrorMap::new()),
non_modrs_mods: Lock::new(vec![]),
buffered_lints: Lock::new(vec![]),
}
}
Expand Down
13 changes: 6 additions & 7 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ pub struct ParseSess {
pub raw_identifier_spans: Lock<Vec<Span>>,
/// The registered diagnostics codes
crate registered_diagnostics: Lock<ErrorMap>,
// Spans where a `mod foo;` statement was included in a non-mod.rs file.
// These are used to issue errors if the non_modrs_mods feature is not enabled.
pub non_modrs_mods: Lock<Vec<(ast::Ident, Span)>>,
/// Used to determine and report recursive mod inclusions
included_mod_stack: Lock<Vec<PathBuf>>,
source_map: Lrc<SourceMap>,
Expand All @@ -81,7 +78,6 @@ impl ParseSess {
registered_diagnostics: Lock::new(ErrorMap::new()),
included_mod_stack: Lock::new(vec![]),
source_map,
non_modrs_mods: Lock::new(vec![]),
buffered_lints: Lock::new(vec![]),
}
}
Expand Down Expand Up @@ -113,11 +109,14 @@ pub struct Directory<'a> {
pub ownership: DirectoryOwnership,
}

#[derive(Copy, Clone)]
#[derive(Clone)]
pub enum DirectoryOwnership {
Owned {
// None if `mod.rs`, `Some("foo")` if we're in `foo.rs`
relative: Option<ast::Ident>,
// Module offset from the current directory.
// Starts out as empty in `mod.rs`, starts as `["foo"]` in `foo.rs`.
// Contains one additional element for every inline nested module we've entered.
// e.g. `mod y { mod z { ... } }` in `x.rs` would have `["x", "y", "z"]`
relative: Vec<ast::Ident>,
},
UnownedViaBlock,
UnownedViaMod(bool /* legacy warnings? */),
Expand Down
74 changes: 24 additions & 50 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ impl<'a> Parser<'a> {
recurse_into_file_modules,
directory: Directory {
path: Cow::from(PathBuf::new()),
ownership: DirectoryOwnership::Owned { relative: None }
ownership: DirectoryOwnership::Owned { relative: vec![] }
},
root_module_name: None,
expected_tokens: Vec::new(),
Expand Down Expand Up @@ -6456,8 +6456,12 @@ impl<'a> Parser<'a> {
}
} else {
let old_directory = self.directory.clone();
self.push_directory(id, &outer_attrs);

// Push inline `mod x { ... }`'s `x` onto the `relative` offset of the module
// from the current directory's location. This ensures that `mod x { mod y; }`
// corresponds to `x/y.rs`, not `y.rs`
if let DirectoryOwnership::Owned { relative } = &mut self.directory.ownership {
relative.push(id);
}
self.expect(&token::OpenDelim(token::Brace))?;
let mod_inner_lo = self.span;
let attrs = self.parse_inner_attributes()?;
Expand All @@ -6468,26 +6472,6 @@ 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") {
self.directory.path.to_mut().push(&path.as_str());
self.directory.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
// `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 DirectoryOwnership::Owned { relative } = &mut self.directory.ownership {
if let Some(ident) = relative.take() { // remove the relative offset
self.directory.path.to_mut().push(ident.as_str());
}
}
self.directory.path.to_mut().push(&id.as_str());
}
}

pub fn submod_path_from_attr(attrs: &[Attribute], dir_path: &Path) -> Option<PathBuf> {
if let Some(s) = attr::first_attr_value_str_by_name(attrs, "path") {
let s = s.as_str();
Expand All @@ -6507,21 +6491,20 @@ impl<'a> Parser<'a> {
/// Returns either a path to a module, or .
pub fn default_submod_path(
id: ast::Ident,
relative: Option<ast::Ident>,
relative: &[ast::Ident],
dir_path: &Path,
source_map: &SourceMap) -> ModulePath
{
// If we're in a foo.rs file instead of a mod.rs file,
// we need to look for submodules in
// `./foo/<id>.rs` and `./foo/<id>/mod.rs` rather than
// `./<id>.rs` and `./<id>/mod.rs`.
let relative_prefix_string;
let relative_prefix = if let Some(ident) = relative {
relative_prefix_string = format!("{}{}", ident.as_str(), path::MAIN_SEPARATOR);
&relative_prefix_string
} else {
""
};
// Offset the current directory first by the name of
// the file if not `mod.rs`, then by any nested modules.
// e.g. `mod y { mod z; }` in `x.rs` should look for
// `./x/y/z.rs` and `./x/y/z/mod.rs` rather than
// `./z.rs` and `./z/mod.rs`.
let mut relative_prefix = String::new();
for ident in relative {
relative_prefix.push_str(&ident.as_str());
relative_prefix.push(path::MAIN_SEPARATOR);
}

let mod_name = id.to_string();
let default_path_str = format!("{}{}.rs", relative_prefix, mod_name);
Expand All @@ -6536,14 +6519,14 @@ impl<'a> Parser<'a> {
(true, false) => Ok(ModulePathSuccess {
path: default_path,
directory_ownership: DirectoryOwnership::Owned {
relative: Some(id),
relative: vec![id],
},
warn: false,
}),
(false, true) => Ok(ModulePathSuccess {
path: secondary_path,
directory_ownership: DirectoryOwnership::Owned {
relative: None,
relative: vec![],
},
warn: false,
}),
Expand Down Expand Up @@ -6582,27 +6565,18 @@ impl<'a> Parser<'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
Some(_) => DirectoryOwnership::Owned { relative: None },
Some(_) => DirectoryOwnership::Owned { relative: vec![] },
_ => DirectoryOwnership::UnownedViaMod(true),
},
path,
warn: false,
});
}

let relative = match self.directory.ownership {
DirectoryOwnership::Owned { relative } => {
// Push the usage onto the list of non-mod.rs mod uses.
// This is used later for feature-gate error reporting.
if let Some(cur_file_ident) = relative {
self.sess
.non_modrs_mods.borrow_mut()
.push((cur_file_ident, id_sp));
}
relative
},
let relative = match &self.directory.ownership {
DirectoryOwnership::Owned { relative } => &**relative,
DirectoryOwnership::UnownedViaBlock |
DirectoryOwnership::UnownedViaMod(_) => None,
DirectoryOwnership::UnownedViaMod(_) => &[],
};
let paths = Parser::default_submod_path(
id, relative, &self.directory.path, self.sess.source_map());
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/tokenstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl TokenTree {
// `None` is because we're not interpolating
let directory = Directory {
path: Cow::from(cx.current_expansion.module.directory.as_path()),
ownership: cx.current_expansion.directory_ownership,
ownership: cx.current_expansion.directory_ownership.clone(),
};
macro_parser::parse(cx.parse_sess(), tts, mtch, Some(directory), true)
}
Expand Down
10 changes: 6 additions & 4 deletions src/test/run-pass/modules/mod_dir_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,23 @@
// ignore-pretty issue #37195

mod mod_dir_simple {
#[path = "test.rs"]
#[path = "mod_dir_simple/test.rs"]
pub mod syrup;
}

pub fn main() {
assert_eq!(mod_dir_simple::syrup::foo(), 10);

#[path = "auxiliary"]
mod foo {
#[path = "auxiliary/two_macros_2.rs"]
mod two_macros_2;
}

#[path = "auxiliary"]
mod bar {
macro_rules! m { () => { mod two_macros_2; } }
macro_rules! m { () => {
#[path = "auxiliary/two_macros_2.rs"]
mod two_macros_2;
} }
m!();
}
}
3 changes: 1 addition & 2 deletions src/test/run-pass/modules/mod_dir_path2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@
// run-pass
// ignore-pretty issue #37195

#[path = "mod_dir_simple"]
mod pancakes {
#[path = "test.rs"]
#[path = "mod_dir_simple/test.rs"]
pub mod syrup;
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/run-pass/modules/mod_dir_path3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
// run-pass
// ignore-pretty issue #37195

#[path = "mod_dir_simple"]
mod pancakes {
#[path = "mod_dir_simple/test.rs"]
pub mod test;
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/run-pass/modules/mod_dir_path_multi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
// run-pass
// ignore-pretty issue #37195

#[path = "mod_dir_simple"]
mod biscuits {
#[path = "mod_dir_simple/test.rs"]
pub mod test;
}

#[path = "mod_dir_simple"]
mod gravy {
#[path = "mod_dir_simple/test.rs"]
pub mod test;
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/conditional-compilation/cfg_attr_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#![deny(unused_attributes)] // c.f #35584

mod auxiliary {
#[cfg_attr(any(), path = "nonexistent_file.rs")] pub mod namespaced_enums;
#[cfg_attr(all(), path = "namespaced_enums.rs")] pub mod nonexistent_file;
#[cfg_attr(any(), path = "auxiliary/nonexistent_file.rs")] pub mod namespaced_enums;
#[cfg_attr(all(), path = "auxiliary/namespaced_enums.rs")] pub mod nonexistent_file;
}

#[rustc_error]
Expand Down

0 comments on commit 3af441b

Please sign in to comment.