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+

Gavin Barraclough
Reported 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.
Attachments
Patch to merge LogicalAndNode and LogicalOrNode (8.02 KB, patch)
2008-06-24 09:19 PDT, Gavin Barraclough 		zwarich: review-
DetailsFormatted DiffDiff
With new added LogicalOperator enum goodness. (7.83 KB, patch)
2008-06-25 08:09 PDT, Gavin Barraclough 		zwarich: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Gavin Barraclough
Comment 1 2008-06-24 09:19:58 PDT
Created attachment 21910 [details] Patch to merge LogicalAndNode and LogicalOrNode
Cameron Zwarich (cpst)
Comment 2 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.
Cameron Zwarich (cpst)
Comment 3 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.
Gavin Barraclough
Comment 4 2008-06-25 08:09:54 PDT
Created attachment 21929 [details] With new added LogicalOperator enum goodness.
Cameron Zwarich (cpst)
Comment 5 2008-06-25 10:17:39 PDT
Comment on attachment 21929 [details] With new added LogicalOperator enum goodness. r=me
Gavin Barraclough
Comment 6 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.
Note You need to log in before you can comment on or make changes to this bug.