Search code, repositories, users, issues, pull requests...

cristianoc · 2022-07-07T06:42:26Z

See here: cristianoc/rescript-compiler-experiments@6101661

Once this is stable we can continue and make it point to this syntax commit.

IwanKaramazow · 2022-07-07T06:58:04Z

Are there roundtrip tests for this? Can't remember what tests are auto included in roundtrip.

Yes, printer tests are roundtripped

src/res_core.ml

cristianoc · 2022-07-07T09:29:50Z

src/res_parens.ml

@@ -338,6 +350,8 @@ let jsxChildExpr expr =
      }
        when startsWithMinus x ->
        Parenthesized
+      | _ when ParsetreeViewer.hasAwaitAttribute expr.pexp_attributes ->


Can move to a helper function, though the difference is minimal. Just easier to make sure all the instances are updated at once if something changes in future.

Another thing I was wondering is: this case seems to often go where there is also ParsetreeViewer.filterParsingAttrs attrs. But not always, so not sure what value this observation has.
But I guess in the cases where ParsetreeViewer.filterParsingAttrs attrs is checked, this await check could at least go right after it. Assuming that no cases in between should take precedence.

cristianoc · 2022-07-07T09:32:02Z

src/res_parsetree_viewer.ml

@@ -11,7 +11,7 @@ let arrowType ct =
      process attrsBefore (arg :: acc) typ2
    | {
     ptyp_desc = Ptyp_arrow ((Nolabel as lbl), typ1, typ2);
-     ptyp_attributes = [({txt = "bs"}, _)] as attrs;
+     ptyp_attributes = [({txt = "bs" | "res.async"}, _)] as attrs;
    } ->
      let arg = (attrs, lbl, typ1) in
      process attrsBefore (arg :: acc) typ2


Why is there a separate case for process attributes?
Why not always process attributes and if nothing of interest is found then it's a no-op.
And the following match case disappears.

src/res_parsetree_viewer.ml

cristianoc · 2022-07-07T15:33:20Z

Sweet. Let me sync this up to the async compiler PR.

cristianoc · 2022-07-07T16:21:47Z

Bingo. Everything works now, as far as I can see.

cristianoc · 2022-07-18T06:51:26Z

@IwanKaramazow could you rebase on master? Then we can merge.

operand-expr += `await` unary-expr

operand-expr += `async` es6-arrow-expression

Existing code using async/await as identifiers in e.g. record field names will keep working.

…d as async arrow function

Reasoning: In JavaScript `await` is a keyword *only* inside async functions. ```javascript let await = 1 // OK ✅ async function wait() { let await = 1 // NOT OK ❌ } ``` The plot twist is that the newer browsers (chrome or node) support top-level await. (By implicitly wrapping everything in an async function?) ```javascript let await = 1 // NOT OK, with top level await ❌ ``` This makes me think: * We can replicate JavaScript parser; inside async function expressions, use of `await` as identifier will result into a syntax error - Downside: this feels like a responsibility for another part of the compiler, not the parser? This is already implemented in cristianoc/rescript-compiler-experiments#1, so we would also be doing double work. - Other downside: if we ever allow top-level await, then we just implemented the above for nothing. * Allow `await` as a "non-keyword" everywhere with some "tricks". ```javascript let await = 1 let x = await // Ok, now it gets tricky. Does this start an "await"-expression? = await fetch(url) // could be this = await() // could be this = await await() // or even this = await // or we might just be assigning the identifier `await` to `x` ``` Seems like we can infer the following rules for an `await expression`: - space after `await` - next token is on the same line as `await` - next token indicates the start of a unary expression But this "breaks down" in the case of ```javascript let x = await - 1 // could be a binary expression: "identifier" MINUS "1" // could also be an await expression with a unary expression: `await` `(-1)` ``` These whitespace sensitive rules kinda feel super arbitrary. Which makes me think that introducing `await` as a keyword is an ok compromise.

IwanKaramazow · 2022-07-18T07:41:47Z

@cristianoc done