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'.
This just piggybacks on the infrastructure for handling Env.#variables.
In the future, a better approach would be to simplify the environment creation
and merging by leveraging the SSA property of the new IR -- 1) We don't need to
track IdentifierId per environment as they are all unique 2) Rather than
tracking values, we can just track Identifiers because Identifiers can never
be reassigned.
The semantics of lvalue changes based on whether lvalue.place.memberPath is null
or not. If it's null, then lvalue.place acts as the lvalue for the instruction,
otherwise it's just a reference to the memberPath specified location.
Ideally we'd have an MemberExpression IR that lowers this complex lvalue into a
temporary Place, uses this temporary place and stores back to the
MemberExpression.
Working around for now, will refactor to create a MemberExpression in the future
if necessary for other analysis.
Instead of using Place, use Identifier as the unit of comparison in SSA.
Place is too high level and can not be substituted for other Places (even those
with the same Identifier) as Place contain higher level metadata such as
memberPath.
Currently, HIR doesn't load global idenfifiers into a temporary Place which
means our SSA transform breaks when it tries to lookup this global identifier.
Instead of throwing, let's log and return the old place. This works for now but
will probably break when we start mutating globals, but at that point our HIR
builder will need fixes.
Joe Savona
Lauren Tan
Jan Kassens
Sathya Gunasekaran
Joseph Savona
Robert Zhang