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

SI-6810 Disallow EOL in char literal #4590

Merged
merged 3 commits into from Aug 24, 2015
Merged

Conversation

som-snytt
Copy link
Contributor

It's clear that char literals are one-lined like normal
string literals.

By the same token, pun intended, char literals accept
unicode escapes the same as string literals, including
\u000A.

This commit adds the usual exclusions (CR, NL, SU).

The spec is outdated in outlawing chars that are not
"printable", in particular, the ASCII control codes.
The original intention may have been that the ordinary
string escapes are required, such as "\b\n". Note that
some common escapes are absent, such as "\a".

Review by @adriaanm who wears specs

It's clear that char literals are one-lined like normal
string literals.

By the same token, pun intended, char literals accept
unicode escapes the same as string literals, including
`\u000A`.

This commit adds the usual exclusions (CR, NL, SU).

The spec is outdated in outlawing chars that are not
"printable", in particular, the ASCII control codes.
The original intention may have been that the ordinary
string escapes are required, such as "\b\n". Note that
some common escapes are absent, such as "\a".
@scala-jenkins scala-jenkins added this to the 2.12.0-M2 milestone Jun 29, 2015
Emphasize that literal parsing accepts Unicode escapes
as if they were escaped. In particular, a newline
represented by its Unicode escape does not terminate
the line in the middle of a literal.
@adriaanm adriaanm self-assigned this Jun 29, 2015
@adriaanm
Copy link
Contributor

This LGTM, but as we're currently flying blind (community build not yet green) and very close to a wall (M2 needs to be out before I fire up the grill this weekend), I'm pushing to M3. The grapevine has chatter of M3 by end of August.

@adriaanm adriaanm modified the milestones: 2.12.0-M3, 2.12.0-M2 Jun 29, 2015
@som-snytt
Copy link
Contributor Author

NP complete as in no problemo.

@lrytz
Copy link
Member

lrytz commented Jul 16, 2015

started a community build on this - also for me to get used to the new community build infra https://scala-ci.typesafe.com/job/scala-2.12.x-integrate-community-build/73/parameters/

@lrytz
Copy link
Member

lrytz commented Jul 16, 2015

LGTM. the failures in the community build run are unrelated:

@SethTisue
Copy link
Member

@adriaanm merge?

@som-snytt
Copy link
Contributor Author

I don't remember why this was worth a PR or what it was about, but I propose it as an easy merge b/c who cares. Cut down on the queue a bit.

For future reference, I suggest the Scala team only accept PRs that make strings harder to use, because people shouldn't be using strings so much anyway.

lrytz added a commit that referenced this pull request Aug 24, 2015
SI-6810 Disallow EOL in char literal
@lrytz lrytz merged commit 3d62009 into scala:2.12.x Aug 24, 2015
@lrytz
Copy link
Member

lrytz commented Aug 24, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants