[clang] [clang][NFC] Reduce one of JSON dump tests (PR #195026)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 30 05:52:56 PDT 2026


AaronBallman wrote:

> I think we have really opposite views about what's difficult to maintain here:
> 
> To me the problem about the manually crafted textual AST dump tests is that:
> 
>     * They are really tedious to update.

Having maintained these files for years, I find the manually crafted AST dump tests to be significantly easier to update. They often require fewer updates when the dumper changes because they're not as sensitive to AST dumping changes due to being more targeted.

>     * They often don't capture enough surrounding context to be useful or readable, and don't avoid false matches on other surrounding tests.

This is the opposite of my problem with the over-testing of AST dumps; those often have so much useless context that it becomes impossible to see what's being tested. Neither situation is ideal, but I find "too much context" to be worse than "too little context" on balance.

> The JSON dump tests avoid those problems, and you really only run a script to update them, and then check the differences on the diff viewer. I'd rather be doing that than struggling with the keyboard to update these tests.

I think it introduces more problems than it avoids unless you filter the tests far better. As an example, you are testing that we correctly match against `__builtin_ms_va_list` which has absolutely nothing to do with anything in the test file. This kind of needless over-testing makes the test harder to read, slower to run, and harder to maintain.

> Regarding the random test update noise, the JSON tests can easily be improved by removing the line / byte-offsets from most tests, and that can be done simply changing the test generator script.

That scratches the surface of the problems of using the test generator script; there's still the issue of it dumping nodes entirely unrelated to what's being tested. We have some amount of filtering for that, but not very much. Perhaps the script could be updated in some way to work with AST matchers so you can filter by more than just identifiers, but all of this is exploratory and I continue to believe this PR is good incremental progress in the direction we want to go (modulo any accidentally dropped test coverage, but it's still unclear to me what exactly that is).

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


More information about the cfe-commits mailing list