Skip to content

Conversation

@aochagavia
Copy link
Contributor

@aochagavia aochagavia commented Nov 9, 2018

Related: #6 (some validators are still missing), fixes #27

"PrefixExpr": (),
"RangeExpr": (),
"BinExpr": (),
"String": (),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any way we can avoid the name String here (bikeshed: StringLit, StringExpr etc.)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure... The token is called STRING and as far as I know the generator requires that you use the token name in CamelCase. Maybe @matklad knows?

// the newline, and all whitespace at the beginning of the next line are ignored
match self.peek() {
Some('\n') | Some('\r') => {
self.skip_whitespace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I right in assuming that this will eat the sequence: "\r\n" for windows?

Copy link
Contributor

Choose a reason for hiding this comment

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

\n is whitespace playground link, so it should do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does

}
}

#[cfg(test)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome

@kjeremy
Copy link
Contributor

kjeremy commented Nov 9, 2018

Is there any way we can void redefining String? Maybe it should be StringLiteral or something?

// the newline, and all whitespace at the beginning of the next line are ignored
match self.peek() {
Some('\n') | Some('\r') => {
self.skip_whitespace();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this eat a string of the form

"\
<all whitespace>
more text"

where there is an entirely whitespace line. See that \n is whitespace playground link.

The current behaviour appears to be that, unless I'm mistaken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will indeed eat all the whitespace, including new lines, as does the Rust compiler. See this playground link

@aochagavia
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Nov 10, 2018
222: Validate string literals r=aochagavia a=aochagavia

Related: #6 (some validators are still missing), fixes #27

Co-authored-by: Adolfo Ochagavía <[email protected]>
@bors
Copy link
Contributor

bors bot commented Nov 10, 2018

Build succeeded

@bors bors bot merged commit 3b4c02c into rust-lang:master Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prototype validators API

3 participants