[PATCH] D51192: Fix reported range of partial token replacement
Stephen Kelly via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 6 12:36:12 PDT 2018
steveire added a comment.
In https://reviews.llvm.org/D51192#1226282, @aaron.ballman wrote:
> In https://reviews.llvm.org/D51192#1226257, @steveire wrote:
>
> > How? This is 'private' code. I don't think it's worth changing that or creating a test with a huge amount of infrastructure in order to test this indirectly.
>
>
> This is changing the observable behavior of the tool, so it should have tests unless they're impossible to write.
Yes. The current behavior is not tested. I agree that tests are better than no tests.
>> Am I missing something?
>
> I'd probably pipe the diagnostic output to a temporary file that gets FileChecked with strict whitespace to see if the underlines from the diagnostic match the expected locations. We do this in a few Clang tests, like SemaCXX\struct-class-redecl.cpp or Misc\wrong-encoding.c.
Doesn't this require building-in a new check to clang-tidy which exists only for the purpose of the test? Otherwise how would a test similar to `SemaCXX\struct-class-redecl.cpp` work? What would be in the `RUN` line?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51192
More information about the cfe-commits
mailing list