Bug 24723 - Use the proper definition of RETURN_PAIR on !MSVC
Summary: Use the proper definition of RETURN_PAIR on !MSVC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P2 Normal
Assignee: Mike Hommey
URL:
Keywords:
Depends on:
Blocks: 24724
  Show dependency treegraph
 
Reported: 2009-03-20 12:12 PDT by Mike Hommey
Modified: 2009-05-09 19:03 PDT (History)
0 users

See Also:


Attachments
patch (767 bytes, patch)
2009-03-20 12:13 PDT, Mike Hommey
no flags Details | Formatted Diff | Diff
patch (1.41 KB, patch)
2009-03-20 12:15 PDT, Mike Hommey
no flags Details | Formatted Diff | Diff
patch v2 (1.11 KB, patch)
2009-03-24 15:07 PDT, Mike Hommey
gns: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Hommey 2009-03-20 12:12:59 PDT
See https://lists.webkit.org/pipermail/webkit-dev/2009-March/007121.html for some details.

I'll be attaching a patch here.
Comment 1 Mike Hommey 2009-03-20 12:13:37 PDT
Created attachment 28793 [details]
patch
Comment 2 Mike Hommey 2009-03-20 12:15:19 PDT
Created attachment 28794 [details]
patch

Better with an actual patch, not only the changelog
Comment 3 Jan Alonzo 2009-03-20 18:31:19 PDT
Landed in r41886. Cheers,
Comment 4 Gustavo Noronha (kov) 2009-03-21 11:17:42 PDT
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
Comment 5 Gustavo Noronha (kov) 2009-03-21 11:18:40 PDT
Reopening because the change was reverted.
Comment 6 Mike Hommey 2009-03-24 15:07:45 PDT
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 7 Gustavo Noronha (kov) 2009-05-09 10:20:07 PDT
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;
Comment 8 Geoffrey Garen 2009-05-09 18:49:18 PDT
(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.
Comment 9 Gustavo Noronha (kov) 2009-05-09 19:03:22 PDT
(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!