[PATCH] D17134: [clang-tidy] Fix an assert failure of ForStmt in `readability-braces-around-statements` check.

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 12 05:46:47 PST 2016


alexfh added inline comments.

================
Comment at: test/clang-tidy/readability-braces-around-statements-assert-failure.cpp:8
@@ +7,3 @@
+
+#include <vector>
+
----------------
Tests shouldn't include any library headers. This is problematic in many different ways:

  * standard library can be different in different setups, which may cause test breakages;
  * including system headers may not work in some test setups;
  * including system headers may lead to significant increase in testing time;
  * there's no way to test something with different library implementations in the same test setup.

This is why we usually model APIs needed for the test in the tests (e.g. see test/clang-tidy/misc-inaccurate-erase.cpp). In this specific case the check doesn't  need an actual std::vector<>, it just needs to simulate a specific compile error that would lead to a specific incompleteness in the AST. So, please, reduce the test case (the largest reduction would be the removal of `#include <vector>`) ;)

================
Comment at: test/clang-tidy/readability-braces-around-statements-assert-failure.cpp:16
@@ -6,2 +15,2 @@
   }
 }
----------------
hokein wrote:
> > Interesting. Does creduce fail to further reduce the test?
> 
> I reduce the test case manually. 
> 
> Now I use FileCheck with `-implicit-check-not` parameter to ignore all compilation errors.
> I reduce the test case manually.

That's why I was asking about creduce. It can do a much better job with a much less manual effort ;)


http://reviews.llvm.org/D17134





More information about the cfe-commits mailing list