[PATCH] D127518: [Diagnostics] Fix inconsistent shift-overflow warnings in C++20

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:11423-11425
+  // Don't warn in C++2A mode (and newer), as signed left shifts
+  // always wrap and never overflow.
+  if (S.getLangOpts().CPlusPlus20)
----------------
I think this should also be checking `|| isSignedOverflowDefined()` -- if signed overflow is defined, then all the rest of the diagnostics shouldn't be triggered because the behavior is defined.

(Eventually, we should remove the check for `CPlusPlus20` and use ONLY `isSignedOverflowDefined()` -- that should account for the current language mode instead of requiring us to check additional predicates. But that doesn't have to be fixed up as part of this patch.)


================
Comment at: clang/lib/Sema/SemaExpr.cpp:11430
   // then, the behavior is undefined before C++2a. Warn about it.
-  if (Left.isNegative() && !S.getLangOpts().isSignedOverflowDefined() &&
-      !S.getLangOpts().CPlusPlus20) {
+  if (Left.isNegative() && !S.getLangOpts().isSignedOverflowDefined()) {
     S.DiagRuntimeBehavior(Loc, LHS.get(),
----------------
Then the `isSignedOverflowDefined()` check can be removed here.


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

https://reviews.llvm.org/D127518