Bug 145480 - Refactoring HandlerInfo and UnlinkedHandlerInfo.
Summary: Refactoring HandlerInfo and UnlinkedHandlerInfo.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-29 16:32 PDT by Mark Lam
Modified: 2015-06-01 15:01 PDT (History)
1 user (show)

See Also:


Attachments
the patch. (8.15 KB, patch)
2015-05-29 16:38 PDT, Mark Lam
benjamin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.