Bug 137771 - CSS JIT: Introduce addressToStackReference
Summary: CSS JIT: Introduce addressToStackReference
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-10-15 23:50 PDT by Yusuke Suzuki
Modified: 2014-10-16 22:17 PDT (History)
1 user (show)

See Also:


Attachments
Patch (10.08 KB, patch)
2014-10-15 23:53 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (10.08 KB, patch)
2014-10-16 00:15 PDT, Yusuke Suzuki
benjamin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2014-10-15 23:50:55 PDT
CSS JIT: Introduce addressToStackReference
Comment 1 Yusuke Suzuki 2014-10-15 23:53:43 PDT
Created attachment 239934 [details]
Patch
Comment 2 Yusuke Suzuki 2014-10-16 00:15:33 PDT
Created attachment 239938 [details]
Patch
Comment 3 Benjamin Poulain 2014-10-16 21:27:07 PDT
Comment on attachment 239938 [details]
Patch

That's a great idea, this cleans up the code and make it harder to misuse the stack.

I think the name addressToStackReference can be confusing. What do you think of:
-stackReferenceAddress()
-addressOfStackReference()
or simply
-addressOf(StackReference)
Comment 4 Yusuke Suzuki 2014-10-16 22:09:01 PDT
(In reply to comment #3)
> Comment on attachment 239938 [details]
> Patch
> 
> That's a great idea, this cleans up the code and make it harder to misuse
> the stack.
> 
> I think the name addressToStackReference can be confusing. What do you think
> of:
> -stackReferenceAddress()
> -addressOfStackReference()
> or simply
> -addressOf(StackReference)

`addressOf` looks very nice :), its name and parameter's type describes the role of the function clearly.
I'll apply this change and land the patch.
Comment 5 Yusuke Suzuki 2014-10-16 22:17:49 PDT
Committed r174810: <http://trac.webkit.org/changeset/174810>