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 d compiler abstractions #6356

Merged
merged 17 commits into from Mar 11, 2020

Conversation

dcbaker
Copy link
Member

@dcbaker dcbaker commented Dec 16, 2019

This should fix the D compilers in a number of cases. It removes the code to treat D like it's own linker (in reality LDC and DMD do something much more annoying, they invoke gcc/clang behind our backs). I've also fixed a few other MacOS related bugs.

I've tested gdc, ldc, and dmd on Linux, and dmd and ldc on macos and windows, and there's no regressions in the test coverage from master. I've fixed all of the tests on macOS, and added ldc to the macOS image in travis so we can get some test coverage there. I've also added a couple of tests to help regressions.

Fixes: #6075
Fixes: #6359

@dcbaker
Copy link
Member Author

dcbaker commented Dec 16, 2019

@michaelbadcrumble, @GoaLitiuM, Could one or both of you test this for me and see if it works in real use cases, not just our rather limited test case coverage (assuming the CI all comes back green)?

@dreamer-coding-555
Copy link
Contributor

dreamer-coding-555 commented Dec 16, 2019

Tested the fix on a dummy hello world program and does not fail to compiler the project, also compiled a shared and static library. So far it is doing good, but there are object files like tmpq7u7cra2.o and tmpng4k3r4y.o are still being generated in the project root directory.

@jpakkane
Copy link
Member

That probably needs a fix similar to this one here.

@jpakkane
Copy link
Member

FAILED: test-urld.exe 
link @test-urld.exe.rsp
Microsoft (R) Incremental Linker Version 14.16.27034.0
Copyright (C) Microsoft Corporation.  All rights reserved.

-of=test-urld.exe test-urld@exe/test.d.obj "/nologo" "-m64" "/DEBUG" "/PDB:test-urld.pdb" "-L=phobos64.lib" "-L=C:\\Users\\VssAdministrator\\AppData\\Local\\dub\\packages\\urld-2.1.1\\urld\\.dub\\build\\library-debug-windows-x86_64-dmd_2089-F2DBBF418391CF118A711870EACD2E3A\\urld.lib" 
LINK : warning LNK4044: unrecognized option '/of=test-urld.exe'; ignored
LINK : warning LNK4044: unrecognized option '/m64'; ignored
LINK : warning LNK4044: unrecognized option '/L=phobos64.lib'; ignored
LINK : warning LNK4044: unrecognized option '/L=C:\\Users\\VssAdministrator\\AppData\\Local\\dub\\packages\\urld-2.1.1\\urld\\.dub\\build\\library-debug-windows-x86_64-dmd_2089-F2DBBF418391CF118A711870EACD2E3A\\urld.lib'; ignored
LINK : fatal error LNK1104: cannot open file 'phobos64.lib'

@dcbaker
Copy link
Member Author

dcbaker commented Dec 19, 2019

Looks like we need to set up link.exe to be invoked by ldc/dmd instead of invoke directly. I don't have my windows machine today, so I'll look at it it tomorrow and update the MR then.

@dreamer-coding-555
Copy link
Contributor

dreamer-coding-555 commented Dec 22, 2019

Any new development regarding the d compiler abstractions @dcbaker ?

@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from e445403 to 2585068 Compare March 6, 2020 17:26
@dcbaker dcbaker requested a review from jpakkane as a code owner March 6, 2020 17:26
@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from dd81686 to 972dfb0 Compare March 6, 2020 19:30
This was never really true of the D compilers, it made them more
complicated than necessary and was incorrect in many cases. Removing it
causes no regressions on Linux, at least in our rather limited test
cases).
DMD and LDC are a real pain to use as linkers. On Unices they invoke the C
compiler as the linker, just like meson does. This means we have to figure out
what C compiler they're using and try to pass valid arguments to that compiler
if the D compiler doesn't understand the linker arguments we want to pass. In
this case that means gcc or clang. We can use-the -Xcc to pass arguments
directly to the C compiler without dmd/ldc getting involved, so we'll use that.
This allows fixing tests that produce .dylib's on macOS and .so's on elf
Unices.
This breaks LDC and DMD, so just don't do it.
@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from 972dfb0 to 432135d Compare March 6, 2020 20:11
@dcbaker
Copy link
Member Author

dcbaker commented Mar 7, 2020

I've rebased this and fixed a large number of issues. There are still some I haven't fixed on windows. I've run out of time for this week though, so I'll come back and look at it more next week. Unless someone wants to point out the obvious reason it isn't working :)

@lgtm-com
Copy link

lgtm-com bot commented Mar 7, 2020

This pull request introduces 1 alert and fixes 1 when merging 5fb18d0 into 6ac4053 - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes

fixed alerts:

  • 1 for Conflicting attributes in base classes

@dcbaker dcbaker added this to the 0.54.0 milestone Mar 9, 2020
This allows the harness to apply the version correctly, putting it in the right
place, dropping the right amount of numbers, etc.

pdb taking a version allows it to be more easily copied from the
shared_lib type.
So we have some dlang testing on macos.
@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from 5fb18d0 to 94ab8b5 Compare March 9, 2020 20:19
@lgtm-com
Copy link

lgtm-com bot commented Mar 9, 2020

This pull request introduces 1 alert and fixes 1 when merging 94ab8b5 into 912d7b7 - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes

fixed alerts:

  • 1 for Conflicting attributes in base classes

DMD is awful in a lot of ways. To change the linker you set an
environment variable, which is pretty much impossible for us.
This is the argument to name the implib when using the Visual Studio
Linker. This is needed by LDC and DMD when using link.exe or
lld-link.exe on windows, and is really a linker argument not a compiler
argument.
It was impossible to figure out what was coming from where before.
This is needed when mixing D and C code, as it's possible to end up
witha  combination of linkers and compilres such that C produces pdb
files but D does not.
@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from 94ab8b5 to 7470617 Compare March 9, 2020 23:55
@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2020

This pull request introduces 1 alert and fixes 1 when merging 7470617 into 912d7b7 - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes

fixed alerts:

  • 1 for Conflicting attributes in base classes

@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2020

This pull request introduces 1 alert and fixes 1 when merging b55d0d3 into 1bd1f98 - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes

fixed alerts:

  • 1 for Conflicting attributes in base classes

@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from 62c2fe4 to 9f59d4a Compare March 10, 2020 17:25
@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2020

This pull request introduces 1 alert and fixes 1 when merging 9f59d4a into 1bd1f98 - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes

fixed alerts:

  • 1 for Conflicting attributes in base classes

@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from 9f59d4a to fba6eeb Compare March 10, 2020 21:12
@dcbaker
Copy link
Member Author

dcbaker commented Mar 10, 2020

I don't have my macbook today, so I'm just kinda shooting blind at fixing the problems on macos.

@lgtm-com
Copy link

lgtm-com bot commented Mar 10, 2020

This pull request introduces 1 alert and fixes 1 when merging fba6eeb into 1bd1f98 - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes

fixed alerts:

  • 1 for Conflicting attributes in base classes

Some compilers that act as linker drivers (dmd and ldc) need to split
arguments that GCC combines with , (ie, -Wl,-foo,bar -> -L=-foo -L=bar).
As such we need to detect that the previous argument contained -soname,
and not wrap that in a --start-group/--end-group

This modifies the shared library test to demonstrate the problem, with a
test case.

Fixes mesonbuild#6359
@dcbaker dcbaker force-pushed the fix-d-compiler-abstractions branch from fba6eeb to d017243 Compare March 11, 2020 16:27
@dcbaker
Copy link
Member Author

dcbaker commented Mar 11, 2020

I now have my macbook and I think I finally have it all working (assuming I didn't break Linux or windows)

@lgtm-com
Copy link

lgtm-com bot commented Mar 11, 2020

This pull request introduces 1 alert and fixes 1 when merging d017243 into 69e9d32 - view on LGTM.com

new alerts:

  • 1 for Conflicting attributes in base classes

fixed alerts:

  • 1 for Conflicting attributes in base classes

@dcbaker
Copy link
Member Author

dcbaker commented Mar 11, 2020

Finally, I think this is good to go! This should have ldc and dmd working on windows, Linux, and mac, plus gdc working on linux.

@jpakkane
Copy link
Member

Great work. This looks like it was all different kinds of terrible.

@jpakkane jpakkane merged commit 88e40c7 into mesonbuild:master Mar 11, 2020
@dcbaker dcbaker deleted the fix-d-compiler-abstractions branch March 11, 2020 23:34
@dcbaker
Copy link
Member Author

dcbaker commented Mar 11, 2020

It wasn't too bad, apart from dmd. That is an infuriating compiler to deal with. And the fact that I'd buggered it up pretty bad and we weren't testing on macos so it had all kinds of broken. We should probably add a test for dmd on macos and gdc on linux. Not sure if travis is the right place for that or if we should use github actions?

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