On Thu, Jun 4, 2015 at 4:43 AM, scott douglass <[email protected]> wrote: >> 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".)
The preferred pattern in Clang is to have the member initializer use the same name as the data member (if you look around the code base, you will see it frequently). ~Aaron > 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
