[llvm-branch-commits] [clang] 0e4d236 - [OpenCL] Warn about side effects for unevaluated vec_step arg

[Sven van Haastregt via llvm-branch-commits]

Author: Sven van Haastregt
Date: 2021-01-05T11:51:10Z
New Revision: 0e4d2361b817bd16f97cd45d5792017edc3891ee

URL: https://github.com/llvm/llvm-project/commit/0e4d2361b817bd16f97cd45d5792017edc3891ee
DIFF: https://github.com/llvm/llvm-project/commit/0e4d2361b817bd16f97cd45d5792017edc3891ee.diff

LOG: [OpenCL] Warn about side effects for unevaluated vec_step arg

The argument to the `vec_step` builtin is not evaluated.  Hoist the
diagnostic for this in `Sema::CheckUnaryExprOrTypeTraitOperand` such
that it comes before `Sema::CheckVecStepTraitOperandType`.

A minor side-effect of this change is that it also produces the
warning for `co_await` and `co_yield` as `sizeof` arguments now, which
seems to be reasonable given that the warning is emitted for `typeid`
already.

Differential Revision: https://reviews.llvm.org/D91348

Added: 
    

Modified: 
    clang/lib/Sema/SemaExpr.cpp
    clang/test/SemaCXX/coroutines.cpp
    clang/test/SemaOpenCL/vec_step.cl

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 28f4c5bbf19b..d8bfd3f8b46a 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -4089,7 +4089,7 @@ bool Sema::CheckUnaryExprOrTypeTraitOperand(Expr *E,
 
   bool IsUnevaluatedOperand =
       (ExprKind == UETT_SizeOf || ExprKind == UETT_AlignOf ||
-       ExprKind == UETT_PreferredAlignOf);
+       ExprKind == UETT_PreferredAlignOf || ExprKind == UETT_VecStep);
   if (IsUnevaluatedOperand) {
     ExprResult Result = CheckUnevaluatedOperand(E);
     if (Result.isInvalid())
@@ -4097,6 +4097,16 @@ bool Sema::CheckUnaryExprOrTypeTraitOperand(Expr *E,
     E = Result.get();
   }
 
+  // The operand for sizeof and alignof is in an unevaluated expression context,
+  // so side effects could result in unintended consequences.
+  // Exclude instantiation-dependent expressions, because 'sizeof' is sometimes
+  // used to build SFINAE gadgets.
+  // FIXME: Should we consider instantiation-dependent operands to 'alignof'?
+  if (IsUnevaluatedOperand && !inTemplateInstantiation() &&
+      !E->isInstantiationDependent() &&
+      E->HasSideEffects(Context, false))
+    Diag(E->getExprLoc(), diag::warn_side_effects_unevaluated_context);
+
   if (ExprKind == UETT_VecStep)
     return CheckVecStepTraitOperandType(*this, ExprTy, E->getExprLoc(),
                                         E->getSourceRange());
@@ -4133,16 +4143,6 @@ bool Sema::CheckUnaryExprOrTypeTraitOperand(Expr *E,
     return true;
   }
 
-  // The operand for sizeof and alignof is in an unevaluated expression context,
-  // so side effects could result in unintended consequences.
-  // Exclude instantiation-dependent expressions, because 'sizeof' is sometimes
-  // used to build SFINAE gadgets.
-  // FIXME: Should we consider instantiation-dependent operands to 'alignof'?
-  if (IsUnevaluatedOperand && !inTemplateInstantiation() &&
-      !E->isInstantiationDependent() &&
-      E->HasSideEffects(Context, false))
-    Diag(E->getExprLoc(), diag::warn_side_effects_unevaluated_context);
-
   if (CheckObjCTraitOperandConstraints(*this, ExprTy, E->getExprLoc(),
                                        E->getSourceRange(), ExprKind))
     return true;

diff  --git a/clang/test/SemaCXX/coroutines.cpp b/clang/test/SemaCXX/coroutines.cpp
index f354b1f2a4bb..d7208f9a1010 100644
--- a/clang/test/SemaCXX/coroutines.cpp
+++ b/clang/test/SemaCXX/coroutines.cpp
@@ -328,6 +328,7 @@ void unevaluated() {
                         // expected-warning at -1 {{declaration does not declare anything}}
   sizeof(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
                       // expected-error at -1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
+                      // expected-warning at -2 {{expression with side effects has no effect in an unevaluated context}}
   typeid(co_await a); // expected-error {{'co_await' cannot be used in an unevaluated context}}
                       // expected-warning at -1 {{expression with side effects has no effect in an unevaluated context}}
                       // expected-warning at -2 {{expression result unused}}
@@ -335,6 +336,7 @@ void unevaluated() {
                         // expected-warning at -1 {{declaration does not declare anything}}
   sizeof(co_yield 2); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
                       // expected-error at -1 {{invalid application of 'sizeof' to an incomplete type 'void'}}
+                      // expected-warning at -2 {{expression with side effects has no effect in an unevaluated context}}
   typeid(co_yield 3); // expected-error {{'co_yield' cannot be used in an unevaluated context}}
                       // expected-warning at -1 {{expression with side effects has no effect in an unevaluated context}}
                       // expected-warning at -2 {{expression result unused}}

diff  --git a/clang/test/SemaOpenCL/vec_step.cl b/clang/test/SemaOpenCL/vec_step.cl
index 0a2f6ef30e76..afb6dc94d92e 100644
--- a/clang/test/SemaOpenCL/vec_step.cl
+++ b/clang/test/SemaOpenCL/vec_step.cl
@@ -29,4 +29,6 @@ void foo(int3 arg1, int8 arg2) {
   int res13 = vec_step(*incomplete1); // expected-error {{'vec_step' requires built-in scalar or vector type, '__private struct S' invalid}}
   int res14 = vec_step(int16*); // expected-error {{'vec_step' requires built-in scalar or vector type, '__private int16 *' invalid}}
   int res15 = vec_step(void(void)); // expected-error {{'vec_step' requires built-in scalar or vector type, 'void (void)' invalid}}
+
+  int res_no_effect = vec_step(auto3++); // expected-warning {{expression with side effects has no effect in an unevaluated context}}
 }