[PATCH] some StmtExprs do not have side-effects

[scott douglass]

Refine Expr::HasSideEffects to correct identify StmtExprs that do not have side-effects.

[Depends on http://reviews.llvm.org/D10210.]

http://reviews.llvm.org/D10211

Files:
  include/clang/AST/EvaluatedExprVisitor.h
  lib/AST/Expr.cpp
  test/Sema/exprs.c

Index: include/clang/AST/EvaluatedExprVisitor.h
===================================================================
--- include/clang/AST/EvaluatedExprVisitor.h
+++ include/clang/AST/EvaluatedExprVisitor.h
@@ -28,6 +28,7 @@
 /// of its potentially-evaluated subexpressions, recursively.
 template<template <typename> class Ptr, typename ImplClass>
 class EvaluatedExprVisitorBase : public StmtVisitorBase<Ptr, ImplClass, void> {
+protected:
   const ASTContext &Context;
 
 public:
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2881,6 +2881,32 @@
   return false;
 }
 
+namespace {
+  /// \brief Look for any side effects within a Stmt.
+  class SideEffectFinder : public ConstEvaluatedExprVisitor<SideEffectFinder>
+  {
+    typedef ConstEvaluatedExprVisitor<SideEffectFinder> Inherited;
+    const bool IncludePossibleEffects;
+    bool HasSideEffects;
+
+  public:
+    explicit SideEffectFinder(const ASTContext &Context, bool IncludePossibleEffects_)
+      : Inherited(Context),
+        IncludePossibleEffects(IncludePossibleEffects_), HasSideEffects(false) { }
+
+    bool getHasSideEffects() const { return HasSideEffects; }
+
+    void VisitExpr(const Expr *E) {
+      if (!E->HasSideEffects(Context, IncludePossibleEffects)) {
+        Inherited::VisitStmt(E);
+        return;
+      }
+
+      HasSideEffects = true;
+    }
+  };
+}
+
 bool Expr::HasSideEffects(const ASTContext &Ctx,
                           bool IncludePossibleEffects) const {
   // In circumstances where we care about definite side effects instead of
@@ -2967,14 +2993,20 @@
   case CompoundAssignOperatorClass:
   case VAArgExprClass:
   case AtomicExprClass:
-  case StmtExprClass:
   case CXXThrowExprClass:
   case CXXNewExprClass:
   case CXXDeleteExprClass:
   case ExprWithCleanupsClass:
     // These always have a side-effect.
     return true;
 
+  case StmtExprClass: {
+    // StmtExprs have a side-efect if any substatement does.
+    SideEffectFinder Finder(Ctx, IncludePossibleEffects);
+    Finder.Visit(cast<StmtExpr>(this)->getSubStmt());
+    return Finder.getHasSideEffects();
+  }
+
   case ParenExprClass:
   case ArraySubscriptExprClass:
   case MemberExprClass:
Index: test/Sema/exprs.c
===================================================================
--- test/Sema/exprs.c
+++ test/Sema/exprs.c
@@ -251,3 +251,14 @@
   if (&test22)
     (void) 0;
 }
+
+int stmtexpr_fn();
+void stmtexprs(int i) {
+  // No "side effects" warnig for the first two:
+  __builtin_assume( ({ 1; }) ); // expected-warning {{use of GNU statement expression extension}}
+  __builtin_assume( ({ if (i) { (void)0; }; 42; }) ); // expected-warning {{use of GNU statement expression extension}}
+  __builtin_assume( ({ if (i) ({ // expected-warning {{use of GNU statement expression extension}} \
+                                 // expected-warning {{use of GNU statement expression extension}} \
+                                 // expected-warning {{the argument to '__builtin_assume' has side effects that will be discarded}}
+                                 stmtexpr_fn(); }); 1; }) );
+}

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D10211.27032.patch
Type: text/x-patch
Size: 3177 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150603/023261f4/attachment.bin>