[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:28
+void UnaryStaticAssertCheck::check(const MatchFinder::MatchResult &Result) {
+
+  const auto *MatchedDecl =
----------------
Can remove the empty line.


================
Comment at: clang-tidy/modernize/UnaryStaticAssertCheck.cpp:36
+
+  if (AssertMessage->getString() != "")
+    return;
----------------
I would hoist this up to the previous if statement and use `!AssertMessage->getLength()`. This eliminates the possibility of an assert failing because the string literal is not an ASCII string.

You should add a test with `L""`, which I would expect to also be replaced.


================
Comment at: clang-tidy/modernize/UnaryStaticAssertCheck.cpp:40
+  diag(MatchedDecl->getLocation(),
+       "use unary 'static_assert' when the assert message is unused")
+      << FixItHint::CreateRemoval(AssertMessage->getSourceRange());
----------------
How about:

use unary 'static_assert' when the string literal is an empty string


================
Comment at: clang-tidy/modernize/UnaryStaticAssertCheck.h:19
+
+/// Replaces static assert declaration with empty message
+/// with their unary version.
----------------
static assert -> a static_assert
with empty -> with an empty


================
Comment at: clang-tidy/modernize/UnaryStaticAssertCheck.h:20
+/// Replaces static assert declaration with empty message
+/// with their unary version.
+///
----------------
with the unary version.


================
Comment at: docs/clang-tidy/checks/modernize-unary-static-assert.rst:6-7
+
+The check finds ``static_assert`` declarations with empty messages, and
+replaces them with an unary ``static_assert`` declaration.
+
----------------
This mixes singular and plural a bit oddly. How about:

The check diagnoses any static_assert declaration with an empty string literal and provides a fix-it to replace the declaration with a single-argument static_assert declaration.


================
Comment at: docs/clang-tidy/checks/modernize-unary-static-assert.rst:9
+
+The check is only applicable for C++17 code.
+
----------------
C++17 and later


https://reviews.llvm.org/D35257