Add type checking for -s settings (#16539)

For almost all types there is single type that can be inferred from the
default value.

There are a few exceptions, where a single settings couple have more
than one type (for example `PTHREAD_POOL_SIZE` can be an integer or
string).  For now, I simply exclude those few settings from the type
checking.

For now, I've not tried to add any extra annotations to the settings
file.

We could possibly followup by adding explicit annotations and/or support
for polymorphic settings, but this change is big with with minimal
impact.

Fixes: #16440

Author: sbc100

ChangeLog.md

@@ -20,6 +20,10 @@ See docs/process.md for more on how version tagging works.
 
 3.1.8
 -----
+- Command line settings (`-s`) are now type checked.  For example, passing a
+  string to a boolean setting will now generate an error (e.g.
+  `-sEXIT_RUNTIME=foo`).  Previously, the value of `foo` would have have been
+  interpreted as non-zero and accepted as valid. (#16539)
 - A warning (limited-postlink-optimizations) was added that gets shown when
   binaryen optimizations are limited due to DWARF information being requested.
   Several binaryen passed are not compatible with the preservation of DWARF

emcc.py

@@ -376,25 +376,17 @@ def apply_settings(user_settings):
     else:
       value = value.replace('\\', '\\\\')
 
-    existing = getattr(settings, user_key, None)
-    expect_list = type(existing) == list
+    expected_type = settings.types.get(key)
 
-    if filename and expect_list and value.strip()[0] != '[':
+    if filename and expected_type == list and value.strip()[0] != '[':
       # Prefer simpler one-line-per value parser
       value = parse_symbol_list_file(value)
     else:
       try:
-        value = parse_value(value, expect_list)
+        value = parse_value(value, expected_type)
       except Exception as e:
         exit_with_error('a problem occurred in evaluating the content after a "-s", specifically "%s=%s": %s', key, value, str(e))
 
-    # Do some basic type checking by comparing to the existing settings.
-    # Sadly we can't do this generically in the SettingsManager since there are settings
-    # that so change types internally over time.
-    # We only currently worry about lists vs non-lists.
-    if expect_list != (type(value) == list):
-      exit_with_error('setting `%s` expects `%s` but got `%s`' % (user_key, type(existing), type(value)))
-
     setattr(settings, user_key, value)
 
     if key == 'EXPORTED_FUNCTIONS':
@@ -1808,7 +1800,7 @@ def phase_linker_setup(options, state, newargs, user_settings):
     exit_with_error('--emrun is not compatible with MINIMAL_RUNTIME')
 
   if options.use_closure_compiler:
-    settings.USE_CLOSURE_COMPILER = options.use_closure_compiler
+    settings.USE_CLOSURE_COMPILER = 1
 
   if settings.CLOSURE_WARNINGS not in ['quiet', 'warn', 'error']:
     exit_with_error('Invalid option -s CLOSURE_WARNINGS=%s specified! Allowed values are "quiet", "warn" or "error".' % settings.CLOSURE_WARNINGS)
@@ -3876,7 +3868,7 @@ def parse_symbol_list_file(contents):
   return [v.strip() for v in values]
 
 
-def parse_value(text, expect_list):
+def parse_value(text, expected_type):
   # Note that using response files can introduce whitespace, if the file
   # has a newline at the end. For that reason, we rstrip() in relevant
   # places here.
@@ -3931,14 +3923,20 @@ def parse_string_list(text):
       return []
     return parse_string_list_members(text)
 
-  if expect_list or (text and text[0] == '['):
+  if expected_type == list or (text and text[0] == '['):
     # if json parsing fails, we fall back to our own parser, which can handle a few
     # simpler syntaxes
     try:
       return json.loads(text)
     except ValueError:
       return parse_string_list(text)
 
+  if expected_type == float:
+    try:
+      return float(text)
+    except ValueError:
+      pass
+
   try:
     if text.startswith('0x'):
       base = 16

emscripten.py

@@ -136,11 +136,11 @@ def update_settings_glue(metadata):
   if settings.MEMORY64:
     assert '--enable-memory64' in settings.BINARYEN_FEATURES
 
-  settings.HAS_MAIN = settings.MAIN_MODULE or settings.STANDALONE_WASM or 'main' in settings.WASM_EXPORTS
+  settings.HAS_MAIN = bool(settings.MAIN_MODULE) or settings.STANDALONE_WASM or 'main' in settings.WASM_EXPORTS
 
   # When using dynamic linking the main function might be in a side module.
   # To be safe assume they do take input parametes.
-  settings.MAIN_READS_PARAMS = metadata['mainReadsParams'] or settings.MAIN_MODULE
+  settings.MAIN_READS_PARAMS = metadata['mainReadsParams'] or bool(settings.MAIN_MODULE)
 
   if settings.STACK_OVERFLOW_CHECK and not settings.SIDE_MODULE:
     settings.EXPORTED_RUNTIME_METHODS += ['writeStackCookie', 'checkStackCookie']

src/library.js

@@ -2568,7 +2568,7 @@ LibraryManager.library = {
     name = UTF8ToString(name);
 
     var ret = getCompilerSetting(name);
-    if (typeof ret == 'number') return ret;
+    if (typeof ret == 'number' || typeof ret == 'boolean') return ret;
 
     if (!_emscripten_get_compiler_setting.cache) _emscripten_get_compiler_setting.cache = {};
     var cache = _emscripten_get_compiler_setting.cache;

src/parseTools.js

@@ -918,11 +918,10 @@ function makeRetainedCompilerSettings() {
   const ret = {};
   for (const x in global) {
     if (!ignore.has(x) && x[0] !== '_' && x == x.toUpperCase()) {
-      try {
-        if (typeof global[x] == 'number' || typeof global[x] == 'string' || this.isArray()) {
-          ret[x] = global[x];
-        }
-      } catch (e) {}
+      const value = global[x];
+      if (typeof value == 'number' || typeof value == 'boolean' || typeof value == 'string' || Array.isArray(x)) {
+        ret[x] = value;
+      }
     }
   }
   return ret;