Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(2)

Issue 3007803002: Migrate block 162 to Dart 2.0.

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 2 weeks ago by jcollins
Modified:
2 weeks, 6 days ago
Reviewers:
Bob Nystrom, eernst
CC:
reviews_dartlang.org, dart2now-team_google.com
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Migrate block 162 to Dart 2.0. Many changes large and small in this CL, most involve type_variable_bounds*_.test. Also please check on my use of the dynamic type in type_variable_function_type_test. I think it is correct, but not entirely sure. BUG=

Patch Set 1 #

Total comments: 63
Unified diffs Side-by-side diffs Delta from patch set Stats (+715 lines, -1988 lines) Patch
M tests/language/language.status View 2 chunks +0 lines, -7 lines 2 comments Download
M tests/language/language_analyzer2.status View 2 chunks +0 lines, -6 lines 3 comments Download
M tests/language/language_dart2js.status View 4 chunks +0 lines, -39 lines 4 comments Download
M tests/language/language_kernel.status View 1 chunk +0 lines, -1 line 1 comment Download
D tests/language/type_promotion_parameter_test.dart View 1 chunk +0 lines, -208 lines 1 comment Download
D tests/language/type_propagation2_test.dart View 1 chunk +0 lines, -24 lines 1 comment Download
D tests/language/type_propagation3_test.dart View 1 chunk +0 lines, -57 lines 1 comment Download
D tests/language/type_propagation_assert_assignable_test.dart View 1 chunk +0 lines, -47 lines 1 comment Download
D tests/language/type_propagation_in_for_update_test.dart View 1 chunk +0 lines, -28 lines 1 comment Download
D tests/language/type_propagation_phi_test.dart View 1 chunk +0 lines, -23 lines 1 comment Download
D tests/language/type_propagation_test.dart View 1 chunk +0 lines, -43 lines 1 comment Download
D tests/language/type_variable_bounds2_test.dart View 1 chunk +0 lines, -63 lines 1 comment Download
D tests/language/type_variable_bounds3_test.dart View 1 chunk +0 lines, -18 lines 1 comment Download
D tests/language/type_variable_bounds4_test.dart View 1 chunk +0 lines, -72 lines 1 comment Download
D tests/language/type_variable_bounds_test.dart View 1 chunk +0 lines, -75 lines 1 comment Download
D tests/language/type_variable_closure2_test.dart View 1 chunk +0 lines, -18 lines 1 comment Download
D tests/language/type_variable_closure3_test.dart View 1 chunk +0 lines, -18 lines 1 comment Download
D tests/language/type_variable_closure4_test.dart View 1 chunk +0 lines, -19 lines 1 comment Download
D tests/language/type_variable_closure_test.dart View 1 chunk +0 lines, -27 lines 1 comment Download
D tests/language/type_variable_conflict2_test.dart View 1 chunk +0 lines, -56 lines 1 comment Download
D tests/language/type_variable_conflict_test.dart View 1 chunk +0 lines, -41 lines 1 comment Download
D tests/language/type_variable_field_initializer2_test.dart View 1 chunk +0 lines, -21 lines 1 comment Download
D tests/language/type_variable_field_initializer_closure2_test.dart View 1 chunk +0 lines, -21 lines 1 comment Download
D tests/language/type_variable_field_initializer_closure_test.dart View 1 chunk +0 lines, -19 lines 1 comment Download
D tests/language/type_variable_field_initializer_test.dart View 1 chunk +0 lines, -19 lines 1 comment Download
D tests/language/type_variable_function_type_test.dart View 1 chunk +0 lines, -25 lines 1 comment Download
D tests/language/type_variable_identifier_expression_test.dart View 1 chunk +0 lines, -23 lines 1 comment Download
D tests/language/type_variable_initializer_test.dart View 1 chunk +0 lines, -24 lines 1 comment Download
M tests/language_2/language_2_analyzer.status View 1 chunk +90 lines, -1 line 0 comments Download
M tests/language_2/language_2_dart2js.status View 5 chunks +113 lines, -0 lines 1 comment Download
M tests/language_2/language_2_flutter.status View 1 chunk +6 lines, -0 lines 0 comments Download
M tests/language_2/language_2_precompiled.status View 1 chunk +85 lines, -0 lines 0 comments Download
M tests/language_2/language_2_vm.status View 1 chunk +85 lines, -0 lines 0 comments Download
A tests/language_2/type_promotion_parameter_test.dart View 1 chunk +208 lines, -0 lines 1 comment Download
A + tests/language_2/type_propagation2_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_propagation3_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_propagation_assert_assignable_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_propagation_in_for_update_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_propagation_phi_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_propagation_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A tests/language_2/type_variable_bounds2_test.dart View 1 chunk +63 lines, -0 lines 10 comments Download
A + tests/language_2/type_variable_bounds3_test.dart View 1 chunk +1 line, -1 line 3 comments Download
A tests/language_2/type_variable_bounds4_test.dart View 1 chunk +53 lines, -0 lines 8 comments Download
A + tests/language_2/type_variable_bounds_test.dart View 4 chunks +13 lines, -13 lines 5 comments Download
A + tests/language_2/type_variable_closure2_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_closure3_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_closure4_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_closure_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_conflict2_test.dart View 1 chunk +12 lines, -12 lines 1 comment Download
A + tests/language_2/type_variable_conflict_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_field_initializer2_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_field_initializer_closure2_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_field_initializer_closure_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_field_initializer_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
A + tests/language_2/type_variable_function_type_test.dart View 1 chunk +1 line, -1 line 0 comments Download
A + tests/language_2/type_variable_identifier_expression_test.dart View 1 chunk +1 line, -1 line 0 comments Download
A + tests/language_2/type_variable_initializer_test.dart View 0 chunks +-1 lines, --1 lines 0 comments Download
M tests/language_strong/language_strong.status View 1 chunk +0 lines, -9 lines 0 comments Download
D tests/language_strong/type_promotion_parameter_test.dart View 1 chunk +0 lines, -208 lines 0 comments Download
D tests/language_strong/type_propagation2_test.dart View 1 chunk +0 lines, -24 lines 0 comments Download
D tests/language_strong/type_propagation3_test.dart View 1 chunk +0 lines, -57 lines 0 comments Download
D tests/language_strong/type_propagation_assert_assignable_test.dart View 1 chunk +0 lines, -47 lines 0 comments Download
D tests/language_strong/type_propagation_in_for_update_test.dart View 1 chunk +0 lines, -28 lines 0 comments Download
D tests/language_strong/type_propagation_phi_test.dart View 1 chunk +0 lines, -23 lines 0 comments Download
D tests/language_strong/type_propagation_test.dart View 1 chunk +0 lines, -43 lines 0 comments Download
D tests/language_strong/type_variable_bounds2_test.dart View 1 chunk +0 lines, -63 lines 0 comments Download
D tests/language_strong/type_variable_bounds3_test.dart View 1 chunk +0 lines, -18 lines 0 comments Download
D tests/language_strong/type_variable_bounds4_test.dart View 1 chunk +0 lines, -72 lines 0 comments Download
D tests/language_strong/type_variable_bounds_test.dart View 1 chunk +0 lines, -75 lines 0 comments Download
D tests/language_strong/type_variable_closure2_test.dart View 1 chunk +0 lines, -31 lines 0 comments Download
D tests/language_strong/type_variable_closure_test.dart View 1 chunk +0 lines, -27 lines 0 comments Download
D tests/language_strong/type_variable_conflict2_test.dart View 1 chunk +0 lines, -57 lines 0 comments Download
D tests/language_strong/type_variable_conflict_test.dart View 1 chunk +0 lines, -41 lines 0 comments Download
D tests/language_strong/type_variable_field_initializer_closure_test.dart View 1 chunk +0 lines, -19 lines 0 comments Download
D tests/language_strong/type_variable_field_initializer_test.dart View 1 chunk +0 lines, -19 lines 0 comments Download
D tests/language_strong/type_variable_function_type_test.dart View 1 chunk +0 lines, -25 lines 0 comments Download
D tests/language_strong/type_variable_identifier_expression_test.dart View 1 chunk +0 lines, -23 lines 0 comments Download
D tests/language_strong/type_variable_initializer_test.dart View 1 chunk +0 lines, -24 lines 0 comments Download
Trybot results: Sign in to try more bots
Commit queue not available (can’t edit this change).

Messages

Total messages: 7 (1 generated)
jcollins
1 month, 2 weeks ago (2017-08-29 20:34:05 UTC) #2
Bob Nystrom
Adding Leaf as a reviewer, at least for type_variable_bounds2_test.dart. I'd really like Erik or Leaf ...
1 month, 2 weeks ago (2017-08-30 20:02:44 UTC) #3
eernst
Looked through all of these (takes time ;-), and there are lots of things that ...
1 month, 2 weeks ago (2017-09-04 17:05:02 UTC) #4
Bob Nystrom
On 2017/09/04 17:05:02, eernst wrote: > There are several cases where it is not trivial ...
1 month, 1 week ago (2017-09-05 22:32:01 UTC) #5
eernst
Hi, trying to catch up with this CL, is it correct that it hasn't been ...
1 month, 1 week ago (2017-09-08 11:20:28 UTC) #6
jcollins
2 weeks, 6 days ago (2017-09-28 21:16:29 UTC) #7
Implemented review comments in
https://dart-review.googlesource.com/c/sdk/+/9360.

https://codereview.chromium.org/3007803002/diff/1/tests/language_2/type_varia...
File tests/language_2/type_variable_bounds3_test.dart (right):

https://codereview.chromium.org/3007803002/diff/1/tests/language_2/type_varia...
tests/language_2/type_variable_bounds3_test.dart:9: class B<X, Y> {
On 2017/09/04 17:05:00, eernst wrote:
> On 2017/08/30 20:02:44, Bob Nystrom wrote:
> > Is "Y" used for anything? Why is it here?
> 
> Agreed that `Y` seems to be noise-only here.
> 
> Another matter is that every test involving a type argument might be handled
> differently when (1) there is only that type argument, (2) there are several
> type arguments, and the one we are working with is first/last/in-the-middle,
> etc.
> 
> We could use code inspection techniques to check that every tool treats every
> type argument exactly the same, and in that case we could eliminate all this
> noise. But even though it seems to be a reasonable expectation, I don't think
we
> can maintain such a discipline (we aren't actually going to check after every
> commit that every tool still treats type variables identically).
> 
> We could also insist that every test must try all cases in this respect, but
> that would create a huge number of very similar tests.
> 
> Concrete coverage data would be even better, of course, but my conclusion
right
> now is that we should probably allow for a certain amount of noise of this
kind,
> rather than trying to eliminate it whenever we see it.
> 
> So I'd be happy if we just leave it alone, and if we act in the same way
> whenever we see small amounts of "noise" like this.

+1.  I've resisted dropping noise since I noticed when I did so once, a dart2js
test that was crashing magically started working fine.  There are the knowledge
of debuggers gone before us embedded in some of that noise -- the original
author presumably had some cause to do it -- and without concrete coverage data
I'm wary of removing noise too proactively.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 81bcdb8aa