https://github.com/ilovepi commented:

Overall I think the testing here is an OK start, but it would be good to test 
properties more thoughtfully. That's a bit hard given the simplistic node 
structures. 

I think it would be beneficial if these nodes could do things like insert new 
children, update text, etc.  Maybe that's the responsibility of another data 
structure to you, but its very typical to have at least an interface to 
add/remove children and provide ways to performa the basic operations. That 
would make this patch a lot more complete. Alternativly, I'd be fine if the 
plan was to do that in another change.

https://github.com/llvm/llvm-project/pull/205609
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to