Bug 901138: Add Debugger.Script.prototype.startColumn r=jimb

I copy-pasted the implementation of Debugger.Script.prototype.startLine, and added a test and documenation.  To make it work, I made JSScript::column_ mandatory, like lineno_.  The only place where lineno_ was set and it was not was in JSScript::fullyInitFromEmitter, which copies the line number from BytecodeEmitter::firstLine, which is itself set in BytecodeEmitter's constructors.  I followed the easiest path and added a new column field to BytecodeEmitter and all of its constructors.

Differential Revision: https://phabricator.services.mozilla.com/D37157

--HG--
extra : moz-landing-system : lando

js/src/debugger/Script.cpp

@@ -325,6 +325,25 @@ bool DebuggerScript::getStartLine(JSContext* cx, unsigned argc, Value* vp) {
   return true;
 }
 
+struct DebuggerScript::GetStartColumnMatcher {
+  using ReturnType = uint32_t;
+
+  ReturnType match(HandleScript script) { return script->column(); }
+  ReturnType match(Handle<LazyScript*> lazyScript) {
+    return lazyScript->column();
+  }
+  ReturnType match(Handle<WasmInstanceObject*> wasmInstance) { return 0; }
+};
+
+/* static */
+bool DebuggerScript::getStartColumn(JSContext* cx, unsigned argc, Value* vp) {
+  THIS_DEBUGSCRIPT_REFERENT(cx, argc, vp, "(get startColumn)", args, obj,
+                            referent);
+  GetStartColumnMatcher matcher;
+  args.rval().setNumber(referent.match(matcher));
+  return true;
+}
+
 struct DebuggerScript::GetLineCountMatcher {
   JSContext* cx_;
   double totalLines;
@@ -2119,6 +2138,7 @@ const JSPropertySpec DebuggerScript::properties_[] = {
     JS_PSG("displayName", getDisplayName, 0),
     JS_PSG("url", getUrl, 0),
     JS_PSG("startLine", getStartLine, 0),
+    JS_PSG("startColumn", getStartColumn, 0),
     JS_PSG("lineCount", getLineCount, 0),
     JS_PSG("source", getSource, 0),
     JS_PSG("sourceStart", getSourceStart, 0),

js/src/debugger/Script.h

@@ -64,6 +64,7 @@ class DebuggerScript : public NativeObject {
   static bool getDisplayName(JSContext* cx, unsigned argc, Value* vp);
   static bool getUrl(JSContext* cx, unsigned argc, Value* vp);
   static bool getStartLine(JSContext* cx, unsigned argc, Value* vp);
+  static bool getStartColumn(JSContext* cx, unsigned argc, Value* vp);
   static bool getLineCount(JSContext* cx, unsigned argc, Value* vp);
   static bool getSource(JSContext* cx, unsigned argc, Value* vp);
   static bool getSourceStart(JSContext* cx, unsigned argc, Value* vp);
@@ -110,6 +111,7 @@ class DebuggerScript : public NativeObject {
 
   class SetPrivateMatcher;
   struct GetStartLineMatcher;
+  struct GetStartColumnMatcher;
   struct GetLineCountMatcher;
   class GetSourceMatcher;
   class GetFormatMatcher;

js/src/doc/Debugger/Debugger.Script.md

@@ -151,6 +151,31 @@ from its prototype:
     which this script's code starts, within the file or document named by
     `url`.
 
+`startColumn`
+:   **If the instance refers to a `JSScript`**, the zero-indexed number of the
+    column at which this script's code starts, within the file or document
+    named by `url`.  For functions, this is the start of the function's
+    arguments:
+    ```language-js
+    function f() { ... }
+    //        ^ start (column 10)
+    let g = x => x*x;
+    //      ^ start (column 8)
+    let h = (x) => x*x;
+    //      ^ start (column 8)
+    ```
+    For default class constructors, it is the start of the `class` keyword:
+    ```language-js
+    let MyClass = class { };
+    //            ^ start (column 14)
+    ```
+    For scripts from other sources, such as `eval` or the `Function`
+    constructor, it is typically 0:
+    ```language-js
+    let f = new Function("  console.log('hello world');");
+    //                    ^ start (column 0, from the string's perspective)
+    ```
+
 `lineCount`
 :   **If the instance refers to a `JSScript`**, the number of lines this
     script's code occupies, within the file or document named by `url`.

js/src/frontend/BytecodeCompiler.cpp

@@ -460,7 +460,8 @@ bool BytecodeCompiler::emplaceEmitter(Maybe<BytecodeEmitter>& emitter,
                                                  ? BytecodeEmitter::SelfHosting
                                                  : BytecodeEmitter::Normal;
   emitter.emplace(/* parent = */ nullptr, parser, sharedContext, script,
-                  /* lazyScript = */ nullptr, options.lineno, emitterMode);
+                  /* lazyScript = */ nullptr, options.lineno, options.column,
+                  emitterMode);
   return emitter->init();
 }
 
@@ -746,7 +747,7 @@ JSScript* frontend::CompileGlobalBinASTScript(
 
   sourceObj->source()->setBinASTSourceMetadata(metadata);
 
-  BytecodeEmitter bce(nullptr, &parser, &globalsc, script, nullptr, 0);
+  BytecodeEmitter bce(nullptr, &parser, &globalsc, script, nullptr, 0, 0);
 
   if (!bce.init()) {
     return nullptr;
@@ -968,9 +969,9 @@ static bool CompileLazyFunctionImpl(JSContext* cx, Handle<LazyScript*> lazy,
   }
 
   BytecodeEmitter bce(/* parent = */ nullptr, &parser, pn->funbox(), script,
-                      lazy, pn->pn_pos, BytecodeEmitter::LazyFunction,
-                      fieldInitializers);
-  if (!bce.init()) {
+                      lazy, lazy->lineno(), lazy->column(),
+                      BytecodeEmitter::LazyFunction, fieldInitializers);
+  if (!bce.init(pn->pn_pos)) {
     return false;
   }
 
@@ -1051,10 +1052,11 @@ bool frontend::CompileLazyBinASTFunction(JSContext* cx,
 
   FunctionNode* pn = parsed.unwrap();
 
-  BytecodeEmitter bce(nullptr, &parser, pn->funbox(), script, lazy, pn->pn_pos,
+  BytecodeEmitter bce(nullptr, &parser, pn->funbox(), script, lazy,
+                      lazy->lineno(), lazy->column(),
                       BytecodeEmitter::LazyFunction);
 
-  if (!bce.init()) {
+  if (!bce.init(pn->pn_pos)) {
     return false;
   }
 

js/src/frontend/BytecodeEmitter.cpp

@@ -94,17 +94,19 @@ static bool ParseNodeRequiresSpecialLineNumberNotes(ParseNode* pn) {
 
 BytecodeEmitter::BytecodeEmitter(
     BytecodeEmitter* parent, SharedContext* sc, HandleScript script,
-    Handle<LazyScript*> lazyScript, uint32_t lineNum, EmitterMode emitterMode,
+    Handle<LazyScript*> lazyScript, uint32_t line, uint32_t column,
+    EmitterMode emitterMode,
     FieldInitializers fieldInitializers /* = FieldInitializers::Invalid() */)
     : sc(sc),
       cx(sc->cx_),
       parent(parent),
       script(cx, script),
       lazyScript(cx, lazyScript),
-      bytecodeSection_(cx, lineNum),
+      bytecodeSection_(cx, line),
       perScriptData_(cx),
       fieldInitializers_(fieldInitializers),
-      firstLine(lineNum),
+      firstLine(line),
+      firstColumn(column),
       emitterMode(emitterMode) {
   MOZ_ASSERT_IF(emitterMode == LazyFunction, lazyScript);
 
@@ -117,10 +119,10 @@ BytecodeEmitter::BytecodeEmitter(
 BytecodeEmitter::BytecodeEmitter(BytecodeEmitter* parent,
                                  BCEParserHandle* handle, SharedContext* sc,
                                  HandleScript script,
-                                 Handle<LazyScript*> lazyScript,
-                                 uint32_t lineNum, EmitterMode emitterMode,
+                                 Handle<LazyScript*> lazyScript, uint32_t line,
+                                 uint32_t column, EmitterMode emitterMode,
                                  FieldInitializers fieldInitializers)
-    : BytecodeEmitter(parent, sc, script, lazyScript, lineNum, emitterMode,
+    : BytecodeEmitter(parent, sc, script, lazyScript, line, column, emitterMode,
                       fieldInitializers) {
   parser = handle;
   instrumentationKinds = parser->options().instrumentationKinds;
@@ -129,10 +131,10 @@ BytecodeEmitter::BytecodeEmitter(BytecodeEmitter* parent,
 BytecodeEmitter::BytecodeEmitter(BytecodeEmitter* parent,
                                  const EitherParser& parser, SharedContext* sc,
                                  HandleScript script,
-                                 Handle<LazyScript*> lazyScript,
-                                 uint32_t lineNum, EmitterMode emitterMode,
+                                 Handle<LazyScript*> lazyScript, uint32_t line,
+                                 uint32_t column, EmitterMode emitterMode,
                                  FieldInitializers fieldInitializers)
-    : BytecodeEmitter(parent, sc, script, lazyScript, lineNum, emitterMode,
+    : BytecodeEmitter(parent, sc, script, lazyScript, line, column, emitterMode,
                       fieldInitializers) {
   ep_.emplace(parser);
   this->parser = ep_.ptr();
@@ -146,6 +148,11 @@ void BytecodeEmitter::initFromBodyPosition(TokenPos bodyPosition) {
 
 bool BytecodeEmitter::init() { return perScriptData_.init(cx); }
 
+bool BytecodeEmitter::init(TokenPos bodyPosition) {
+  initFromBodyPosition(bodyPosition);
+  return init();
+}
+
 template <typename T>
 T* BytecodeEmitter::findInnermostNestableControl() const {
   return NestableControl::findNearest<T>(innermostNestableControl);
@@ -5764,10 +5771,14 @@ MOZ_NEVER_INLINE bool BytecodeEmitter::emitFunction(
       fieldInitializers = setupFieldInitializers(classContentsIfConstructor);
     }
 
+    uint32_t innerScriptLine, innerScriptColumn;
+    parser->errorReporter().lineAndColumnAt(funNode->pn_pos.begin,
+                                            &innerScriptLine,
+                                            &innerScriptColumn);
     BytecodeEmitter bce2(this, parser, funbox, innerScript,
-                         /* lazyScript = */ nullptr, funNode->pn_pos,
-                         nestedMode, fieldInitializers);
-    if (!bce2.init()) {
+                         /* lazyScript = */ nullptr, innerScriptLine,
+                         innerScriptColumn, nestedMode, fieldInitializers);
+    if (!bce2.init(funNode->pn_pos)) {
       return false;
     }
 

js/src/frontend/BytecodeEmitter.h

@@ -102,7 +102,9 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
   mozilla::Maybe<EitherParser> ep_ = {};
   BCEParserHandle* parser = nullptr;
 
-  unsigned firstLine = 0; /* first line, for JSScript::initFromEmitter */
+  // First line and column, for JSScript::initFromEmitter.
+  unsigned firstLine = 0;
+  unsigned firstColumn = 0;
 
   uint32_t maxFixedSlots = 0; /* maximum number of fixed frame slots so far */
 
@@ -175,7 +177,7 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
   // Internal constructor, for delegation use only.
   BytecodeEmitter(
       BytecodeEmitter* parent, SharedContext* sc, JS::Handle<JSScript*> script,
-      JS::Handle<LazyScript*> lazyScript, uint32_t lineNum,
+      JS::Handle<LazyScript*> lazyScript, uint32_t line, uint32_t column,
       EmitterMode emitterMode,
       FieldInitializers fieldInitializers = FieldInitializers::Invalid());
 
@@ -194,60 +196,27 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
   BytecodeEmitter(
       BytecodeEmitter* parent, BCEParserHandle* parser, SharedContext* sc,
       JS::Handle<JSScript*> script, JS::Handle<LazyScript*> lazyScript,
-      uint32_t lineNum, EmitterMode emitterMode = Normal,
+      uint32_t line, uint32_t column, EmitterMode emitterMode = Normal,
       FieldInitializers fieldInitializers = FieldInitializers::Invalid());
 
   BytecodeEmitter(
       BytecodeEmitter* parent, const EitherParser& parser, SharedContext* sc,
       JS::Handle<JSScript*> script, JS::Handle<LazyScript*> lazyScript,
-      uint32_t lineNum, EmitterMode emitterMode = Normal,
+      uint32_t line, uint32_t column, EmitterMode emitterMode = Normal,
       FieldInitializers fieldInitializers = FieldInitializers::Invalid());
 
   template <typename Unit>
   BytecodeEmitter(
       BytecodeEmitter* parent, Parser<FullParseHandler, Unit>* parser,
       SharedContext* sc, JS::Handle<JSScript*> script,
-      JS::Handle<LazyScript*> lazyScript, uint32_t lineNum,
+      JS::Handle<LazyScript*> lazyScript, uint32_t line, uint32_t column,
       EmitterMode emitterMode = Normal,
       FieldInitializers fieldInitializers = FieldInitializers::Invalid())
       : BytecodeEmitter(parent, EitherParser(parser), sc, script, lazyScript,
-                        lineNum, emitterMode, fieldInitializers) {}
-
-  // An alternate constructor that uses a TokenPos for the starting
-  // line and that sets functionBodyEndPos as well.
-  BytecodeEmitter(
-      BytecodeEmitter* parent, BCEParserHandle* parser, SharedContext* sc,
-      JS::Handle<JSScript*> script, JS::Handle<LazyScript*> lazyScript,
-      TokenPos bodyPosition, EmitterMode emitterMode = Normal,
-      FieldInitializers fieldInitializers = FieldInitializers::Invalid())
-      : BytecodeEmitter(parent, parser, sc, script, lazyScript,
-                        parser->errorReporter().lineAt(bodyPosition.begin),
-                        emitterMode, fieldInitializers) {
-    initFromBodyPosition(bodyPosition);
-  }
-
-  BytecodeEmitter(
-      BytecodeEmitter* parent, const EitherParser& parser, SharedContext* sc,
-      JS::Handle<JSScript*> script, JS::Handle<LazyScript*> lazyScript,
-      TokenPos bodyPosition, EmitterMode emitterMode = Normal,
-      FieldInitializers fieldInitializers = FieldInitializers::Invalid())
-      : BytecodeEmitter(parent, parser, sc, script, lazyScript,
-                        parser.errorReporter().lineAt(bodyPosition.begin),
-                        emitterMode, fieldInitializers) {
-    initFromBodyPosition(bodyPosition);
-  }
-
-  template <typename Unit>
-  BytecodeEmitter(
-      BytecodeEmitter* parent, Parser<FullParseHandler, Unit>* parser,
-      SharedContext* sc, JS::Handle<JSScript*> script,
-      JS::Handle<LazyScript*> lazyScript, TokenPos bodyPosition,
-      EmitterMode emitterMode = Normal,
-      FieldInitializers fieldInitializers = FieldInitializers::Invalid())
-      : BytecodeEmitter(parent, EitherParser(parser), sc, script, lazyScript,
-                        bodyPosition, emitterMode, fieldInitializers) {}
+                        line, column, emitterMode, fieldInitializers) {}
 
   MOZ_MUST_USE bool init();
+  MOZ_MUST_USE bool init(TokenPos bodyPosition);
 
   template <typename T>
   T* findInnermostNestableControl() const;

js/src/jit-test/tests/debug/Script-startColumn.js

@@ -0,0 +1,89 @@
+// Script.prototype.startColumn returns the correct column for all scripts.
+
+const g = newGlobal({newCompartment: true, useWindowProxy: true});
+const dbg = Debugger(g);
+const obj = dbg.addDebuggee(g);
+
+function test(f, expected) {
+    const object = obj.makeDebuggeeValue(f);
+    assertEq(object.callable, true);
+    console.log("object name: " + object.displayName);
+    assertEq(object.script.startColumn, expected);
+}
+
+g.eval(`
+function f1() { }
+`);
+
+test(g.f1, 11);
+g.eval(`
+var f2 = function({ a, b, c }, d, e, ...more) { };
+`);
+test(g.f2, 17);
+
+g.eval(`
+var f3 = function *() { };
+`);
+test(g.f3, 19);
+g.eval(`
+var f4 = async function
+  () { };
+`);
+test(g.f4, 2);
+
+g.eval(`
+var f5 = (a, b) => a + b;
+`);
+test(g.f5, 9);
+
+g.eval(`
+var f6 = a => a + 1;
+`);
+test(g.f6, 9);
+
+g.eval(`
+var MyClass = class {
+    method() { }
+};
+var myInstance = new MyClass();
+`);
+test(g.myInstance.method, 10);
+test(g.myInstance.constructor, 14);
+
+g.eval(`
+function f8() {
+    return function f8Inner() { }
+}
+`);
+test(g.f8, 11);
+test(g.f8(), 27);
+
+g.eval(`
+var f9 = new Function(\"\");
+`);
+test(g.f9, 0);
+
+let hit = 0;
+let column;
+dbg.onDebuggerStatement = function (frame) {
+    column = frame.script.startColumn;
+    hit += 1;
+}
+
+g.eval(`    debugger;`);
+assertEq(column, 0);
+assertEq(hit, 1);
+
+const location = { fileName: "column.js", lineNumber: 1, columnNumber: 1 };
+hit = 0;
+g.evaluate(`    debugger;`, location);
+assertEq(column, 1);
+assertEq(hit, 1);
+
+g.evaluate(`var f10 = function () { };`, location);
+test(g.f10, 20);
+
+g.evaluate(`
+var f11 = function () { };
+`, location);
+test(g.f11, 19);

js/src/vm/JSFunction.cpp

@@ -1662,10 +1662,9 @@ bool JSFunction::createScriptForLazilyInterpretedFunction(JSContext* cx,
 
     // Try to insert the newly compiled script into the lazy script cache.
     if (canRelazify) {
-      // A script's starting column isn't set by the bytecode emitter, so
-      // specify this from the lazy script so that if an identical lazy
-      // script is encountered later a match can be determined.
-      script->setColumn(lazy->column());
+      // If an identical lazy script is encountered later a match can be
+      // determined based on line and column number.
+      MOZ_ASSERT(lazy->column() == script->column());
 
       // Remember the lazy script on the compiled script, so it can be
       // stored on the function again in case of re-lazification.

js/src/vm/JSScript.cpp

@@ -4101,6 +4101,7 @@ bool JSScript::fullyInitFromEmitter(JSContext* cx, HandleScript script,
 
   // Initialize POD fields
   script->lineno_ = bce->firstLine;
+  script->column_ = bce->firstColumn;
 
   // Initialize script flags from BytecodeEmitter
   script->setFlag(ImmutableFlags::Strict, bce->sc->strict());

js/src/vm/JSScript.h

@@ -2343,8 +2343,6 @@ class JSScript : public js::BaseScript {
 
   size_t mainOffset() const { return immutableScriptData()->mainOffset; }
 
-  void setColumn(size_t column) { column_ = column; }
-
   // The fixed part of a stack frame is comprised of vars (in function and
   // module code) and block-scoped locals (in all kinds of code).
   size_t nfixed() const { return immutableScriptData()->nfixed; }