Only consider built-in compound assignment operators for -Wunused-but-set-*

[sberg]

Summary

At least LibreOffice has, for mainly historic reasons that would be hard to change now, a class Any with an overloaded operator >>= that semantically does not assign to the LHS but rather extracts into the (by-reference) RHS. Which thus caused false positive -Wunused-but-set-parameter and -Wunused-but-set-variable after those have been introduced recently.
This change is more conservative about the assumed semantics of overloaded operators, excluding compound assignment operators but keeping plain operator = ones. At least for LibreOffice, that strikes a good balance of not producing false positives but still finding lots of true ones.

[xbolva00]

gcc also ignores it?

[sberg]

In D103949#2807490, @xbolva00 wrote:

gcc also ignores it?

For reasons that I never looked into, GCC never warned for any of these cases. The Clang implementation appears to be way more aggressive (in a positive sense) to begin with.

[xbolva00]

Ok thanks for info. We should follow gcc in this case.

[mbenfield]

clang/lib/Sema/SemaExprCXX.cpp

7811–7816

Would you mind elaborating on the need for this code? IIUC, you're concerned about overloaded operators, but won't such operators always be covered by the CXXOperatorCallExpr case below?

[sberg]

clang/lib/Sema/SemaExprCXX.cpp

7811–7816

That's what I would initially have thought too (see my unanswered https://lists.llvm.org/pipermail/llvm-dev/2021-June/150875.html "[llvm-dev] BinaryOperator vs. CXXOperatorCallExpr in template code"; but which, I notice now, I accidentally sent to llvm-dev rather than cfe-dev).

But e.g. the template f4 test code in warn-unused-but-set-variables-cpp.cpp turns the += into a BinaryOperator.

[mbenfield]

LGTM

clang/lib/Sema/SemaExprCXX.cpp

ExprResult Sema::ActOnNoexceptExpr(SourceLocation KeyLoc, SourceLocation,
                                    Expr *Operand, SourceLocation RParen) {
   return BuildCXXNoexceptExpr(KeyLoc, Operand, RParen);
 }
 
 static void MaybeDecrementCount(
     Expr *E, llvm::DenseMap<const VarDecl *, int> &RefsMinusAssignments) {
   DeclRefExpr *LHS = nullptr;
   if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
-    if (!BO->isAssignmentOp())
+    if (BO->getLHS()->getType()->isDependentType() ||
+        BO->getRHS()->getType()->isDependentType()) {
+      if (BO->getOpcode() != BO_Assign)
+        return;
+    } else if (!BO->isAssignmentOp())
       return;
     LHS = dyn_cast<DeclRefExpr>(BO->getLHS());
   } else if (CXXOperatorCallExpr *COCE = dyn_cast<CXXOperatorCallExpr>(E)) {
-    if (!COCE->isAssignmentOp())
+    if (COCE->getOperator() != OO_Equal)
       return;
     LHS = dyn_cast<DeclRefExpr>(COCE->getArg(0));
   }
   if (!LHS)
     return;
   VarDecl *VD = dyn_cast<VarDecl>(LHS->getDecl());
   if (!VD)
     return;

clang/test/SemaCXX/warn-unused-but-set-variables-cpp.cpp

 // RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s
 
 struct S {
   int i;
 };
 
 struct __attribute__((warn_unused)) SWarnUnused {
   int j;
+  void operator +=(int);
 };
 
 int f0() {
   int y; // expected-warning{{variable 'y' set but not used}}
   y = 0;
 
   int z __attribute__((unused));
   z = 0;

 
 void f2() {
   // Don't warn for either of these cases.
   constexpr int x = 2;
   const int y = 1;
   char a[x];
   char b[y];
 }
+
+void f3(int n) {
+  // Don't warn for overloaded compound assignment operators.
+  SWarnUnused swu;
+  swu += n;
+}
+
+template<typename T> void f4(T n) {
+  // Don't warn for (potentially) overloaded compound assignment operators in
+  // template code.
+  SWarnUnused swu;
+  swu += n;
+}