Skip to content
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

The async keyword should not be followed by a newline #1515

Merged
merged 3 commits into from
Oct 4, 2023
Merged

Conversation

vouillon
Copy link
Member

@vouillon vouillon commented Oct 3, 2023

Fixes #1514

@vouillon vouillon requested a review from hhugo October 3, 2023 11:41
@xvw
Copy link
Contributor

xvw commented Oct 3, 2023

Hi @vouillon
I have try your fix on https://github.com/funkywork/yourbones/tree/main/examples and I still have an issue related to that kind of line:

this.connection.open(a),a=this.connection),this.connection=this.setConnection(a),await

in https://gist.github.com/xvw/767a6e2ad72bff091b594a542d3cc744
So I guess that the fix should be applied to await too.

@vouillon
Copy link
Member Author

vouillon commented Oct 3, 2023

@xvw I missed a case. An async at the end of the line is not recognized as a keyword, and thus we get an error at the following await.

@hhugo
Copy link
Member

hhugo commented Oct 3, 2023

@vouillon, I've added some tests and added more fixes. I also rebased your branch.

@xvw
Copy link
Contributor

xvw commented Oct 3, 2023

When I try locally I have the following issue

16:58:26.439 Uncaught SyntaxError: missing ) in parenthetical main.bc.js:4875

# Line
this.connection.open(a),a=this.connection),this.connection=this.setConnection(a),await

Using a pretty-printer, I have this line

open(a = this.connection) {
                        // After this comment
                        this.connection === a && this.connection.connected || (this.connection.connected && this.close(), "string" == typeof a && (await this.connection.open(a), a = this.connection), this.connection = this.setConnection(a), await this.connection.open(), this.registerEventListeners(), this.events.emit("connect"))
                        // Before this comment
                    }

https://gist.githubusercontent.com/xvw/7ea94ae5657df668f948b2f024b52ba1/raw/a1f58b468efdf27306d4539c1dd1fb27c09e27ed/stubs.js (line 18651)

@hhugo
Copy link
Member

hhugo commented Oct 3, 2023

Trying your example from above, I see

ReferenceError: Cannot access 'b' before initialization
    at e (main.bc.js:411:7)
    at d (main.bc.js:2638:21)
    at d.next (<anonymous>)
    at main.bc.js:2646:56
    at new Promise (<anonymous>)
    at au (main.bc.js:2638:165)
    at window.DAppClient.<anonymous> (main.bc.js:2690:9)
    at Generator.next (<anonymous>)
    at j (main.bc.js:2661:14)
Promise.catch (async)
(anonymous) @ main.bc.js:2682
(anonymous) @ main.bc.js:2667
b9 @ main.bc.js:2660
initSDK @ main.bc.js:2680
ae @ main.bc.js:2676
Se @ main.bc.js:2807
window.DAppClient @ main.bc.js:6060
(anonymous) @ main.bc.js:11750
(anonymous) @ main.bc.js:11760

@xvw
Copy link
Contributor

xvw commented Oct 3, 2023

Trying your example from above, I see

ReferenceError: Cannot access 'b' before initialization
    at e (main.bc.js:411:7)
    at d (main.bc.js:2638:21)
    at d.next (<anonymous>)
    at main.bc.js:2646:56
    at new Promise (<anonymous>)
    at au (main.bc.js:2638:165)
    at window.DAppClient.<anonymous> (main.bc.js:2690:9)
    at Generator.next (<anonymous>)
    at j (main.bc.js:2661:14)
Promise.catch (async)
(anonymous) @ main.bc.js:2682
(anonymous) @ main.bc.js:2667
b9 @ main.bc.js:2660
initSDK @ main.bc.js:2680
ae @ main.bc.js:2676
Se @ main.bc.js:2807
window.DAppClient @ main.bc.js:6060
(anonymous) @ main.bc.js:11750
(anonymous) @ main.bc.js:11760

From yourbones/examples ?
You need to build the stubs before compilation using make js_stubs.

@hhugo
Copy link
Member

hhugo commented Oct 3, 2023

I now just see

main.bc.js:11614 Request_permissions_rejection

Just checking: have you pinned js_of_ocaml-compiler ? what commit do you use ?

@hhugo
Copy link
Member

hhugo commented Oct 3, 2023

I see a different line for line number 4875

window&&void

@xvw
Copy link
Contributor

xvw commented Oct 3, 2023

You need to run the application through a webserver (ie: python3 -m http.server --directory . ).
Yes I have opam update + opam upgrade

js_of_ocaml             5.3.0        Compiler from OCaml bytecode to JavaScript
js_of_ocaml-compiler    5.3.0        pinned to version 5.3.0 at git+https://github.com/ocsigen/js_of_ocaml.git#async-fix
js_of_ocaml-lwt         5.3.0        Compiler from OCaml bytecode to JavaScript
js_of_ocaml-ppx         5.3.0        Compiler from OCaml bytecode to JavaScript

@xvw
Copy link
Contributor

xvw commented Oct 3, 2023

Ok, using a fixed commit (9302343) it is working! Thanks a lot!

@hhugo hhugo merged commit cdb6d8c into master Oct 4, 2023
15 checks passed
@hhugo hhugo deleted the async-fix branch October 4, 2023 08:41
hhugo pushed a commit to hhugo/opam-repository that referenced this pull request Dec 4, 2023
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
mseri pushed a commit to ocaml/opam-repository that referenced this pull request Dec 6, 2023
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Compiling using --profile=release leads to runtime errors
3 participants