> On Wed, Jun 3, 2015 at 6:45 AM, scott douglass <[email protected]>
> 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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
