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

Issue 2125823002: Handle redirects to unresolved redirects (Closed)

Created:
4 years, 5 months ago by Johnni Winther
Modified:
4 years, 5 months ago
Reviewers:
Harry Terkelsen
CC:
reviews_dartlang.org
Base URL:
https://github.com/dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Handle redirects to unresolved redirects The solves the handling of redirecting factories that redirects to other redirecting factories whos immediate redirection target is unresolved. For instance class C { factory C.a() = C.b; factory C.b() = Unresolved; } The implementation normalizes the properties 'immediateRedirectionTarget', 'redirectionDeferredPrefix', 'effectiveTarget', 'effectiveTargetType', and 'isEffectiveTargetMalformed' by always storing the information on the implementation element. R=het@google.com Committed: https://github.com/dart-lang/sdk/commit/1f1e00f0126c2d62a893b87c1b649115c4e45974

Patch Set 1 #

Total comments: 7
Unified diffs Side-by-side diffs Delta from patch set Stats (+182 lines, -63 lines) Patch
M pkg/compiler/lib/src/elements/modelx.dart View 4 chunks +87 lines, -22 lines 4 comments Download
M pkg/compiler/lib/src/resolution/members.dart View 1 chunk +2 lines, -5 lines 0 comments Download
M pkg/compiler/lib/src/resolution/resolution.dart View 3 chunks +12 lines, -19 lines 1 comment Download
M pkg/compiler/lib/src/serialization/element_serialization.dart View 1 chunk +2 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/serialization/equivalence.dart View 1 chunk +7 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/serialization/keys.dart View 1 chunk +2 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/serialization/modelz.dart View 3 chunks +11 lines, -2 lines 0 comments Download
M tests/compiler/dart2js/patch_test.dart View 3 chunks +27 lines, -2 lines 0 comments Download
M tests/compiler/dart2js/serialization/equivalence_test.dart View 4 chunks +24 lines, -2 lines 2 comments Download
M tests/compiler/dart2js/serialization/test_data.dart View 2 chunks +2 lines, -2 lines 0 comments Download
M tests/language/factory5_test.dart View 1 chunk +0 lines, -1 line 0 comments Download
A + tests/language/factory6_test.dart View 1 chunk +6 lines, -8 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
Johnni Winther
4 years, 5 months ago (2016-07-06 11:16:44 UTC) #2
Harry Terkelsen
lgtm lgtm! I really like this refactoring https://codereview.chromium.org/2125823002/diff/1/pkg/compiler/lib/src/elements/modelx.dart File pkg/compiler/lib/src/elements/modelx.dart (right): https://codereview.chromium.org/2125823002/diff/1/pkg/compiler/lib/src/elements/modelx.dart#newcode412 pkg/compiler/lib/src/elements/modelx.dart:412: throw new ...
4 years, 5 months ago (2016-07-06 16:40:07 UTC) #3
Johnni Winther
Committed patchset #1 (id:1) manually as 1f1e00f0126c2d62a893b87c1b649115c4e45974 (presubmit successful).
4 years, 5 months ago (2016-07-08 09:14:26 UTC) #5
Johnni Winther
4 years, 5 months ago (2016-07-08 09:17:26 UTC) #6
Message was sent while issue was closed.
https://codereview.chromium.org/2125823002/diff/1/pkg/compiler/lib/src/elemen...
File pkg/compiler/lib/src/elements/modelx.dart (right):

https://codereview.chromium.org/2125823002/diff/1/pkg/compiler/lib/src/elemen...
pkg/compiler/lib/src/elements/modelx.dart:412: throw new
UnsupportedError("setImmediateRedirectionTarget=");
On 2016/07/06 16:40:07, Harry Terkelsen wrote:
> nit: remove the '='

Done.

https://codereview.chromium.org/2125823002/diff/1/pkg/compiler/lib/src/elemen...
pkg/compiler/lib/src/elements/modelx.dart:2307: if (isPatched) {
On 2016/07/06 16:40:07, Harry Terkelsen wrote:
> given how every method in this class has a different behavior if it is patched
> or not, I wonder if it would be worth it to have a PatchedConstructorElementX
> class...

Unfortunately we don't know if it is going to be patched at creation time :(

https://codereview.chromium.org/2125823002/diff/1/tests/compiler/dart2js/seri...
File tests/compiler/dart2js/serialization/equivalence_test.dart (right):

https://codereview.chromium.org/2125823002/diff/1/tests/compiler/dart2js/seri...
tests/compiler/dart2js/serialization/equivalence_test.dart:854: check(element1,
element2, 'messageKind', element1.messageKind, element2.messageKind);
On 2016/07/06 16:40:07, Harry Terkelsen wrote:
> dartfmt

Really ?!? ;)

Powered by Google App Engine
This is Rietveld 408576698