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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2139003002: Tighten check disallowing use of initializing formals (fixes #26855). (Closed)

Created:
4 years, 5 months ago by regis
Modified:
4 years, 5 months ago
Reviewers:
siva, hausner
CC:
reviews_dartlang.org, vm-dev_dartlang.org, hausner
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Tighten check disallowing use of initializing formals (fixes #26855). Add regression test. Add exception for analyzer (filed an issue). R=asiva@google.com Committed: https://github.com/dart-lang/sdk/commit/455110c6ee2d47c3aa526c0a4a259de80f48117f

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+34 lines, -2 lines) Patch
M runtime/vm/parser.h View 1 chunk +1 line, -2 lines 0 comments Download
M runtime/vm/parser.cc View 1 chunk +6 lines, -0 lines 2 comments Download
M tests/language/language_analyzer2.status View 1 chunk +1 line, -0 lines 0 comments Download
A tests/language/regress_26855_test.dart View 1 chunk +26 lines, -0 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
regis
4 years, 5 months ago (2016-07-11 22:04:39 UTC) #2
siva
lgtm
4 years, 5 months ago (2016-07-11 22:27:51 UTC) #3
regis
Committed patchset #1 (id:1) manually as 455110c6ee2d47c3aa526c0a4a259de80f48117f (presubmit successful).
4 years, 5 months ago (2016-07-11 22:30:29 UTC) #5
hausner
DBQ(uestion) https://codereview.chromium.org/2139003002/diff/1/runtime/vm/parser.cc File runtime/vm/parser.cc (right): https://codereview.chromium.org/2139003002/diff/1/runtime/vm/parser.cc#newcode3396 runtime/vm/parser.cc:3396: ReportError(param.name_pos, DBC: why does this check not cover ...
4 years, 5 months ago (2016-07-14 09:02:46 UTC) #7
regis
4 years, 5 months ago (2016-07-14 17:00:48 UTC) #8
Message was sent while issue was closed. 
https://codereview.chromium.org/2139003002/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2139003002/diff/1/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:3396: ReportError(param.name_pos,
On 2016/07/14 09:02:45, hausner wrote:
> DBC: why does this check not cover the cases in the regression test? 

There are 14 different call sites to ParseFormalParameterList in the parser.
This check only covers one call to ParseFormalParameterList on line 3377. The
cases of the regression test are parsed by other calls to 
ParseFormalParameterList. I did not want to duplicate this code everywhere and
found a common place to do the check.

Your question makes me realize that this check is actually redundant now, since
an error would be reported by the call at line 3384. I'll send you a cl removing
it. Thanks.

Powered by Google App Engine
This is Rietveld 408576698