Skip to content

Remove unused variables. NFC - #8972

Merged
sbc100 merged 1 commit into
mainfrom
remove_unused
Aug 5, 2026
Merged

Remove unused variables. NFC#8972
sbc100 merged 1 commit into
mainfrom
remove_unused

Conversation

@sbc100

@sbc100 sbc100 commented Aug 5, 2026

Copy link
Copy Markdown
Member

These were preventing the latest version of llvm from building binaryen.

This was triggered by llvm/llvm-project#203084

See https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8674381472972128657/+/u/Build_Binaryen/stdout

These were preventing the latest version of llvm from building binaryen.
I guess the unused variable checker got better.
@sbc100
sbc100 requested a review from a team as a code owner August 5, 2026 18:01
@sbc100
sbc100 requested review from kripken and removed request for a team August 5, 2026 18:01

@kripken kripken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is this, Go? 😆

@sbc100

sbc100 commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

What is this, Go? 😆

Don't we expect clang to report these unused locals? We do build with -Wall which includes -Wunused right? Why were the unused local Name entry variables still not reported (I just noticed them incidentally).

@sbc100
sbc100 enabled auto-merge (squash) August 5, 2026 18:11
@kripken

kripken commented Aug 5, 2026

Copy link
Copy Markdown
Member

Yeah, I'm not complaining, this is expected as clang gets better at detecting this. I'm not sure why it detects some but not all.

@kripken

kripken commented Aug 5, 2026

Copy link
Copy Markdown
Member

Now if clang can also get to Go's compile time speed... that would be nice.

@sbc100
sbc100 merged commit e821d17 into main Aug 5, 2026
16 checks passed
@sbc100
sbc100 deleted the remove_unused branch August 5, 2026 18:47
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