Fix checks to bit flags of PreParserExpression

Fix checks to bit flags of PreParserExpression

This fixes checks on the "code_" member of PreParserExpression, in order
to make methods IsThis(), IsThisProperty(), IsProperty(), IsCall() and
IsValidReferenceExpression() work correctly.

BUG=v8:3456
LOG=
R=svenpanne@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=22562

[wingo, 2014-07-23 12:54:55]

https://codereview.chromium.org/410873004/diff/1/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/410873004/diff/1/src/preparser.h#newcode683
src/preparser.h:683: bool IsThis() const { return (code_ & kThisExpression) ==
kThisExpression; }
Since the kThisExpression et al are just bitflags I would write "return code_ &
kThisExpression" instead, without the ==.  That's just me though.  Looks good to
this non-owner, please pass on to an owner.

[Sven Panne, 2014-07-23 13:26:06]

LGTM, wowhead.com has been resurrected by this patch, I'll land it for you.

https://codereview.chromium.org/410873004/diff/1/src/preparser.h
File src/preparser.h (right):

https://codereview.chromium.org/410873004/diff/1/src/preparser.h#newcode683
src/preparser.h:683: bool IsThis() const { return (code_ & kThisExpression) ==
kThisExpression; }
On 2014/07/23 12:54:55, wingo wrote:
> Since the kThisExpression et al are just bitflags I would write "return code_
&
> kThisExpression" instead, without the ==.  That's just me though.  Looks good
to
> this non-owner, please pass on to an owner.

For consistency, I would leave it as it is. All this bit-fiddling has to be
abstracted, anyway. Using one enum for different purposes without any
abstraction is not nice...

[Sven Panne, 2014-07-23 13:29:31]

Committed patchset #1 manually as r22562 (presubmit successful).

[aperez, 2014-07-23 13:36:50]

On 2014/07/23 13:26:06, Sven Panne wrote:
> LGTM, http://wowhead.com has been resurrected by this patch, I'll land it for
you.
> 
> https://codereview.chromium.org/410873004/diff/1/src/preparser.h
> File src/preparser.h (right):
> 
> https://codereview.chromium.org/410873004/diff/1/src/preparser.h#newcode683
> src/preparser.h:683: bool IsThis() const { return (code_ & kThisExpression) ==
> kThisExpression; }
> On 2014/07/23 12:54:55, wingo wrote:
> > Since the kThisExpression et al are just bitflags I would write "return
code_
> &
> > kThisExpression" instead, without the ==.  That's just me though.  Looks
good
> to
> > this non-owner, please pass on to an owner.
> 
> For consistency, I would leave it as it is. All this bit-fiddling has to be
> abstracted, anyway. Using one enum for different purposes without any
> abstraction is not nice...

Agreed, while making the fix I was thinking that it would be nice to use the
BitField template from “src/utils.h”... anyway, the idea was to have this fix
applied ASAP, so let's leave cleanups for another day.

Thanks for landing this CL for me.

[Sven Panne, 2014-07-23 13:50:07]

On 2014/07/23 13:36:50, aperez wrote:
> Agreed, while making the fix I was thinking that it would be nice to use the
> BitField template from “src/utils.h”... anyway, the idea was to have this fix
> applied ASAP, so let's leave cleanups for another day.

Yes, using BitField would be much better, although I don't fully understand why
we don't simply use C++'s enums + bit fields: Even with BitField you have to
write error-prone boilerplate code, which would be automatically and safely be
generated by the C++ compiler instead of your brain. But that's a topic for a
separate discussion...