[clang-tools-extra] [clang] [clang-tidy] fix match for binaryOperator in ExprMutationAnalyzer for misc-const-correctness (PR #70559)

2023-11-12 Thread Julian Schmidt via cfe-commits

5chmidti wrote:

> In tests we usually use `-fno-delayed-template-parsing` to overcome this 
> issue with templates and windows.

Done. I changed the test in `ExprMutationAnalyzerTest.cpp` to use the flag, 
`const-correctness-templates.cpp` already had it, and I changed the tested code 
back to what it was before (-formatting).

https://github.com/llvm/llvm-project/pull/70559
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang] [clang-tidy] fix match for binaryOperator in ExprMutationAnalyzer for misc-const-correctness (PR #70559)

2023-11-08 Thread Julian Schmidt via cfe-commits

5chmidti wrote:

> how about adding a function call like `f()`? will this can prevent clang 
> to emit empty function body?

Yep, thanks. I went this route, but I had to define a conversion operator to 
compile and keep the `<<` a `BinaryOperator`. The CI now passes

https://github.com/llvm/llvm-project/pull/70559
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang] [clang-tidy] fix match for binaryOperator in ExprMutationAnalyzer for misc-const-correctness (PR #70559)

2023-11-08 Thread Julian Schmidt via cfe-commits

https://github.com/5chmidti updated 
https://github.com/llvm/llvm-project/pull/70559

>From b29eb35fe8597ceefc4c615817174181a16f3c4c Mon Sep 17 00:00:00 2001
From: Julian Schmidt <44101708+5chmi...@users.noreply.github.com>
Date: Sat, 28 Oct 2023 18:08:51 +0200
Subject: [PATCH 1/3] [clang-tidy] fix match for binaryOperator in
 ExprMutationAnalyzer for misc-const-correctness

The `ExprMutationAnalyzer`s matcher of `binaryOperator`s
that contained the variable expr, were previously narrowing the
variable to be type dependent, when the `binaryOperator` should
have been narrowed as dependent.
The variable we are trying to find mutations for does
not need to be the dependent type, the other operand of
the `binaryOperator` could be dependent.

Fixes #57297
---
 clang-tools-extra/docs/ReleaseNotes.rst   |  5 +
 .../checkers/misc/const-correctness-templates.cpp | 11 +++
 clang/docs/ReleaseNotes.rst   |  3 +++
 clang/lib/Analysis/ExprMutationAnalyzer.cpp   |  6 +++---
 clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp | 11 +++
 5 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 5ce38ab48bf295f..bb75c9a3ce08125 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -308,6 +308,11 @@ Changes in existing checks
   ` check to avoid false positive 
when
   using pointer to member function.
 
+- Improved :doc:`misc-const-correctness
+  ` check to not warn on uses in
+  type-dependent binary operators, when the variable that is being
+  looked at, is not the dependent operand.
+
 - Improved :doc:`misc-include-cleaner
   ` check by adding option
   `DeduplicateFindings` to output one finding per symbol occurrence, avoid
diff --git 
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
index 7b5ccabdd6ef611..415bb06020b9dc3 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp
@@ -20,3 +20,14 @@ void instantiate_template_cases() {
   type_dependent_variables();
   type_dependent_variables();
 }
+
+namespace gh57297{
+struct Stream {};
+// Explicitly not declaring a (templated) stream operator
+// so the `<<` is a `binaryOperator` with a dependent type.
+template  void f() {
+  T t;
+  Stream stream;
+  stream << t;
+}
+} // namespace gh57297
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2595737e8b3b143..fb9afc0646ac8cb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -683,6 +683,9 @@ Bug Fixes to AST Handling
 - Fixed a bug where RecursiveASTVisitor fails to visit the
   initializer of a bitfield.
   `Issue 64916 `_
+- Fixed a bug where ``ExprMutationAnalyzer`` did not find a potential mutation
+  for uses in type-dependent binary operators, when the variable that is being
+  looked at, is not the dependent operand.
 
 Miscellaneous Bug Fixes
 ^^^
diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp 
b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
index f2e1beb025423cf..624a643cc60e4ba 100644
--- a/clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -296,9 +296,9 @@ const Stmt *ExprMutationAnalyzer::findDirectMutation(const 
Expr *Exp) {
   // resolved and modelled as `binaryOperator` on a dependent type.
   // Such instances are considered a modification, because they can modify
   // in different instantiations of the template.
-  binaryOperator(hasEitherOperand(
-  allOf(ignoringImpCasts(canResolveToExpr(equalsNode(Exp))),
-isTypeDependent(,
+  binaryOperator(
+  
hasEitherOperand(ignoringImpCasts(canResolveToExpr(equalsNode(Exp,
+  isTypeDependent()),
   // Within class templates and member functions the member expression 
might
   // not be resolved. In that case, the `callExpr` is considered to be a
   // modification.
diff --git a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp 
b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
index c0a398394093c48..cfa3c535ce35292 100644
--- a/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ b/clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -343,6 +343,17 @@ TEST(ExprMutationAnalyzerTest, UnresolvedOperator) {
   EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
+TEST(ExprMutationAnalyzerTest, DependentOperatorWithNonDependentOperand) {
+  // gh57297
+  // Explicitly not declaring a (templated) stream operator
+  // so the `<<` is a `binaryOperator` with a dependent type.
+  const auto AST = buildASTFromCode("struct Stream {}; template  "
+

[clang-tools-extra] [clang] [clang-tidy] fix match for binaryOperator in ExprMutationAnalyzer for misc-const-correctness (PR #70559)

2023-11-06 Thread Qizhi Hu via cfe-commits

jcsxky wrote:

how about adding a function call like `f()`? will this can prevent clang 
to emit empty function body?

https://github.com/llvm/llvm-project/pull/70559
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang] [clang-tidy] fix match for binaryOperator in ExprMutationAnalyzer for misc-const-correctness (PR #70559)

2023-11-03 Thread Julian Schmidt via cfe-commits

5chmidti wrote:

I found the issue, but I have not looked for a solution.
See https://godbolt.org/z/or4EnMfWj and #71203.
When using clang with `-target` to specify `x86_64-pc-win32` and 
`x86_64-linux`, the body for the function `f` is `NULL` in the AST with the 
windows target. This only happens for `-target x86_64-windows` pre `c++20`.

Although the tested code does not directly depend on being `c++20` (it is valid 
in `c++11`), maybe I constrain the tests in this pr to be in `c++20` mode until 
the issue is fixed? What do you think?

https://github.com/llvm/llvm-project/pull/70559
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] [clang] [clang-tidy] fix match for binaryOperator in ExprMutationAnalyzer for misc-const-correctness (PR #70559)

2023-10-30 Thread Qizhi Hu via cfe-commits

jcsxky wrote:




> The top-level Linux and Windows tests work, but inside the `Trigger 
> Build`/`clang-ci` run, the windows test step fails. I'm not sure what the 
> difference is between these two Windows tests.

This is interesting! Generally speaking, AST of the code in Windows and Linux 
platform is identical with clang compiled. Probably need debuging in Windows.

https://github.com/llvm/llvm-project/pull/70559
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits