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

Conversation

nrc
Copy link
Member

@nrc nrc commented May 17, 2017

This is useful if parsing from stdin or a String and don't want to try and read in a module from another file. Instead we just leave a stub in the AST.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

This is useful if parsing from stdin or a String and don't want to try and read in a module from another file. Instead we just leave a stub in the AST.
@aidanhs aidanhs added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 18, 2017
@carols10cents
Copy link
Member

ping @nikomatsakis, pinging you on IRC too!

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Looks good. r=me with or without changing the order of the arguments to Parser::new

@@ -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

@@ -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.

@nikomatsakis nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2017
@nrc
Copy link
Member Author

nrc commented May 24, 2017

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented May 24, 2017

📌 Commit a256630 has been approved by nikomatsakis

@nrc nrc added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 24, 2017
Add an option to the parser to avoid parsing out of line modules

This is useful if parsing from stdin or a String and don't want to try and read in a module from another file. Instead we just leave a stub in the AST.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 24, 2017
Add an option to the parser to avoid parsing out of line modules

This is useful if parsing from stdin or a String and don't want to try and read in a module from another file. Instead we just leave a stub in the AST.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 24, 2017
Add an option to the parser to avoid parsing out of line modules

This is useful if parsing from stdin or a String and don't want to try and read in a module from another file. Instead we just leave a stub in the AST.
bors added a commit that referenced this pull request May 24, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 25, 2017
Add an option to the parser to avoid parsing out of line modules

This is useful if parsing from stdin or a String and don't want to try and read in a module from another file. Instead we just leave a stub in the AST.
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request May 25, 2017
Add an option to the parser to avoid parsing out of line modules

This is useful if parsing from stdin or a String and don't want to try and read in a module from another file. Instead we just leave a stub in the AST.
bors added a commit that referenced this pull request May 25, 2017
@bors bors merged commit a256630 into rust-lang:master May 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants