[PATCH] D35257: [clang-tidy] Add new modernize use unary assert check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/modernize/UnaryStaticAssertCheck.cpp:32
+
+  if (!AssertMessage || AssertMessage->getLength())
+    return;
----------------
barancsuk wrote:
> aaron.ballman wrote:
> > I think this should be `!AssertMessage->getLength()`
> It works correctly without the negation, otherwise the tests fail. 
Sorry about the brain hiccup, you're correct.


================
Comment at: test/clang-tidy/modernize-unary-static-assert.cpp:16
+  // CHECK-FIXES: {{^}}  FOO{{$}}
+  static_assert(sizeof(a) <= 17, MSG);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use unary 'static_assert' when
----------------
This probably should not diagnose, but definitely should not provide a fixit -- just because the macro is empty doesn't mean it will *always* be empty. Consider:
```
#if FOO
#define MSG ""
#else
#define MSG "Not empty"
#endif
```
I think diagnosing this case is just as likely to be a false-positive as not diagnosing, so my preference is to be silent instead of chatty. However, maybe there are other opinions.


https://reviews.llvm.org/D35257