Page MenuHomePhabricator

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

Authored by sberg on Jun 9 2021, 2:19 AM.

Details

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.

Diff Detail

Event Timeline

sberg requested review of this revision.Jun 9 2021, 2:19 AM
sberg created this revision.

gcc also ignores it?

sberg added a comment.Jun 9 2021, 3:24 AM

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.

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

mbenfield added inline comments.Jun 9 2021, 9:08 AM
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 added inline comments.Jun 9 2021, 11:11 PM
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.

This revision is now accepted and ready to land.Jun 10 2021, 9:31 AM
This revision was landed with ongoing or failed builds.Jun 13 2021, 11:04 PM
This revision was automatically updated to reflect the committed changes.