Bug 19751

Summary: Merge LogicalAndNode and LogicalOrNode to LogicalOpNode
Product: WebKit Reporter: Gavin Barraclough <barraclough>
Component: JavaScriptCoreAssignee: 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 Flags
Patch to merge LogicalAndNode and LogicalOrNode
zwarich: review-
With new added LogicalOperator enum goodness. zwarich: review+

Description Gavin Barraclough 2008-06-24 09:19:05 PDT
Post Squirrelfish there is no longer a performance gain in having separate nodes for 'and' and 'or' – merging these will simplify & reduce code duplication in the parse tree.

Patch following.
Comment 1 Gavin Barraclough 2008-06-24 09:19:58 PDT
Created attachment 21910 [details]
Patch to merge LogicalAndNode and LogicalOrNode
Comment 2 Cameron Zwarich (cpst) 2008-06-24 21:08:57 PDT
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.
Comment 3 Cameron Zwarich (cpst) 2008-06-24 21:37:56 PDT
Sam pointed out that I didn't suggest a good name for the new enum. LogicalOperator seems like a good choice to me.
Comment 4 Gavin Barraclough 2008-06-25 08:09:54 PDT
Created attachment 21929 [details]
With new added LogicalOperator enum goodness.
Comment 5 Cameron Zwarich (cpst) 2008-06-25 10:17:39 PDT
Comment on attachment 21929 [details]
With new added LogicalOperator enum goodness.

r=me
Comment 6 Gavin Barraclough 2008-07-17 05:06:42 PDT
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.