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

Issue 10961074: Separate type for dynamic in mirrors.dart2js (Closed)

Created:
8 years, 3 months ago by Johnni Winther
Modified:
8 years, 3 months ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Separate type for dynamic in mirrors.dart2js Committed: https://code.google.com/p/dart/source/detail?r=12769

Patch Set 1 #

Patch Set 2 : Identity used on dynamic type #

Total comments: 2

Patch Set 3 : Updated cf. comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+48 lines, -48 lines) Patch
M pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart View 1 2 11 chunks +48 lines, -48 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Johnni Winther
8 years, 3 months ago (2012-09-24 09:49:04 UTC) #1
Lasse Reichstein Nielsen
LGTM https://codereview.chromium.org/10961074/diff/2001/pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart File pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart (right): https://codereview.chromium.org/10961074/diff/2001/pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart#newcode1238 pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:1238: bool get isObject => false; Why not put ...
8 years, 3 months ago (2012-09-24 10:41:09 UTC) #2
Johnni Winther
8 years, 3 months ago (2012-09-24 10:58:18 UTC) #3
https://codereview.chromium.org/10961074/diff/2001/pkg/dartdoc/lib/src/mirror...
File pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart (right):

https://codereview.chromium.org/10961074/diff/2001/pkg/dartdoc/lib/src/mirror...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:1238: bool get isObject =>
false;
On 2012/09/24 10:41:09, Lasse Reichstein Nielsen wrote:
> Why not put these "false" getters on the supertype, and only override the true
> ones in subclasses? That seems like a quadratic->linear space optimization, if
> nothing else. (And it's easier to maintain - you only need to fix two places
> when adding a new one).

Done.

Powered by Google App Engine
This is Rietveld 408576698