GH-111485: Separate out parsing, analysis and code-gen phases #112299
Conversation
markshannon
commented
Nov 21, 2023
•
markshannon
commented
- Issue: The code generated for the interpreter(s) is too fragile #111485
gvanrossum
changed the title
There was a problem hiding this comment.
It's more of a rewrite (or everything besides the parser and lexer) than a refactor. :-) It does feel cleaner, so I support forging ahead here.
General: I'd like it to use argparse, have correct type annotations everywhere, and be formatted using Black.
I'd like to understand why _PUSH_FRAME and the macros using it are now considered not to have an output stack effect (or one less, for the macros). Given that the generated output is unchanged for these, I am assuming that you're no longer checking that all the members of a family have the same stack effect?
One concern I have with creating separate top-level command-line utilities is that in my experience, the bulk of the processing time is in the parser -- which means that if you have five utilities to generate each of the five output files, you end up waiting five times as long for make regen-cases or its equivalent. This should be easy to address given your architecture. Please don't forget to update README.md.
I didn't try to understand your stack.py carefully, but it deserves a lot of scrutiny. In the past fixes there usually were obvious when the generated code was different -- but you've moved things around in the output just enough (even taking the case-sorting out of the equation) that it's hard to review everything without glazing over.
markshannon
changed the title
| return self._size | ||
|
|
||
|
|
||
| Part = Uop | Skip |
There was a problem hiding this comment.
Skip is "unused cache entry", but here it's used as a uop?
There was a problem hiding this comment.
Where is Part used as a Uop?
There was a problem hiding this comment.
uops: list[Part] in line 133
There was a problem hiding this comment.
It is a list of parts, and used as such. I'll change the name.
| allow_redefinition = True | ||
| implicit_reexport = True |
There was a problem hiding this comment.
Suggestion: move these lines of the config file above the # ...And be strict: comment on line 9, since they don't relate to increasing the strictness of mypy
| from typing import Optional | ||
|
|
||
|
|
||
| @dataclass |
There was a problem hiding this comment.
I would be tempted to do this, for readability. I agree with Guido's opinion that it's hard to read when instances of this class are constructed using positional arguments
| @dataclass | |
| @dataclass(kw_only=True) |
…f tier 1 code generator (pythonGH-112299)
…f tier 1 code generator (pythonGH-112299)
