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

Updated string literal design based on #199 #732

Merged
merged 10 commits into from
Aug 27, 2021
Merged

Updated string literal design based on #199 #732

merged 10 commits into from
Aug 27, 2021

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Aug 11, 2021

I've modified the text from the proposal slightly, focusing the overview more on a design setup, but mostly kept the details. One important thing here is I noticed that raw tab characters are disallowed -- this was a little buried before, and I've now updated the list of characters allowed in a string to exclude tabs. Additionally, I've noted \0D in the list of escapes as explicitly invalid. Can you skim through this and make sure it seems like a reasonable change?

@jonmeow jonmeow requested a review from zygoloid August 11, 2021 18:17
@jonmeow jonmeow requested review from a team as code owners August 11, 2021 18:17
@google-cla google-cla bot added the cla: yes PR meets CLA requirements according to bot. label Aug 11, 2021
docs/design/lexical_conventions/string_literals.md Outdated Show resolved Hide resolved
docs/design/lexical_conventions/string_literals.md Outdated Show resolved Hide resolved
docs/design/lexical_conventions/string_literals.md Outdated Show resolved Hide resolved
jonmeow added a commit that referenced this pull request Aug 12, 2021
Noticed this while working on #732, seemed easiest to split changes though.
@jonmeow jonmeow requested a review from zygoloid August 25, 2021 22:14
Copy link
Contributor

@zygoloid zygoloid 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. A couple of optional tweaks; feel free to ignore them if you prefer.

@jonmeow jonmeow merged commit c5590f9 into carbon-language:trunk Aug 27, 2021
@jonmeow jonmeow deleted the docs-string branch August 27, 2021 22:43
jonmeow added a commit that referenced this pull request Aug 30, 2021
This is based on discussion on #732: that we should probably parse the invalid whitespace, then reject it as part of string validation, rather than having different parses. I worry the question of "how is this parsed" may lead to subtly unexpected results if we aren't consistent, so I'm switching the logic from the lexer to the unescape library (and also adjusting the list of rejected whitespace).
chandlerc pushed a commit that referenced this pull request Jun 28, 2022
Noticed this while working on #732, seemed easiest to split changes though.
chandlerc pushed a commit that referenced this pull request Jun 28, 2022
I've modified the text from the proposal slightly, focusing the overview more on a design setup, but mostly kept the details. One important thing here is I noticed that raw tab characters are disallowed -- this was a little buried before, and I've now updated the list of characters allowed in a string to exclude tabs. Additionally, I've noted `\0D` in the list of escapes as explicitly invalid.

Co-authored-by: Richard Smith <[email protected]>
chandlerc pushed a commit that referenced this pull request Jun 28, 2022
This is based on discussion on #732: that we should probably parse the invalid whitespace, then reject it as part of string validation, rather than having different parses. I worry the question of "how is this parsed" may lead to subtly unexpected results if we aren't consistent, so I'm switching the logic from the lexer to the unescape library (and also adjusting the list of rejected whitespace).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes PR meets CLA requirements according to bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants