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

Allow "rethrow" to end a switch case #27650

Closed
munificent opened this issue Oct 21, 2016 · 7 comments
Closed

Allow "rethrow" to end a switch case #27650

munificent opened this issue Oct 21, 2016 · 7 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).

Comments

@munificent
Copy link
Member

Here's a goofy corner of the language:

try {
  ...
} catch (err) {
  switch (err) {
    case 1:
      rethrow;
    case 2:
      print("Another case.");
  }
}

Analyzer reports:

[error] The last statement of the 'case' should be 'break', 'continue', 'return' or 'throw'. (...temp.dart, line 20, col 7)

This is correct. The spec does not mention allow rethrow to end a clause. It probably should?

@munificent munificent added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Oct 21, 2016
@bwilkerson
Copy link
Member

See also #21822

@lrhn
Copy link
Member

lrhn commented Oct 21, 2016

The current specification is too restrictive to be really helpful.
It doesn't have a proper analysis, and without that kind of intelligence behind the warnings, they do occasionally look pretty stupid. Giving warnings that the user can clearly see are wrong is not being helpful.

Personally, I would just remove the warning and the throw at the end of switch cases, and just break the switch when a case's statements end. Simpler for everybody, except those that expect fall-through, they won't get any warnings.

@floitschG
Copy link
Contributor

In the long term I agree with Lasse.
However, it seems like a very easy fix to just add rethrow to the list of accepted switch breaks.

@kmillikin
Copy link

Also see #7537

lrhn added a commit that referenced this issue Oct 25, 2016
Add `rethrow` as one of the statements that are allowed to end a switch case
without a warning.
Change wording to not claim that `throw` is a statement.
Also allow wrapping case statements in a block statement without causing
more warnings. Currently, a `case 4: { break; }` is required to warn because
the last statement is a block statement, not a break statement.

Fixes issue #27650
BUG= http://dartbug.com/27650
R=eernst@google.com, floitsch@google.com, rnystrom@google.com

Review URL: https://codereview.chromium.org/2447613003 .
@lrhn lrhn closed this as completed Oct 27, 2016
@munificent
Copy link
Member Author

This is fixed in the spec, but have the implementations implemented it? Have we documented it? Written tests?

@lrhn
Copy link
Member

lrhn commented Nov 3, 2016

It's also fixed in the analyzer (1322877). Since this is entirely a warning issue, that's the extend of the implementation.

There is a test (1d4657f).

As for documentation, it is in the spec, and it is (now) in the CHANGELOG.md (be4b4c6)

It seems dart2js doesn't check anyway, so I ignored it.

@munificent
Copy link
Member Author

it is (now) in the CHANGELOG.md (be4b4c6)

Groovy, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).
Projects
None yet
Development

No branches or pull requests

5 participants