Skip to content

JS: Google Closure Compiler hard errors (fixes #24602)

Serge S. Gulin requested to merge gulin.serge/ghc:wip/google_closure into master

Still WIP but any review comments are welcome! Still in makeup process, the best time to add review comments.

You can test:

$ ../ghc-js/_build/stage1/bin/javascript-unknown-ghcjs-ghc -fforce-recomp HelloJS.hs
$ google-closure-compiler --debug --warning_level VERBOSE --formatting PRETTY_PRINT --isolation_mode IIFE --assume_function_wrapper --emit_use_strict --externs ./HelloJS.jsexe/all.js.externs --compilation_level ADVANCED_OPTIMIZATIONS ./HelloJS.jsexe/all.js --js_output_file ./HelloJS.jsexe/all.min.debug.js
...
0 error(s), 683 warning(s), 94.2% typed

The series of commits is large. Let's break it on independent parts and connect commits to errors on which they are targeted.

JS: fix typos and namings (fixes #24602 (closed))

Errors fixed by this commit 
./HelloJS.jsexe/all.js:9747:68: ERROR - [JSC_UNDEFINED_VARIABLE] variable null_ is undeclared
  9747|     var p = (((ptr_d).arr && (ptr_d).arr[off]) ? (ptr_d).arr[off] : null_); var o = (ptr_d).dv.getInt32(off,true);;
                                                                            ^^^^^
./HelloJS.jsexe/all.js:12808:11: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$vt_int is undeclared
  12808|       case h$vt_int:
                    ^^^^^^^^
./HelloJS.jsexe/all.js:10273:26: ERROR - [JSC_UNDEFINED_VARIABLE] variable j is undeclared
  10273|                       for(j=x.length-1;j>=0;j--) {
                                   ^
./HelloJS.jsexe/all.js:12247:9: ERROR - [JSC_UNDEFINED_VARIABLE] variable ptr is undeclared
  12247|   return ptr = h$initHeapBufferLen(str_d, str_o, str_d.len);
                  ^^^
./HelloJS.jsexe/all.js:13834:33: ERROR - [JSC_UNDEFINED_VARIABLE] variable arr is undeclared
  13834|   return h$charCodeArrayToString(arr);
                                          ^^^
./HelloJS.jsexe/all.js:13925:12: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$EINVAL is undeclared
  13925|   h$errno = h$EINVAL;
                     ^^^^^^^^

You may noted that I've also changed term of

, global "h$vt_double" ||= toJExpr IntV

IntV

  WaitReadOp  -> \[] [fd] -> pure $ PRPrimCall $ returnS (app "h$waidRead" [fd])

h$waidRead

I think these were typos.

JS: trivial checks for variable presence (fixes #24602 (closed))

Errors fixed by this commit 
./HelloJS.jsexe/all.js:8432:27: ERROR - [JSC_UNDEFINED_VARIABLE] variable Buffer is undeclared
  8432|         h$fs.read(real_fd, Buffer.alloc(n), 0, n, pos, function(err, bytesRead, nbuf) {
                                   ^^^^^^
./HelloJS.jsexe/all.js:8495:8: ERROR - [JSC_UNDEFINED_VARIABLE] variable putstr is undeclared
  8495|         putstr(h$decodeUtf8(buf, n, buf_offset));
                ^^^^^^
./HelloJS.jsexe/all.js:8499:8: ERROR - [JSC_UNDEFINED_VARIABLE] variable printErr is undeclared
  8499|         printErr(h$decodeUtf8(buf, n, buf_offset));
                ^^^^^^^^
./HelloJS.jsexe/all.js:8507:37: ERROR - [JSC_UNDEFINED_VARIABLE] variable debug is undeclared
  8507|     var h$base_stderrLeftover = { f: debug, val: null };
                                             ^^^^^
./HelloJS.jsexe/all.js:7795:8: ERROR - [JSC_UNDEFINED_VARIABLE] variable java is undeclared
  7795|         java.lang.System.out.print(s);
                ^^^^
./HelloJS.jsexe/all.js:12498:25: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$setCcs_e is undeclared
  12498|     if(h$stack[h$sp] !== h$setCcs_e) {
                                  ^^^^^^^^^^

JS: fs module imported twice (by emscripten and by ghc-internal)

Errors fixed by this commit 
./HelloJS.jsexe/all.js:57067:4: ERROR - [JSC_VAR_MULTIPLY_DECLARED_ERROR] Variable fs declared more than once. First occurrence: ./HelloJS.jsexe/all.js:7767:8
  57067| var fs, nodePath;

Better solution is to use some JavaScript module system. It will be investigated at other issues.

JS: thread.js requires hfds and hfdReady to be declared for static code analysis

Errors fixed by this commit 
./HelloJS.jsexe/all.js:14112:2: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$fds is undeclared
  14112|   h$fds[fd].waitRead.push(h$currentThread);
           ^^^^^
./HelloJS.jsexe/all.js:33154:5: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$fdReady is undeclared
  33154| h$r1=h$fdReady(a,1,f,(c>>>0),0);
              ^^^^^^^^^

I am not sure does it enough for now or not. I've just copied some old pieces of GHCJS from publicly available sources. Also I didn't put details to h$fds. I took minimal and left only its object initialization: var h$fds = {};.

So, any shedding light is important here. Does it OK for now or it requires better attention?

JS: heap and stack overflows reporting defined as js hard failure (fixes #24602 (closed))

Errors fixed by this commit 
./HelloJS.jsexe/all.js:47403:0: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$reportStackOverflow is undeclared
  47403| h$reportStackOverflow(c);
         ^^^^^^^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:47407:8: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$reportHeapOverflow is undeclared
  47407| case(2):h$reportHeapOverflow();
                 ^^^^^^^^^^^^^^^^^^^^

These errors were treated as a hard failure for browser application. The fix is trivial: just throw error. Please, suggest better way if needed due to STG semantics.

Stubs

Errors fixed by these commits. They are common by sense. 
./HelloJS.jsexe/all.js:12941:22: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$checkInvariants_e is undeclared
  12941|       h$stack[h$sp] = h$checkInvariants_e;
                               ^^^^^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:24517:13: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$stg_cloneMyStackzh is undeclared
  24517| if(c) {var g=h$stg_cloneMyStackzh();
                      ^^^^^^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:24518:6: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$stg_decodeStackzh is undeclared
  24518| var i=h$stg_decodeStackzh(g);
               ^^^^^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:25814:6: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$libdwLookupLocation is undeclared
  25814| var h=h$libdwLookupLocation(d,e,a,c,f,g);
               ^^^^^^^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:26282:21: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$libdwPoolRelease is undeclared
  26282| h$r3=h$mkFunctionPtr(h$libdwPoolRelease);
                              ^^^^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:26290:2: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$libdwPoolTake is undeclared
  26290| a=h$libdwPoolTake();
           ^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:26307:2: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$libdwGetBacktrace is undeclared
  26307| c=h$libdwGetBacktrace(a,b);
           ^^^^^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:26335:21: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$backtraceFree is undeclared
  26335| h$r3=h$mkFunctionPtr(h$backtraceFree);
                              ^^^^^^^^^^^^^^^
./HelloJS.jsexe/all.js:37760:5: ERROR - [JSC_UNDEFINED_VARIABLE] variable h$lookupIPE is undeclared
  37760| h$r1=h$lookupIPE(a,c,d,0);
              ^^^^^^^^^^^

Stubs commits:

  • STM: h$stmCheckInvariants
  • RTS/Libdw: h$libdwLookupLocation, h$libdwPoolRelease, h$libdwPoolTake, h$libdwGetBacktrace, h$backtraceFree, h$libdwLookupLocation, h$libdwPoolRelease, h$libdwPoolTake, h$libdwGetBacktrace, h$backtraceFree
  • RTS/StackCloningDecoding: h$stg_cloneMyStackzh, h$stg_decodeStackzh
  • RTS h$lookupIPE

These errors were fixed just by introducing stubbed functions with throw for further implementation. Again, please let me known which functions should be actually ported (and from where) in this commit, I could try to do that.

Here I think there is a good reason to squash them into single commit if we do not need to apply actual implementation in this MR. Squashed.

Google Closure Externs support

The error from which this mini-journey was started 
./HelloJS.jsexe/all.js:57052:22: ERROR - [JSC_UNDEFINED_VARIABLE] variable __dirname is undeclared
  57052|     scriptDirectory = __dirname + '/';
                               ^^^^^^^^^

It is the heaviest part of this MR by sense:

  • JS: Add special preprocessor for js files due of needing to keep jsdoc comments (fixes #24602 (closed))
  • JS: Add externs to linker (fixes #24602 (closed))

Our js files have defined google closure compiler types at jsdoc entries but these jsdoc entries are removed by cpp preprocessor. I considered that reusing them in javascript-backend would be a nice thing. Right now haskell processor uses -traditional option to deal with comments and // operators. Its application has the long story: 1997, 1999, 2000, 2001, 2003 1, 2003 2, 2003 3, 2013, 2014 1, 2014 2, 2014 3, 2014 4, 2015, 2022 first usage in context of javascript-backend, 2023 1, 2023 2, 2023 3, 2024, . But now there are following compiler options: -C and -CC. You can read about them at GCC and CLang. It seems that -CC works better for javascript jsdoc than -traditional. At least it leaves /* ... */ comments w/o changes.

After enabling jsdoc and built-in google closure compiler types I was needed to deal with the following:

  1. Define NodeJS-environment types. I've just copied minimal set of externs from semi-official repo.
  2. Define Emscripten-environment types: HEAP8. Emscripten already provides some externs in our code but it supposed to be run in some module system. And its definitions do not work well in plain bundle.
  3. We have some functions which purpose is to add to functions some contextual information via function properties. These functions should be marked as modifies to let google closure compiler remove calls if these functions are not used actually by call graph. Such functions are: h$o, h$sti, h$init_closure, h$setObjInfo.
  4. STG primitives such as registries and stuff from GHC.StgToJS. dXX properties were already present at externs generator function but by unknown reason they are started from 7, not from 1. This message is related: // fixme does closure compiler bite us here?.
Edited by Serge S. Gulin

Merge request reports