neogeny / TatSu

竜 TatSu generates Python parsers from grammars in a variation of EBNF
https://tatsu.readthedocs.io/
Other
408 stars 48 forks source link

[walkers] Make implementation of NodeWalker more regular #274

Closed dnicolodi closed 2 years ago

dnicolodi commented 2 years ago

NodeWalker defines a new() method that takes the unusual step of modifying the class definition received as first argument adding a _walker_cache class member. This means that all NodeWalker subclass instances share the same _walker_cache and that this dictionary in reset every time a new NodeWalker instance is created.

It may be that this implementation was an attempt to add instance initialization without requiting subclasses to have to call super().init() in their initialization. However, this results in somehow bizarre behavior.

Changing the implementation to obtain a more regular and expected behavior is worth the minuscule compatibility breakage of requiring sublasses to call the parent init() method. Failing to do so manifests with an obvious error and the fix is straightforward.

Fixes #266.

dnicolodi commented 2 years ago

As I explained in the comment to #275, I think that the new metaclass implementation does not have any advantage over the much simpler implementation here.

dnicolodi commented 2 years ago

Ops. Please disregard the previous comment, I was confused by the comment in the code. The cache dictionary is not copied.