Bug 145480

Summary: Refactoring HandlerInfo and UnlinkedHandlerInfo.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch. benjamin: review+

Description Mark Lam 2015-05-29 16:32:53 PDT
HandlerInfo and UnlinkedHandlerInfo have common parts, but are not currently expressed as 2 unrelated structs that happen to have near identical fields.  We can refactor them to better express their relationship.  We can also add some convenience functions to make the code that uses them a little more readable.
Comment 1 Mark Lam 2015-05-29 16:38:56 PDT
Created attachment 253935 [details]
the patch.
Comment 2 Benjamin Poulain 2015-05-29 16:50:43 PDT
Comment on attachment 253935 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=253935&action=review

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:132
> +        UnlinkedHandlerInfo info(start, end, range.tryData->target->bind(), range.tryData->targetScopeDepth);

No need for cast?
Comment 3 Mark Lam 2015-05-29 17:16:35 PDT
(In reply to comment #2)
> Comment on attachment 253935 [details]
> the patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=253935&action=review
> 
> > Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:132
> > +        UnlinkedHandlerInfo info(start, end, range.tryData->target->bind(), range.tryData->targetScopeDepth);
> 
> No need for cast?

My local builds and the bots seem to agree that we don't need the cast, but it might be resulting in noisy warning on the Windows port.  I'll restore them before landing.
Comment 4 Mark Lam 2015-05-29 17:19:58 PDT
Thanks for the review.  Landed in r185022: <http://trac.webkit.org/r185022>.
Comment 5 Csaba Osztrogonác 2015-06-01 04:23:14 PDT
Comment on attachment 253935 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=253935&action=review

> Source/JavaScriptCore/bytecode/HandlerInfo.h:51
> +    void initialize(const UnlinkedHandlerInfo& unlinkedInfo, size_t nonLocalScopeDepth, CodeLocationLabel label)

Using CodeLocationLabel is incorrect here, because it is defined inside ENABLE(ASSEMBLER) guard.
Now !ENABLE(ASSEMBLER) build is broken due to this issue. ( !ENABLE(JIT) && !ENABLE(YARR_JIT) )

Could you check it?
Comment 6 Mark Lam 2015-06-01 15:01:07 PDT
The CodeLocationLabel issue is fixed in https://bugs.webkit.org/show_bug.cgi?id=145515.