Neil-N4 wrote:

> One of the issues I have with the tests is that they don't test anything 
> other than perhaps initialization for the constructor. That's sort of OK to 
> start, but I do find the notion that we have a data structure that can't set 
> any of its own properties to be a bit strange. Its not unherad of, as we do 
> have POD structs around in the codebase, but its rather atypical. I'd expect 
> that if you had operations you wanted to test in the parser, these classes 
> would need to implement something to support that. those basic operations are 
> what I'd expect to see here getting tested.
> 
> I don't want to stop this up too much, so I'm going to defer to the other 
> reviewers, since there isn't' anything glaringly wrong yet, and this code is 
> not used yet.

Nodes are write once by the parser and read only after that, same as clang ast. 
why would mutation methods make sense here?

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