Skip to content
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

Add an option to the parser to avoid parsing out of line modules #42071

Merged
merged 1 commit into from
May 25, 2017
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
2 changes: 1 addition & 1 deletion src/libsyntax/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ impl Attribute {
pub fn parse<'a, T, F>(&self, sess: &'a ParseSess, mut f: F) -> PResult<'a, T>
where F: FnMut(&mut Parser<'a>) -> PResult<'a, T>,
{
let mut parser = Parser::new(sess, self.tokens.clone(), None, false);
let mut parser = Parser::new(sess, self.tokens.clone(), None, false, false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you just using false here because we don't expect inline modules in attributes? In other words, we don't expect this option to matter here one way or the other, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct

let result = f(&mut parser)?;
if parser.token != token::Eof {
parser.unexpected()?;
Expand Down
8 changes: 6 additions & 2 deletions src/libsyntax/ext/tt/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,13 @@ fn inner_parse_loop(sess: &ParseSess,
Success(())
}

pub fn parse(sess: &ParseSess, tts: TokenStream, ms: &[TokenTree], directory: Option<Directory>)
pub fn parse(sess: &ParseSess,
tts: TokenStream,
ms: &[TokenTree],
directory: Option<Directory>,
recurse_into_modules: bool)
-> NamedParseResult {
let mut parser = Parser::new(sess, tts, directory, true);
let mut parser = Parser::new(sess, tts, directory, recurse_into_modules, true);
let mut cur_eis = SmallVector::one(initial_matcher_pos(ms.to_owned(), parser.span.lo));
let mut next_eis = Vec::new(); // or proceed normally

Expand Down
4 changes: 2 additions & 2 deletions src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ fn generic_extension<'cx>(cx: &'cx mut ExtCtxt,
path: cx.current_expansion.module.directory.clone(),
ownership: cx.current_expansion.directory_ownership,
};
let mut p = Parser::new(cx.parse_sess(), tts, Some(directory), false);
let mut p = Parser::new(cx.parse_sess(), tts, Some(directory), true, false);
p.root_module_name = cx.current_expansion.module.mod_path.last()
.map(|id| id.name.as_str().to_string());

Expand Down Expand Up @@ -192,7 +192,7 @@ pub fn compile(sess: &ParseSess, features: &RefCell<Features>, def: &ast::Item)
ast::ItemKind::MacroDef(ref body) => body.clone().into(),
_ => unreachable!(),
};
let argument_map = match parse(sess, body, &argument_gram, None) {
let argument_map = match parse(sess, body, &argument_gram, None, true) {
Success(m) => m,
Failure(sp, tok) => {
let s = parse_failure_msg(tok);
Expand Down
25 changes: 23 additions & 2 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,9 @@ pub fn parse_stream_from_source_str(name: String, source: String, sess: &ParseSe
// Create a new parser from a source string
pub fn new_parser_from_source_str(sess: &ParseSess, name: String, source: String)
-> Parser {
filemap_to_parser(sess, sess.codemap().new_filemap(name, source))
let mut parser = filemap_to_parser(sess, sess.codemap().new_filemap(name, source));
parser.recurse_into_file_modules = false;
parser
}

/// Create a new parser, handling errors as appropriate
Expand Down Expand Up @@ -218,7 +220,7 @@ pub fn filemap_to_stream(sess: &ParseSess, filemap: Rc<FileMap>) -> TokenStream

/// Given stream and the `ParseSess`, produce a parser
pub fn stream_to_parser(sess: &ParseSess, stream: TokenStream) -> Parser {
Parser::new(sess, stream, None, false)
Parser::new(sess, stream, None, true, false)
}

/// Parse a string representing a character literal into its final form.
Expand Down Expand Up @@ -1032,4 +1034,23 @@ mod tests {
Err(_) => panic!("could not get snippet"),
}
}

// This tests that when parsing a string (rather than a file) we don't try
// and read in a file for a module declaration and just parse a stub.
// See `recurse_into_file_modules` in the parser.
#[test]
fn out_of_line_mod() {
let sess = ParseSess::new(FilePathMapping::empty());
let item = parse_item_from_source_str(
"foo".to_owned(),
"mod foo { struct S; mod this_does_not_exist; }".to_owned(),
&sess,
).unwrap().unwrap();

if let ast::ItemKind::Mod(ref m) = item.node {
assert!(m.items.len() == 2);
} else {
panic!();
}
}
}
15 changes: 13 additions & 2 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ pub struct Parser<'a> {
pub obsolete_set: HashSet<ObsoleteSyntax>,
/// Used to determine the path to externally loaded source files
pub directory: Directory,
/// Whether to parse sub-modules in other files.
pub recurse_into_file_modules: 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.
Expand All @@ -190,6 +192,7 @@ pub struct Parser<'a> {
pub cfg_mods: bool,
}


struct TokenCursor {
frame: TokenCursorFrame,
stack: Vec<TokenCursorFrame>,
Expand Down Expand Up @@ -439,6 +442,7 @@ impl<'a> Parser<'a> {
pub fn new(sess: &'a ParseSess,
tokens: TokenStream,
directory: Option<Directory>,
recurse_into_file_modules: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really care, but adding the new argument "next-to-last" seems a bit error-prone, as I imagine people may be inclined add the new argument at the end of the list. More generally, having a lot of booleans is a crappy API, it'd be sort of nice to make this a builder-style thing... but I guess that's for another PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I put it here to be next to the directory option since they are related. My first attempt tried to combine the two into an enum, but it was really grotty. Builder would be best I guess, but yeah, later.

desugar_doc_comments: bool)
-> Self {
let mut parser = Parser {
Expand All @@ -450,6 +454,7 @@ impl<'a> Parser<'a> {
prev_token_kind: PrevTokenKind::Other,
restrictions: Restrictions::empty(),
obsolete_set: HashSet::new(),
recurse_into_file_modules: recurse_into_file_modules,
directory: Directory { path: PathBuf::new(), ownership: DirectoryOwnership::Owned },
root_module_name: None,
expected_tokens: Vec::new(),
Expand All @@ -467,12 +472,14 @@ impl<'a> Parser<'a> {
let tok = parser.next_tok();
parser.token = tok.tok;
parser.span = tok.sp;

if let Some(directory) = directory {
parser.directory = directory;
} else if parser.span != syntax_pos::DUMMY_SP {
parser.directory.path = PathBuf::from(sess.codemap().span_to_filename(parser.span));
parser.directory.path.pop();
}

parser.process_potential_macro_variable();
parser
}
Expand Down Expand Up @@ -3921,6 +3928,7 @@ impl<'a> Parser<'a> {
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,
Expand Down Expand Up @@ -5254,7 +5262,7 @@ impl<'a> Parser<'a> {
let id = self.parse_ident()?;
if self.check(&token::Semi) {
self.bump();
if in_cfg {
if in_cfg && self.recurse_into_file_modules {
// This mod is in an external file. Let's go get it!
let ModulePathSuccess { path, directory_ownership, warn } =
self.submod_path(id, &outer_attrs, id_span)?;
Expand All @@ -5281,10 +5289,12 @@ impl<'a> Parser<'a> {
} else {
let old_directory = self.directory.clone();
self.push_directory(id, &outer_attrs);

self.expect(&token::OpenDelim(token::Brace))?;
let mod_inner_lo = self.span;
let attrs = self.parse_inner_attributes()?;
let module = self.parse_mod_items(&token::CloseDelim(token::Brace), mod_inner_lo)?;

self.directory = old_directory;
Ok((id, ItemKind::Mod(module), Some(attrs)))
}
Expand Down Expand Up @@ -5347,7 +5357,8 @@ impl<'a> Parser<'a> {
fn submod_path(&mut self,
id: ast::Ident,
outer_attrs: &[ast::Attribute],
id_sp: Span) -> PResult<'a, ModulePathSuccess> {
id_sp: Span)
-> PResult<'a, ModulePathSuccess> {
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()) {
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/tokenstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl TokenTree {
path: cx.current_expansion.module.directory.clone(),
ownership: cx.current_expansion.directory_ownership,
};
macro_parser::parse(cx.parse_sess(), tts, mtch, Some(directory))
macro_parser::parse(cx.parse_sess(), tts, mtch, Some(directory), true)
}

/// Check if this TokenTree is equal to the other, regardless of span information.
Expand Down