Noticed this while running test262 tests that many variables were throwing an
invariant for being undefined. This includes things like the special `arguments`
object, a global `assert` function used by test262, etc.
- Adds a shallow git submodule for test262 as the tests aren't available as an
npm module - To run all tests: `yarn test262:all`. Note that this chunks up the
tests by test262 folder as there are over 50k+ tests and the test harness only
accepts arrays of filepaths which exceeds arg limits - To run a specific test:
`yarn test262 test262/test/folder/file.js`. You can also pass globs which
expand into an array of filepaths: `yarn test262 test262/test/folder/**/*.js` -
More instructions for the test-harness can be found here:
https://github.com/bterlson/test262-harness
I noticed on @kassens's #771 that despite running LeaveSSA there are still cases
where we still reassign to a unique identifier: functions that have reassignment
but no phi nodes, such as:
```javascript
function foo() {
let x$1 = 0;
x$2 = x$1 + 1;
}
```
Here SSA form rewrote the second statement's LHS, but bc there's no phi node we
can't recover what the original was supposed to be (`x = x + 1`). This was my
oversight when suggesting the simpler LeaveSSA algorithm, it works for
eliminating phis but not other reassignments. The only alternative to removing
SSA form is to add assignment statements, which we obviously don't want to do
since that generates bloat.
This PR addresses the issue by adding an additional, optional property to
`Identifier` called `preSsaId` that starts off null. When entering SSA we save
the original id in this property and update id to a new SSA value. LeaveSSA does
the inverse, setting id = preSsaId and nulling out the latter. This means that
an identifier can always be uniquely identified by its `id` value at any point
in the compiler, while it's trivial to correctly undo SSA form.
```typescript
type Identifier = {
// Unique value for each original identifier
id: IdentifierId;
// The original, un-mangled variable name if this was a variable present in the
source (null if it's generated)
name: string | null;
// When in SSA mode, this is set to the original, pre-SSA `id` value
preSsaId: IdentifierId | null;
}
```
Implements assignment expressions with operators other than `=` (such as `+=`)
by lowering to an assignment.
I think this isn't fully correct for something like `a.b.c += 1`, but it seems
like there's more gaps in object accesses.
- Get rid of `indent` as it was making the code hard to read - Remove
unnecessary 2nd iteration over blocks - Remove extra newline between the bb
subgraphs and the jumps section - Remove trailing spaces - Remove newlines
between each subgraph and jump
We might need to revisit the tabs vs. options split, but for now this just adds
a checkbox toggle that outputs codegenned JS instead of HIR in the HIR tab. Open
to ideas to organize this in the future...
This PR adds a new section to fixture tests, which renders the HIR into
a visualization using mermaid.js syntax which can then be embedded
directly into markdown.
The nice thing about the mermaid syntax is that it's quite readable, so
if desired we could replace the current basic block textual output with
the mermaid block. I'm opting to append it for now and wait for feedback
if we want to keep both or replace.
To view the graphs in your editor, download an extension that
adds mermaid.js support:
https://mermaid-js.github.io/mermaid/#/integrations?id=editor-plugins. In vscode
you can use this plugin by right clicking on "Open Preview" on any expect.md
file. No extra dependencies are required for GitHub which should have builtin
support for mermaid in markdown
Instead of hacking into the babel plugin passes, this now takes a new approach
for the HIR tab:
- The different passes are exported from the babel plugin (for simplicity)
- The tab actually runs the compiler steps based on local config in the tab.
- Re-purposed the CompilerFlags component to configure what passes to run. This
is currently mostly causing different errors, but could be useful going forward
as a direction.
The `--watch` argument forwarding wasn't working anymore because `yarn build`
doesn't have the `tsc` command at the end anymore. There's maybe something that
can be done to forward the watch argument, just call `tsc --watch` directly.
Small adjustment to the previous PR for a special case:
```javascript
while (cond) {
break;
}
```
The loop body is an indirection to the fallthrough, so shrink() collapses that
and makes the while.loop === while.fallthrough. We now detect that this is the
case in codegen and correctly emit a `break` rather than trying to write the
fallthrough block inside the loop.
Adds a new 'while' terminal variant, which will be a model for other loop
terminals, and adds support for the entire compilation pipeline through codegen.
To understand the structure of the terminal consider this input:
```javascript
let x = 0;
while (x) {
x = foo(x);
}
return x;
```
We currently lower this to ifs and gotos:
```
bb0: precursor to loop
let x = 0;
goto(break) bb1; // <-- **The new terminal replaces this**
bb1: test block, whether to (re-)enter the loop
if (x) consequent=bb2 alternate=bb3;
bb2: loop body
x = foo(x);
goto(continue) bb1;
bb3: fallthrough after the loop
return x
```
This representation correctly models the semantics of while statements, but
loses the high-level information that there was a loop. The new 'while' terminal
replaces the first 'goto(break) bb1'. Conceptually, the 'while' terminal means
"enter the starting point of a while loop". In this example the terminal would
look like this:
```
{
kind: 'while',
testBlock: 'bb1', // the basic block that checks whether to enter the loop or
not
loop: 'bb2', // the block containing the loop body
fallthrough: 'bb3' // the block that goes after the loop
}
```
Most passes will only look at 'testBlock', ie they will treat this terminal as a
simple goto:testBlock. However, codegen uses the full information in the
terminal to reconstruct the loop. My previous PR, #755, added a mechanism to be
smart about when to emit or not emit `break` statements; this PR improves upon
that to accurately emit the minimal break and continue statements: ie omitting
entirely where they are extraneous, emitting unlabeled break/continue when
sufficient, and falling back to labeled break/continue only where strictly
necessary. The logic is very much analogous to IR construction.
Alternative approach to #750. We now store the original identifier on the Phi
node, then rewrite every BasicBlock's identifiers to reference the original id
instead of the SSA'd id.
This solves the shadowing problem and also lets us omit adding copies of
instructions.
The approach is very similar to what BuildHIR does to resolve break and continue
targets during IR construction:
* We annotate goto targets as either a break or a continue (during HIR
construction). This is necessary to reconstruct the right kind in codegen.
* Codegen continues to work by traversing the IR as if it were a tree, relying
on the `fallthrough` branches of if/switch to be able to visit the
consequent/alternate recursively and then emit the fallthrough branch.
* We track a Set of blocks that are scheduled to be emitted by some parent in
the tree. Nested ifs may all have the same fallthrough branch, which we only
want to emit once. This set helps us to know that a parent is already going to
emit some block, such that children can skip it.
* We also keep a stack of break targets that are in scope, and use this to
convert gotos appropriately, as either a break, continue, or nothing at all (for
example a switch case that falls through has no explicit syntax to model this
fall-through, the only option is to emit nothing for the goto).
* Then, if/switch have to carefully check whether each branch should be emitted
or not. For example, if the alternate is already scheduled to be emitted (by a
parent), then we emit a block with a break statement instead.
* Switch in particular is tricky, because we need to know that subsequent cases
are scheduled, but only for preceding blocks. So we visit the cases in reverse
order (not surprisingly, we do the same thing during IR construction for similar
reasons!).
The bookkeeping is a bit finicky but this works reliably. There are some cases
where we could try to emit an unlabeled break instead of a labeled break, or
avoid emitting a label at all (if nothing will explicitly break to that label),
but overall the generated code is readable enough that i'm inclined to ship and
iterate. I'm open to feedback though, as always!
Reverts #726 which added an early optimization to the SSAify pass in skipping
over phi creation if only one unique operand. This is no longer necessary with
the addition of a phi elimination pass added in #739.
This is an alternate take on phi elimination to the one we pursued over VC w
@poteto driving. This version exploits the RPO ordering of blocks to do phi
elimination in a single pass when there are no loops, and to minimize repeated
visits when there are loops. The main difference is when redundant phis are
removed. Rather than eagerly walking through the CFG for each pruned phi to
rewrite its uses, we build up a mapping of rewritten identifiers. As we walk
through subsequent instructions, we rewrite each place based on that mapping. We
continue cycling through the blocks so long as a given iteration *both* added
new rewrites (meaning there may be subsequent uses to rewrite) *and* there are
back-edges. With no loops this results in a single visit of each block and of
each instruction, but even with loops this is bounded.
This diff adds styling to the compiler options editor. The floating input/output
toggle button on small screens now spans the bottom of the screen, so that it
doesn't block the compiler options.
Height overflows when adjusting screen size are complicated by Monaco Editor and
will be addressed in a later diff.
Test plan:
Start Playground and see the latest look of the compiler options editor beneath
the output section.
Removes all the `path: NodePath` values from various IR node types, replacing
them with `loc: SourceLocation`. This type is an alias for babel's source
location type plus a "generated" variant. The "OtherStatement" kind also used
the `path` to print back the original AST (since we don't look into these);
instead, we now capture the underlying `node` and emit that as-is during
codegen.
While I was doing this, i also fixed up the places where we had passed a null
path; the vast majority have a clear place we can pull a location from. For
example, `a ?? b` syntax creates some places/instructions for the `a != null`,
but those can all point back to the `a` location.
This commit introduces a small update to our fixture tests to allow certain jest
`test` modifiers to be added to fixture tests via a leading prefix in the
fixture name.
For example, `only.my-fixture.js` is the equivalent of writing
`test.only("my-fixture")`.
This currently basically lowers the code into the equivalent of
```
const vLeft = <left>;
const vNull = null;
const vCond = vLeft != vNull;
vCond ? vLeft : <right>
```
I created a temporary `Place` to hold the `null` constant value because the
binary operator in HIR accepts only `Place`s. Not sure if this is the preferred
approach. Alternatives I could think of:
- Allow constants as an alternative to Place?
- A `NotNull` operator for `<x> != null`
- Some other extension to the HIR?
## Proper Detection of Out-of-order Functions
The no-use-before-define rule from ESLint has a strange behavior in which it
treats variables differently than functions:
```javascript
function foo() {
return bar(X);
}
const X = null;
function bar(x) {}
```
By default, `bar(x)` has two errors: one because X is used before defined, and
once because `bar` is used before defined. The rule has an option `{variables:
false}` which only enables validation when the variable is from the same "scope"
as the reference, the net result of which is it means it doesn't report spurious
errors such as X being undefined. There is _also_ a `{functions: false}` option,
but for some reason that doesn't work the same way, it just turns off all
validation of references that came from functions. So enabling that option would
suppress the (spurious) error on invoking `bar()` above, but causes the rule to
miss invalid code such as:
```
function foo() {
return bar();
function bar() {}
}
```
This PR adds a fork of the rule that makes `{functions: false}` behave similarly
to `{variables: false}`, which should help avoid some of the spurious errors i
saw internally. The rule is exported from Forget itself, which will make it
easier to consume internally, in tests, and in the playground.
## Targeting the validation to Forget functions
Even with the above, there are still some false positives coming from code such
as:
```javascript
const x = foo();
function foo() {}
```
This PR changes codegen to ensure that the output of a function _always_ has the
body starting with 'use forget'. The ESLint rule then only looks at function
declarations/expressions whose body starts with that expression. The new unit
test confirms that the validation finds invalid reorderings even on functions
that weren't explicitly tagged as 'use forget'.
Lauren Tan
Joseph Savona
Jan Kassens
Joe Savona
Sathya Gunasekaran
Robert Zhang