Skip to content

Conversation

@aheejin
Copy link
Member

@aheejin aheejin commented Dec 26, 2025

This fixes various typos in the codebase. The work is done using Gemini and manually checked and fixed by me. This does not touch third party code or files with external licenses.

This fixes various typos in the codebase. The work is done using Gemini
and manually checked and fixed by me. This does not touch third party
code or files with external licenses.
@aheejin aheejin requested review from kripken and sbc100 December 26, 2025 23:56
This is an automatic change generated by tools/maint/rebaseline_tests.py.

The following (6) test expectation files were updated by
running the tests with `--rebaseline`:

```
test/codesize/audio_worklet_wasm.expected.js updated
test/codesize/hello_wasm_worker_wasm.expected.js updated
test/codesize/test_codesize_file_preload.expected.js updated
test/codesize/test_codesize_minimal_O0.expected.js updated
codesize/test_minimal_runtime_code_size_audio_worklet.json: 6157 => 6157 [+0 bytes / +0.00%]
codesize/test_unoptimized_code_size.json: 180994 => 180979 [-15 bytes / -0.01%]

Average change: -0.00% (-0.01% - +0.00%)
```
aheejin added a commit to emscripten-core/llvm-project that referenced this pull request Dec 27, 2025
This does not include llvm-libc changes, which is now synced with the
upstream main branch.

This includes changes from
emscripten-core/emscripten#26014, which has not
yet landed.
Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Nice!

This is great. I'm curious how you used the llm to produce these changes. Is this something we can turn into a script to run periodically or on checkin maybe?

Can you split out the changes the effect actual code (I think I've tagged them all), then mark this as NFC in the title?

break;
}
case 0x000330005: { // GLFW_RAW_MOUSE_MOTION
case 0x00033005: { // GLFW_RAW_MOUSE_MOTION
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks different to the other changes in this PR since its actually a bugfix with a functional effect. Can we split this one out into its own change? Is it possible to test is? Maybe we don't have any good coverage of this code?

default:
#if OPENAL_DEBUG
dbg(`alBufferData() called with invalid format ${format}`;
dbg(`alBufferData() called with invalid format ${format}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change and the one below are fixes for syntax errors. Can you split these out too?

// Drop now unneeded references to from the Module object in this Worker,
// these are not needed anymore.
props.wasm = props.memMemory = 0;
props.wasm = props.wasmMemory = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an actual bugfix too. Can you split it out?

#include <unistd.h>
#include <fcntl.h>
#include <fcntl.h>
#include <unistd.h>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an interesting one. Do you know why it picked up on this but didn't remove unused includes in the rest of the codebase. This somehow seems different fixing typos.

Copy link
Member Author

@aheejin aheejin Dec 31, 2025

Choose a reason for hiding this comment

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

I just asked it to fix typos. I don't know what Gemini was thinking 🤷🏻.
I think Gemini removed them not because they were unused but because they were duplicated with the includes right above them. It didn't need to analyze the whole codebase to check if they were unused. And these two lines are probably too small a change to warrant a separate PR I guess?

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to #26024

elif arg == '-mno-sign-ext':
feature_matrix.disable_feature(feature_matrix.Feature.SIGN_EXT)
elif arg == '-mnontrappting-fptoint':
elif arg == '-mnontrapping-fptoint':
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks like a bugfix too.

aheejin added a commit to emscripten-core/llvm-project that referenced this pull request Dec 31, 2025
This does not include llvm-libc changes, which is now synced with the
upstream main branch.

This includes changes from
emscripten-core/emscripten#26014, which has not
yet landed.
@aheejin
Copy link
Member Author

aheejin commented Dec 31, 2025

This is great. I'm curious how you used the llm to produce these changes. Is this something we can turn into a script to run periodically or on checkin maybe?

Unfortunately, it wasn't a magic prompt that we can run periodically. This just started as a fun PR that I thought I would be able to do in a hour or something. First I asked Gemini something like "Fix any typos you find in the codebase, excluding third party code". Then it just searched common misspellings in the codebase using grep and only fixed them, resulting in maybe only a couple dozen fixes.

This was done by... asking Gemini to actually read files using @ command and repeatedly ask it to fix any typos in there. Because of the context window size, I had to use dozens of sessions and split the codebase into many chunks and spoon-fed each of them. This was one of the prompts I used:

"Read ??? and fix all typos you find. Don't just look for commonly misspelled words; actually read each file and fix typos. Read files from start to finish (not just first 2000 lines)."

??? can be a list of of code paths that Gemini can handle at once, like, @test/*.py (Actually even this can be too large because of many large files)

Anyway this ended up taking a lot longer than expected and I regretted starting it 🫠. I think doing this again is not really a good use of somebody's time. Or in a few years Gemini will be a lot better with 50x context window so maybe we really can do this in a single prompt.

Can you split out the changes the effect actual code (I think I've tagged them all), then mark this as NFC in the title?

Even after splitting those real code bugfixes out, I'm not sure we can mark this NFC, because there are still many error message fixes in Python files (which are code changes too) and docs changes (.rst files).

Will close this and try to split the PR into several categories instead.

@aheejin aheejin closed this Dec 31, 2025
aheejin added a commit to aheejin/emscripten that referenced this pull request Dec 31, 2025
Spun out of emscripten-core#26014. This fixes typos in docs, `settings.js` (because the
comments here are copied into `settings_reference.rst`), and header
strings in `update_settings_docs.py`.
aheejin added a commit to aheejin/emscripten that referenced this pull request Dec 31, 2025
Spun out of emscripten-core#26014. This fixes typos in docs, `settings.js` (because the
comments here are copied into `settings_reference.rst`), and header
strings in `update_settings_docs.py`.
@aheejin aheejin mentioned this pull request Dec 31, 2025
aheejin added a commit to aheejin/emscripten that referenced this pull request Dec 31, 2025
This fixes typos in code comments.

Split out of emscripten-core#26014.
aheejin added a commit to aheejin/emscripten that referenced this pull request Dec 31, 2025
This fixes typos in error / log messages.

Split out of emscripten-core#26014.
aheejin added a commit to aheejin/emscripten that referenced this pull request Dec 31, 2025
This fixes typos in error / log messages.

Split out of emscripten-core#26014.
aheejin added a commit to aheejin/emscripten that referenced this pull request Dec 31, 2025
This fixes typos in code that's NFC. This includes cases like typos in
method or parameter names or whitespace fixes.

This is "mostly" NFC because method name changes in `test_***.py` result
in test name changes.

Split out of emscripten-core#26014.
aheejin added a commit to aheejin/emscripten that referenced this pull request Dec 31, 2025
sbc100 pushed a commit that referenced this pull request Dec 31, 2025
sbc100 pushed a commit that referenced this pull request Dec 31, 2025
This fixes typos in code comments.

Split out of #26014.
@sbc100
Copy link
Collaborator

sbc100 commented Dec 31, 2025

Yes, hopefully soon there will just be bot we can use that can run over all repos that does this kind of thing automatically.

aheejin added a commit that referenced this pull request Dec 31, 2025
This fixes typos in docs, `settings.js` (because the comments here are
copied into `settings_reference.rst`), and header strings in
`update_settings_docs.py`.

Split out of #26014.
@aheejin aheejin deleted the fix_typos branch December 31, 2025 21:58
aheejin added a commit that referenced this pull request Dec 31, 2025
This fixes typos in error / log messages.

Split out of #26014.
aheejin added a commit that referenced this pull request Dec 31, 2025
This fixes typos in code that's NFC. This includes cases like typos in
method or parameter names or whitespace fixes.

This is "mostly" NFC because method name changes in `test_***.py` result
in test name changes.

Split out of #26014.
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