Add SourceLocationMixin and SourceLocationBase. This allows FileLocation to avoid extending SourceLocation at all, which avoids unused line, column, and sourceUrl fields. This produces a speed improvement of approximately 5% in the YAML parser, and will likely do more in code that uses locations more heavily relative to spans. R=rnystrom@google.com Review URL: https://codereview.chromium.org//1307123004 .
diff --git a/pkgs/source_span/CHANGELOG.md b/pkgs/source_span/CHANGELOG.md index 6bd18f1..ab13bda 100644 --- a/pkgs/source_span/CHANGELOG.md +++ b/pkgs/source_span/CHANGELOG.md
@@ -1,5 +1,9 @@ # 1.2.0 +* **Deprecated:** Extending `SourceLocation` directly is deprecated. Instead, + extend the new `SourceLocationBase` class or mix in the new + `SourceLocationMixin` mixin. + * Dramatically improve the performance of `FileLocation`. # 1.1.6
diff --git a/pkgs/source_span/lib/source_span.dart b/pkgs/source_span/lib/source_span.dart index 89b1650..9666dc2 100644 --- a/pkgs/source_span/lib/source_span.dart +++ b/pkgs/source_span/lib/source_span.dart
@@ -6,6 +6,7 @@ export "src/file.dart"; export "src/location.dart"; +export "src/location_mixin.dart"; export "src/span.dart"; export "src/span_exception.dart"; export "src/span_mixin.dart";
diff --git a/pkgs/source_span/lib/src/file.dart b/pkgs/source_span/lib/src/file.dart index c2b97e7..95fa92c 100644 --- a/pkgs/source_span/lib/src/file.dart +++ b/pkgs/source_span/lib/src/file.dart
@@ -8,6 +8,7 @@ import 'dart:typed_data'; import 'location.dart'; +import 'location_mixin.dart'; import 'span.dart'; import 'span_mixin.dart'; import 'span_with_context.dart'; @@ -212,17 +213,19 @@ /// and column values based on its offset and the contents of [file]. /// /// A [FileLocation] can be created using [SourceFile.location]. -class FileLocation extends SourceLocation { +class FileLocation extends SourceLocationMixin implements SourceLocation { /// The [file] that [this] belongs to. final SourceFile file; + final int offset; Uri get sourceUrl => file.url; int get line => file.getLine(offset); int get column => file.getColumn(offset); - FileLocation._(this.file, int offset) - : super(offset) { - if (offset > file.length) { + FileLocation._(this.file, this.offset) { + if (offset < 0) { + throw new RangeError("Offset may not be negative, was $offset."); + } else if (offset > file.length) { throw new RangeError("Offset $offset must not be greater than the number " "of characters in the file, ${file.length}."); }
diff --git a/pkgs/source_span/lib/src/location.dart b/pkgs/source_span/lib/src/location.dart index 024c6e2..afb37c7 100644 --- a/pkgs/source_span/lib/src/location.dart +++ b/pkgs/source_span/lib/src/location.dart
@@ -6,7 +6,13 @@ import 'span.dart'; -// A class that describes a single location within a source file. +// TODO(nweiz): Use SourceLocationMixin once we decide to cut a release with +// breaking changes. See SourceLocationMixin for details. + +/// A class that describes a single location within a source file. +/// +/// This class should not be extended. Instead, [SourceLocationBase] should be +/// extended instead. class SourceLocation implements Comparable<SourceLocation> { /// URL of the source containing this location. /// @@ -85,3 +91,10 @@ String toString() => '<$runtimeType: $offset $toolString>'; } + +/// A base class for source locations with [offset], [line], and [column] known +/// at construction time. +class SourceLocationBase extends SourceLocation { + SourceLocationBase(int offset, {sourceUrl, int line, int column}) + : super(offset, sourceUrl: sourceUrl, line: line, column: column); +}
diff --git a/pkgs/source_span/lib/src/location_mixin.dart b/pkgs/source_span/lib/src/location_mixin.dart new file mode 100644 index 0000000..5aa0de5 --- /dev/null +++ b/pkgs/source_span/lib/src/location_mixin.dart
@@ -0,0 +1,51 @@ +// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +library source_span.location_mixin; + +import 'location.dart'; +import 'span.dart'; + +// Note: this class duplicates a lot of functionality of [SourceLocation]. This +// is because in order for SourceLocation to use SourceLocationMixin, +// SourceLocationMixin couldn't implement SourceLocation. In SourceSpan we +// handle this by making the class itself non-extensible, but that would be a +// breaking change for SourceLocation. So until we want to endure the pain of +// cutting a release with breaking changes, we duplicate the code here. + +/// A mixin for easily implementing [SourceLocation]. +abstract class SourceLocationMixin implements SourceLocation { + String get toolString { + var source = sourceUrl == null ? 'unknown source' : sourceUrl; + return '$source:${line + 1}:${column + 1}'; + } + + int distance(SourceLocation other) { + if (sourceUrl != other.sourceUrl) { + throw new ArgumentError("Source URLs \"${sourceUrl}\" and " + "\"${other.sourceUrl}\" don't match."); + } + return (offset - other.offset).abs(); + } + + SourceSpan pointSpan() => new SourceSpan(this, this, ""); + + int compareTo(SourceLocation other) { + if (sourceUrl != other.sourceUrl) { + throw new ArgumentError("Source URLs \"${sourceUrl}\" and " + "\"${other.sourceUrl}\" don't match."); + } + return offset - other.offset; + } + + bool operator ==(other) => + other is SourceLocation && + sourceUrl == other.sourceUrl && + offset == other.offset; + + int get hashCode => sourceUrl.hashCode + offset; + + String toString() => '<$runtimeType: $offset $toolString>'; +} +
diff --git a/pkgs/source_span/pubspec.yaml b/pkgs/source_span/pubspec.yaml index 7ffbc64..25de799 100644 --- a/pkgs/source_span/pubspec.yaml +++ b/pkgs/source_span/pubspec.yaml
@@ -1,5 +1,5 @@ name: source_span -version: 1.2.0-dev +version: 1.2.0 author: Dart Team <misc@dartlang.org> description: A library for identifying source spans and locations. homepage: https://github.com/dart-lang/source_span