[libcxx-commits] [PATCH] D137804: [libc++][clang-tidy] Enable readability-simplify-boolean-expr

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

I am not against this, but it's not clear to me that this really increases readability that much, since 50% of the changes suggested by the tool are not improvements IMO.



================
Comment at: libcxx/include/charconv:253
         {
-            if (!('0' <= *__p && *__p <= '9'))
+            if ('0' > *__p || *__p > '9')
                 break;
----------------
I don't find the new one to be more readable. The old one read more naturally as "if `*__p` is not between `'0'` and `'9'`". The new one isn't as clear.


================
Comment at: libcxx/include/fstream:754
         {
-            _LIBCPP_ASSERT ( !(__extbufnext_ == NULL && (__extbufend_ != __extbufnext_)), "underflow moving from NULL" );
+            _LIBCPP_ASSERT (__extbufnext_ != NULL || __extbufend_ == __extbufnext_, "underflow moving from NULL" );
             if (__extbufend_ != __extbufnext_)
----------------
Here too, I don't find the new formulation more readable.


================
Comment at: libcxx/include/locale:4012
         {
-             _LIBCPP_ASSERT(!(__extbufnext_ == NULL && (__extbufend_ != __extbufnext_)), "underflow moving from NULL" );
+             _LIBCPP_ASSERT(__extbufnext_ != NULL || __extbufend_ == __extbufnext_, "underflow moving from NULL" );
              if (__extbufend_ != __extbufnext_)
----------------
Same.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137804/new/

https://reviews.llvm.org/D137804