Skip to content

Simplify settings type checking. NFC#26153

Closed
sbc100 wants to merge 1 commit intoemscripten-core:mainfrom
sbc100:settings_type_check
Closed

Simplify settings type checking. NFC#26153
sbc100 wants to merge 1 commit intoemscripten-core:mainfrom
sbc100:settings_type_check

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jan 23, 2026

All settings should have known types.

All settings should have known types.
@sbc100 sbc100 force-pushed the settings_type_check branch from 26a5a80 to 0467c1b Compare January 23, 2026 01:44
@sbc100 sbc100 requested review from dschuff and kripken January 23, 2026 01:44
if not expected_type:
# These settings have a variable type so cannot be easily type checked, or they
# are legacy settings not declared in settings.js (and therefore not having any type).
if name in ('USE_PTHREADS', 'EXECUTABLE', 'SUPPORT_LONGJMP', 'PTHREAD_POOL_SIZE', 'SEPARATE_DWARF', 'LTO', 'MODULARIZE'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is USE_PTHREADS added?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because, as mentioned in the comment, its no declared in settings.js. Its deprecated setting.

I have a follow up to add explicit types to all of these.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, I didn't remember we deprecated it...

@sbc100 sbc100 closed this Jan 24, 2026
@sbc100 sbc100 deleted the settings_type_check branch January 24, 2026 00:36
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.

2 participants