[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> > > @alanzhao1 do you think it's reasonable for the workaround to only apply 
> > > to code in system headers, or does the NDK get included as regular 
> > > headers generally?
> > 
> > 
> > Having it only apply to system headers should be OK - in our case, chrome 
> > imports these headers as part of a `--sysroot`, which IIRC treats warnings 
> > the same way as `-isystem`
> > > Do you need us to revert the changes while we resolve this?
> > 
> > 
> > If you can revert while working on a solution, that would be helpful.
> 
> We (Chrome) no longer need a revert - we patched the NDK locally.

Thank you for letting us know!

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Eli Friedman via cfe-commits]

efriedma-quic wrote:

Posted #99579.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Alan Zhao via cfe-commits]

alanzhao1 wrote:

> > @alanzhao1 do you think it's reasonable for the workaround to only apply to 
> > code in system headers, or does the NDK get included as regular headers 
> > generally?
> 
> Having it only apply to system headers should be OK - in our case, chrome 
> imports these headers as part of a `--sysroot`, which IIRC treats warnings 
> the same way as `-isystem`
> 
> > Do you need us to revert the changes while we resolve this?
> 
> If you can revert while working on a solution, that would be helpful.

We (Chrome) no longer need a revert - we patched the NDK locally.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Eli Friedman via cfe-commits]

efriedma-quic wrote:

I think the issue is the usage of "Info.EvalStatus.Diag" to determine whether 
we consider the expression an ICE; that's going to lead to unpredictable 
results, and it's not how we handle things anywhere else.  Not sure what the 
goal of that change is.

Separately, Sema::AddInitializerToDecl should probably print diagnostics when 
it checks for an ICE, instead of just emitting the generic 
diag::err_in_class_initializer_non_constant/diag::ext_in_class_initializer_non_constant.
  Any maybe we should consider enabling ext_in_class_initializer_non_constant 
by default.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Alexander Kornienko via cfe-commits]

alexfh wrote:

Now there's a different and more subtle side-effect of this patch: 
https://gcc.godbolt.org/z/YP3EfGern

The invalid expression `1 << 59` leads to the compiler silently ignoring the 
in-class initializer of the static data member.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Alexander Kornienko via cfe-commits]

alexfh wrote:

I see. Thanks for the explanation and for the suggestion on how to fix that. 
Actually, the only instance I've seen, had already been fixed upstream 
(https://hg.openjdk.org/jdk/jdk/rev/71495d579a65), so the impact of this 
compiler behavior change is quite low in my part of the universe.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> Just to confirm: is this the intentional outcome of this patch?

Before C++20, left shift of -1 does not produce a mathematical result that's 
within the range of representable values for the result type, so it's undefined 
behavior, and therefore not a core constant expression, and therefore invalid 
as the initializer for an enumeration constant. So I believe the behavior is 
expected and correct; you can see the UB diagnostic in other contexts: 
https://gcc.godbolt.org/z/bqj6xhzzr and GCC agrees with Clang in the 
enumeration case: https://gcc.godbolt.org/z/qjKzeaYTz

> the diagnostic - "expression is not an integral constant expression" - 
> doesn't sound helpful (how is it not constant or not integral?). And what's 
> worse, is that it's an error rather than a warning that can be disabled.

It has integral constants but it's not an integral constant expression as far 
as the language is concerned. The note tells you why it's not a valid integral 
constant expression. As for warning vs error; the standard makes the code 
ill-formed and we treat that as an error generally speaking.

If you can't enable C++20 mode, you can still get the same value in C++17 and 
earlier with well-formed code: https://gcc.godbolt.org/z/3ra83joqs

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Alexander Kornienko via cfe-commits]

alexfh wrote:

Just to confirm: is this the intentional outcome of this patch? 
https://gcc.godbolt.org/z/Kf9YK1qM3
```
enum {
a = 1<<12,
b = (-1)<<13
};
```
```
:3:9: error: expression is not an integral constant expression
3 | b = (-1)<<13
  | ^~~~
:3:13: note: left shift of negative value -1
3 | b = (-1)<<13
  | ^
```

Though technically, it's UB before C++20, the diagnostic - "expression is not 
an integral constant expression" - doesn't sound helpful (how is it not 
constant or not integral?). And what's worse, is that it's an error rather than 
a warning that can be disabled.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Stephan Bergmann via cfe-commits]

stbergmann wrote:

> That is intentional; `strlen` is not a valid constant expression in C, and so 
> that is technically a VLA, except we have an extension to fold it back into a 
> constant.

Thanks for confirming.  From the commit message and the changes to 
`clang/docs/ReleaseNotes.rst` it just didn't look like this newly emitted 
warning was intended by the commit.  But now I notice the changes to 
`clang/test/Sema/builtins.c` that do acknowledge this newly emitted warning.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> This started to cause
> 
> ```
> $ cat test.c
> #include 
> void f(void) {
> char a[strlen("x")];
> (void) a;
> }
> ```
> 
> ```
> $ clang -Wall -fsyntax-only test.c
> test.c:3:12: warning: variable length array folded to constant array as an 
> extension [-Wgnu-folding-constant]
> 3 | char a[strlen("x")];
>   |^~~
> 1 warning generated.
> ```
> 
> to emit a warning, and I'm not sure that's intentional?

That is intentional; `strlen` is not a valid constant expression in C, and so 
that is technically a VLA, except we have an extension to fold it back into a 
constant.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Stephan Bergmann via cfe-commits]

stbergmann wrote:

This started to cause
```
$ cat test.c
#include 
void f(void) {
char a[strlen("x")];
(void) a;
}
```
```
$ clang -Wall -fsyntax-only test.c
test.c:3:12: warning: variable length array folded to constant array as an 
extension [-Wgnu-folding-constant]
3 | char a[strlen("x")];
  |^~~
1 warning generated.
```
to emit a warning, and I'm not sure that's intentional?

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Alan Zhao via cfe-commits]

alanzhao1 wrote:

> @alanzhao1 do you think it's reasonable for the workaround to only apply to 
> code in system headers, or does the NDK get included as regular headers 
> generally? 

Having it only apply to system headers should be OK - in our case, chrome 
imports these headers as part of a `--sysroot`, which IIRC treats warnings the 
same way as `-isystem`

>  Do you need us to revert the changes while we resolve this?

If you can revert while working on a solution, that would be helpful.

> > @budimirarandjelovicsyrmia, can you please add a workaround so that the NDK 
> > code can continue to compile?
> 
> I will investigate this.

FYI I noticed that gcc can compile this with `-fpermissive`

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

budimirarandjelovicsyrmia wrote:

> @budimirarandjelovicsyrmia, can you please add a workaround so that the NDK 
> code can continue to compile?

I will investigate this.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

> The android NDK code is wrong, but this no longer compiles (even with 
> -Wno-error).

Well that's unfortunate! Thank you for letting us know.

@alanzhao1 do you think it's reasonable for the workaround to only apply to 
code in system headers, or does the NDK get included as regular headers 
generally? Do you need us to revert the changes while we resolve this?

@budimirarandjelovicsyrmia, can you please add a workaround so that the NDK 
code can continue to compile?

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

budimirarandjelovicsyrmia wrote:

> @budimirarandjelovicsyrmia -- do you need someone to land these changes on 
> your behalf?

I don't understand question. I think that I answered correctly to previous 
comments.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

AaronBallman wrote:

@budimirarandjelovicsyrmia -- do you need someone to land these changes on your 
behalf?

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 0dd1a3e2b45a8c918217e62fe844b91dbb51df8e Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  7 --
 clang/test/Sema/builtins.c |  4 +--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 104 insertions(+), 22 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cfec1cd14a6fc..6adf57da42e65 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -705,6 +705,8 @@ Improvements to Clang's diagnostics
 
 - For the ARM target, calling an interrupt handler from another function is 
now an error. #GH95359.
 
+- Clang now diagnoses integer constant expressions that are folded to a 
constant value as an extension in more circumstances. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e0c9ef68cb448..0aeac9d03eed3 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2859,6 +2859,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2882,6 +2885,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 63496b327da4d..d6b85cbcaf56b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11078,7 +11078,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11093,7 +11093,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11126,7 +11126,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -16964,11 +16964,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if (Notes.size() == 1 && Notes[0].second.getDiagID() 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Timm Baeder via cfe-commits]

https://github.com/tbaederr approved this pull request.


https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -686,6 +686,8 @@ Improvements to Clang's diagnostics
 - Clang no longer emits a "no previous prototype" warning for Win32 entry 
points under ``-Wmissing-prototypes``.
   Fixes #GH94366.
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863

AaronBallman wrote:

```suggestion
- Clang now diagnoses integer constant expressions that are folded to a 
constant value as an extension in more circumstances . Fixes #GH59863
```

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -277,7 +277,9 @@ void dr258(void) {
 void dr261(void) {
   /* This is still an integer constant expression despite the overflow. */

AaronBallman wrote:

Probably good to update this comment to mention "extension".

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman approved this pull request.

LGTM aside from a few small things to correct. @tbaederr are you happy with the 
state of things as well?

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman edited 
https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

budimirarandjelovicsyrmia wrote:

Ping @AaronBallman @shafik @tbaederr 

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From d5f3ea5c5bc3ee94ed72e529482e9df4a8770848 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 ++-
 clang/test/Sema/builtins.c |  4 +--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 102 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cd3a4c2b1be1a..e06de2de9a75d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -686,6 +686,8 @@ Improvements to Clang's diagnostics
 - Clang no longer emits a "no previous prototype" warning for Win32 entry 
points under ``-Wmissing-prototypes``.
   Fixes #GH94366.
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e0c9ef68cb448..0aeac9d03eed3 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2859,6 +2859,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2882,6 +2885,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 45991e66b3e43..dd1191bef4d3e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11084,7 +11084,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11099,7 +11099,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11132,7 +11132,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -16970,11 +16970,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
+  

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 9e4309a805f31096d72cb21cd266175cac5b07c1 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 ++-
 clang/test/Sema/builtins.c |  6 +++--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 104 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9ed3ff4507671..d09cec913fd16 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -626,6 +626,8 @@ Improvements to Clang's diagnostics
   used rather than when they are needed for constant evaluation or when code 
is generated for them.
   The check is now stricter to prevent crashes for some unsupported 
declarations (Fixes #GH95495).
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab87..5196d85d2a985 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2881,6 +2884,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4a2f3a65eac96..0c7d0fc6173e1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17138,11 +17138,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if (Notes.size() == 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From f8b8175f074c4caba44517ac9ea2714d50c5e86e Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 ++-
 clang/test/Sema/builtins.c |  6 +++--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 104 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9ed3ff4507671..d09cec913fd16 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -626,6 +626,8 @@ Improvements to Clang's diagnostics
   used rather than when they are needed for constant evaluation or when code 
is generated for them.
   The check is now stricter to prevent crashes for some unsupported 
declarations (Fixes #GH95495).
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab87..5196d85d2a985 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2881,6 +2884,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4a2f3a65eac96..0c7d0fc6173e1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17138,11 +17138,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if (Notes.size() == 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 9e4309a805f31096d72cb21cd266175cac5b07c1 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 ++-
 clang/test/Sema/builtins.c |  6 +++--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 104 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9ed3ff4507671..d09cec913fd16 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -626,6 +626,8 @@ Improvements to Clang's diagnostics
   used rather than when they are needed for constant evaluation or when code 
is generated for them.
   The check is now stricter to prevent crashes for some unsupported 
declarations (Fixes #GH95495).
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab87..5196d85d2a985 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2881,6 +2884,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4a2f3a65eac96..0c7d0fc6173e1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17138,11 +17138,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if (Notes.size() == 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia reopened 
https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From a66ca21ad79df2385fbaec12344f9c16cc3c5b83 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 17 +
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 +++-
 clang/test/Sema/builtins.c |  6 --
 clang/test/Sema/code_align.c   |  7 ---
 clang/test/Sema/constant-builtins-2.c  |  8 
 clang/test/Sema/shift-count-negative.c | 10 ++
 clang/test/Sema/shift-count-overflow.c |  7 +++
 clang/test/Sema/shift-negative-value.c | 10 ++
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/enum.cpp|  6 --
 clang/test/SemaCXX/shift.cpp   |  2 +-
 13 files changed, 73 insertions(+), 20 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab87..80c4886e5d94a 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2881,6 +2884,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4a2f3a65eac96..9f5ec9f432df9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17138,11 +17138,20 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size() && !Diagnoser.Suppress) {
+  Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+  return ExprError();
+}
+
 return E;
   }
 
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 36de32a93da95..530c216722cc3 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* c89only-error {{static assertion 
expression is not an integral constant expression}}
+   c89only-note {{left shift of 
negative 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 9e4309a805f31096d72cb21cd266175cac5b07c1 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 ++-
 clang/test/Sema/builtins.c |  6 +++--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 104 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9ed3ff4507671c..d09cec913fd162 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -626,6 +626,8 @@ Improvements to Clang's diagnostics
   used rather than when they are needed for constant evaluation or when code 
is generated for them.
   The check is now stricter to prevent crashes for some unsupported 
declarations (Fixes #GH95495).
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab874..5196d85d2a9852 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2881,6 +2884,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4a2f3a65eac96f..0c7d0fc6173e1a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17138,11 +17138,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 39c314b060959578ccfe70c5ef2aa5aba5688c11 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 17 +
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 +++-
 clang/test/Sema/builtins.c |  6 --
 clang/test/Sema/code_align.c   |  7 ---
 clang/test/Sema/constant-builtins-2.c  |  8 
 clang/test/Sema/shift-count-negative.c | 10 ++
 clang/test/Sema/shift-count-overflow.c |  7 +++
 clang/test/Sema/shift-negative-value.c | 10 ++
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/enum.cpp|  6 --
 clang/test/SemaCXX/shift.cpp   |  2 +-
 13 files changed, 73 insertions(+), 20 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab874..80c4886e5d94ad 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2881,6 +2884,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4a2f3a65eac96f..9f5ec9f432df93 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17138,11 +17138,20 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size() && !Diagnoser.Suppress) {
+  Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+  return ExprError();
+}
+
 return E;
   }
 
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 36de32a93da95d..530c216722cc3b 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* c89only-error {{static assertion 
expression is not an integral constant expression}}
+   c89only-note {{left shift of 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From f19ddd1d381d7e1c79b1d841070deb461f442eb7 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 ++-
 clang/test/Sema/builtins.c |  6 +++--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 104 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9c8f8c4a4fbaf..c8d1633406fd3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -612,6 +612,8 @@ Improvements to Clang's diagnostics
   used rather than when they are needed for constant evaluation or when code 
is generated for them.
   The check is now stricter to prevent crashes for some unsupported 
declarations (Fixes #GH95495).
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fe4b9a569ab87..5196d85d2a985 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2858,6 +2858,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2881,6 +2884,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 4a2f3a65eac96..0c7d0fc6173e1 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11256,7 +11256,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11271,7 +11271,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11304,7 +11304,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17138,11 +17138,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if (Notes.size() == 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 975000d3a8bfff223111bc5c119294d6fea929ae Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/docs/ReleaseNotes.rst|  2 ++
 clang/lib/AST/ExprConstant.cpp |  7 ++
 clang/lib/Sema/SemaExpr.cpp| 35 +++---
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 ++-
 clang/test/Sema/builtins.c |  6 +++--
 clang/test/Sema/constant-builtins-2.c  | 12 ++---
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  9 +++
 clang/test/Sema/shift-negative-value.c | 13 ++
 clang/test/Sema/vla-2.c|  6 +++--
 clang/test/SemaCXX/enum.cpp| 16 +++-
 clang/test/SemaCXX/shift.cpp   |  2 +-
 14 files changed, 104 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index cf1ba02cbc4b2..95ce9f2c332ac 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -569,6 +569,8 @@ Improvements to Clang's diagnostics
 - Clang no longer emits a "declared here" note for a builtin function that has 
no declaration in source.
   Fixes #GH93369.
 
+- Clang now diagnoses non-C++11 integer constant expressions. Fixes #GH59863
+
 Improvements to Clang's time-trace
 --
 
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index af1f18aa8ef24..afb1838e572b1 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2856,6 +2856,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2879,6 +2882,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus11)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 44f886bf54e3a..887f8355fad25 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11246,7 +11246,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11261,7 +11261,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11294,7 +11294,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17130,11 +17130,38 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.empty())
+  return E;
+
+// If our only note is the usual "invalid subexpression" note, just point
+// the caret at its location rather than producing an essentially
+// redundant note.
+if (Notes.size() == 1 && Notes[0].second.getDiagID() ==
+  

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From a80047859d20f0fbb591f6c8a561468ce966f845 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 16 
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 +++-
 clang/test/Sema/builtins.c |  6 --
 clang/test/Sema/constant-builtins-2.c  | 12 
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 
 clang/test/Sema/shift-count-overflow.c |  6 ++
 clang/test/Sema/shift-negative-value.c |  9 +
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/enum.cpp| 15 ---
 clang/test/SemaCXX/shift.cpp   |  2 +-
 13 files changed, 74 insertions(+), 22 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index af1f18aa8ef24..0926bcf258f58 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2856,6 +2856,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2879,6 +2882,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 44f886bf54e3a..fd1c8284b5ff0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11246,7 +11246,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11261,7 +11261,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11294,7 +11294,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17130,11 +17130,19 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size()) {
+  Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+}
+
 return E;
   }
 
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 36de32a93da95..252dc9329c4ca 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* expected-warning {{expression is 
not an integer constant expression; folding it to a constant is a GNU 
extension}}
+   expected-note {{left shift of 
negative value -1}} */
  

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 6d5fdc817db7216577429e5949bbaa7e6cd3648f Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 16 
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 +++-
 clang/test/Sema/builtins.c |  6 --
 clang/test/Sema/constant-builtins-2.c  | 12 
 clang/test/Sema/integer-overflow.c |  2 ++
 clang/test/Sema/shift-count-negative.c |  8 
 clang/test/Sema/shift-count-overflow.c |  6 ++
 clang/test/Sema/shift-negative-value.c |  9 +
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/enum.cpp|  7 +--
 clang/test/SemaCXX/shift.cpp   |  2 +-
 13 files changed, 71 insertions(+), 17 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index af1f18aa8ef24..0926bcf258f58 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2856,6 +2856,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2879,6 +2882,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 44f886bf54e3a..fd1c8284b5ff0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11246,7 +11246,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11261,7 +11261,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11294,7 +11294,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17130,11 +17130,19 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size()) {
+  Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+}
+
 return E;
   }
 
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 36de32a93da95..252dc9329c4ca 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* expected-warning {{expression is 
not an integer constant expression; folding it to a constant is a GNU 
extension}}
+   expected-note {{left shift of 
negative value -1}} */
  

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* c89only-error {{static assertion 
expression is not an integral constant expression}}

budimirarandjelovicsyrmia wrote:

This was error in all C language modes. At first, I added diagnosing this error 
in C mode (file SemaExpr.cpp, lines between 1732-1750, function 
diagnoseNotICE). After suggestion, I changed it to pedantically emit warning 
(substitute diagnoseNotICE with diagnoseFold, same part of code).

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From caea9286c405b8dc0e71383efed256beaa5134f0 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 16 
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 +++-
 clang/test/Sema/builtins.c |  6 --
 clang/test/Sema/constant-builtins-2.c  | 10 ++
 clang/test/Sema/shift-count-negative.c |  8 
 clang/test/Sema/shift-count-overflow.c |  6 ++
 clang/test/Sema/shift-negative-value.c |  9 +
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/enum.cpp|  6 --
 clang/test/SemaCXX/shift.cpp   |  2 +-
 12 files changed, 66 insertions(+), 17 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index af1f18aa8ef24..0926bcf258f58 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2856,6 +2856,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2879,6 +2882,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 44f886bf54e3a..fd1c8284b5ff0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11246,7 +11246,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11261,7 +11261,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11294,7 +11294,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17130,11 +17130,19 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size()) {
+  Diagnoser.diagnoseFold(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+}
+
 return E;
   }
 
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 36de32a93da95..252dc9329c4ca 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* expected-warning {{expression is 
not an integer constant expression; folding it to a constant is a GNU 
extension}}
+   expected-note {{left shift of 
negative value -1}} */
  _Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -277,7 +277,9 @@ void dr258(void) {
 void dr261(void) {
   /* This is still an integer constant expression despite the overflow. */
   enum e1 {
-ex1 = __INT_MAX__ + 1  /* expected-warning {{overflow in expression; 
result is -2'147'483'648 with type 'int'}} */
+ex1 = __INT_MAX__ + 1  /* expected-warning {{overflow in expression; 
result is -2'147'483'648 with type 'int'}}
+  c89only-error {{expression is not an integer 
constant expression}}

AaronBallman wrote:

Same question here as to why this is an error in C89.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* c89only-error {{static assertion 
expression is not an integral constant expression}}

AaronBallman wrote:

Something seems wrong here -- why is this an error only in C89 mode? This 
should be pedantically diagnosed as a warning, not an error (in all C language 
modes).

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 69c6fc02cef92c0dd0eda0a30daa82856552ef1c Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 17 +
 clang/test/C/drs/dr0xx.c   |  3 ++-
 clang/test/C/drs/dr2xx.c   |  4 +++-
 clang/test/Sema/builtins.c |  6 --
 clang/test/Sema/code_align.c   |  7 ---
 clang/test/Sema/constant-builtins-2.c  |  8 
 clang/test/Sema/shift-count-negative.c | 10 ++
 clang/test/Sema/shift-count-overflow.c |  7 +++
 clang/test/Sema/shift-negative-value.c | 10 ++
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/enum.cpp|  6 --
 clang/test/SemaCXX/shift.cpp   |  2 +-
 13 files changed, 73 insertions(+), 20 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index af1f18aa8ef24..0926bcf258f58 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2856,6 +2856,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2879,6 +2882,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 44f886bf54e3a..c77a0e511a3a8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11246,7 +11246,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11261,7 +11261,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11294,7 +11294,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17130,11 +17130,20 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size() && !Diagnoser.Suppress) {
+  Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+  return ExprError();
+}
+
 return E;
   }
 
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 36de32a93da95..530c216722cc3 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,8 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* c89only-error {{static assertion 
expression is not an integral constant expression}}
+   c89only-note {{left shift of 
negative 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 0ed347381cdd9e639ab5026dcf244ca4d7fcb6e3 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 17 +
 clang/test/C/drs/dr0xx.c   |  2 +-
 clang/test/Sema/shift-count-negative.c | 10 ++
 clang/test/Sema/shift-count-overflow.c |  7 +++
 clang/test/Sema/shift-negative-value.c | 10 ++
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/shift.cpp   |  1 -
 8 files changed, 52 insertions(+), 8 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index af1f18aa8ef24..0926bcf258f58 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2856,6 +2856,9 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
   else if (LHS.countl_zero() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2879,6 +2882,10 @@ static bool handleIntIntBinOp(EvalInfo , const 
BinaryOperator *E,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 44f886bf54e3a..c77a0e511a3a8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11246,7 +11246,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11261,7 +11261,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftSize)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11294,7 +11294,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17130,11 +17130,20 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size() && !Diagnoser.Suppress) {
+  Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+  return ExprError();
+}
+
 return E;
   }
 
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 36de32a93da95..05045344efd91 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -430,7 +430,7 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* Undefined behavior since C99 */
  _Static_assert(1 << 3 == 1u << 3u, "fail"); /* Shift of a positive signed 
value does sensible things. */
 }
 
diff --git a/clang/test/Sema/shift-count-negative.c 
b/clang/test/Sema/shift-count-negative.c
new file mode 100644
index 0..34d51294b43d4
--- /dev/null
+++ b/clang/test/Sema/shift-count-negative.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify=expected,c 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

@@ -11444,9 +11444,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   llvm::APSInt Right = RHSResult.Val.getInt();
 
   if (Right.isNegative()) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+if (S.ExprEvalContexts.back().isConstantEvaluated())
+  S.Diag(Loc, diag::warn_shift_negative) << RHS.get()->getSourceRange();
+else
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_negative)
+  << RHS.get()->getSourceRange());

budimirarandjelovicsyrmia wrote:

Diagnose is caught in handleIntIntBinOp() and is emitted for C++ in 
VerifyIntegerConstantExpression(). There are two reasons why diagnostic could 
be emitted in case of C++:
1) evaluating expression as rvalue fails or evaluation result has side 
effects or it is not an integer; evaluating expression as rvalue fails if 
function handleIntIntBinOp() returns false
2) folding is allowed (CanFold == AllowFoldKind::AllowFold); this value 
depends on place where constant expression is defined; eg. in case on enum 
folding is allowed and in case of static assertion it is not allowed
Also, diagnostic would not be emitted if Diagnoser is suppresed.

However, in C case there are no diagnostic for integer constant expression.

So I enabled diagnosing in C case (if Diagnoser is not suppressed). In function 
EvaluateKnownConstIntCheckOverflow() it is expected that evaluating rvalue is 
successful, so I keep handleIntIntBinOp() returning true.
For C++ diagnostic, I modified handleIntIntBinOp() to return false.

As folding is allowed for enum and not for static assertion, I added in tests 
to check both enum and static assertion diagnosing.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 550bdebeb5ee3e305495407063a7d33c640a333c Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  7 +++
 clang/lib/Sema/SemaExpr.cpp| 17 +
 clang/test/AST/Interp/shifts.cpp   |  4 +---
 clang/test/C/drs/dr0xx.c   |  2 +-
 clang/test/Sema/shift-count-negative.c | 10 ++
 clang/test/Sema/shift-count-overflow.c |  7 +++
 clang/test/Sema/shift-negative-value.c | 10 ++
 clang/test/Sema/vla-2.c|  6 --
 clang/test/SemaCXX/shift.cpp   |  1 -
 9 files changed, 53 insertions(+), 11 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 78cfecbec9fd3..8b26ba30c2bbd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2816,6 +2816,9 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
   else if (LHS.countLeadingZeros() < SA)
 Info.CCEDiag(E, diag::note_constexpr_lshift_discards);
 }
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS << SA;
 return true;
   }
@@ -2839,6 +2842,10 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
 << RHS << E->getType() << LHS.getBitWidth();
+
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+Info.getLangOpts().CPlusPlus)
+  return false;
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c2..70afced7a32e7 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11446,7 +11446,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11462,7 +11462,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Right.uge(LeftBits)) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11495,7 +11495,7 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   if (Left.isNegative()) {
 S.DiagRuntimeBehavior(Loc, LHS.get(),
   S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+  << LHS.get()->getSourceRange());
 return;
   }
 
@@ -17322,11 +17322,20 @@ Sema::VerifyIntegerConstantExpression(Expr *E, 
llvm::APSInt *Result,
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
+SmallVector Notes;
 if (Result)
-  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context, );
 if (!isa(E))
   E = Result ? ConstantExpr::Create(Context, E, APValue(*Result))
  : ConstantExpr::Create(Context, E);
+
+if (Notes.size() && !Diagnoser.Suppress) {
+  Diagnoser.diagnoseNotICE(*this, DiagLoc) << E->getSourceRange();
+  for (const PartialDiagnosticAt  : Notes)
+Diag(Note.first, Note.second);
+  return ExprError();
+}
+
 return E;
   }
 
diff --git a/clang/test/AST/Interp/shifts.cpp b/clang/test/AST/Interp/shifts.cpp
index b1df7b85cc9f2..b2b2cb60f4a61 100644
--- a/clang/test/AST/Interp/shifts.cpp
+++ b/clang/test/AST/Interp/shifts.cpp
@@ -33,9 +33,7 @@ namespace shifts {
// FIXME: 'implicit conversion' warning missing in 
the new interpreter. \
// cxx17-warning {{shift count >= width of type}} \
// ref-warning {{shift count >= width of type}} \
-   // ref-warning {{implicit conversion}} \
-   // ref-cxx17-warning {{shift count >= width of 
type}} \
-   // ref-cxx17-warning {{implicit conversion}}
+   // ref-cxx17-warning {{shift count >= width of 
type}}
 c = 1 >> 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Timm Baeder via cfe-commits]

@@ -11444,9 +11444,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   llvm::APSInt Right = RHSResult.Val.getInt();
 
   if (Right.isNegative()) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+if (S.ExprEvalContexts.back().isConstantEvaluated())
+  S.Diag(Loc, diag::warn_shift_negative) << RHS.get()->getSourceRange();
+else
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_negative)
+  << RHS.get()->getSourceRange());

tbaederr wrote:

The constant interpreter already diagnoses this:
https://github.com/llvm/llvm-project/blob/51089e360e37962c7841fe0a494ba9fb5368bab2/clang/lib/AST/ExprConstant.cpp#L2850-L2851

and for the test case with the `(-1 << 29)` enum value, this is evaluated in 
`Sema::VerifyIntegerConstantExpresssion()` and it's also diagnosed here:
https://github.com/llvm/llvm-project/blob/51089e360e37962c7841fe0a494ba9fb5368bab2/clang/lib/Sema/SemaExpr.cpp#L18158-L18159

But the diagnostic doesn't show up. Not sure why. If I modify the constant 
interpreter to `return false` in the negative LHS case, I get the expected 
diagnostic:
```console
/home/tbaeder/test.cpp:2:9: error: expression is not an integral constant 
expression
2 | X = (-1<<29) // expected-warning {{shifting a negative signed value 
is undefined}}
  | ^~~~
/home/tbaeder/test.cpp:2:12: note: left shift of negative value -1
2 | X = (-1<<29) // expected-warning {{shifting a negative signed value 
is undefined}}
  |^
```


https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -11444,9 +11444,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   llvm::APSInt Right = RHSResult.Val.getInt();
 
   if (Right.isNegative()) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+if (S.ExprEvalContexts.back().isConstantEvaluated())
+  S.Diag(Loc, diag::warn_shift_negative) << RHS.get()->getSourceRange();
+else
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_negative)
+  << RHS.get()->getSourceRange());

AaronBallman wrote:

I still don't think this is the correct fix -- I think the issue is that 
ExprConstant.cpp needs to be taught about these as undefined behaviors. Notice 
how GCC correctly emits the pedantic diagnostic but Clang does not: 
https://godbolt.org/z/arb8Y9G9q

When I step through in the debugger, it seems that we do catch the issue in 
`handleIntIntBinOp()` but we're suppressing diagnostics for it in C more 
broadly. That seems more likely to be the root cause of the issue. (Basically, 
we want the diagnostics to be split between SemaExpr.cpp for runtime reachable 
diagnostics and ExprConstant.cpp for constant expressions.)

CC @tbaederr for opinions on constant expression behavior here.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -2805,7 +2805,7 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
 if (SA != RHS) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();

AaronBallman wrote:

FWIW, you should be using `clang-format-diff` so that you're only formatting 
the changes made in the PR: 
https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

@@ -2805,7 +2805,7 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
 if (SA != RHS) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();

budimirarandjelovicsyrmia wrote:

Reverted changes. Just to mention that 'git clang-format' proposed spaces in 
this comment and comments below.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From 17fb77d094018db20de0ac2a65861338006f9d68 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/Sema/SemaExpr.cpp| 29 +++---
 clang/test/AST/Interp/shifts.cpp   |  7 ++
 clang/test/C/drs/dr0xx.c   |  2 +-
 clang/test/Sema/shift-count-negative.c |  8 ++
 clang/test/Sema/shift-count-overflow.c |  6 +
 clang/test/Sema/shift-negative-value.c |  8 ++
 clang/test/Sema/vla-2.c|  9 ---
 clang/test/SemaCXX/cxx2a-explicit-bool.cpp |  2 +-
 clang/test/SemaCXX/shift.cpp   |  1 -
 9 files changed, 52 insertions(+), 20 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c27..4fef5163c657f8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11444,9 +11444,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   llvm::APSInt Right = RHSResult.Val.getInt();
 
   if (Right.isNegative()) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_negative)
-<< RHS.get()->getSourceRange());
+if (S.ExprEvalContexts.back().isConstantEvaluated())
+  S.Diag(Loc, diag::warn_shift_negative) << RHS.get()->getSourceRange();
+else
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_negative)
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11460,9 +11463,14 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
   llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
   if (Right.uge(LeftBits)) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+if (S.ExprEvalContexts.back().isConstantEvaluated()
+&& !S.getLangOpts().CPlusPlus11)
+  S.Diag(Loc, diag::warn_shift_gt_typewidth)
+<< RHS.get()->getSourceRange();
+else
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_gt_typewidth)
+  << RHS.get()->getSourceRange());
 return;
   }
 
@@ -11493,9 +11501,12 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   // If LHS does not have a non-negative value then, the
   // behavior is undefined before C++2a. Warn about it.
   if (Left.isNegative()) {
-S.DiagRuntimeBehavior(Loc, LHS.get(),
-  S.PDiag(diag::warn_shift_lhs_negative)
-<< LHS.get()->getSourceRange());
+if (S.ExprEvalContexts.back().isConstantEvaluated())
+  S.Diag(Loc, diag::warn_shift_lhs_negative) << 
LHS.get()->getSourceRange();
+else
+  S.DiagRuntimeBehavior(Loc, LHS.get(),
+S.PDiag(diag::warn_shift_lhs_negative)
+  << LHS.get()->getSourceRange());
 return;
   }
 
diff --git a/clang/test/AST/Interp/shifts.cpp b/clang/test/AST/Interp/shifts.cpp
index b1df7b85cc9f2b..c11ba0f6f3a19e 100644
--- a/clang/test/AST/Interp/shifts.cpp
+++ b/clang/test/AST/Interp/shifts.cpp
@@ -33,13 +33,10 @@ namespace shifts {
// FIXME: 'implicit conversion' warning missing in 
the new interpreter. \
// cxx17-warning {{shift count >= width of type}} \
// ref-warning {{shift count >= width of type}} \
-   // ref-warning {{implicit conversion}} \
-   // ref-cxx17-warning {{shift count >= width of 
type}} \
-   // ref-cxx17-warning {{implicit conversion}}
+   // ref-cxx17-warning {{shift count >= width of 
type}}
 c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of 
type}} \
// cxx17-warning {{shift count >= width of type}} \
-   // ref-warning {{shift count >= width of type}} \
-   // ref-cxx17-warning {{shift count >= width of 
type}}
+   // ref-warning {{shift count >= width of type}}
 c = 1 << c;
 c <<= 0;
 c >>= 0;
diff --git a/clang/test/C/drs/dr0xx.c b/clang/test/C/drs/dr0xx.c
index 6a3717f0729b60..6611b2b65264fa 100644
--- a/clang/test/C/drs/dr0xx.c
+++ b/clang/test/C/drs/dr0xx.c
@@ -426,7 +426,7 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -11428,7 +11428,7 @@ static bool isScopedEnumerationType(QualType T) {
   return false;
 }
 
-static void DiagnoseBadShiftValues(Sema& S, ExprResult , ExprResult ,
+static void DiagnoseBadShiftValues(Sema , ExprResult , ExprResult ,

AaronBallman wrote:

```suggestion
static void DiagnoseBadShiftValues(Sema& S, ExprResult , ExprResult ,
```
Backing out an unrelated formatting change.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -2805,7 +2805,7 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
 if (SA != RHS) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();

AaronBallman wrote:

Only unrelated whitespace changes in the file, so I think the whole file can be 
reverted.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman edited 
https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -11523,9 +11525,9 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
 
   S.Diag(Loc, diag::warn_shift_result_gt_typewidth)
-<< HexResult.str() << Result.getMinSignedBits() << LHSType
-<< Left.getBitWidth() << LHS.get()->getSourceRange()
-<< RHS.get()->getSourceRange();
+  << HexResult.str() << Result.getMinSignedBits() << LHSType
+  << Left.getBitWidth() << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();

AaronBallman wrote:

```suggestion
<< HexResult.str() << Result.getMinSignedBits() << LHSType
<< Left.getBitWidth() << LHS.get()->getSourceRange()
<< RHS.get()->getSourceRange();
```
Backing out more unrelated formatting changes.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -19959,7 +19961,10 @@ bool Sema::DiagRuntimeBehavior(SourceLocation Loc, 
ArrayRef Stmts,
   case ExpressionEvaluationContext::ConstantEvaluated:
   case ExpressionEvaluationContext::ImmediateFunctionContext:
 // Relevant diagnostics should be produced by constant evaluation.
-break;
+
+// Solution below works for warnings detected in
+// Sema::DiagnoseBadShiftValues()
+return DiagIfReachable(Loc, Stmts, PD);

AaronBallman wrote:

This seems incorrect to me -- `DiagRuntimeBehavior()` is used to diagnose 
runtime issues, so we should not be emitting diagnostics for constant 
evaluations.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

@@ -2836,9 +2837,11 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of the
 // shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS)
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {

budimirarandjelovicsyrmia wrote:

Same

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

@@ -2803,9 +2803,10 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of
 // the shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS) {
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {

budimirarandjelovicsyrmia wrote:

As I didn't find valid reason, I reversed this change.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From c9ef2f27f4b633a5862c4a46d6940ce830a7bde1 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp |  4 ++--
 clang/lib/Sema/SemaExpr.cpp| 21 +
 clang/test/AST/Interp/shifts.cpp   |  7 ++-
 clang/test/C/drs/dr0xx.c   |  2 +-
 clang/test/Sema/shift-count-negative.c |  8 
 clang/test/Sema/shift-count-overflow.c |  6 ++
 clang/test/Sema/shift-negative-value.c |  8 
 clang/test/Sema/vla-2.c|  9 ++---
 clang/test/SemaCXX/cxx2a-explicit-bool.cpp |  2 +-
 clang/test/SemaCXX/shift.cpp   |  1 -
 10 files changed, 47 insertions(+), 21 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 78cfecbec9fd36..51ff18e59eb590 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2805,7 +2805,7 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
 if (SA != RHS) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
 } else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
   // C++11 [expr.shift]p2: A signed left shift must have a non-negative
   // operand, and must not overflow the corresponding unsigned type.
@@ -2838,7 +2838,7 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
 if (SA != RHS)
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c27..3efafd9bbb3613 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11428,7 +11428,7 @@ static bool isScopedEnumerationType(QualType T) {
   return false;
 }
 
-static void DiagnoseBadShiftValues(Sema& S, ExprResult , ExprResult ,
+static void DiagnoseBadShiftValues(Sema , ExprResult , ExprResult ,
SourceLocation Loc, BinaryOperatorKind Opc,
QualType LHSType) {
   // OpenCL 6.3j: shift values are effectively % word size of LHS (more 
defined),
@@ -11460,9 +11460,11 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
   llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
   if (Right.uge(LeftBits)) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+if (!S.getLangOpts().CPlusPlus11) {
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_gt_typewidth)
+<< RHS.get()->getSourceRange());
+}
 return;
   }
 
@@ -11523,9 +11525,9 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
 
   S.Diag(Loc, diag::warn_shift_result_gt_typewidth)
-<< HexResult.str() << Result.getMinSignedBits() << LHSType
-<< Left.getBitWidth() << LHS.get()->getSourceRange()
-<< RHS.get()->getSourceRange();
+  << HexResult.str() << Result.getMinSignedBits() << LHSType
+  << Left.getBitWidth() << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
 }
 
 /// Return the resulting type when a vector is shifted
@@ -19959,7 +19961,10 @@ bool Sema::DiagRuntimeBehavior(SourceLocation Loc, 
ArrayRef Stmts,
   case ExpressionEvaluationContext::ConstantEvaluated:
   case ExpressionEvaluationContext::ImmediateFunctionContext:
 // Relevant diagnostics should be produced by constant evaluation.
-break;
+
+// Solution below works for warnings detected in
+// Sema::DiagnoseBadShiftValues()
+return DiagIfReachable(Loc, Stmts, PD);
 
   case ExpressionEvaluationContext::PotentiallyEvaluated:
   case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
diff --git a/clang/test/AST/Interp/shifts.cpp b/clang/test/AST/Interp/shifts.cpp
index b1df7b85cc9f2b..c11ba0f6f3a19e 100644
--- a/clang/test/AST/Interp/shifts.cpp
+++ b/clang/test/AST/Interp/shifts.cpp
@@ -33,13 +33,10 @@ namespace shifts {
// FIXME: 'implicit conversion' warning missing in 
the new interpreter. \
// 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Timm Baeder via cfe-commits]

tbaederr wrote:

There are two questions above about removing a diagnostic that you haven't 
answered yet

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

budimirarandjelovicsyrmia wrote:

Ping @AaronBallman 

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From c89be5f77740d0800339cca189312800f567ffb9 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp | 11 +++
 clang/lib/Sema/SemaExpr.cpp| 21 +
 clang/test/AST/Interp/shifts.cpp   |  7 ++-
 clang/test/C/drs/dr0xx.c   |  2 +-
 clang/test/Sema/shift-count-negative.c |  8 
 clang/test/Sema/shift-count-overflow.c |  6 ++
 clang/test/Sema/shift-negative-value.c |  8 
 clang/test/Sema/vla-2.c|  9 ++---
 clang/test/SemaCXX/cxx2a-explicit-bool.cpp |  2 +-
 clang/test/SemaCXX/shift.cpp   |  1 -
 10 files changed, 52 insertions(+), 23 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 78cfecbec9fd36..52ee2655c18f99 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2803,9 +2803,10 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of
 // the shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS) {
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
+  return false;
 } else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
   // C++11 [expr.shift]p2: A signed left shift must have a non-negative
   // operand, and must not overflow the corresponding unsigned type.
@@ -2836,9 +2837,11 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of the
 // shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS)
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
+  return false;
+}
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c27..3efafd9bbb3613 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11428,7 +11428,7 @@ static bool isScopedEnumerationType(QualType T) {
   return false;
 }
 
-static void DiagnoseBadShiftValues(Sema& S, ExprResult , ExprResult ,
+static void DiagnoseBadShiftValues(Sema , ExprResult , ExprResult ,
SourceLocation Loc, BinaryOperatorKind Opc,
QualType LHSType) {
   // OpenCL 6.3j: shift values are effectively % word size of LHS (more 
defined),
@@ -11460,9 +11460,11 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
   llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
   if (Right.uge(LeftBits)) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+if (!S.getLangOpts().CPlusPlus11) {
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_gt_typewidth)
+<< RHS.get()->getSourceRange());
+}
 return;
   }
 
@@ -11523,9 +11525,9 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
 
   S.Diag(Loc, diag::warn_shift_result_gt_typewidth)
-<< HexResult.str() << Result.getMinSignedBits() << LHSType
-<< Left.getBitWidth() << LHS.get()->getSourceRange()
-<< RHS.get()->getSourceRange();
+  << HexResult.str() << Result.getMinSignedBits() << LHSType
+  << Left.getBitWidth() << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
 }
 
 /// Return the resulting type when a vector is shifted
@@ -19959,7 +19961,10 @@ bool Sema::DiagRuntimeBehavior(SourceLocation Loc, 
ArrayRef Stmts,
   case ExpressionEvaluationContext::ConstantEvaluated:
   case ExpressionEvaluationContext::ImmediateFunctionContext:
 // Relevant diagnostics should be produced by constant evaluation.
-break;
+
+// Solution below works for warnings detected in
+// Sema::DiagnoseBadShiftValues()
+return DiagIfReachable(Loc, Stmts, PD);
 
   case ExpressionEvaluationContext::PotentiallyEvaluated:
   case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
diff 

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

budimirarandjelovicsyrmia wrote:

> The patch summary doesn't really explain why the changes are needed. Can you 
> explain what problem you're solving (perhaps link to an issue if this is 
> fixing one that was reported by someone)?

Main goal of this patch is to enable diagnosing and emitting warnings related 
to shift operator that are already found by GCC.

Original proposal fix and comments were posted on Pharbricator 
([link](https://reviews.llvm.org/D141192)). This patch continues that patch.
Original issue was posted 
[here](https://github.com/llvm/llvm-project/issues/59863).

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -426,7 +426,7 @@ void dr081(void) {
   /* Demonstrate that we don't crash when left shifting a signed value; that's
* implementation defined behavior.
*/
- _Static_assert(-1 << 1 == -2, "fail"); /* Didn't shift a zero into the "sign 
bit". */
+ _Static_assert(-1 << 1 == -2, "fail"); /* expected-warning {{shifting a 
negative signed value is undefined}} */

AaronBallman wrote:

Please fix the comment above; it's no longer implementation-defined behavior, 
it's undefined since C99

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -2836,9 +2837,11 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of the
 // shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS)
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {

AaronBallman wrote:

Same here?

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+X = (-1<<29) //expected-warning {{shifting a negative signed value is 
undefined}}

AaronBallman wrote:

```suggestion
X = (-1<<29) // expected-warning {{shifting a negative signed value is 
undefined}}
```

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman commented:

The patch summary doesn't really explain why the changes are needed. Can you 
explain what problem you're solving (perhaps link to an issue if this is fixing 
one that was reported by someone)?

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -2803,9 +2803,10 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of
 // the shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS) {
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {

AaronBallman wrote:

Why are we removing this diagnostic in C++98 and C?

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-count-negative %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-count-negative %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+X = (1<<-29) //expected-warning {{shift count is negative}}

AaronBallman wrote:

```suggestion
X = (1<<-29) // expected-warning {{shift count is negative}}
```

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Aaron Ballman via cfe-commits]

https://github.com/AaronBallman edited 
https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia updated 
https://github.com/llvm/llvm-project/pull/70307

From c9b85c8e435790d8b4a42340f3963d852e7d65ae Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp | 11 +++
 clang/lib/Sema/SemaExpr.cpp| 21 +
 clang/test/AST/Interp/shifts.cpp   |  7 ++-
 clang/test/C/drs/dr0xx.c   |  2 +-
 clang/test/Sema/shift-count-negative.c |  8 
 clang/test/Sema/shift-count-overflow.c |  6 ++
 clang/test/Sema/shift-negative-value.c |  8 
 clang/test/Sema/vla-2.c|  9 ++---
 clang/test/SemaCXX/cxx2a-explicit-bool.cpp |  2 +-
 clang/test/SemaCXX/shift.cpp   |  1 -
 10 files changed, 52 insertions(+), 23 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 78cfecbec9fd363..52ee2655c18f994 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2803,9 +2803,10 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of
 // the shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS) {
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
+  return false;
 } else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
   // C++11 [expr.shift]p2: A signed left shift must have a non-negative
   // operand, and must not overflow the corresponding unsigned type.
@@ -2836,9 +2837,11 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of the
 // shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS)
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
+  return false;
+}
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c27a..3efafd9bbb36135 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11428,7 +11428,7 @@ static bool isScopedEnumerationType(QualType T) {
   return false;
 }
 
-static void DiagnoseBadShiftValues(Sema& S, ExprResult , ExprResult ,
+static void DiagnoseBadShiftValues(Sema , ExprResult , ExprResult ,
SourceLocation Loc, BinaryOperatorKind Opc,
QualType LHSType) {
   // OpenCL 6.3j: shift values are effectively % word size of LHS (more 
defined),
@@ -11460,9 +11460,11 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
   llvm::APInt LeftBits(Right.getBitWidth(), LeftSize);
   if (Right.uge(LeftBits)) {
-S.DiagRuntimeBehavior(Loc, RHS.get(),
-  S.PDiag(diag::warn_shift_gt_typewidth)
-<< RHS.get()->getSourceRange());
+if (!S.getLangOpts().CPlusPlus11) {
+  S.DiagRuntimeBehavior(Loc, RHS.get(),
+S.PDiag(diag::warn_shift_gt_typewidth)
+<< RHS.get()->getSourceRange());
+}
 return;
   }
 
@@ -11523,9 +11525,9 @@ static void DiagnoseBadShiftValues(Sema& S, ExprResult 
, ExprResult ,
   }
 
   S.Diag(Loc, diag::warn_shift_result_gt_typewidth)
-<< HexResult.str() << Result.getMinSignedBits() << LHSType
-<< Left.getBitWidth() << LHS.get()->getSourceRange()
-<< RHS.get()->getSourceRange();
+  << HexResult.str() << Result.getMinSignedBits() << LHSType
+  << Left.getBitWidth() << LHS.get()->getSourceRange()
+  << RHS.get()->getSourceRange();
 }
 
 /// Return the resulting type when a vector is shifted
@@ -19959,7 +19961,10 @@ bool Sema::DiagRuntimeBehavior(SourceLocation Loc, 
ArrayRef Stmts,
   case ExpressionEvaluationContext::ConstantEvaluated:
   case ExpressionEvaluationContext::ImmediateFunctionContext:
 // Relevant diagnostics should be produced by constant evaluation.
-break;
+
+// Solution below works for warnings detected in
+// Sema::DiagnoseBadShiftValues()
+return DiagIfReachable(Loc, Stmts, PD);
 
   case ExpressionEvaluationContext::PotentiallyEvaluated:
   case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

@@ -11773,7 +11786,25 @@ QualType Sema::CheckShiftOperands(ExprResult , 
ExprResult ,
   isScopedEnumerationType(RHSType)) {
 return InvalidOperands(Loc, LHS, RHS);
   }
-  DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+
+  BadShiftValueKind BSVKind =
+  DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+  if (ExprEvalContexts.back().isConstantEvaluated()) {

budimirarandjelovicsyrmia wrote:

This solution originally was posted on 
[Phabricator](https://reviews.llvm.org/D141192). I reviewed commit and found 
that it makes more sense to diagnose bad shift values inside function 
DiagnoseBadShiftValues(...) instead in mentioned IF. If there are no more 
questions, I will edit this part of code.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Shafik Yaghmour via cfe-commits]

@@ -11773,7 +11786,25 @@ QualType Sema::CheckShiftOperands(ExprResult , 
ExprResult ,
   isScopedEnumerationType(RHSType)) {
 return InvalidOperands(Loc, LHS, RHS);
   }
-  DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+
+  BadShiftValueKind BSVKind =
+  DiagnoseBadShiftValues(*this, LHS, RHS, Loc, Opc, LHSType);
+  if (ExprEvalContexts.back().isConstantEvaluated()) {

shafik wrote:

This feels pretty hacky but perhaps I am not fully understanding the solution.

Can you update the description with more details on why this is a good approach 
to the problem.

https://github.com/llvm/llvm-project/pull/70307
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Emit bad shift warnings (PR #70307)

[Budimir Aranđelović via cfe-commits]

https://github.com/budimirarandjelovicsyrmia created 
https://github.com/llvm/llvm-project/pull/70307

Diagnose bad shifts and emit warnings

From 894d6503b0e1ced73bab65b0454abfc65d5b8a99 Mon Sep 17 00:00:00 2001
From: budimirarandjelovicsyrmia 
Date: Thu, 26 Oct 2023 10:39:52 +0200
Subject: [PATCH] [clang] Emit bad shift warnings

---
 clang/lib/AST/ExprConstant.cpp | 11 ++--
 clang/lib/Sema/SemaExpr.cpp| 59 +-
 clang/test/AST/Interp/shifts.cpp   |  7 +--
 clang/test/C/drs/dr0xx.c   |  2 +-
 clang/test/Sema/shift-count-negative.c |  8 +++
 clang/test/Sema/shift-count-overflow.c |  6 +++
 clang/test/Sema/shift-negative-value.c |  8 +++
 clang/test/Sema/vla-2.c|  9 ++--
 clang/test/SemaCXX/cxx2a-explicit-bool.cpp |  2 +-
 clang/test/SemaCXX/shift.cpp   |  1 -
 10 files changed, 84 insertions(+), 29 deletions(-)
 create mode 100644 clang/test/Sema/shift-count-negative.c
 create mode 100644 clang/test/Sema/shift-count-overflow.c
 create mode 100644 clang/test/Sema/shift-negative-value.c

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 78cfecbec9fd363..52ee2655c18f994 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2803,9 +2803,10 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of
 // the shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS) {
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
+  return false;
 } else if (LHS.isSigned() && !Info.getLangOpts().CPlusPlus20) {
   // C++11 [expr.shift]p2: A signed left shift must have a non-negative
   // operand, and must not overflow the corresponding unsigned type.
@@ -2836,9 +2837,11 @@ static bool handleIntIntBinOp(EvalInfo , const Expr 
*E, const APSInt ,
 // C++11 [expr.shift]p1: Shift width must be less than the bit width of the
 // shifted type.
 unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
-if (SA != RHS)
+if (SA != RHS && Info.getLangOpts().CPlusPlus11) {
   Info.CCEDiag(E, diag::note_constexpr_large_shift)
-<< RHS << E->getType() << LHS.getBitWidth();
+  << RHS << E->getType() << LHS.getBitWidth();
+  return false;
+}
 Result = LHS >> SA;
 return true;
   }
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 94f52004cf6c27a..8d3768c06915ce9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -11428,26 +11428,35 @@ static bool isScopedEnumerationType(QualType T) {
   return false;
 }
 
-static void DiagnoseBadShiftValues(Sema& S, ExprResult , ExprResult ,
-   SourceLocation Loc, BinaryOperatorKind Opc,
-   QualType LHSType) {
+enum BadShiftValueKind {
+  BSV_Ok,
+  BSV_ShiftIsNegative,
+  BSV_ShiftLHSIsNegative,
+  BSV_ShiftSizeGTTypeWidth,
+};
+
+static BadShiftValueKind DiagnoseBadShiftValues(Sema , ExprResult ,
+ExprResult ,
+SourceLocation Loc,
+BinaryOperatorKind Opc,
+QualType LHSType) {
   // OpenCL 6.3j: shift values are effectively % word size of LHS (more 
defined),
   // so skip remaining warnings as we don't want to modify values within Sema.
   if (S.getLangOpts().OpenCL)
-return;
+return BSV_Ok;
 
   // Check right/shifter operand
   Expr::EvalResult RHSResult;
   if (RHS.get()->isValueDependent() ||
   !RHS.get()->EvaluateAsInt(RHSResult, S.Context))
-return;
+return BSV_Ok;
   llvm::APSInt Right = RHSResult.Val.getInt();
 
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
 << RHS.get()->getSourceRange());
-return;
+return BSV_ShiftIsNegative;
   }
 
   QualType LHSExprType = LHS.get()->getType();
@@ -11463,12 +11472,12 @@ static void DiagnoseBadShiftValues(Sema& S, 
ExprResult , ExprResult ,
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
 << RHS.get()->getSourceRange());
-return;
+return BSV_ShiftSizeGTTypeWidth;
   }
 
   // FIXME: We probably need to handle fixed point types specially here.
   if (Opc != BO_Shl || LHSExprType->isFixedPointType())
-return;
+return BSV_Ok;
 
   // When left shifting an ICE which is signed, we can check for overflow which
   // according to C++ standards prior