Page MenuHomePhabricator

Run ClangTidy tests in all C++ language modes
ClosedPublic

Authored by gribozavr on May 20 2019, 1:23 AM.

Details

Summary

I inspected every test and did one of the following:

  • changed the test to run in all language modes,
  • added a comment explaining why the test is only applicable in a certain mode,
  • limited the test to language modes where it passes and added a FIXME to fix the checker or the test.

Diff Detail

Repository
rL LLVM

Event Timeline

gribozavr created this revision.May 20 2019, 1:23 AM

Wow, a lot of work!

I did not check all test files, but I saw that you explicitly enabled c++11,14,17 but not 98. Is there a reason for that? I think we should test that standard too.

alexfh accepted this revision.May 20 2019, 2:12 AM

That's a quite impressive amount of work. Thanks! Looks good.

Wow, a lot of work!

I did not check all test files, but I saw that you explicitly enabled c++11,14,17 but not 98. Is there a reason for that? I think we should test that standard too.

Having seen the amount of work out into this I wouldn't demand c++98 coverage from Dimitri. It's a huge improvement already and c++98 coverage is a rather niche thing there days. I believe, someone with genuine need for C++98 support can work on that.

This revision is now accepted and ready to land.May 20 2019, 2:12 AM

c++98 coverage is a rather niche thing there days. I believe, someone with genuine need for C++98 support can work on that.

+1, that's exactly the reason why I skipped 98. C++98 is 21 years old now (or 16 if you count from C++03). Most projects that build with a modern toolchain and care about code quality enough to run static analysis tools have migrated to C++11. Many ClangTidy checkers have not even been tested *ever* on 98, many (modernize-*) don't apply in 98.

We wouldn't refuse patches to annotate tests with C++98 support or fix bugs in C++98 mode, but I believe as far as ClangTidy is concerned C++98 is a niche use case, so I personally find it difficult to justify spending my time on it.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2019, 2:24 AM