Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(352)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2205263002: Implement fall-through errors. (Closed)

Created:
4 years, 4 months ago by ahe
Modified:
4 years, 4 months ago
Reviewers:
kasperl
CC:
rasta-dart+reviews_googlegroups.com
Base URL:
git@github.com:dart-lang/rasta.git@errors
Target Ref:
refs/heads/master
Project:
Rasta
Visibility:
Public.

Description

Implement fall-through errors. Fixes #71. BUG=https://github.com/dart-lang/rasta/issues/71 R=kasperl@google.com Committed: https://github.com/dart-lang/rasta/commit/eeff0b8400f23865fc3bf1e07947eedc7eb55be7

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+238 lines, -0 lines) Patch
M dart_vm_standalone/rasta_errors.dart View 1 chunk +4 lines, -0 lines 0 comments Download
M lib/kernel.dart View 1 chunk +4 lines, -0 lines 0 comments Download
A lib/kernel/fall_through_visitor.dart View 1 chunk +97 lines, -0 lines 3 comments Download
M lib/kernel_visitor.dart View 2 chunks +11 lines, -0 lines 0 comments Download
A test/kernel/regression/switch_execution_case_t02.dart View 1 chunk +60 lines, -0 lines 0 comments Download
A test/kernel/regression/switch_execution_case_t02.dart.txt View 1 chunk +62 lines, -0 lines 0 comments Download

Depends on Patchset:

Issue 2204943003 Patch 1

Messages

Total messages: 7 (2 generated)
ahe
4 years, 4 months ago (2016-08-03 12:49:10 UTC) #2
kasperl
LGTM. https://codereview.chromium.org/2205263002/diff/1/lib/kernel/fall_through_visitor.dart File lib/kernel/fall_through_visitor.dart (right): https://codereview.chromium.org/2205263002/diff/1/lib/kernel/fall_through_visitor.dart#newcode50 lib/kernel/fall_through_visitor.dart:50: return node.statements.isEmpty || node.statements.last.accept(this); Wouldn't it be better ...
4 years, 4 months ago (2016-08-04 11:20:39 UTC) #3
ahe
https://codereview.chromium.org/2205263002/diff/1/lib/kernel/fall_through_visitor.dart File lib/kernel/fall_through_visitor.dart (right): https://codereview.chromium.org/2205263002/diff/1/lib/kernel/fall_through_visitor.dart#newcode50 lib/kernel/fall_through_visitor.dart:50: return node.statements.isEmpty || node.statements.last.accept(this); On 2016/08/04 11:20:38, kasperl wrote: ...
4 years, 4 months ago (2016-08-04 12:05:45 UTC) #4
ahe
Committed patchset #1 (id:1) manually as eeff0b8400f23865fc3bf1e07947eedc7eb55be7 (presubmit successful).
4 years, 4 months ago (2016-08-04 12:07:22 UTC) #6
ahe
4 years, 4 months ago (2016-08-04 12:27:27 UTC) #7
Message was sent while issue was closed. 
https://codereview.chromium.org/2205263002/diff/1/lib/kernel/fall_through_vis...
File lib/kernel/fall_through_visitor.dart (right):

https://codereview.chromium.org/2205263002/diff/1/lib/kernel/fall_through_vis...
lib/kernel/fall_through_visitor.dart:50: return node.statements.isEmpty ||
node.statements.last.accept(this);
On 2016/08/04 12:05:45, ahe wrote:
> On 2016/08/04 11:20:38, kasperl wrote:
> > Wouldn't it be better to loop through the statements and visit them and if
you
> > get a false result, you stop and return false. Otherwise, you return true?
> 
> Thank you, good idea. I'll do that in a follow-up.

Addressed in CL 2213933002.

Powered by Google App Engine
This is Rietveld 408576698