Refactor for Dialect.
#99
Conversation
| self.apply_kw = apply_kw | ||
| self.opcode_lookup = dict() | ||
| self.multi_op_fn = ChainableMultiOpFn(self.opcode_lookup, multi_op_fn) | ||
| self.to_python = to_python |
There was a problem hiding this comment.
I thought the convention these days was to declare members, with types and sometimes initial values in the class body, rather than just assigning them in the __init__() function.
| from .casts import int_to_bytes | ||
| from .dialect import ConversionFn, Dialect, new_dialect, opcode_table_for_backend | ||
|
|
||
| KEYWORDS = ( |
There was a problem hiding this comment.
is the dialect responsible for compiling clvm code into byte-code too? if not, we could just put the full function names in this list, removing the need for OP_REWRITE and some of the logic in op_atom_to_imp_table
| from .types import CLVMObjectType, ConversionFn, MultiOpFn, OperatorDict | ||
|
|
||
|
|
||
| OP_REWRITE = { |
There was a problem hiding this comment.
this seems like it also would belong in the chia-specific dialect, no?
| @@ -1,4 +1,4 @@ | |||
| brun --backend=python -c -v '(x)' | |||
| brun -c -v '(x)' | |||
There was a problem hiding this comment.
this would cause the clvm_rs tests to fail, wouldn't it?
| self.to_python = to_python | ||
|
|
||
| def update(self, d: OperatorDict) -> None: | ||
| self.opcode_lookup.update(d) |
There was a problem hiding this comment.
given that you have this update() function, why do you need to be able to chain the multi_op_fn with ChainableMultiOpFn? it looks more complex than it needs to be
| quote_kw: bytes, apply_kw: bytes, strict: bool, to_python: ConversionFn | ||
| ) -> Dialect: | ||
| unknown_op_callback = ( | ||
| clvm_rs.NATIVE_OP_UNKNOWN_STRICT |
There was a problem hiding this comment.
are these flags or magic-values exported by the rust bindings, to indicate it's a native rust function?
| # Test construction from an already existing OperatorDict | ||
| o2 = OperatorDict(o) | ||
| self.assertEqual(o2.apply_atom, 1) | ||
| self.assertEqual(o2.quote_atom, 2) |
There was a problem hiding this comment.
OperatorDict is still a type that exists. It's not clear why it shouldn't have a test
No description provided.