JonasToth added inline comments.
================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:13
+
+#include <iostream>
+#include <limits>
----------------
aaron.ballman wrote:
> Including <iostream> is forbidden by our coding standard. However, it doesn't
> appear to be used in the source file, either.
Yes, forgot it from debugging.
================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:91-92
+ // Context.getTypeSize(T) returns the number of bits T uses.
+ // Calculates the number of discrete values that are representable by this
+ // type.
+ return T->isIntegralType(Context) ? twoPow(Context.getTypeSize(T))
----------------
aaron.ballman wrote:
> I don't think this comment adds value.
i dropped all of it. The function doc should be clear enough, is it?
================
Comment at: clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp:94
+ return T->isIntegralType(Context) ? twoPow(Context.getTypeSize(T))
+ : twoPow(0ul);
+}
----------------
aaron.ballman wrote:
> aaron.ballman wrote:
> > No need for the `ul` suffix -- the type will undergo implicit conversion
> > with or without the suffix.
> Why call toPow instead of just returning the value `1` directly?
True!
================
Comment at: test/clang-tidy/hicpp-multiway-paths-covered.cpp:445
+ }
+}
----------------
aaron.ballman wrote:
> For fun, can you add a switch over a value of type `bool`?
> ```
> void f(bool b) {
> switch (b) {
> case true:
> }
>
> switch (b) {
> default:
> }
>
> switch (b) {
> case true:
> case false:
> }
> }
> ```
There is already a warning in the frontend (-Wswitch-bool) that will complain
about boolean conditions in `switch`.
I will implement the logic anyway. I think its more userfriendly to get this
warning. One might have code style that allows switch for bools.
https://reviews.llvm.org/D37808
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
