Refactor script position calculation

src/js/messages.js

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // -------------------------------------------------------------------
 
 (function(global, utils) {
 
 %CheckIsBootstrapping();
 
@@ skipping 207 matching lines @@
 }
 
 
 /**
  * Find a line number given a specific source position.
  * @param {number} position The source position.
  * @return {number} 0 if input too small, -1 if input too large,
        else the line number.
  */
 function ScriptLineFromPosition(position) {
-  var lower = 0;
-  var upper = this.lineCount() - 1;
-  var line_ends = this.line_ends;
-
-  // We'll never find invalid positions so bail right away.
-  if (position > line_ends[upper]) {
-    return -1;
-  }
-
-  // This means we don't have to safe-guard indexing line_ends[i - 1].
-  if (position <= line_ends[0]) {
-    return 0;
-  }
-
-  // Binary search to find line # from position range.
-  while (upper >= 1) {
-    var i = (lower + upper) >> 1;
-
-    if (position > line_ends[i]) {
-      lower = i + 1;
-    } else if (position <= line_ends[i - 1]) {
-      upper = i - 1;
-    } else {
-      return i;
-    }
-  }
-
-  return -1;
+  var info = %ScriptPositionInfo(this, position, false);
+  return (info == null) ? -1 : info.line;
 }
 
 
 /**
  * Get information on a specific source position.
  * @param {number} position The source position
  * @param {boolean} include_resource_offset Set to true to have the resource
  *     offset added to the location
  * @return {SourceLocation}
  *     If line is negative or not in the source null is returned.
  */
 function ScriptLocationFromPosition(position,
                                     include_resource_offset) {
-  var line = this.lineFromPosition(position);
-  if (line == -1) return null;
+  var info = %ScriptPositionInfo(this, position, !!include_resource_offset);
+  if (info == null) return null;
 
-  // Determine start, end and column.
-  var line_ends = this.line_ends;
-  var start = line == 0 ? 0 : line_ends[line - 1] + 1;
-  var end = line_ends[line];
-  if (end > 0 && %_StringCharAt(this.source, end - 1) === '\r') {
-    end--;
-  }
-  var column = position - start;
-
-  // Adjust according to the offset within the resource.
-  if (include_resource_offset) {
-    line += this.line_offset;
-    if (line == this.line_offset) {
-      column += this.column_offset;
-    }
-  }
-
-  return new SourceLocation(this, position, line, column, start, end);
+  return new SourceLocation(this, position, info.line, info.column,

[Yang, 2016/05/18 13:23:08]

Can we simply return 'info'? Do we need line_start and line_end for anything
other than formatting the string in SourceLocationSourceText? Is
SourceLocation.start and SourceLocation.end used anywhere? If not, we can simply
omit line_start/line_end and simply have %ScriptPositionInfo store the string
slice for the source text on 'info'. Substrings are fairly cheap to create
anyways.

[jgruber1, 2016/05/19 08:05:24]

Done. I'll remove ScripLocationFromPosition itself in an upcoming CL.

+          info.line_start, info.line_end);
 }
 
 
 /**
  * Get information on a specific source line and column possibly offset by a
  * fixed source position. This function is used to find a source position from
  * a line and column position. The fixed source position offset is typically
  * used to find a source position in a function based on a line and column in
  * the source for the function alone. The offset passed will then be the
  * start position of the source for the function within the full script source.
  * @param {number} opt_line The line within the source. Default value is 0
  * @param {number} opt_column The column in within the line. Default value is 0
  * @param {number} opt_offset_position The offset from the begining of the
  *     source from where the line and column calculation starts.
  *     Default value is 0
  * @return {SourceLocation}
  *     If line is negative or not in the source null is returned.
  */
 function ScriptLocationFromLine(opt_line, opt_column, opt_offset_position) {

[Yang, 2016/05/18 13:23:08]

This function is also only used inside V8. Let's move this thing to the runtime
entirely and only call the runtime function in V8. Let's not expose it on the
Script object anymore.

[jgruber1, 2016/05/19 08:05:24]

Will do in an upcoming commit unless you'd prefer it in this one.

   // Default is the first line in the script. Lines in the script is relative
   // to the offset within the resource.
   var line = 0;
   if (!IS_UNDEFINED(opt_line)) {
     line = opt_line - this.line_offset;
   }
 
   // Default is first column. If on the first line add the offset within the
   // resource.
   var column = opt_column || 0;
   if (line == 0) {
     column -= this.column_offset;
   }
 
   var offset_position = opt_offset_position || 0;
   if (line < 0 || column < 0 || offset_position < 0) return null;
   if (line == 0) {
     return this.locationFromPosition(offset_position + column, false);
   } else {
     // Find the line where the offset position is located.
     var offset_line = this.lineFromPosition(offset_position);
 
     if (offset_line == -1 || offset_line + line >= this.lineCount()) {
       return null;
     }
 
     return this.locationFromPosition(
-        this.line_ends[offset_line + line - 1] + 1 + column);  // line > 0 here.
+        %ScriptLineStartPosition(this, offset_line + line) + column);
   }
 }
 
 
 /**
  * Get a slice of source code from the script. The boundaries for the slice is
  * specified in lines.
  * @param {number} opt_from_line The first line (zero bound) in the slice.
  *     Default is 0
  * @param {number} opt_to_column The last line (zero bound) in the slice (non
@@ skipping 13 matching lines @@
   if (from_line < 0) from_line = 0;
   if (to_line > this.lineCount()) to_line = this.lineCount();
 
   // Check parameters.
   if (from_line >= this.lineCount() ||
       to_line < 0 ||
       from_line > to_line) {
     return null;
   }
 
-  var line_ends = this.line_ends;
-  var from_position = from_line == 0 ? 0 : line_ends[from_line - 1] + 1;
-  var to_position = to_line == 0 ? 0 : line_ends[to_line - 1] + 1;
+  var from_position = %ScriptLineStartPosition(this, from_line);
+  var to_position = %ScriptLineStartPosition(this, to_line);
 
   // Return a source slice with line numbers re-adjusted to the resource.
   return new SourceSlice(this,

[Yang, 2016/05/18 13:23:08]

The SourceSlice object seems not to be used anywhere in Chrome, and in V8 only
in a test case. Let's remove that test case and this whole SourceSlice thing.

[jgruber1, 2016/05/19 08:05:24]

What about DebugCommandProcessor.prototype.sourceRequest_?

                          from_line + this.line_offset,
                          to_line + this.line_offset,
-                          from_position, to_position);
+                         from_position, to_position);
 }
 
 
 function ScriptSourceLine(opt_line) {

[Yang, 2016/05/18 13:23:08]

Seems like this function is only used inside V8 (please verify). Can we turn
this whole thing into a runtime function and and use that runtime function at
call sites and not expose this on the Script object anymore?

[jgruber1, 2016/05/19 08:05:24]

Ditto (upcoming commit).

   // Default is the first line in the script. Lines in the script are relative
   // to the offset within the resource.
   var line = 0;
   if (!IS_UNDEFINED(opt_line)) {
     line = opt_line - this.line_offset;
   }
 
   // Check parameter.
   if (line < 0 || this.lineCount() <= line) {
     return null;
   }
 
   // Return the source line.
-  var line_ends = this.line_ends;
-  var start = line == 0 ? 0 : line_ends[line - 1] + 1;
-  var end = line_ends[line];
+  var start = %ScriptLineStartPosition(this, line);
+  var end = %ScriptLineEndPosition(this, line);
   return %_Call(StringSubstring, this.source, start, end);
 }
 
 
 /**
  * Returns the number of source lines.
  * @return {number}
  *     Number of source lines.
  */
 function ScriptLineCount() {
   // Return number of source lines.
-  return this.line_ends.length;
+  return %ScriptLineCount(this);

[Yang, 2016/05/18 13:23:09]

This also doesn't seem to be used outside of V8. Let's not expose this anymore.

[jgruber1, 2016/05/19 08:05:24]

Ditto.

 }
 
 
-/**
- * Returns the position of the nth line end.
- * @return {number}
- *     Zero-based position of the nth line end in the script.
- */
-function ScriptLineEnd(n) {
-  return this.line_ends[n];
-}
-
-
 /**
  * If sourceURL comment is available returns sourceURL comment contents.
  * Otherwise, script name is returned. See
  * http://fbug.googlecode.com/svn/branches/firebug1.1/docs/ReleaseNotes_1.1.txt
  * and Source Map Revision 3 proposal for details on using //# sourceURL and
  * deprecated //@ sourceURL comment to identify scripts that don't have name.
  *
  * @return {?string} script name if present, value for //# sourceURL comment or
  * deprecated //@ sourceURL comment otherwise.
  */
 function ScriptNameOrSourceURL() {
   if (this.source_url) return this.source_url;
   return this.name;
 }
 
 
 utils.SetUpLockedPrototype(Script, [
     "source",
     "name",
     "source_url",
     "source_mapping_url",
-    "line_ends",
     "line_offset",
     "column_offset"
   ], [
     "lineFromPosition", ScriptLineFromPosition,
     "locationFromPosition", ScriptLocationFromPosition,
     "locationFromLine", ScriptLocationFromLine,
     "sourceSlice", ScriptSourceSlice,
     "sourceLine", ScriptSourceLine,
     "lineCount", ScriptLineCount,
     "nameOrSourceURL", ScriptNameOrSourceURL,
-    "lineEnd", ScriptLineEnd
   ]
 );
 
 
 /**
  * Class for source location. A source location is a position within some
  * source with the following properties:
  *   script   : script object for the source
  *   line     : source line number
  *   column   : source column within the line
@@ skipping 578 matching lines @@
 utils.Export(function(to) {
   to.ErrorToString = ErrorToString;
   to.MakeError = MakeError;
   to.MakeRangeError = MakeRangeError;
   to.MakeSyntaxError = MakeSyntaxError;
   to.MakeTypeError = MakeTypeError;
   to.MakeURIError = MakeURIError;
 });
 
 });