gh-90928: Improve static initialization of keywords tuple in AC #95907
+2,751
−6,802
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
Deduce num_keywords from f.parameters
erlend-aasland
requested review from
ethanfurman,
rhettinger,
,
berkerpeksag,
pganssle,
abalkin,
tiran,
1st1 and
asvetlov
as code owners
erlend-aasland
removed request for
,
berkerpeksag,
1st1,
asvetlov,
tiran,
abalkin,
pganssle,
rhettinger and
ethanfurman
ericsnowcurrently
requested changes
Aug 11, 2022
Thanks for working on this! Mostly LGTM.
Also, can you drop "num_keywords" from template_dict now?
|
When you're done making the requested changes, leave the comment: |
It is already dropped :) |
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @ericsnowcurrently: please review the changes made to this pull request. |
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
erlend-aasland
reviewed
Aug 12, 2022
|
Thanks for reviewing; highly appreciated. I'll make a quick adjustment before landing. |
- inline len(keywords) - use NUM_KEYWORDS constant for improved readability in generated code
|
I'm done with the last round of adjustments. I ended up adding both of your last suggestions. Thanks again. |
|
FYI, I'll wait with merging until Kumar has finished his review. |
|
Thanks for the reviews! |
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.
Calculate
num_keywordsduring template generation instead of leaving it to the preprocessor. This makes for much smaller and easier to read (FWIW) AC output.See discussion in #95860 (comment)