Move definitions of expression rules. #162
Conversation
|
ah it breaks blame. will fix it. |
odashi
force-pushed
the
refactor-expr-codegen
branch
from
725289f to
e34b62c
Compare
odashi
force-pushed
the
refactor-expr-codegen
branch
from
e34b62c to
3ae62c1
Compare
|
I see you've marked as blocked by #161, but feel free to merge if working. I can resolve any conflicts in my branch later on. |
|
@ZibingZhang Sure, removed the reference. |
| (ast.Attribute(), expression_rules._INF_PRECEDENCE), | ||
| ], | ||
| ) | ||
| def test_get_precedence(tree: ast.AST, precedence: int) -> None: |
There was a problem hiding this comment.
Half the tests call this argument tree and the other half node. Personally I prefer node but don't really care but would make it easier to understand tests if the same variable name was used across them.
There was a problem hiding this comment.
renamed to node anyway since the name has been used in codegens. tree is used in frontend though. I prefer this name a bit because it slightly means "entire tree" rather than the fragment. (But this is just a bike-shedding problem I guess)
|
|
||
|
|
||
| # name => left_syntax, right_syntax, is_wrapped | ||
| BUILTIN_FUNCS: dict[str, FunctionRule] = { |
There was a problem hiding this comment.
Assuming this constant doesn't have a _ prefix since it's imported from another file. In that case, shouldn't _MATH_SYMBOLS in math_symbols.py also have it's _ prefix removed?
There was a problem hiding this comment.
moved _MATH_SYMBOLS to expression_rules.MATH_SYMBOLS
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "tree,precedence", |
There was a problem hiding this comment.
tree -> node (or node -> tree)?
Overview
This change doesn't introduce any new features.
This change moves all rule definitions for
ExpressionCodegentoexpression_rules.py.CC @ZibingZhang
Details
References
Blocked by