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

Try any expression #8071

Merged
merged 1 commit into from Jun 18, 2019
Merged

Conversation

som-snytt
Copy link
Contributor

Previously, parsing try was special-cased for expressions
beginning with paren or brace. Perhaps that was to avoid odd
sights such as try (1,2,3) or try { case _ => }, but it
also broke try { 1 } + 1 finally ().

This commit takes an arbitrary expression to try.

This is just the parser change for try from #4400

Fixes scala/bug#5887

Previously, parsing `try` was special-cased for expressions
beginning with paren or brace. Perhaps that was to avoid odd
sights such as `try (1,2,3)` or `try { case _ => }`, but it
also broke `try { 1 } + 1 finally ()`.

This commit takes an arbitrary expression to `try`.
@scala-jenkins scala-jenkins added this to the 2.13.1 milestone May 18, 2019
@hrhino
Copy link
Member

hrhino commented May 18, 2019

Try any patch twice.

case FINALLY => in.nextToken(); expr()
case _ => EmptyTree
case FINALLY => in.nextToken() ; expr()
case _ => EmptyTree
Copy link
Member

Choose a reason for hiding this comment

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

We don't need scalafmt, just somfmt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd changed it to if-else in the earlier patch, but now I always prefer match. This is how I typed it back in.


// was: value isDefinedAt is not a member of Int
// now: required: PartialFunction[Throwable,?]
//def f = try ??? catch 22
Copy link
Member

Choose a reason for hiding this comment

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

I think that's a good error message. Why not test it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That will come back with the change to catch, which I'll attempt thricely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pardon, this is the third time, not so much charmed as cut off at the knees. The next try for catch will be a fourth.

@SethTisue SethTisue merged commit bf5fefa into scala:2.13.x Jun 18, 2019
@som-snytt som-snytt deleted the issue/5887-2.13-try-only branch June 18, 2019 14:42
@szeiger szeiger added the release-notes worth highlighting in next release notes label Sep 9, 2019
@som-snytt
Copy link
Contributor Author

@hrhino this is when I laughed again at your previous comment.

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