Fix closure warnings in JS library code

For some reason closure doesn't report these real issues in the current configuration but they show up in the `MODULARIZE_INSTANCE` configuration that I'm working on. Most of these issues consist of adding default parameters to functions declared on objects such as `FS`. In addition there are a couple of other changes that are all real fixes: - ErrnoError unified between old FS and wasm FS. This should have been done as part of emscripten-core#21149 - Default initializer for `canvasResizedCallbackTargetThread` removed since `JSEvents.getTargetThreadForEventCallback()` will always return undefined when called with no arguments - Extra arguments to copyIndexedColorData removed in library_sdl.js since it only ever called with one arg. This change is basically code size neutral with a few tests coming in a byte or two heavier and others a byte or two lighter.
sbc100 · Apr 1, 2024 · 9dd0183 · 9dd0183
1 parent a8b489d
commit 9dd0183
Show file tree

Hide file tree

Showing 27 changed files with 99 additions and 118 deletions.
diff --git a/src/library_browser.js b/src/library_browser.js
@@ -54,6 +54,9 @@ var LibraryBrowser = {
       timingValue: 0,
       currentFrameNumber: 0,
       queue: [],
+#if USE_CLOSURE_COMPILER
+      expectedBlockers: 0,
+#endif
       pause() {
         Browser.mainLoop.scheduler = null;
         // Incrementing this signals the previous main loop that it's now become old, and it must return.
@@ -628,7 +631,7 @@ var LibraryBrowser = {
       Browser.resizeListeners.forEach((listener) => listener(canvas.width, canvas.height));
     },
 
-    setCanvasSize(width, height, noUpdates) {
+    setCanvasSize(width, height, noUpdates = false) {
       var canvas = Module['canvas'];
       Browser.updateCanvasDimensions(canvas, width, height);
       if (!noUpdates) Browser.updateResizeListeners();
@@ -658,7 +661,7 @@ var LibraryBrowser = {
       Browser.updateResizeListeners();
     },
 
-    updateCanvasDimensions(canvas, wNative, hNative) {
+    updateCanvasDimensions(canvas, wNative = 0, hNative = 0) {
       if (wNative && hNative) {
         canvas.widthNative = wNative;
         canvas.heightNative = hNative;

diff --git a/src/library_fs.js b/src/library_fs.js
@@ -51,7 +51,6 @@ FS.staticInit();` +
     devices: {},
     streams: [],
     nextInode: 1,
-    nameTable: null,
     currentPath: '/',
     initialized: false,
     // Whether we are currently ignoring permissions. Useful when preparing the
@@ -62,40 +61,10 @@ FS.staticInit();` +
 #if FS_DEBUG
     trackingDelegate: {},
 #endif
-    ErrnoError: null, // set during init
     genericErrors: {},
     filesystems: null,
     syncFSRequests: 0, // we warn if there are multiple in flight at once
-
-#if ASSERTIONS
-    ErrnoError: class extends Error {
-#else
-    ErrnoError: class {
-#endif
-      // We set the `name` property to be able to identify `FS.ErrnoError`
-      // - the `name` is a standard ECMA-262 property of error objects. Kind of good to have it anyway.
-      // - when using PROXYFS, an error can come from an underlying FS
-      // as different FS objects have their own FS.ErrnoError each,
-      // the test `err instanceof FS.ErrnoError` won't detect an error coming from another filesystem, causing bugs.
-      // we'll use the reliable test `err.name == "ErrnoError"` instead
-      constructor(errno) {
-#if ASSERTIONS
-        super(ERRNO_MESSAGES[errno]);
-#endif
-        // TODO(sbc): Use the inline member declaration syntax once we
-        // support it in acorn and closure.
-        this.name = 'ErrnoError';
-        this.errno = errno;
-#if ASSERTIONS
-        for (var key in ERRNO_CODES) {
-          if (ERRNO_CODES[key] === errno) {
-            this.code = key;
-            break;
-          }
-        }
-#endif
-      }
-    },
+    ErrnoError: SharedErrnoError,
 
     FSStream: class {
       constructor() {
@@ -304,7 +273,7 @@ FS.staticInit();` +
       // if we failed to find it in the cache, call into the VFS
       return FS.lookup(parent, name);
     },
-    createNode(parent, name, mode, rdev) {
+    createNode(parent, name, mode, rdev = 0) {
 #if ASSERTIONS
       assert(typeof parent == 'object')
 #endif
@@ -670,13 +639,13 @@ FS.staticInit();` +
       return parent.node_ops.mknod(parent, name, mode, dev);
     },
     // helpers to create specific types of nodes
-    create(path, mode) {
+    create(path, mode = undefined) {
       mode = mode !== undefined ? mode : 438 /* 0666 */;
       mode &= {{{ cDefs.S_IALLUGO }}};
       mode |= {{{ cDefs.S_IFREG }}};
       return FS.mknod(path, mode, 0);
     },
-    mkdir(path, mode) {
+    mkdir(path, mode = undefined) {
       mode = mode !== undefined ? mode : 511 /* 0777 */;
       mode &= {{{ cDefs.S_IRWXUGO }}} | {{{ cDefs.S_ISVTX }}};
       mode |= {{{ cDefs.S_IFDIR }}};
@@ -688,7 +657,7 @@ FS.staticInit();` +
       return FS.mknod(path, mode, 0);
     },
     // Creates a whole directory tree chain if it doesn't yet exist
-    mkdirTree(path, mode) {
+    mkdirTree(path, mode = undefined) {
       var dirs = path.split('/');
       var d = '';
       for (var i = 0; i < dirs.length; ++i) {
@@ -701,7 +670,7 @@ FS.staticInit();` +
         }
       }
     },
-    mkdev(path, mode, dev) {
+    mkdev(path, mode, dev = undefined) {
       if (typeof dev == 'undefined') {
         dev = mode;
         mode = 438 /* 0666 */;
@@ -906,7 +875,7 @@ FS.staticInit();` +
       }
       return PATH_FS.resolve(FS.getPath(link.parent), link.node_ops.readlink(link));
     },
-    stat(path, dontFollow) {
+    stat(path, dontFollow = false) {
       var lookup = FS.lookupPath(path, { follow: !dontFollow });
       var node = lookup.node;
       if (!node) {
@@ -920,7 +889,7 @@ FS.staticInit();` +
     lstat(path) {
       return FS.stat(path, true);
     },
-    chmod(path, mode, dontFollow) {
+    chmod(path, mode, dontFollow = false) {
       var node;
       if (typeof path == 'string') {
         var lookup = FS.lookupPath(path, { follow: !dontFollow });
@@ -943,7 +912,7 @@ FS.staticInit();` +
       var stream = FS.getStreamChecked(fd);
       FS.chmod(stream.node, mode);
     },
-    chown(path, uid, gid, dontFollow) {
+    chown(path, uid, gid, dontFollow = false) {
       var node;
       if (typeof path == 'string') {
         var lookup = FS.lookupPath(path, { follow: !dontFollow });
@@ -1009,7 +978,7 @@ FS.staticInit();` +
         timestamp: Math.max(atime, mtime)
       });
     },
-    open(path, flags, mode) {
+    open(path, flags, mode = undefined) {
       if (path === "") {
         throw new FS.ErrnoError({{{ cDefs.ENOENT }}});
       }
@@ -1187,7 +1156,7 @@ FS.staticInit();` +
 #endif
       return bytesRead;
     },
-    write(stream, buffer, offset, length, position, canOwn) {
+    write(stream, buffer, offset, length, position = undefined, canOwn = false) {
 #if ASSERTIONS
       assert(offset >= 0);
 #endif
@@ -1460,7 +1429,7 @@ FS.staticInit();` +
 #endif
       };
     },
-    init(input, output, error) {
+    init(input = undefined, output = undefined, error = undefined) {
 #if ASSERTIONS
       assert(!FS.init.initialized, 'FS.init was previously called. If you want to initialize later with custom parameters, remove any earlier calls (note that one is automatically added to the generated code)');
 #endif
@@ -1499,7 +1468,7 @@ FS.staticInit();` +
       }
       return ret.object;
     },
-    analyzePath(path, dontResolveLastLink) {
+    analyzePath(path, dontResolveLastLink = false) {
       // operate from within the context of the symlink's target
       try {
         var lookup = FS.lookupPath(path, { follow: !dontResolveLastLink });
@@ -1570,7 +1539,7 @@ FS.staticInit();` +
         FS.chmod(node, mode);
       }
     },
-    createDevice(parent, name, input, output) {
+    createDevice(parent, name, input, output = undefined) {
       var path = PATH.join2(typeof parent == 'string' ? parent : FS.getPath(parent), name);
       var mode = FS_getMode(!!input, !!output);
       if (!FS.createDevice.major) FS.createDevice.major = 64;

diff --git a/src/library_fs_shared.js b/src/library_fs_shared.js
@@ -4,6 +4,36 @@
  * SPDX-License-Identifier: MIT
  */
 
+#if ASSERTIONS
+class SharedErrnoError extends Error {
+#else
+class SharedErrnoError {
+#endif
+  // We set the `name` property to be able to identify `FS.ErrnoError`
+  // - the `name` is a standard ECMA-262 property of error objects. Kind of good to have it anyway.
+  // - when using PROXYFS, an error can come from an underlying FS
+  // as different FS objects have their own FS.ErrnoError each,
+  // the test `err instanceof FS.ErrnoError` won't detect an error coming from another filesystem, causing bugs.
+  // we'll use the reliable test `err.name == "ErrnoError"` instead
+  constructor(errno) {
+#if ASSERTIONS
+    super(ERRNO_MESSAGES[errno]);
+#endif
+    // TODO(sbc): Use the inline member declaration syntax once we
+    // support it in acorn and closure.
+    this.name = 'ErrnoError';
+    this.errno = errno;
+#if ASSERTIONS
+    for (var key in ERRNO_CODES) {
+      if (ERRNO_CODES[key] === errno) {
+        this.code = key;
+        break;
+      }
+    }
+#endif
+  }
+}
+
 addToLibrary({
   $preloadPlugins: "{{{ makeModuleReceiveExpr('preloadPlugins', '[]') }}}",
 
@@ -50,7 +80,7 @@ addToLibrary({
     '$FS_handledByPreloadPlugin',
 #endif
   ],
-  $FS_createPreloadedFile: (parent, name, url, canRead, canWrite, onload, onerror, dontCreateFile, canOwn, preFinish) => {
+  $FS_createPreloadedFile: (parent, name, url, canRead, canWrite, onload, onerror, dontCreateFile, canOwn = false, preFinish = undefined) => {
     // TODO we should allow people to just pass in a complete filename instead
     // of parent and name being that we just join them anyways
     var fullname = name ? PATH_FS.resolve(PATH.join2(parent, name)) : parent;

diff --git a/src/library_glemu.js b/src/library_glemu.js
@@ -69,7 +69,6 @@ var LibraryGLEmulation = {
     fogStart: 0,
     fogEnd: 1,
     fogDensity: 1.0,
-    fogColor: null,
     fogMode: 0x800, // GL_EXP
     fogEnabled: false,
 
@@ -2127,7 +2126,7 @@ var LibraryGLEmulation = {
       return renderer;
     },
 
-    createRenderer(renderer) {
+    createRenderer() {
       var useCurrProgram = !!GL.currProgram;
       var hasTextures = false;
       for (var i = 0; i < GLImmediate.MAX_TEXTURES; i++) {
@@ -2995,7 +2994,7 @@ var LibraryGLEmulation = {
       }
     },
 
-    flush(numProvidedIndexes, startIndex = 0, ptr = 0) {
+    flush(numProvidedIndexes = 0, startIndex = 0, ptr = 0) {
 #if ASSERTIONS
       assert(numProvidedIndexes >= 0 || !numProvidedIndexes);
 #endif

diff --git a/src/library_html5.js b/src/library_html5.js
@@ -94,8 +94,7 @@ var LibraryHTML5 = {
         return true;
       }
       // Test if the given call was already queued, and if so, don't add it again.
-      for (var i in JSEvents.deferredCalls) {
-        var call = JSEvents.deferredCalls[i];
+      for (var call of JSEvents.deferredCalls) {
         if (call.targetFunction == targetFunction && arraysHaveEqualContent(call.argsList, argsList)) {
           return;
         }
@@ -106,7 +105,10 @@ var LibraryHTML5 = {
         argsList
       });
 
-      JSEvents.deferredCalls.sort((x,y) => x.precedence < y.precedence);
+      // sortcallback(x, y):
+      // A negative value indicates that x should come before y.
+      // A positive value indicates that x should come after y.
+      JSEvents.deferredCalls.sort((x,y) => y.precedence - x.precedence);
     },
 
     // Erases all deferred calls to the given target function from the queue list.
@@ -151,10 +153,9 @@ var LibraryHTML5 = {
     // Removes all event handlers on the given DOM element of the given type.
     // Pass in eventTypeString == undefined/null to remove all event handlers
     // regardless of the type.
-    removeAllHandlersOnTarget: (target, eventTypeString) => {
+    removeAllHandlersOnTarget: (target) => {
       for (var i = 0; i < JSEvents.eventHandlers.length; ++i) {
-        if (JSEvents.eventHandlers[i].target == target &&
-          (!eventTypeString || eventTypeString == JSEvents.eventHandlers[i].eventTypeString)) {
+        if (JSEvents.eventHandlers[i].target) {
            JSEvents._removeHandler(i--);
          }
       }
@@ -1512,9 +1513,6 @@ var LibraryHTML5 = {
         filteringMode: {{{ makeGetValue('fullscreenStrategy', C_STRUCTS.EmscriptenFullscreenStrategy.filteringMode, 'i32') }}},
         canvasResizedCallback: {{{ makeGetValue('fullscreenStrategy', C_STRUCTS.EmscriptenFullscreenStrategy.canvasResizedCallback, 'i32') }}},
         canvasResizedCallbackUserData: {{{ makeGetValue('fullscreenStrategy', C_STRUCTS.EmscriptenFullscreenStrategy.canvasResizedCallbackUserData, 'i32') }}},
-#if PTHREADS
-        canvasResizedCallbackTargetThread: JSEvents.getTargetThreadForEventCallback(),
-#endif
         target,
         softFullscreen: true
     };

diff --git a/src/library_nodefs.js b/src/library_nodefs.js
@@ -53,9 +53,9 @@ addToLibrary({
 #if ASSERTIONS
       assert(ENVIRONMENT_IS_NODE);
 #endif
-      return NODEFS.createNode(null, '/', NODEFS.getMode(mount.opts.root), 0);
+      return NODEFS.createNode(null, '/', NODEFS.getMode(mount.opts.root));
     },
-    createNode(parent, name, mode, dev) {
+    createNode(parent, name, mode) {
       if (!FS.isDir(mode) && !FS.isFile(mode) && !FS.isLink(mode)) {
         throw new FS.ErrnoError({{{ cDefs.EINVAL }}});
       }
@@ -169,7 +169,7 @@ addToLibrary({
         return NODEFS.createNode(parent, name, mode);
       },
       mknod(parent, name, mode, dev) {
-        var node = NODEFS.createNode(parent, name, mode, dev);
+        var node = NODEFS.createNode(parent, name, mode);
         // create the backing node for this in the fs root as well
         var path = NODEFS.realPath(node);
         try {
@@ -320,7 +320,7 @@ addToLibrary({
         return { ptr, allocated: true };
       },
       msync(stream, buffer, offset, length, mmapFlags) {
-        NODEFS.stream_ops.write(stream, buffer, 0, length, offset, false);
+        NODEFS.stream_ops.write(stream, buffer, 0, length, offset);
         // should we check if bytesWritten and length are the same?
         return 0;
       }

diff --git a/src/library_openal.js b/src/library_openal.js
@@ -103,7 +103,7 @@ var LibraryOpenAL = {
     // represents the queue of buffers scheduled for physical playback. These two queues are
     // distinct because of the differing semantics of OpenAL and web audio. Some changes
     // to OpenAL parameters, such as pitch, may require the web audio queue to be flushed and rescheduled.
-    scheduleSourceAudio: (src, lookahead) => {
+    scheduleSourceAudio: (src, lookahead = false) => {
       // See comment on scheduleContextAudio above.
       if (Browser.mainLoop.timingMode === {{{ cDefs.EM_TIMING_RAF }}} && document['visibilityState'] != 'visible') {
         return;

diff --git a/src/library_sdl.js b/src/library_sdl.js
@@ -116,8 +116,6 @@ var LibrarySDL = {
     TOUCH_DEFAULT_ID: 0, // Our default deviceID for touch events (we get nothing from the browser)
 
     eventHandler: null,
-    eventHandlerContext: null,
-    eventHandlerTemp: 0,
 
     // DOM code ==> SDL code. See
     // https://developer.mozilla.org/en/Document_Object_Model_%28DOM%29/KeyboardEvent
@@ -346,7 +344,7 @@ var LibrarySDL = {
 
     translateRGBAToColor: (r, g, b, a) => r | g << 8 | b << 16 | a << 24,
 
-    makeSurface(width, height, flags, usePageCanvas, source, rmask, gmask, bmask, amask) {
+    makeSurface(width, height, flags, usePageCanvas, source = 0, rmask = 0, gmask = 0, bmask = 0, amask = 0) {
       var is_SDL_HWSURFACE = flags & 0x00000001;
       var is_SDL_HWPALETTE = flags & 0x00200000;
       var is_SDL_OPENGL = flags & 0x04000000;
@@ -438,7 +436,7 @@ var LibrarySDL = {
 
     // Copy data from the C++-accessible storage to the canvas backing
     // for surface with HWPALETTE flag(8bpp depth)
-    copyIndexedColorData(surfData, rX, rY, rW, rH) {
+    copyIndexedColorData(surfData) {
       // HWPALETTE works with palette
       // set by SDL_SetColors
       if (!surfData.colors) {
@@ -448,10 +446,10 @@ var LibrarySDL = {
       var fullWidth  = Module['canvas'].width;
       var fullHeight = Module['canvas'].height;
 
-      var startX  = rX || 0;
-      var startY  = rY || 0;
-      var endX    = (rW || (fullWidth - startX)) + startX;
-      var endY    = (rH || (fullHeight - startY)) + startY;
+      var startX  = 0;
+      var startY  = 0;
+      var endX    = (fullWidth - startX) + startX;
+      var endY    = (fullHeight - startY) + startY;
 
       var buffer  = surfData.buffer;
 

diff --git a/src/library_sockfs.js b/src/library_sockfs.js
@@ -140,7 +140,7 @@ addToLibrary({
       // these functions aren't actually sock_ops members, but we're
       // abusing the namespace to organize them
       //
-      createPeer(sock, addr, port) {
+      createPeer(sock, addr, port = undefined) {
         var ws;
 
         if (typeof addr == 'object') {