[PATCH] some StmtExprs do not have side-effects

[Aaron Ballman]

On Thu, Jun 4, 2015 at 4:43 AM, scott douglass <sdouglass at arm.com> wrote:
>> 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".)

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.