-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Scopes and variable shadowing #37
Comments
I've had to revise my proposal after realizing something I had overlooked: I wanted to attach binding information (mapping original variables to generated expressions) to the original sources but (the body of) an original function can appear multiple times in the generated source due to function inlining: original:
generated:
When you're paused in the first line of the generated source you'd expect the debugger to show that you're paused in the second line of the original source and an original scope with a variable So my revised proposal is to add scope information to the generated source with the following information for each scope:
Points to discuss:
TBD:
|
Some takeaways from the naming meeting:
|
I mentioned the next case: // ORIGINAL SOURCE:
const foo = 5
const bar = 6
console.log(foo + bar)
// GENERATED SOURCE:
/* after a constant folding could looks like this: */
console.log(11)
/* ^ at this point of debugging we don't have any information about the original values of `foo` and `bar` variables */ The question is, do we want to have the ability to describe debug information for such cases? |
@JSMonk In this case the sourcemap could specify the bindings as |
Here are some more examples (I specify the ranges for the scopes as <startLine>:<startColumn> - <endLine>:<endColumn>, with 1-based lines and columns) Removed scopesOriginal: 1 {
2 let x = 1;
3 console.log(x);
4 {
5 let x = 2;
6 console.log(x);
7 }
8 console.log(x);
9 } Generated: 1 {
2 var x1 = 1;
3 console.log(x1);
4 var x2 = 2;
5 console.log(x2);
6 console.log(x1);
7 } Scopes:
Inlined functionsOriginal: 1 function foo(condition) {
2 log(condition ? "yes" : "no");
3 }
4 foo(true);
5 foo(false); Generated: 1 log("yes");
2 log("no"); Scopes:
Variable shadowingOriginal: 1 function outer(num) {
2 function inner(value) {
3 const value_plus_one = value + 1;
4 console.log(value_plus_one);
5 }
6 const num_plus_one = num + 1;
7 inner(num_plus_one);
8 }
9 outer(1); Generated: 1 function f(a) {
2 function g(a) {
3 const b = a + 1;
4 console.log(b);
5 }
6 const b = a + 1;
7 g(b);
8 }
9 f(1); Scopes:
|
I've created an example implementation here for computing the original scopes using the scope information encoded in the sourcemap and added two of the examples above. |
Btw, I've just realized that with this proposal it's also possible to "map" values. |
I have added a
|
@hbenl I believe there should also be a source index to indicate the file containing the source scope. |
@JSMonk The scope locations are in the generated source and there is exactly one generated source for a sourcemap. The sourcemap's |
@jkup Sure, add them to this issue. |
Tobias' spec notes: https://gist.github.com/sokra/97a53a869b9a421accadbc9681cb26f3 |
Here's a particularly tricky example, merging two modules into one and then inlining a function from one module into the other module (produced with rollup and terser and then pretty-printed): Original files:
1 import { f } from "./two";
2 let num = 42;
3 f(num);
4 console.log(num++);
1 let increment = 1;
2 export function f(x) {
3 console.log(x + increment++);
4 } Generated file: 1 let l = 1;
2 let o = 42;
3 var e;
4 (e = o),
5 console.log(e + l++),
6 console.log(o++); When paused at the
With sourcemaps it should show two frames:
The scopes information in the sourcemap would be:
I've also added this example to my example implementation but will have to update the implementation to correctly support function inlining (i.e. recreate the frame that was "lost" due to inlining). |
I stumbled over two examples where the current "bindings per scope" approach is not that great for debuggers. The first one is the following: original: (function foo() {
let someVar = 'hello';
console.log('hello');
someVar = 'world';
console.log('world');
})(); generated: console.log('hello');
console.log('world'); The current proposal would call for one entry in The second example is taken from https://szuend.github.io/scope-proposal-examples/01_simple/simple.html original: function addWithMultiply(arg1, arg2, arg3) {
const intermediate = arg1 + arg2;
if (arg3 !== undefined) {
const result = intermediate * arg3;
return result;
}
return intermediate;
}
addWithMultiply(2, 3, 4); generated: function n(n,t,e){const r=n+t;if(e!==undefined){const n=r*e;return n}return r}n(2,3,4) The current proposal calls for the following binding sections for the function and block scope respectively: "bindings": [["arg1", "n"], ["arg2", "t"], ["arg3", "e"], ["intermediate", "r"]],
"bindings": [["result", "n"]] Let's say we pause on the Both of these examples together with issue #61 lead me to believe that we should consider using live ranges instead of "per scope bindings". With live ranges we would specify all the generated ranges where a original variable is available and what expression to use for each range. This would make generators more complicated since they have to figure out liveness, but debuggers become "trivial" since for any given generated position they only have to check the source map for all live ranges at that position to figure out the set of available original variables. |
About the second example:
Not quite: So the debugger just needs to look up |
Sorry for the confusion. I was a bit hung up on how all of this currently works in Chrome DevTools and V8. There we need to distinguish between a debugger's scope view and arbitrary expression evaluation at a breakpoint. The former is also already possible in Chrome DevTools (similar to how you pointed out it works in Firefox). It'll even source map the name already and show you For expression evaluation Chrome DevTools currently only does a search&replace of variable names from original to compiled, which runs in the aforementioned shadowing problem. We don't provide V8 yet with source mapped scope information (but I aim to fix this once the proposal is further along). With only a search&replace scheme it is currently tricky to figure out what is a valid substitution due to liveness. Note though that any tool that does constant propagation can't properly express that with "per scope" bindings. |
Exactly. The debugger would need to be able to evaluate an expression "in an outer scope" but I haven't seen any debuggers that offer this functionality. It can be emulated though: to evaluate an expression |
I was thinking we could modify CDPs |
Sure, I just wanted to point out that this can be implemented without adding special support to the debugger protocol. |
The env proposal would add information about scopes and bindings in the original source to a sourcemap. That proposal satisfies all the scenarios listed here but there is one scenario it doesn't support: variable shadowing, i.e. a variable declared in an outer scope that is shadowed by another variable with the same name in an inner scope. This scenario is quite common because javascript minifiers reuse variable names aggressively.
To recap: The env proposal would add an
env
field encoding the scopes and bindings:Imagine that this original code
was transpiled to
The scope information encoded according to the env proposal would contain the following binding information (I'm omitting the bindings for the functions here):
value
=>a
,value_plus_one
=>b
num
=>a
,num_plus_one
=>b
When the debugger is paused at
console.log(b);
, the scopes would be:a
=>2
,b
=>3
a
=>1
,b
=>2
Now the debugger tries to reconstruct the original scopes: for
num_plus_one
in the outer function scope it would evaluateb
and get3
but the correct value is2
.To get the correct value, the debugger would need to understand that it should take the value of
b
in the outer function scope.In this example it seems rather obvious from which scope the value of
b
should be taken but in general, with deeper nesting of scopes and the transpiler adding and/or removing scopes there's no way for the debugger to know which generated scope it should use.This could be fixed by adding information about the generated scopes to the sourcemap (in addition to the information about original scopes): to find the value for a variable in a given original scope the debugger would translate the start and end locations of that scope to generated locations and then find the innermost generated scope that contains those locations. It would then evaluate the expression from the binding information in that scope. Note, however, that debuggers usually don't support evaluating in an outer scope of the one that execution is currently paused in, so the debugger would have to "emulate" this evaluation. This would probably only be feasible for very simple expressions, e.g.
x
,obj.prop
andarr[idx]
, but this would cover a lot of ground already and go beyond the current capabilities of javascript debuggers I've worked with (in the best case they support what can be done by only supporting variable names as binding expressions).Note that the information about generated scopes would only need to contain the start and end locations of the scopes, but not their type, name and bindings (although I would optionally keep the type information because I imagine it could be useful).
The text was updated successfully, but these errors were encountered: