Issue 11729004: Refactor location computation in dart2js mirrors.

Issue 11729004: Refactor location computation in dart2js mirrors. (Closed)

Created:
7 years, 11 months ago by Johnni Winther

Modified:
7 years, 11 months ago

Reviewers:
ahe

CC:
reviews_dartlang.org

Base URL:
https://dart.googlecode.com/svn/branches/bleeding_edge/dart

Visibility:
Public.

More Reviews

Description

Refactor location computation in dart2js mirrors. Committed: https://code.google.com/p/dart/source/detail?r=16631

Patch Set 1 #

Total comments: 4

Patch Set 2 : Updated cf. comments. Location test added. #

Total comments: 3

Created: 7 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+119 lines, -93 lines)			Patch
M	sdk/lib/_internal/compiler/implementation/elements/elements.dart	View	1	1 chunk	+5 lines, -0 lines	1 comment	Download
M	sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart	View	1	11 chunks	+98 lines, -93 lines	1 comment	Download
M	sdk/lib/_internal/compiler/implementation/patch_parser.dart	View	1	1 chunk	+2 lines, -0 lines	1 comment	Download
M	tests/compiler/dart2js/mirrors_test.dart	View	1	1 chunk	+14 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

ahe

LGTM, assuming you'll get rid of the call to parseNode in a follow-up CL. https://codereview.chromium.org/11729004/diff/1/sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart ...

7 years, 11 months ago (2013-01-03 18:58:21 UTC) #2

Johnni Winther

7 years, 11 months ago (2013-01-04 09:57:55 UTC) #3

https://codereview.chromium.org/11729004/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart
(right):

https://codereview.chromium.org/11729004/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart:516: Node
node = _element.parseNode(mirrors.compiler);
On 2013/01/03 18:58:21, ahe wrote:
> This is brittle as it can cause a parse error.  Ideally, you'd be able to
> extract a span directly from the element.  For example, PartialClassElement
has
> two fields, beginToken and endToken.
> 
> The "original" partial elements (PartialFunctionElement and
PartialClassElement
> both record both begin and end tokens.  Partial elements that have been added
> later do not (PartialTypedefElement and PartialMetadataAnnotation).  However,
it
> should be easy to fix that.

Added a TODO.

https://codereview.chromium.org/11729004/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart:721: Token
getBeginToken() {
On 2013/01/03 18:58:21, ahe wrote:
> Document what you do here.

Done.

https://codereview.chromium.org/11729004/diff/7005/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/elements/elements.dart (right):

https://codereview.chromium.org/11729004/diff/7005/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/elements/elements.dart:2168: 
Needed in this CL and not in https://codereview.chromium.org/11726005/

https://codereview.chromium.org/11729004/diff/7005/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart
(right):

https://codereview.chromium.org/11729004/diff/7005/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/mirrors/dart2js_mirror.dart:1720: 
The changes below are from https://codereview.chromium.org/11571058/ on which
this CL is based.

https://codereview.chromium.org/11729004/diff/7005/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/patch_parser.dart (right):

https://codereview.chromium.org/11729004/diff/7005/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/patch_parser.dart:506: Token get
beginToken => null;
Needed in this CL and not in https://codereview.chromium.org/11726005/

Expand Messages | Collapse Messages