Remove EM_LOG_C_STACK feature of emscripten_log #23553
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sbc100
force-pushed
the
EM_LOG_C_STACK
branch
from
060da25 to
9bdce5d
Compare
sbc100
changed the title
sbc100
force-pushed
the
EM_LOG_C_STACK
branch
from
9bdce5d to
7fee72f
Compare
kripken
reviewed
Feb 3, 2025
sbc100
force-pushed
the
EM_LOG_C_STACK
branch
from
7fee72f to
e8b476b
Compare
|
@kripken do you think its worth trying to resurect this feature? It seems like it was some kind of user-space source-map parser.. I'm not sure it worth the effort given that nobody has asked for it so many years. |
sbc100
force-pushed
the
EM_LOG_C_STACK
branch
from
e8b476b to
d38a79f
Compare
|
I agree it's not worth keeping given the lack of users. Have people been using it with wasm2js perhaps though? If not sgtm to remove. |
Source maps do not work with wasm2js ( |
kripken
reviewed
Feb 3, 2025
ChangeLog.md
Outdated
Comment on lines
28
to
30
| - The `EM_LOG_C_STACK` flag to `emscripten_log` was deprecated and the helper | ||
| file on which it was based (`emscripten-source-map.min.js`) deteted. It | ||
| seems this feature has been broken for at least 4 years now. (#23553) |
There was a problem hiding this comment.
Suggested change
| - The `EM_LOG_C_STACK` flag to `emscripten_log` was deprecated and the helper | |
| file on which it was based (`emscripten-source-map.min.js`) deteted. It | |
| seems this feature has been broken for at least 4 years now. (#23553) | |
| - The `EM_LOG_C_STACK` flag to `emscripten_log` was deprecated and the helper | |
| file on which it was based (`emscripten-source-map.min.js`) deleted. This | |
| feature (userspace source map parsing in logs) was not ported to wasm | |
| source maps, so it has not worked, and we have had no requests for it. This | |
| has no impact on browser devtools support of source maps (which work | |
| fully). (#23553) |
There was a problem hiding this comment.
Done. With a little re-phrasing of my own.
This was supported by `emscripten-source-map.min.js` which seems like minified source map parser, but I don't think this feature has worked for at least 4 years. This feature was part of `emscripten_log` since it was first added in 26d6ad3 but I know of anyone that ever used it. Fixes: emscripten-core#13089
sbc100
force-pushed
the
EM_LOG_C_STACK
branch
from
d38a79f to
cecd6a6
Compare
kripken
approved these changes
Feb 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was supported by
emscripten-source-map.min.jswhich seems like minified source map parser, but I don't think this feature has worked for at least 4 years since we switched to using wasm and wasm source maps.This feature was part of
emscripten_logsince it was first added in 26d6ad3 but I know of anyone that ever used it.Fixes: #13089