Merge SourceLocation and Source

Merge SourceLocation and Source

Committed: https://code.google.com/p/dart/source/detail?r=14454

[kasperl, 2012-11-01 10:13:29]

LGTM with comments:

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
File pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart (right):

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:634: var sourceFile =
_script.file is SourceFile;
I'd factor out the computation part into a computeLine method and cache the
result on first access.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:642: index =
sourceText.indexOf('\n', index) + 1;
I don't think this works on all platforms. I'd recommend looking for a pattern
like 

    const RegExp("\r\n?|\n\r?")

instead. See https://codereview.chromium.org/11174003.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:648: int get column {
I'd factor out the computation part into a computeColumn method and cache the
result on first access.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:656: int index = offset-1;
offset - 1 (spacing)

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:666: return column;
Hmm. This looks broken. Isn't this self-recursive? Shouldn't this be
columnNumber? Is this tested?

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:1057:
mirrors.compiler.spanFromElement(_type.element));
Indentation is off now.

[Johnni Winther, 2012-11-01 12:01:58]

PTAL

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
File pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart (right):

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:634: var sourceFile =
_script.file is SourceFile;
On 2012/11/01 10:13:29, kasperl wrote:
> I'd factor out the computation part into a computeLine method and cache the
> result on first access.

Done.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:642: index =
sourceText.indexOf('\n', index) + 1;
On 2012/11/01 10:13:29, kasperl wrote:
> I don't think this works on all platforms. I'd recommend looking for a pattern
> like 
> 
>     const RegExp("\r\n?|\n\r?")
> 
> instead. See https://codereview.chromium.org/11174003.

Done.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:648: int get column {
On 2012/11/01 10:13:29, kasperl wrote:
> I'd factor out the computation part into a computeColumn method and cache the
> result on first access.

Done.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:656: int index = offset-1;
On 2012/11/01 10:13:29, kasperl wrote:
> offset - 1 (spacing)

Done.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:666: return column;
On 2012/11/01 10:13:29, kasperl wrote:
> Hmm. This looks broken. Isn't this self-recursive? Shouldn't this be
> columnNumber? Is this tested?

Yes. Fixed.

https://codereview.chromium.org/11275097/diff/1/pkg/dartdoc/lib/src/mirrors/d...
pkg/dartdoc/lib/src/mirrors/dart2js_mirror.dart:1057:
mirrors.compiler.spanFromElement(_type.element));
On 2012/11/01 10:13:29, kasperl wrote:
> Indentation is off now.

Done.

[kasperl, 2012-11-01 12:21:28]

LGTM.

[ahe, 2012-11-01 13:05:22]

https://codereview.chromium.org/11275097/diff/3002/lib/compiler/implementatio...
File lib/compiler/implementation/source_file.dart (right):

https://codereview.chromium.org/11275097/diff/3002/lib/compiler/implementatio...
lib/compiler/implementation/source_file.dart:30: starts.add(match.start + 1);
What is the performance overhead of this? Perhaps the scanner should record this
information.

[Johnni Winther, 2012-11-01 15:27:13]

https://codereview.chromium.org/11275097/diff/3002/lib/compiler/implementatio...
File lib/compiler/implementation/source_file.dart (right):

https://codereview.chromium.org/11275097/diff/3002/lib/compiler/implementatio...
lib/compiler/implementation/source_file.dart:30: starts.add(match.start + 1);
On 2012/11/01 13:05:22, ahe wrote:
> What is the performance overhead of this? Perhaps the scanner should record
this
> information.

Enormous. I'll revert.