Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9cb38ffbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mapping = {} | ||
| repeated = self.repeated_strings_by_scope.get(id(node), ()) | ||
| if repeated: | ||
| mapping = {value: next(self.generator) for value in repeated} |
There was a problem hiding this comment.
Reserve hoisted names against non-Name bindings
RepeatedStringRewriter allocates helper identifiers with next(self.generator), but that generator is seeded from VariableNameCollector.visit_Name, so bindings that are not ast.Name nodes (for example lambda/function parameters and names declared via global/nonlocal) are not reserved. When a collision occurs, hoisted literals can silently change runtime behavior (e.g. lambda b: ("hello", "hello") becomes (b, b)) or make valid input fail to compile (SyntaxError: name 'b' is assigned to before global declaration).
Useful? React with 👍 / 👎.
What changed
examples/pyminify.pyoutput and README compression comparisonsWhy
String hoisting was one of the optimizations we had backed out during the TexSoup hardening pass. This brings part of it back without reopening the broader scope-ordering and public-surface problems from module and class scopes.
Validation
.venv/bin/python -m pytest.venv/bin/python scripts/regenerate_examples.py --check78 passed