[libcxx-commits] [PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

Fri Jan 6 09:07:24 PST 2023

fahadnayyar marked 2 inline comments as done.
fahadnayyar added a comment.

In D139114#4023456 <https://reviews.llvm.org/D139114#4023456>, @aaron.ballman wrote:

> It looks like this change breaks libc++ (see the precommit CI failures) by making more changes than expected. I'm now seeing `implicit conversion changes signedness` diagnostics where we didn't previously get them. Is that expected and intentional? (I think it may be a fix: https://godbolt.org/z/hTaaf8c5P so I'm adding the libc++ folks just in case they disagree.)
>
> Also, these changes should come with an entry in the release notes.

I've made some change to suppress the warning in libcxx, please have a look and let me know if the change can break the semantics of the function in any way. Since gcc also give warning on this example, I guess we should include this behaviour in clang as well.

================
Comment at: clang/lib/Sema/SemaChecking.cpp:13400-13401

+  // Check for implicit conversion loss of precision form 64-to-32 for compound
+  // statements.
+  if (E->getLHS()->getType()->isIntegerType() &&
----------------
aaron.ballman wrote:
> This comment isn't quite accurate, right? It's checking for any kind of implicit conversion issue (such as changing signs even if the integer widths stay the same).
Changed the comment!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114