Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Oct 27, 2025

I proposed removing this internal setting in #25661 but it turns out it was useful for externally managed JS library files.

However, I don't think it should be tested as part of emscripten_get_compiler_setting.c since C/C++ can already get the compiler version using VERSION macros. . There should be no need for the runtime checks.

Indeed I hope we can stop exposing any internal settings to emscripten_get_compiler_setting: #25663

Also, add test for @juj's specific use case for EMSCRIPTEN_VERSION in a JS library file.

@sbc100 sbc100 requested a review from juj October 27, 2025 18:01
@sbc100 sbc100 force-pushed the emscripten_get_compiler_setting branch from 3a38538 to 4f8e3c5 Compare October 27, 2025 18:02
@sbc100 sbc100 requested review from dschuff and kripken October 27, 2025 18:04
I proposed removing this internal setting in emscripten-core#25661 but it turns
out it was useful for externally managed JS library files.

However, I don't think it should be tested as part of
emscripten_get_compiler_setting.c since C/C++ can already get the
compiler version using VERSION macros, no need for the runtime check.

Indeed I hope we can stop exposing any internal settings to
emscripten_get_compiler_setting: emscripten-core#25663
@sbc100 sbc100 force-pushed the emscripten_get_compiler_setting branch from 4f8e3c5 to 67de53b Compare October 27, 2025 18:42
@sbc100 sbc100 merged commit f69a8f6 into emscripten-core:main Oct 27, 2025
34 checks passed
@sbc100 sbc100 deleted the emscripten_get_compiler_setting branch October 27, 2025 20:54
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