justincady wrote:

I understand the concern. I suspect we just view the value of the explicit test 
differently. As someone previously unfamiliar with the clangd codebase, I was 
surprised that I couldn't find any validation of on-disk representation (at 
least, not in the way I was expecting it). Adding this test was helpful in 
growing my understanding, and I thought desirable to keep (even if there is 
already implicit coverage via round-trip testing).

> I am simply trying to understand what specific potential problem is not 
> covered by the existing tests, where a bug might occur that will be missed by 
> the existing tests.

There may or may not be a potential problem exclusively identified by this 
test. I don't think I can prove that either way. I'm happy to update the test 
to validate the full path as you pointed out, but I also don't want to waste 
anyone's time. If you and other maintainers would rather not add the test I'll 
close this PR.

Again, I apologize for making things more complex in my attempt to simplify a 
future review. Thanks again for taking a look.

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

Reply via email to