Bug 24723

Summary: Use the proper definition of RETURN_PAIR on !MSVC
Product: WebKit Reporter: Mike Hommey <mh+webkit>
Component: JavaScriptCoreAssignee: 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 Flags
patch
none
patch
none
patch v2 gustavo: review-

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!