[Webkit-unassigned] [Bug 50015] Move regex parsing and fallback handling to runtime/RegExp.cpp

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Gavin Barraclough <barraclough at apple.com>  2010-11-29 23:37:13 PST ---
Hi Peter,

A couple of cosmetic things, we should probably remove the compile method (this can now just fold back up into the constructor, since PCRE is gone, and we should probably also remove the following comment from the header:
  // FIXME: Just decompile m_regExp instead of storing this.
(not a direction I think we're headed in).

There is one way I think you could improve this patch though, see what you think.  Checks like this seem a little over complicated, particularly on a hot path:
> if (!!m_representation->m_regExpJITCode || (m_useInterpreter && m_representation->m_regExpBytecode))
(okay, it's just a couple more branches, but it could be simpler).
I'd suggest instead of adding m_useInterpreter to add an enum to track the state of the regex.  At the point of construction the RegExp always goes into one of three states - either (A) it has a parse error, or (B) if the JIT is enabled & the regex is supported it will be JIT compiled, else (C) it will be byte-compiled.  In match that if check becomes something like 'if (m_state != ParseError)'

In the longer term, it would be nice to only construct RegExp objects after successfully parsing, and have RegExp::create just return 0 on failure (with an error string as an output parameter).  Having duff RegExp objects that can't actually be run seems odd, and I think in all cases, at least within the JS languages use, errors should be reported (an thrown immediately) at construction time anyway, so there is likely no way to get a handle to a JS object that is referencing a RegExp that failed to compile.  Anyway, this would be a bigger change & is not important now.

What do you think of adding an enum to explicitly delineate the states?

cheers, G.

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