[PATCH] some StmtExprs do not have side-effects

scott douglass sdouglass at arm.com
Thu Jun 4 01:43:03 PDT 2015


> On Wed, Jun 3, 2015 at 6:45 AM, scott douglass <sdouglass at arm.com>
> wrote:
> > 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

Thanks for the review!  I'll do the changes for your other comments in a new patch, but I have one question, below:

> > +    typedef ConstEvaluatedExprVisitor<SideEffectFinder> Inherited;
> > +    const bool IncludePossibleEffects;
> > +    bool HasSideEffects;
> > +
> > +  public:
> > +    explicit SideEffectFinder(const ASTContext &Context, bool
> > + IncludePossibleEffects_)
> 
> No trailing underscore in the param name, please.
> 
> > +      : Inherited(Context),
> > +        IncludePossibleEffects(IncludePossibleEffects_),
> > + HasSideEffects(false) { }

Ok, but I'd like to avoid the having the member initializer "IncludePossibleEffects(IncludePossibleEffects)" because it's confusing.  Is there a "preferred way" of avoiding that?  (If not, I'll just rename the parameter to "IncludePossible".)

Also, if you also have the time to look over the prerequisite patch, http://reviews.llvm.org/D10210, that would be helpful.







More information about the cfe-commits mailing list