[jsdbg2] Debugger.Environment.prototype.type should return more detailed information

NEW
Unassigned

Status

()

enhancement
P3
normal
2 years ago
16 days ago

People

(Reporter: jimb, Unassigned)

Tracking

(Blocks 1 bug, {good-first-bug, helpwanted})

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fix-optional)

Details

At the moment, the Debugger.Environment.prototype.type accessor property distinguishes only between "declarative", "with", and "object" environments. This causes clients that want to help users recognize scopes to resort to other means to make finer discriminations, which is often inefficient and inaccurate.

For example, here's some code implementing the Firefox devtools protocol's environment actor:

    // What is this environment's type?
    if (this.obj.type == "declarative") {
      form.type = this.obj.callee ? "function" : "block";
    } else {
      form.type = this.obj.type;
    }

http://searchfox.org/mozilla-central/rev/5a60492a53667fc61a62af1847d005a210b7a4f6/devtools/server/actors/environment.js#45-50

On environments with callees, checking the callee property here causes the allocation of a Debugger.Object which is used as a boolean value and abandoned. Meanwhile, the implementation of D.E.p.type is throwing away exactly the details the environment actor wants:

bool
DebugEnvironmentProxy::isForDeclarative() const
{
    EnvironmentObject& e = environment();
    return e.is<CallObject>() ||
           e.is<VarEnvironmentObject>() ||
           e.is<ModuleEnvironmentObject>() ||
           e.is<WasmInstanceEnvironmentObject>() ||
           e.is<WasmFunctionCallObject>() ||
           e.is<LexicalEnvironmentObject>();
}

http://searchfox.org/mozilla-central/rev/5a60492a53667fc61a62af1847d005a210b7a4f6/js/src/vm/EnvironmentObject.cpp#2371-2381

It's reasonable to be concerned about exposing implementation details via the Debugger API, but most of these distinctions look like they can be defined in terms of the source language.
Priority: -- → P3
Blocks: js-devtools
Here are some notes from a November call where we discussed this:
https://docs.google.com/document/d/1QZyTzte5UvQsqN3ad38Q2RqIEJgTWsRqZgsYU4VEP5E/edit
Here is a quick patch based on my memory of our call in November. I did not try to compile this so, it is safe to say that it will likely contain lots of syntax errors. The main thing i'm interested in though is if this captures the primary goals:

1. make environment->type() more fine grained
2. simplify the environment actor

https://github.com/jasonLaster/gecko-dev/compare/bookmarks/central...jasonLaster:js-env-type

Jim, let me know how close this was to what you remember...
I'm not going to do an implementation of this, but I did look it over to see how complex it would be, and I wanted to share what I could see.

I think that patch is a step in the right direct. My main concern was that it only covers a few of the cases that the debugger's own AST visitor marks, so I was curious to see where things diverged. As you might guess, the main thing is that, from a user standpoint, it would be nice to be able to have more specific information about where a lexical scope is used. Some places in the engine have custom scopes to handle spec edge cases, but there a lots of places where things are generic lexical scopes. While that's fine for the engine, it's not ideal if the goal is for users to easily see what scopes map to.

Here's a list of the scopes we handle in the devtools AST visitor, compared to what I could see in the engine.

* Global
  * ScopeKind::Global
* Lexical Global
  * Emitted in a standard lexical scope
* Class Inner
  * Emitted in a standard lexical scope
* For-loop lexical capture
  * Emitted in a standard lexical scope
* Switch
  * Emitted in a standard lexical scope
* Block
  * Emitted in a standard lexical scope
* Module
  * ScopeKind::Module
* Named function expression
  * ScopeKind::NamedLambda
  * ScopeKind::StrictNamedLambda
* Function 
  * ScopeKind::Function
  * ScopeKind::ParameterExpressionVar
  * ScopeKind::FunctionBodyVar
* Lexical Function Body
  * ScopeKind::Lexical
* Catch
  * ScopeKind::SimpleCatch
  * ScopeKind::Catch
* With
  * ScopeKind::With
* Class Field Inititializer
  * Doesn't exist in engine

The other issue is that not all of these ScopeKind types are exposed in their respective environment types, so that might also need to be passed through somewhere.
Blocks: dbg-scopes
Keywords: helpwanted
Keywords: good-first-bug
Blocks: dbg-68
No longer blocks: dbg-68
Whiteboard: [debugger-mvp]
Whiteboard: [debugger-mvp]
You need to log in before you can comment on or make changes to this bug.