Summary: | Use the proper definition of RETURN_PAIR on !MSVC | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mike Hommey <mh+webkit> | ||||||||
Component: | JavaScriptCore | Assignee: | Mike Hommey <mh+webkit> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | ||||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 24724 | ||||||||||
Attachments: |
|
Description
Mike Hommey
2009-03-20 12:12:59 PDT
Created attachment 28793 [details]
patch
Created attachment 28794 [details]
patch
Better with an actual patch, not only the changelog
Comment on attachment 28794 [details] patch this patch should probably be reworked to account for this change: https://bugs.webkit.org/attachment.cgi?id=28824 Reopening because the change was reverted. Created attachment 28911 [details]
patch v2
I think this patch should be safe. I see no reason why any 32-bits platforms couldn't use the second definition, while 64-bits platforms (only x86-64 at the moment) use the first one.
It makes things simpler on the long term, IMHO.
Comment on attachment 28911 [details] patch v2 > -// The Mac compilers are fine with this, > -#if PLATFORM(MAC) > +#if PLATFORM(X86_64) I think you want something like this here: #if PLATFORM(X86_64) || PLATFORM(MAC) That's because your patch will disable the usage of this struct for x86-macs, which is not desired in my experience with similar patches. > struct VoidPtrPair { > void* first; > void* second; (In reply to comment #7) > (From update of attachment 28911 [details] [review]) > > -// The Mac compilers are fine with this, > > -#if PLATFORM(MAC) > > +#if PLATFORM(X86_64) This change was already made on TOT. (In reply to comment #8) > (In reply to comment #7) > > (From update of attachment 28911 [details] [review] [review]) > > > -// The Mac compilers are fine with this, > > > -#if PLATFORM(MAC) > > > +#if PLATFORM(X86_64) > > This change was already made on TOT. > My bad. Thanks for looking! |