[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