[Cython] Bug in DictNode

[Dag Sverre Seljebotn]

Martin C. Martin wrote:
> Hi,
>
> Calling get_child_accessors() on a DictNode throws an exception, 
> because, because it doesn't define a child_attrs field (and thus 
> inherits the value None from class Node), so get_child_attrs() checks 
> self.subexprs to see if it's None, but the field doesn't exist.  So, 
> what's the right fix?
>
> - Change get_child_attrs() to look at self.saved_subexpr_nodes instead 
> of self.subexprs?
> - Add "subexprs = None" to ExprNode?
> - Add "subexprs = None" to DictNode?
> - Something else?
>   
Two fixes are needed. If you are simply trying to get some transform 
code running that doesn't involve transforming DictNode then the first 
will suffice for now.

1) Fix the self.subexprs check (probably adding subexprs = None to 
ExprNode would be best)

2) Get DictNode to actually export its children to the tree. I don't 
know this part of the code too well so this might be wrong, but in 
principle this is done by adding to DictNode:

child_attrs = ["key_value_pairs"]

However, there is a problem with this: key_value_pairs doesn't contain 
tree nodes as children but rather tuples. There are many approaches to this:

a) Every tree transform that works with DictNode accepts this as the 
manner of working, and the current tree printers etc. are modified to 
handle this.

b) Probably better: Change the implementation of DictNode to use a list 
of a new DictEntryNode node class (with "key" and "value" subnodes and 
attributes) directly in key_value_pairs. This needs proper testing.

c) Being more careful with existing code, one might override 
get_child_attrs in DictNode instead, and provide an adaptor object that 
wraps the list and returns approprioately wrapped children (basically 
export a list of DictEntryNode, each DictEntryNode representing each 
tuple in the list -- but this is complicated in that changes to the 
exported list should also be mirrored in , so this is probably going to 
be a ~70 line wrapper and not worth it over b - however it won't break 
existing code and so won't need the same degree of testing (for now)).

-- 
Dag Sverre