Summary: | Merge LogicalAndNode and LogicalOrNode to LogicalOpNode | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gavin Barraclough <barraclough> | ||||||
Component: | JavaScriptCore | Assignee: | Gavin Barraclough <barraclough> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | zwarich | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Gavin Barraclough
2008-06-24 09:19:05 PDT
Created attachment 21910 [details]
Patch to merge LogicalAndNode and LogicalOrNode
Comment on attachment 21910 [details]
Patch to merge LogicalAndNode and LogicalOrNode
I like the idea of this patch a lot. I have been thinking of doing this for other node types, including UnaryOpNode, BinaryOpNode, and the ones you have already done.
The only thing I don't like about this patch is that you add more cases to the Operator enum. I would prefer a separate enum for OpLogicalOr and OpLogicalAnd. Change that and I'll review it.
Sam pointed out that I didn't suggest a good name for the new enum. LogicalOperator seems like a good choice to me. Created attachment 21929 [details]
With new added LogicalOperator enum goodness.
Comment on attachment 21929 [details]
With new added LogicalOperator enum goodness.
r=me
Sending JavaScriptCore/ChangeLog Sending JavaScriptCore/kjs/grammar.y Sending JavaScriptCore/kjs/nodes.cpp Sending JavaScriptCore/kjs/nodes.h Sending JavaScriptCore/kjs/nodes2string.cpp Transmitting file data ..... Committed revision 35221. |