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

2023-11-11 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158657

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


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

2023-08-23 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL created this revision.
PiotrZSL added reviewers: alexfh, njames93, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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

Fixes: #24066


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158657

Files:
  clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/static-assert.cpp
@@ -20,6 +20,48 @@
 #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( -  == 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(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(256) == 0, "");
+  // CHECK-FIXES-CXX17: {{^  }}static_assert(static_cast(256) == 0);
+}
+
+}
+
 constexpr bool myfunc(int a, int b) { return a * b == 0; }
 
 typedef __SIZE_TYPE__ size_t;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -216,6 +216,10 @@
   ` check to ignore
   false-positives in unevaluated context (e.g., ``decltype``).
 
+- Improved :doc:`misc-static-assert
+  ` check to ignore false-positives when
+  referring to non-``constexpr`` variables in non-unevaluated context.
+
 - Improved :doc:`modernize-loop-convert
   ` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``.
Index: clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
+++ 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 @@
   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(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits