grok_bin_oct_hex: Some minor improvements #23908
Open
Conversation
This file contains hidden or 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
Clarify some comments.
I noticed that leading zeros are quite common for octal and hex constants. This code is structured for speed, with a partially unrolled loop structured so that it is impossible to overflow the unrolled part. If we get to the end of the unrolled portion, and the accumulated value is still zero, it's because there have been only leading zeroes so far, and instead of dropping into the loop, we can re-enter the unrolled part without having to consider the possibility of overflowing. This allows the next chunk of digits to be processed without branching.
It's easier to understand and code if each flag name is for a single flag.
This clarifies its meaning
Apparently no one has tried to use these before. These flags are to suppress the display of certain warnings, but to instead return that the suppression happened in output flags. The output flags were not getting set. I'm not adding a separate test, because a future commit will cause this feature to be used regularly.
We already have this value in a variable.
tonycoz
reviewed
Nov 10, 2025
The new scheme is is what toke.c already uses for the same purpose, and I think is cleaner
This prevents a future maintainer from accidentally changing this pointer inside the function, where its original value is needed at the end
khwilliamson
force-pushed
the
more_grok_bin
branch
from
7d9b710 to
bbd568b
Compare
tonycoz
approved these changes
Nov 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
These commits change this private function to avoid some recalculations, and speed up the common case where these binary-like numbers have leading zeros. And two flags that should have been returned were not.