[Webkit-unassigned] [Bug 69690] DFG does not have flow-sensitive intraprocedural control flow analysis

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=69690


Gavin Barraclough <barraclough at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #110430|review?                     |review+
               Flag|                            |




--- Comment #34 from Gavin Barraclough <barraclough at apple.com>  2011-10-11 16:45:44 PST ---
(From update of attachment 110430)
View in context: https://bugs.webkit.org/attachment.cgi?id=110430&action=review

r+.  I still don't think AbstractValue is really descriptive enough, but I don't have a better name to suggest.

> Source/JavaScriptCore/dfg/DFGAbstractValue.h:312
> +struct AbstractValue {

AbstractValue doesn't seem to be the right name for this class.  This struct can tell you a type, not a value - e.g. "int", or "object with properties x, y", not '3' or '"Hello, world"'.
This naming is confusing in it's uses, e.g. in GetLocal.

> Source/JavaScriptCore/dfg/DFGNode.h:715
> +    unsigned takenBytecodeOffsetDuringParsing()

This change could probably have been made as a separate patch, breaking up this review a bit would have helped! - probably no point doing so now though.
It's a bit unfortunate that the meaning of the branch's opInfos changes over time, it would be better if we could guard against error here.
We could consider extra state in debug builds that we can ASSERT on, or we could change reflect this in the node types (have a BranchDuringParse/JumpDuringParse that are updated as the successors are set, and never reach generation).

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.