Thread overview
Linux 64bit branch
Jan 12, 2014
Jesse Phillips
Jan 12, 2014
Jacob Carlborg
Jan 12, 2014
Jesse Phillips
Jan 12, 2014
Jesse Phillips
Jan 13, 2014
Jesse Phillips
Jan 13, 2014
Jacob Carlborg
Jan 13, 2014
Jacob Carlborg
Jan 13, 2014
Jacob Carlborg
January 12, 2014
I wanted to mention that I have a branch for the base and gtk repositories which compile under 64bit.

https://github.com/JesseKPhillips/base/tree/64bit
https://github.com/JesseKPhillips/org.eclipse.swt.gtk.linux.x86/tree/64bit

I don't know if changes are correct, in the since that I don't have a grasp on the boundaries with the gtk libraries. They shouldn't affect 32bit though (it also compiles)

As I have not built a working example yet I won't be creating a pull request, the compiler message for snippet 107:

-------
(org-eclipse-swt-program-Program.o): undefined reference to symbol 'gnome_vfs_init'
//usr/lib/x86_64-linux-gnu/libgnomevfs-2.so.0: error adding symbols: DSO missing from command line
-------

Which could be an issue with ordering[1] or missing a library. I don't know if I'll get anything working, but wanted to provide information that there is a compiling lib for 64bit out there.

1. http://stackoverflow.com/questions/19901934/strange-linking-error-dso-missing-from-command-line
January 12, 2014
On 2014-01-12 08:19, Jesse Phillips wrote:
> I wanted to mention that I have a branch for the base and gtk
> repositories which compile under 64bit.
>
> https://github.com/JesseKPhillips/base/tree/64bit
> https://github.com/JesseKPhillips/org.eclipse.swt.gtk.linux.x86/tree/64bit

Thanks, that's great.

> I don't know if changes are correct, in the since that I don't have a
> grasp on the boundaries with the gtk libraries. They shouldn't affect
> 32bit though (it also compiles)

I had a look at the changes. I noticed a couple things:

* Most changes seems to be related to changing "int" to "size_t" or "ptrdiff_t". "int" was used because that's what the Java API uses. Java doesn't even have unsigned types. Is the changes really necessary? Does it require a lot of casts otherwise?

The place where it is correct to change the type is when the SWT scource code look something like this: "(int)/*64*/". That is a hint to a tool SWT uses to automatically translate between 32 and 64bit. These cases should always be replaced with the native type. Be it either a pointer type, size_t or something else.

* Currently DWT is compatible with both D1 and D2. Could you please use "to!(T)(args)" instead and import "tango.util.Convert" when Tango is used to make it compatible with D1 as well. I do intend to drop the support for D1 at some point, either when I merge in DWT-Cocoa or when there is a significant large change making it not practical to do the change both for D1 and D2. Anyway, I want to drop the support in a controlled way, making a D1 branch, make an announcement and so on. I don't think this is a significant large change to break the D1 support. It's easy to make it compatible with both D1 and D2.

* I see that you have used "auto". I prefer we try to avoid using "auto" at all. My experience with using "auto" in DWT is that it's easier to make a mistake when porting. When not using "auto" both the return type of a function and in the expression where it's used need to have the correct type. Also, "auto" doesn't exist in Java and we want to stay as close to the Java source code to make it easier to merge future versions.

> As I have not built a working example yet I won't be creating a pull
> request, the compiler message for snippet 107:
>
> -------
> (org-eclipse-swt-program-Program.o): undefined reference to symbol
> 'gnome_vfs_init'
> //usr/lib/x86_64-linux-gnu/libgnomevfs-2.so.0: error adding symbols: DSO
> missing from command line
> -------
>
> Which could be an issue with ordering[1] or missing a library. I don't
> know if I'll get anything working, but wanted to provide information
> that there is a compiling lib for 64bit out there.

Ok, I see. Have you verified that you have all the necessary libraries installed? They're listed here: https://github.com/d-widget-toolkit/dwt#linux

-- 
/Jacob Carlborg
January 12, 2014
On Sunday, 12 January 2014 at 11:42:30 UTC, Jacob Carlborg wrote:
> * Most changes seems to be related to changing "int" to "size_t" or "ptrdiff_t". "int" was used because that's what the Java API uses. Java doesn't even have unsigned types. Is the changes really necessary? Does it require a lot of casts otherwise?

The main issue is that arr.length is size_t, I thought that that it would be wise not to go against the grain, but I had to back out some of those changes, since in reality it seems that gtk seems to expect count/length in int (I've squashed the commits now).

To reduce casting the internals of the function utilize the D type. But since I was working from compiler errors it is probably just a mix.

> The place where it is correct to change the type is when the SWT scource code look something like this: "(int)/*64*/". That is a hint to a tool SWT uses to automatically translate between 32 and 64bit. These cases should always be replaced with the native type. Be it either a pointer type, size_t or something else.

I'll have to look into that, certainly didn't get that right.

> * Currently DWT is compatible with both D1 and D2. Could you please use "to!(T)(args)" instead and import "tango.util.Convert" when Tango is used to make it compatible with D1 as well. I do intend to drop the support for D1 at some point, either when I merge in DWT-Cocoa or when there is a significant large change making it not practical to do the change both for D1 and D2. Anyway, I want to drop the support in a controlled way, making a D1 branch, make an announcement and so on. I don't think this is a significant large change to break the D1 support. It's easy to make it compatible with both D1 and D2.

Yes, I forgot to mention that this pass didn't concern itself with Tango.

> * I see that you have used "auto". I prefer we try to avoid using "auto" at all. My experience with using "auto" in DWT is that it's easier to make a mistake when porting. When not using "auto" both the return type of a function and in the expression where it's used need to have the correct type. Also, "auto" doesn't exist in Java and we want to stay as close to the Java source code to make it easier to merge future versions.

I'm not sure what having the return type match the type used in the expression has to do with anything. Unless it is floating point or some bit-shift manipulation.

Something had to change, the options are:

1. size_t or ptrdiff_t depending on the result
2. use auto
3. do type conversion

I chose auto because I didn't really care to identify if size_t or ptrdiff_t was needed (D doesn't error on mismatch if you get those wrong) and type conversion would then require type conversion in many later expressions (possibly).

> Ok, I see. Have you verified that you have all the necessary libraries installed? They're listed here: https://github.com/d-widget-toolkit/dwt#linux

Not yet, will be something to look into.
January 12, 2014
On Sunday, 12 January 2014 at 20:13:35 UTC, Jesse Phillips wrote:
> On Sunday, 12 January 2014 at 11:42:30 UTC, Jacob Carlborg
>> Ok, I see. Have you verified that you have all the necessary libraries installed? They're listed here: https://github.com/d-widget-toolkit/dwt#linux
>
> Not yet, will be something to look into.

I was able to get snippets to compile using the recommendation:
http://forum.dlang.org/post/kjscs8$2a9n$1@digitalmars.com

Adding gnomevfs-2.

But the result of running is a segfault so will need to investigate there now. I'll start with your /*64*/ tip.
January 13, 2014
On Sunday, 12 January 2014 at 20:43:24 UTC, Jesse Phillips wrote:
> On Sunday, 12 January 2014 at 20:13:35 UTC, Jesse Phillips wrote:
>> On Sunday, 12 January 2014 at 11:42:30 UTC, Jacob Carlborg
>>> Ok, I see. Have you verified that you have all the necessary libraries installed? They're listed here: https://github.com/d-widget-toolkit/dwt#linux
>>
>> Not yet, will be something to look into.
>
> I was able to get snippets to compile using the recommendation:
> http://forum.dlang.org/post/kjscs8$2a9n$1@digitalmars.com
>
> Adding gnomevfs-2.
>
> But the result of running is a segfault so will need to investigate there now. I'll start with your /*64*/ tip.

I'm not sure that there is anything specific needed where /*64*/ comments exist, the tool would have a lot of work to translate many types and not just a single cast.

But I did find where there is more work. The segfault pointed to:

    org.eclipse.swt.widgets.Display.Display.windowProcFunc3()

Which is defined as:

    private static extern(C) int /*long*/ windowProcFunc3 (int /*long*/ handle, int /*long*/ arg0, int /*long*/ user_data) {

which is assigned as:

    GCallback windowProc3 = cast(GCallback)&windowProcFunc3;

Where GCallback is defined as:

internal.gtk.OS.d
    public alias org.eclipse.swt.internal.c.glib_object.GCallback GCallback;

    alias _BCD_func__2331 GCallback;
    alias void function() _BCD_func__2331;

I don't understand why it is cast to a function which takes no arguments, but suspect that fixing the signatures for /*long*/ is a good direction.
January 13, 2014
On 2014-01-13 05:29, Jesse Phillips wrote:

> I'm not sure that there is anything specific needed where /*64*/
> comments exist, the tool would have a lot of work to translate many
> types and not just a single cast.

Yes, it needs to translate signatures as well, as you mention below.

> But I did find where there is more work. The segfault pointed to:
>
>      org.eclipse.swt.widgets.Display.Display.windowProcFunc3()

It also good to verify that the snippet work for 32bit as well.

> Which is defined as:
>
>      private static extern(C) int /*long*/ windowProcFunc3 (int /*long*/
> handle, int /*long*/ arg0, int /*long*/ user_data) {
>
> which is assigned as:
>
>      GCallback windowProc3 = cast(GCallback)&windowProcFunc3;
>
> Where GCallback is defined as:
>
> internal.gtk.OS.d
>      public alias org.eclipse.swt.internal.c.glib_object.GCallback
> GCallback;
>
>      alias _BCD_func__2331 GCallback;
>      alias void function() _BCD_func__2331;
>
> I don't understand why it is cast to a function which takes no
> arguments, but suspect that fixing the signatures for /*long*/ is a good
> direction.

The original type GCallback[1] is defined to take no arguments and return "void". Although I see that it doesn't mean the callback must take no arguments and return "void".

I have not worked with the Linux port, but in the Mac OS X port they basically merge all callbacks with the same amounts of arguments to a single function. In this case "windowProcFunc3", would handle all callbacks taking three arguments.

Technically I don't think the signature matters, as long as the correct calling convention is used. Where "correct" is whatever is expected from the callback and/or the function invoking the callback.

I don't know which type here is best to use. In the Mac OS X port there is a single type that covers all types except for structs.

https://developer.gnome.org/gobject/stable/gobject-Closures.html#GCallback

-- 
/Jacob Carlborg
January 13, 2014
On 2014-01-12 21:13, Jesse Phillips wrote:

> The main issue is that arr.length is size_t, I thought that that it
> would be wise not to go against the grain, but I had to back out some of
> those changes, since in reality it seems that gtk seems to expect
> count/length in int (I've squashed the commits now).

Since the Java code expects "int" in these cases, I just hope this won't break anything. Although, I have used size_t in the Mac OS X port, trying to make it 32 and 64bit compatible at once.

I'm using this philosophy when I'm porting SWT to D. In prioritizing order:

1. The public API needs to be the same on all platforms and preferably as the Java API.

2. The types and signatures should match the native ones. That means, if the native signature uses a pointer, the D code should use a pointer as well, even if the Java code uses int/long.

3. Try to stay as close to the original Java code as possible. This will make it easier to port future versions.

> To reduce casting the internals of the function utilize the D type. But
> since I was working from compiler errors it is probably just a mix.
>
>> The place where it is correct to change the type is when the SWT
>> scource code look something like this: "(int)/*64*/". That is a hint
>> to a tool SWT uses to automatically translate between 32 and 64bit.
>> These cases should always be replaced with the native type. Be it
>> either a pointer type, size_t or something else.
>
> I'll have to look into that, certainly didn't get that right.

Hmm, these might actually be the cases where it should be "int" on both platforms and a cast is required since it's would otherwise be "long" for 64bit.

You did find the other cases where I know for sure that the type should vary on 32 and 64bit, that is when the code looks like this:

int /*long*/

> Yes, I forgot to mention that this pass didn't concern itself with Tango.

Ok

> I'm not sure what having the return type match the type used in the
> expression has to do with anything. Unless it is floating point or some
> bit-shift manipulation.

Say I have a function like this in the Java code:

int /*long*/ foo ();

And used like this:

int /*long*/ bar = foo();

It has happened to me several times that I used "auto" when declaring "bar". And I changed the signature of "foo" to the correct native type. The only problem is that I used the wrong native type which I later discovered when I translated the declaration of "bar". If I had not used "auto" and I had use the actual correct native type when declaring "bar" the compiler would have a caught the type error. That's just my experience.

> Something had to change, the options are:
>
> 1. size_t or ptrdiff_t depending on the result

I think this is the most correct option.

> 2. use auto
> 3. do type conversion
>
> I chose auto because I didn't really care to identify if size_t or
> ptrdiff_t was needed (D doesn't error on mismatch if you get those
> wrong) and type conversion would then require type conversion in many
> later expressions (possibly).

Yes, see above.

-- 
/Jacob Carlborg
January 13, 2014
On 2014-01-13 10:58, Jacob Carlborg wrote:

> I'm using this philosophy when I'm porting SWT to D. In prioritizing order:
>
> 1. The public API needs to be the same on all platforms and preferably
> as the Java API.
>
> 2. The types and signatures should match the native ones. That means, if
> the native signature uses a pointer, the D code should use a pointer as
> well, even if the Java code uses int/long.
>
> 3. Try to stay as close to the original Java code as possible. This will
> make it easier to port future versions.

> Hmm, these might actually be the cases where it should be "int" on both
> platforms and a cast is required since it's would otherwise be "long"
> for 64bit.
>
> You did find the other cases where I know for sure that the type should
> vary on 32 and 64bit, that is when the code looks like this:
>
> int /*long*/

I double checked these cases with the Java code:

32bit           64bit
int /*long*/    long /*int*/
(int)/*64*/     (int)/*64*/

The reason for (int)/*64*/ is usually because the native type would be 64bit but the Java requires it to be 32bit. By the second rule above, I would translate (int)/*64*/ to the native type.

-- 
/Jacob Carlborg