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

Issue 2954523002: fix #27259, implement covariance checking for strong mode and DDC (Closed)

Created:
3 years, 6 months ago by Jennifer Messerly
Modified:
3 years, 5 months ago
Reviewers:
vsm, Leaf
CC:
dev-compiler+reviews_dartlang.org
Target Ref:
refs/heads/master
Visibility:
Public.

Description

fix #27259, implement covariance checking for strong mode and DDC R=vsm@google.com Committed: https://github.com/dart-lang/sdk/commit/90380dfdfa4c0feb1ed2a896c4ad9fb20774aa64

Patch Set 1 #

Patch Set 2 : rebase #

Total comments: 32

Patch Set 3 : refactor, still working on comments #

Patch Set 4 : add more comments #

Total comments: 8

Patch Set 5 : format #

Total comments: 5

Patch Set 6 : merged and fix an analysis error #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1329 lines, -320 lines) Patch
M pkg/analyzer/lib/src/dart/element/type.dart View 1 2 3 4 5 11 chunks +39 lines, -19 lines 0 comments Download
M pkg/analyzer/lib/src/generated/resolver.dart View 1 2 3 4 5 1 chunk +3 lines, -3 lines 0 comments Download
M pkg/analyzer/lib/src/generated/type_system.dart View 1 2 3 4 5 3 chunks +25 lines, -20 lines 0 comments Download
M pkg/analyzer/lib/src/task/strong/ast_properties.dart View 1 2 3 4 5 3 chunks +56 lines, -9 lines 0 comments Download
M pkg/analyzer/lib/src/task/strong/checker.dart View 1 2 3 4 5 17 chunks +456 lines, -55 lines 0 comments Download
M pkg/analyzer/test/generated/strong_mode_test.dart View 1 2 3 4 5 2 chunks +295 lines, -0 lines 0 comments Download
M pkg/dev_compiler/lib/js/legacy/dart_library.js View 1 2 3 4 5 3 chunks +14 lines, -1 line 0 comments Download
M pkg/dev_compiler/lib/src/compiler/code_generator.dart View 1 2 3 4 5 30 chunks +184 lines, -89 lines 0 comments Download
M pkg/dev_compiler/lib/src/compiler/compiler.dart View 1 2 3 4 5 4 chunks +0 lines, -32 lines 0 comments Download
M pkg/dev_compiler/lib/src/compiler/property_model.dart View 1 2 3 4 5 2 chunks +15 lines, -3 lines 0 comments Download
M pkg/dev_compiler/lib/src/compiler/reify_coercions.dart View 1 2 3 4 5 2 chunks +11 lines, -2 lines 0 comments Download
M pkg/dev_compiler/lib/src/compiler/type_utilities.dart View 1 2 3 4 5 3 chunks +5 lines, -17 lines 0 comments Download
M pkg/dev_compiler/test/browser/language_tests.js View 1 2 3 4 5 1 chunk +0 lines, -4 lines 0 comments Download
M pkg/dev_compiler/test/codegen_expected/closure.js View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
A tests/language_strong/covariant_subtyping_test.dart View 1 2 3 4 5 1 chunk +225 lines, -0 lines 0 comments Download
M tests/language_strong/type_hoisting_test.dart View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
D tests/language_strong/type_no_hoisting_test.dart View 1 2 3 4 5 1 chunk +0 lines, -64 lines 0 comments Download

Messages

Total messages: 17 (2 generated)
Jennifer Messerly
3 years, 6 months ago (2017-06-22 19:07:18 UTC) #2
vsm
https://codereview.chromium.org/2954523002/diff/10001/pkg/dev_compiler/lib/src/compiler/code_generator.dart File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right): https://codereview.chromium.org/2954523002/diff/10001/pkg/dev_compiler/lib/src/compiler/code_generator.dart#newcode704 pkg/dev_compiler/lib/src/compiler/code_generator.dart:704: if (!isImplicit && rules.isSubtypeOf(from, to)) return jsFrom; I'd like ...
3 years, 6 months ago (2017-06-23 15:59:08 UTC) #3
Jennifer Messerly
https://codereview.chromium.org/2954523002/diff/10001/pkg/dev_compiler/lib/src/compiler/code_generator.dart File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right): https://codereview.chromium.org/2954523002/diff/10001/pkg/dev_compiler/lib/src/compiler/code_generator.dart#newcode704 pkg/dev_compiler/lib/src/compiler/code_generator.dart:704: if (!isImplicit && rules.isSubtypeOf(from, to)) return jsFrom; On 2017/06/23 ...
3 years, 6 months ago (2017-06-23 18:36:21 UTC) #4
Jennifer Messerly
Maybe another way to say this: think about callee side checks? class List<T> { add(T ...
3 years, 6 months ago (2017-06-23 18:39:18 UTC) #5
Jennifer Messerly
On 2017/06/23 18:39:18, Jennifer Messerly wrote: > Maybe another way to say this: think about ...
3 years, 6 months ago (2017-06-23 18:43:43 UTC) #6
vsm
This is really cool! Still working my way through this. As discussed offline, can you ...
3 years, 5 months ago (2017-06-26 22:14:20 UTC) #7
Jennifer Messerly
On 2017/06/26 22:14:20, vsm wrote: > This is really cool! Still working my way through ...
3 years, 5 months ago (2017-06-27 20:42:59 UTC) #8
Leaf
This is epic! Some questions below. https://codereview.chromium.org/2954523002/diff/10001/pkg/analyzer/lib/src/dart/element/type.dart File pkg/analyzer/lib/src/dart/element/type.dart (right): https://codereview.chromium.org/2954523002/diff/10001/pkg/analyzer/lib/src/dart/element/type.dart#newcode1127 pkg/analyzer/lib/src/dart/element/type.dart:1127: * Compares two ...
3 years, 5 months ago (2017-06-28 18:12:30 UTC) #9
Jennifer Messerly
thank you Leaf!!! These are excellent observations! Quick reply as I get to work on ...
3 years, 5 months ago (2017-06-28 19:11:03 UTC) #10
Jennifer Messerly
Thanks everyone, please take another look! Leaf gave me a verbal okay to check in ...
3 years, 5 months ago (2017-07-05 20:11:21 UTC) #11
vsm
lgtm https://codereview.chromium.org/2954523002/diff/30001/pkg/analyzer/lib/src/dart/element/type.dart File pkg/analyzer/lib/src/dart/element/type.dart (right): https://codereview.chromium.org/2954523002/diff/30001/pkg/analyzer/lib/src/dart/element/type.dart#newcode1128 pkg/analyzer/lib/src/dart/element/type.dart:1128: * their corresponding parameters match [parameterRelation]. nit: s/their ...
3 years, 5 months ago (2017-07-05 22:57:34 UTC) #12
Jennifer Messerly
Thank you! One of your comments made me think of a nice refactoring in checker.dart ...
3 years, 5 months ago (2017-07-06 01:11:10 UTC) #13
vsm
lgtm! https://codereview.chromium.org/2954523002/diff/40001/pkg/analyzer/lib/src/task/strong/checker.dart File pkg/analyzer/lib/src/task/strong/checker.dart (right): https://codereview.chromium.org/2954523002/diff/40001/pkg/analyzer/lib/src/task/strong/checker.dart#newcode917 pkg/analyzer/lib/src/task/strong/checker.dart:917: /// has a known type arguments, such as ...
3 years, 5 months ago (2017-07-06 15:43:03 UTC) #14
Jennifer Messerly
thanks! https://codereview.chromium.org/2954523002/diff/40001/pkg/analyzer/lib/src/task/strong/checker.dart File pkg/analyzer/lib/src/task/strong/checker.dart (right): https://codereview.chromium.org/2954523002/diff/40001/pkg/analyzer/lib/src/task/strong/checker.dart#newcode917 pkg/analyzer/lib/src/task/strong/checker.dart:917: /// has a known type arguments, such as ...
3 years, 5 months ago (2017-07-06 21:31:05 UTC) #15
Jennifer Messerly
3 years, 5 months ago (2017-07-06 21:31:46 UTC) #17
Message was sent while issue was closed.
Committed patchset #6 (id:50001) manually as
90380dfdfa4c0feb1ed2a896c4ad9fb20774aa64 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698