[webkit-reviews] review granted: [Bug 19408] Constant folding in parser : [Attachment 22997] Fold only *, +, <<, >>, ~ operators (v4)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Aug 28 21:51:02 PDT 2008


Cameron Zwarich (cpst) <cwzwarich at uwaterloo.ca> has granted Gabor Loki
<loki at inf.u-szeged.hu>'s request for review:
Bug 19408: Constant folding in parser
https://bugs.webkit.org/show_bug.cgi?id=19408

Attachment 22997: Fold only *, +, <<, >>, ~ operators (v4)
https://bugs.webkit.org/attachment.cgi?id=22997&action=edit

------- Additional Comments from Cameron Zwarich (cpst)
<cwzwarich at uwaterloo.ca>
I think it would be best to make m1 and m2 more descriptive, like expr1 and
expr2. You don't want them too long, because some of this code is already quite
lengthy, but those would be better than m1 and m2. Also, locHasAssignments
doesn't really make the most sense here. In makeAssignNode(), locHasAssignments
refers to the location of the assignment. For these binary operations, it
should be rightHasAssignments, because we are concerned about the right
expression having assignments. I can make both of these changes when I land the
patch, if you'd like.

I still find the shouldBeRound() thing sort of odd, because the value it prints
is not actually the result of the operation. You could have used different
values that would work out exactly. However, I won't complain too much. More
tests are always welcome, and I can always change it if I want.


More information about the webkit-reviews mailing list