Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fixes #15434: object.d must not import from rt #1450

Merged
merged 1 commit into from
Dec 27, 2015
Merged

Conversation

yazd
Copy link
Contributor

@yazd yazd commented Dec 11, 2015

@yazd
Copy link
Contributor Author

yazd commented Dec 11, 2015

The auto-tester fails for some reason.. uhmm. I cannot reproduce the failure on either a 32-bit or a 64-bit linux.

@schveiguy
Copy link
Member

Seems it was a spurious failure, passed now.

@schveiguy
Copy link
Member

I've not seen externDFunc before, interesting! Is that really the only way to declare an external d function? Almost all other runtime extern functions are hooked via C linkage. I'm not sure which way is better.

@yazd
Copy link
Contributor Author

yazd commented Dec 11, 2015

It is used several times in this file to link to rt functions.

@schveiguy
Copy link
Member

OK, then I guess I just haven't read through it in a while :)
LGTM

@PetarKirov
Copy link
Member

@yazd thanks for fixing this!

Btw I wonder how it worked in debug (no inline) mode.

@schveiguy this is a regression from DMD 2.067 (when the code used to work), so I hope this PR will be merged in time for the next release.

@yazd
Copy link
Contributor Author

yazd commented Dec 18, 2015

@ZombineDev 👍
I think it works with no inline because the compiler only uses the declaration and links directly to the compiled druntime (it doesn't recompile druntime). I might be wrong.

Is there anything else to address to get this pulled?
Thanks.

MartinNowak added a commit that referenced this pull request Dec 27, 2015
Fixes #15434: object.d must not import from rt
@MartinNowak MartinNowak merged commit 2a36f9e into dlang:master Dec 27, 2015
tramker pushed a commit to tramker/druntime that referenced this pull request Jan 27, 2016
[REG2.068] Fix Issue 15434: object.d must not import from rt

Signed-off-by: Martin Krejcirik <mk@krej.cz>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants