Page MenuHomePhabricator

[analyzer] NFC: Use -verify=... in MoveChecker tests.
ClosedPublic

Authored by NoQ on Apr 15 2019, 2:49 PM.

Details

Summary

Just wanted to give a bit more visibility to the underrated technology of writing -verify=a,b,c instead of the #ifdefclutter.

It's also a bit wonky because it seems that you have to write an exponential number of flags to verify in order to have full flexibility (which is why i didn't update the #ifdef DFS test), but it still seems to be much better in most cases.

Diff Detail

Repository
rL LLVM

Event Timeline

NoQ created this revision.Apr 15 2019, 2:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 15 2019, 2:49 PM
Charusso accepted this revision.Apr 15 2019, 3:00 PM

I think this functionality is unused because you would split the file into six to reduce the overhead/scroll and that is it.

It is a cool reveal, could you provide a documentation?

This revision is now accepted and ready to land.Apr 15 2019, 3:00 PM

This is also very useful to test that a given warning is only emitted in c++xx. Funnily when grepping for verify= in test/ most matches are from tests exercising this functionality.

Just wanted to give a bit more visibility to the underrated technology of writing -verify=a,b,c instead of the #ifdefclutter.

It's great to see this getting more use.

It's also a bit wonky because it seems that you have to write an exponential number of flags to verify in order to have full flexibility (which is why i didn't update the #ifdef DFS test), but it still seems to be much better in most cases.

Sometimes you can avoid an explosion of FileCheck prefixes using -D. I'm not aware of anything like -D for -verify.

It is a cool reveal, could you provide a documentation?

The original patch added documentation to -cc1 -help. Doxygen for VerifyDiagnosticConsumer is another possibility except it currently doesn't mention command-line usage at all. Maybe it should?

Sometimes you can avoid an explosion of FileCheck prefixes using -D. I'm not aware of anything like -D for -verify.

I mean a -D that expands a variable within an expected diagnostic.

NoQ added a comment.Apr 16 2019, 6:00 PM

The original patch added documentation to -cc1 -help. Doxygen for VerifyDiagnosticConsumer is another possibility except it currently doesn't mention command-line usage at all. Maybe it should?

I would have definitely discovered this feature earlier if it was mentioned on the doxygen page for VerifyDiagnosticConsumer!

I think this functionality is unused because you would split the file into six to reduce the overhead/scroll and that is it.

The current test tests all 6 modes on all functions, that's a lot of coverage. You can't obtain the same coverage by splitting the file into mode-specific tests; you'd have to duplicate all code in all files, is much harder to maintain than #ifdefs. It would probably still be easier to read, but the constant fear that the files aren't actually identical pretty much defeats the purpose.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2019, 4:16 PM