[clang-tidy] Fix false-positives in misc-static-assert caused by non-constexpr variables

[PiotrZSL]

Ignore false-positives when referring to non-constexpr variables in non-unevaluated context (like decltype, sizeof, ...).

Moved from https://reviews.llvm.org/D158657

Fixes: #24066

[llvmbot]

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Ignore false-positives when referring to non-constexpr variables in non-unevaluated context (like decltype, sizeof, ...).

Moved from https://reviews.llvm.org/D158657

Fixes: #24066

---

Full diff: https://github.com/llvm/llvm-project/pull/77203.diff

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp (+9-1)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5-1)
• (modified) clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp (+42)

diff --git a/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp b/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
index 77763a9f429f2b..35536b47700a65 100644
--- a/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "StaticAssertCheck.h"
+#include "../utils/Matchers.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Expr.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -45,13 +46,20 @@ void StaticAssertCheck::registerMatchers(MatchFinder *Finder) {
       IsAlwaysFalse);
   auto NonConstexprFunctionCall =
       callExpr(hasDeclaration(functionDecl(unless(isConstexpr()))));
+  auto NonConstexprVariableReference =
+      declRefExpr(to(varDecl(unless(isConstexpr()))),
+                  unless(hasAncestor(expr(matchers::hasUnevaluatedContext()))),
+                  unless(hasAncestor(typeLoc())));
+
+  auto NonConstexprCode =
+      expr(anyOf(NonConstexprFunctionCall, NonConstexprVariableReference));
   auto AssertCondition =
       expr(
           anyOf(expr(ignoringParenCasts(anyOf(
                     AssertExprRoot, unaryOperator(hasUnaryOperand(
                                         ignoringParenCasts(AssertExprRoot)))))),
                 anything()),
-          unless(findAll(NonConstexprFunctionCall)))
+          unless(NonConstexprCode), unless(hasDescendant(NonConstexprCode)))
           .bind("condition");
   auto Condition =
       anyOf(ignoringParenImpCasts(callExpr(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 08ade306b5a077..1a6f25cd226f7c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,7 +119,7 @@ Improvements to clang-tidy
 
 - Improved `--dump-config` to print check options in alphabetical order.
 
-- Improved :program:`clang-tidy-diff.py` script. 
+- Improved :program:`clang-tidy-diff.py` script.
     * Return exit code `1` if any :program:`clang-tidy` subprocess exits with
       a non-zero code or if exporting fixes fails.
 
@@ -376,6 +376,10 @@ Changes in existing checks
   <clang-tidy/checks/misc/redundant-expression>` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``).
 
+- Improved :doc:`misc-static-assert
+  <clang-tidy/checks/misc/static-assert>` check to ignore false-positives when
+  referring to non-``constexpr`` variables in non-unevaluated context.
+
 - Improved :doc:`misc-unused-using-decls
   <clang-tidy/checks/misc/unused-using-decls>` check to avoid false positive when
   using in elaborated type.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp
index da4ab9baa3948a..16c6ba60c1925f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp
@@ -20,6 +20,48 @@ void print(...);
 #define my_macro() assert(0 == 1)
 // CHECK-FIXES: #define my_macro() assert(0 == 1)
 
+namespace PR24066 {
+
+void referenceMember() {
+  struct {
+    int A;
+    int B;
+  } S;
+  assert(&S.B - &S.A == 1);
+}
+
+const int X = 1;
+void referenceVariable() {
+  assert(X > 0);
+}
+
+
+constexpr int Y = 1;
+void referenceConstexprVariable() {
+  assert(Y > 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be replaced by static_assert() [misc-static-assert]
+  // CHECK-FIXES-CXX11: {{^  }}static_assert(Y > 0, "");
+  // CHECK-FIXES-CXX17: {{^  }}static_assert(Y > 0);
+}
+
+void useInSizeOf() {
+  char a = 0;
+  assert(sizeof(a) == 1U);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be replaced by static_assert() [misc-static-assert]
+  // CHECK-FIXES-CXX11: {{^  }}static_assert(sizeof(a) == 1U, "");
+  // CHECK-FIXES-CXX17: {{^  }}static_assert(sizeof(a) == 1U);
+}
+
+void useInDecltype() {
+  char a = 0;
+  assert(static_cast<decltype(a)>(256) == 0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found assert() that could be replaced by static_assert() [misc-static-assert]
+  // CHECK-FIXES-CXX11: {{^  }}static_assert(static_cast<decltype(a)>(256) == 0, "");
+  // CHECK-FIXES-CXX17: {{^  }}static_assert(static_cast<decltype(a)>(256) == 0);
+}
+
+}
+
 constexpr bool myfunc(int a, int b) { return a * b == 0; }
 
 typedef __SIZE_TYPE__ size_t;

[justincady]

Context: I have limited experience writing AST matchers, but I saw the request for reviewers. I'm basing this review on that limited experience and the tests put in place with this diff.

On those grounds, these changes LGTM.

[HerrCai0907]

LGTM