Make EXPORTED_RUNTIME_METHODS work for library functions (#17369)

This helps when moving function from runtime to JS library since it
means that users who were previously using `EXPORTED_RUNTIME_METHODS` to
export such symbols will not be effected by the transition.

Author: sbc100

ChangeLog.md

@@ -20,6 +20,11 @@ See docs/process.md for more on how version tagging works.
 
 3.1.16
 ------
+- When JS library functions are included as part of `EXPORTED_RUNTIME_METHODS`
+  it is no longer necessary to also add them to
+  `DEFAULT_LIBRARY_FUNCS_TO_INCLUDE`.  This change allows us to transition
+  runtime functions to JS library functions without the need to folks to add
+  `DEFAULT_LIBRARY_FUNCS_TO_INCLUDE`. (#17369)
 
 3.1.15 - 07/01/2022
 -------------------

src/jsifier.js

@@ -81,6 +81,11 @@ function runJSify(functionsOnly) {
     LibraryManager.load();
 
     const libFuncsToInclude = DEFAULT_LIBRARY_FUNCS_TO_INCLUDE;
+    for (const sym of EXPORTED_RUNTIME_METHODS) {
+      if ('$' + sym in LibraryManager.library) {
+        libFuncsToInclude.push('$' + sym);
+      }
+    }
     if (INCLUDE_FULL_LIBRARY) {
       for (const key in LibraryManager.library) {
         if (!isJsLibraryConfigIdentifier(key)) {

src/modules.js

@@ -344,6 +344,33 @@ function isExportedByForceFilesystem(name) {
          name === 'removeRunDependency';
 }
 
+function isInternalSymbol(ident) {
+  return ident + '__internal' in LibraryManager.library;
+}
+
+// When running with ASSERTIONS enabled we create stubs for each library
+// function that that was not included in the build.  This gives useful errors
+// when library dependencies are missing from `__deps` or depended on without
+// being added to DEFAULT_LIBRARY_FUNCS_TO_INCLUDE
+// TODO(sbc): These errors could potentially be generated at build time via
+// some kind of acorn pass that searched for uses of these missing symbols.
+function addMissingLibraryStubs() {
+  if (!ASSERTIONS) return '';
+  let rtn = '';
+  const librarySymbolSet = new Set(libraryFunctions);
+  for (const ident in LibraryManager.library) {
+    if (typeof LibraryManager.library[ident] === 'function') {
+      if (ident[0] === '$' && !isJsLibraryConfigIdentifier(ident) && !isInternalSymbol(ident)) {
+        const name = ident.substr(1);
+        if (!librarySymbolSet.has(name)) {
+          rtn += `var ${name} = missingLibraryFunc('${name}');\n`;
+        }
+      }
+    }
+  }
+  return rtn;
+}
+
 // export parts of the JS runtime that the user asked for
 function exportRuntime() {
   const EXPORTED_RUNTIME_METHODS_SET = new Set(EXPORTED_RUNTIME_METHODS);
@@ -379,7 +406,6 @@ function exportRuntime() {
         return `unexportedRuntimeFunction('${name}', ${fssymbol});`;
       }
     }
-    return '';
   }
 
   function maybeExportNumber(name) {
@@ -499,7 +525,7 @@ function exportRuntime() {
   // '$ which indicates they are JS methods.
   const runtimeElementsSet = new Set(runtimeElements);
   for (const ident in LibraryManager.library) {
-    if (ident[0] === '$' && !isJsLibraryConfigIdentifier(ident) && !LibraryManager.library[ident + '__internal']) {
+    if (ident[0] === '$' && !isJsLibraryConfigIdentifier(ident) && !isInternalSymbol(ident)) {
       const jsname = ident.substr(1);
       assert(!runtimeElementsSet.has(jsname), 'runtimeElements contains library symbol: ' + ident);
       runtimeElements.push(jsname);
@@ -516,12 +542,12 @@ function exportRuntime() {
     const runtimeNumbersSet = new Set(runtimeNumbers);
     for (const name of EXPORTED_RUNTIME_METHODS_SET) {
       if (!runtimeElementsSet.has(name) && !runtimeNumbersSet.has(name)) {
-        printErr(`warning: invalid item (maybe a typo?) in EXPORTED_RUNTIME_METHODS: ${name}`);
+        printErr(`warning: invalid item in EXPORTED_RUNTIME_METHODS: ${name}`);
       }
     }
   }
   let exports = runtimeElements.map((name) => maybeExport(name));
   exports = exports.concat(runtimeNumbers.map((name) => maybeExportNumber(name)));
-  exports = exports.filter((name) => name != '');
-  return exports.join('\n');
+  exports = exports.filter((name) => name);
+  return exports.join('\n') + '\n' + addMissingLibraryStubs();
 }

src/parseTools.js

@@ -1045,14 +1045,6 @@ function makeRemovedFSAssert(fsName) {
   return `var ${fsName} = '${fsName} is no longer included by default; build with -l${lower}.js';`;
 }
 
-function makeRemovedRuntimeFunction(name) {
-  assert(ASSERTIONS);
-  if (libraryFunctions.includes(name)) {
-    return '';
-  }
-  return `function ${name}() { abort('\`${name}\` is now a library function and not included by default; add it to your library.js __deps or to DEFAULT_LIBRARY_FUNCS_TO_INCLUDE on the command line'); }`;
-}
-
 // Given an array of elements [elem1,elem2,elem3], returns a string "['elem1','elem2','elem3']"
 function buildStringArray(array) {
   if (array.length > 0) {

src/runtime_debug.js

@@ -30,6 +30,10 @@ function unexportedMessage(sym, isFSSybol) {
   return msg;
 }
 
+function missingLibraryFunc(sym) {
+  return () => abort('Call to `' + sym + '` which is a library function and not included by default; add it to your library.js __deps or to DEFAULT_LIBRARY_FUNCS_TO_INCLUDE on the command line');
+}
+
 function unexportedRuntimeSymbol(sym, isFSSybol) {
   if (!Object.getOwnPropertyDescriptor(Module, sym)) {
     Object.defineProperty(Module, sym, {

src/settings.js

@@ -863,10 +863,7 @@ var ASYNCIFY_DEBUG = 0;
 
 // Runtime elements that are exported on Module by default. We used to export
 // quite a lot here, but have removed them all. You should use
-// EXPORTED_RUNTIME_METHODS for things you want to export from the runtime. Note
-// that methods on this list are only exported if they are included (either
-// automatically from linking, or due to being in
-// DEFAULT_LIBRARY_FUNCS_TO_INCLUDE).
+// EXPORTED_RUNTIME_METHODS for things you want to export from the runtime.
 // Note that the name may be slightly misleading, as this is for any JS library
 // element, and not just methods. For example, we can export the FS object by
 // having "FS" in this list.

src/shell.js

@@ -477,7 +477,6 @@ assert(typeof Module['TOTAL_MEMORY'] == 'undefined', 'Module.TOTAL_MEMORY has be
 #if !NODERAWFS
 {{{ makeRemovedFSAssert('NODEFS') }}}
 #endif
-{{{ makeRemovedRuntimeFunction('alignMemory') }}}
 
 #if USE_PTHREADS
 assert(ENVIRONMENT_IS_WEB || ENVIRONMENT_IS_WORKER || ENVIRONMENT_IS_NODE, 'Pthreads do not work in this environment yet (need Web Workers, or an alternative to them)');

tests/other/test_unoptimized_code_size.js.size

@@ -1 +1 @@
-89932
+98207

tests/other/test_unoptimized_code_size_no_asserts.js.size

@@ -1 +1 @@
-59666
+59667

tests/test_core.py

@@ -5680,22 +5680,16 @@ def test_utf(self):
     self.do_core_test('test_utf.c')
 
   def test_utf32(self):
-    if self.get_setting('MINIMAL_RUNTIME'):
-      self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', ['$UTF32ToString', '$stringToUTF32', '$lengthBytesUTF32'])
-    else:
-      self.set_setting('EXPORTED_RUNTIME_METHODS', ['UTF32ToString', 'stringToUTF32', 'lengthBytesUTF32'])
+    self.set_setting('EXPORTED_RUNTIME_METHODS', ['UTF32ToString', 'stringToUTF32', 'lengthBytesUTF32'])
     self.do_runf(test_file('utf32.cpp'), 'OK.')
     self.do_runf(test_file('utf32.cpp'), 'OK.', args=['-fshort-wchar'])
 
   def test_utf16(self):
+    self.set_setting('EXPORTED_RUNTIME_METHODS', ['writeAsciiToMemory'])
     self.do_runf(test_file('core/test_utf16.cpp'), 'OK.')
 
   def test_utf8(self):
-    if self.get_setting('MINIMAL_RUNTIME'):
-      self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', ['$AsciiToString', '$stringToAscii', '$writeAsciiToMemory'])
-    else:
-      self.set_setting('EXPORTED_RUNTIME_METHODS',
-                       ['UTF8ToString', 'stringToUTF8', 'AsciiToString', 'stringToAscii'])
+    self.set_setting('EXPORTED_RUNTIME_METHODS', ['UTF8ToString', 'stringToUTF8', 'AsciiToString', 'stringToAscii', 'writeAsciiToMemory'])
     self.do_runf(test_file('utf8.cpp'), 'OK.')
 
   @also_with_wasm_bigint