Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix 32 bit OSX build #5788

Merged
merged 1 commit into from
May 17, 2016
Merged

fix 32 bit OSX build #5788

merged 1 commit into from
May 17, 2016

Conversation

WalterBright
Copy link
Member

size_t has mangling issues on 32 bit OSX

@MartinNowak
Copy link
Member

MartinNowak commented May 17, 2016

Great, this is currently blocking the nightly builds. Please target stable and mention the bugzilla number in your commit https://issues.dlang.org/show_bug.cgi?id=16000. As this is just a workaround you should exclude the fix from fix Issue 16000, so we won't close the ticket but still cross-reference it.
Also I've assigned you the existing card https://trello.com/c/oUlKisPm/188-issue-16000-linking-issues-on-osx-with-size-t-extern-c-i386-and-clang.

@MartinNowak
Copy link
Member

This bug isn't in stable, it was just recently 2016-05-06 introduced in master.

@yebblies
Copy link
Member

And backend.d

@@ -474,7 +474,7 @@ void DtBuilder::cat(DtBuilder *dtb)
/**************************************
* Repeat a list of dt_t's count times.
*/
void DtBuilder::repeat(dt_t *dt, size_t count)
void DtBuilder::repeat(dt_t *dt, unsigned count)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why d_size_t is not used here as well?

@jacob-carlborg
Copy link
Contributor

The D declaration of DtBuilder::repeat still uses size_t, which doesn't match unsigned on 64bit.

@WalterBright
Copy link
Member Author

@jacob-carlborg I had messed up my files due to a merge, and the wrong files got in this PR. Corrected.

@jacob-carlborg
Copy link
Contributor

LGTM 👌

@jacob-carlborg
Copy link
Contributor

This is kind of a workaround. Any plans to properly support linking to a C++ function with a size_t parameter?

@MartinNowak
Copy link
Member

This is kind of a workaround. Any plans to properly support linking to a C++ function with a size_t parameter?

I don't think we plan to introduce size_t as distinct ("real") type, so it cannot have a different mangling than uint/ulong. Not sure what else we could do, the information that sth. was a size_t is lost pretty early.

@MartinNowak MartinNowak merged commit 7d00095 into dlang:master May 17, 2016
@WalterBright WalterBright deleted the d_size_t branch May 17, 2016 23:08
@MartinNowak
Copy link
Member

Nice, working nightlies again http://nightlies.dlang.org/dmd-2016-05-17/.

@mathias-lang-sociomantic
Copy link
Contributor

I don't think we plan to introduce size_t as distinct ("real") type, so it cannot have a different mangling than uint/ulong. Not sure what else we could do, the information that sth. was a size_t is lost pretty early.

If we want to provide the frontend as a library people can use to write tool, we will ultimately have to go for a less / non-destructive approach, and store the full information.
Also, we already have tstring, so we could consider a similar approach as a temporary solution.

@yebblies
Copy link
Member

Why are we building a 32-bit OSX binary at all?

@jacob-carlborg
Copy link
Contributor

jacob-carlborg commented May 18, 2016

Why are we building a 32-bit OSX binary at all?

@yebblies I know Guillaume Piolat, @p0nce, is still using 32bit to build plugins for audio applications on OS X. What I understand is that users of these audio applications have loads of, I assume old, plugins compiled for 32bit. If they are old and not maintained they might not be able to get a 64bit version of the plugins and therefore cannot move to a 64bit version of the audio application.

The iOS simulator doesn't emulate ARM, instead it runs x86 binaries. Even though there are iOS devices which are 64bit, the Apple Watch is still 32bit, as far as I understand. Although one might argue that one would use LDC to target iOS devices including the simulator.

@yebblies
Copy link
Member

I'm not suggesting dropping OSX32 as a target, just as a host.

@WalterBright
Copy link
Member Author

We already have officially dropped OSX32 as a host. But I find it convenient to build it as a way of finding portability issues, and having the compiler be able to build itself is part of testing the OSX32 support.

@jacob-carlborg
Copy link
Contributor

I'm not suggesting dropping OSX32 as a target, just as a host.

Aha, you mean building the compiler as a 32bit binary. No, there's really no point in doing that when you can cross-compile.

@MartinNowak
Copy link
Member

Why are we building a 32-bit OSX binary at all?

Good question, it's due to the quirkiness of our build script, which was originally written to allow as many configuration options as possible, leading to lots of complicated edge cases.
https://github.com/dlang/installer/blob/77f4523692c50b75f794f198dbdb43c573cf0e61/create_dmd_release/create_dmd_release.d#L379

@yebblies
Copy link
Member

But I find it convenient to build it as a way of finding portability issues

Most of the issues it finds seem to be exactly the issues we dropped it for. My motivation was avoiding the ugly hacks and special cases that we're now adding. If we're going to actively maintain it anyway then we might as well have it as an official host platform.

and having the compiler be able to build itself is part of testing the OSX32 support.

It doesn't have to be, and it isn't on the autotester. It's a weird kind of limbo in between fully supported and unsupported at the moment - but I guess so are solaris and win64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants