https://github.com/ilovepi commented:
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. https://github.com/llvm/llvm-project/pull/205609 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
