[clang] 82f72c1 - [clang][Sema] Re-use existing BinaryOperator if possible (#90625)

[via cfe-commits]

Author: Youngsuk Kim
Date: 2024-05-02T10:10:38-04:00
New Revision: 82f72c1aa8ef36b521a617a291c9f732edca9d12

URL: https://github.com/llvm/llvm-project/commit/82f72c1aa8ef36b521a617a291c9f732edca9d12
DIFF: https://github.com/llvm/llvm-project/commit/82f72c1aa8ef36b521a617a291c9f732edca9d12.diff

LOG: [clang][Sema] Re-use existing BinaryOperator if possible (#90625)

First round of Sema checks were run at initial parsing step. Creating a
new BinaryOperator instance (with the re-built LHS or RHS) will trigger
another round of Sema checks, which can lead to duplicate diagnostic
warning messages.

All we want here is to replace the LHS or RHS with a NonOdrUse version.
Don't create a new BinaryOperator, but simply replace the LHS or RHS of
the given BinaryOperator.

Fixes #45783

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaExpr.cpp
    clang/test/CXX/drs/dr7xx.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 85f7144a27caac..3a68a6d5efe5fa 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -540,6 +540,9 @@ Bug Fixes in This Version
   The values of 0 and 1 block any unrolling of the loop. This keeps the same behavior with GCC.
   Fixes (`#88624 <https://github.com/llvm/llvm-project/issues/88624>`_).
 
+- Clang will no longer emit a duplicate -Wunused-value warning for an expression
+  `(A, B)` which evaluates to glvalue `B` that can be converted to non ODR-use. (#GH45783)
+
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 0c37f43f75401b..0cca02c338954a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19707,18 +19707,17 @@ static ExprResult rebuildPotentialResultsAsNonOdrUsed(Sema &S, Expr *E,
       ExprResult Sub = Rebuild(LHS);
       if (!Sub.isUsable())
         return Sub;
-      LHS = Sub.get();
+      BO->setLHS(Sub.get());
     //   -- If e is a comma expression, ...
     } else if (BO->getOpcode() == BO_Comma) {
       ExprResult Sub = Rebuild(RHS);
       if (!Sub.isUsable())
         return Sub;
-      RHS = Sub.get();
+      BO->setRHS(Sub.get());
     } else {
       break;
     }
-    return S.BuildBinOp(nullptr, BO->getOperatorLoc(), BO->getOpcode(),
-                        LHS, RHS);
+    return ExprResult(BO);
   }
 
   //   -- If e has the form (e1)...

diff  --git a/clang/test/CXX/drs/dr7xx.cpp b/clang/test/CXX/drs/dr7xx.cpp
index 69ee6d6d4e6ae0..0300dae08d6d31 100644
--- a/clang/test/CXX/drs/dr7xx.cpp
+++ b/clang/test/CXX/drs/dr7xx.cpp
@@ -28,10 +28,8 @@ namespace cwg712 { // cwg712: partial
         use(a);
         use((a));
         use(cond ? a : a);
-        // FIXME: should only warn once
         use((cond, a));
         // expected-warning at -1 {{left operand of comma operator has no effect}}
-        // expected-warning at -2 {{left operand of comma operator has no effect}}
 
         (void)a;
         // expected-error at -1 {{reference to local variable 'a' declared in enclosing function 'cwg712::f'}} FIXME