[clang] [clang] fix merging of UsingShadowDecl (PR #80245)

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 1 19:45:34 PST 2024


ChuanqiXu9 wrote:

> I took a look at what sort of complexity unittest would entail here. I don't think it's a good compromise complexity wise, it's a lot of boilerplate just to test a few AST nodes are correctly linked.

But they are much easier to maintain than dumpped AST text. I feel things get pretty clear with unittests. And I don't think there are a lot of complexities. They indeed have some boilerplates. But I think it is worhty to make things get tested. Also the boilerplates can be improved by refactoring and it is not so hurting since they are tests.

> 
> On the other hand, these AST tests don't look particularly out of place compared to a lot of other AST tests we have.

But that is not good. We should try to avoid that.

> 
> However, I have seen that there are a lot of other related merging issues, so I will leave the tangentially related tests for another MR.

I am not sure what's your meaning. What's your intention of testing of this patch itself? And I think it is not maintainable or acceptable to test the dumpped text in the following patches.



https://github.com/llvm/llvm-project/pull/80245


More information about the cfe-commits mailing list