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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to