Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Comments

fix Issue 17829 - core.stdc.errno does not work with -betterC#1917

Merged
WalterBright merged 1 commit intodlang:masterfrom
WalterBright:fix17829
Sep 29, 2017
Merged

fix Issue 17829 - core.stdc.errno does not work with -betterC#1917
WalterBright merged 1 commit intodlang:masterfrom
WalterBright:fix17829

Conversation

@WalterBright
Copy link
Member

This avoids needing druntime by connecting directly to the C implementation of errno. The C function actually returns an int*, but we call it ref int so it automagically dereferences when called.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close Bugzilla Description
17829 core.stdc.errno does not work with -betterC

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Sep 16, 2017
@WalterBright WalterBright force-pushed the fix17829 branch 3 times, most recently from 8fca210 to 323f8fb Compare September 16, 2017 00:39
@WalterBright
Copy link
Member Author

On Linux, this is failing because of:

../src/dmd -conf= -m64 -Irunnable  -inline -odgenerated/runnable -ofgenerated/runnable/test15_2 runnable/test15.d
runnable/test15.d(207): Deprecation: instead of C-style syntax, use D-style syntax `int delegate(in int arg1)[char[]] List1`
runnable/test15.d(208): Deprecation: instead of C-style syntax, use D-style syntax `int[char[]] List2`
runnable/test15.d(1045): Deprecation: instead of C-style syntax, use D-style syntax `float[][] mat`
runnable/test15.d(1047): Deprecation: instead of C-style syntax, use D-style syntax `float[][] newmat`
generated/runnable/test15_2.o: In function `_D3std8typecons__T10RefCountedTSQBe5stdio4File__T6ByLineTaTaZQm4ImplVEQCqQCp24RefCountedAutoInitializei0ZQDj15RefCountedStore4moveMFNaNbNiKQEeZv':
runnable/test15.d:(.text._D3std8typecons__T10RefCountedTSQBe5stdio4File__T6ByLineTaTaZQm4ImplVEQCqQCp24RefCountedAutoInitializei0ZQDj15RefCountedStore4moveMFNaNbNiKQEeZv[_D3std8typecons__T10RefCountedTSQBe5stdio4File__T6ByLineTaTaZQm4ImplVEQCqQCp24RefCountedAutoInitializei0ZQDj15RefCountedStore4moveMFNaNbNiKQEeZv]+0x1a): undefined reference to `getErrno'
runnable/test15.d:(.text._D3std8typecons__T10RefCountedTSQBe5stdio4File__T6ByLineTaTaZQm4ImplVEQCqQCp24RefCountedAutoInitializei0ZQDj15RefCountedStore4moveMFNaNbNiKQEeZv[_D3std8typecons__T10RefCountedTSQBe5stdio4File__T6ByLineTaTaZQm4ImplVEQCqQCp24RefCountedAutoInitializei0ZQDj15RefCountedStore4moveMFNaNbNiKQEeZv]+0x3d): undefined reference to `setErrno'

i.e. it is picking up a reference to getErrno and setErrno. But grepping through druntime and phobos, I cannot find any reference to those names. Could it be there's some other core.stdc.errno being referenced by the autotester?

@ibuclaw
Copy link
Member

ibuclaw commented Sep 18, 2017

You're still compiling in the errno.c file which has these functions. So must be something else.

@WalterBright WalterBright force-pushed the fix17829 branch 4 times, most recently from f271a27 to ca3b2f0 Compare September 28, 2017 00:55
@WalterBright
Copy link
Member Author

WalterBright commented Sep 28, 2017

Also fixed the use of errno functions in core.memory, which were seriously confused.

@WalterBright
Copy link
Member Author

@ibuclaw got the mysterious linux errors fixed.

else
{
static assert(0, "add system errno function");
}
Copy link
Member

@rainers rainers Sep 28, 2017

Choose a reason for hiding this comment

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

You can avoid repeating all the platform details by wrapping it in a function (similar to pureReprintReal in core.demangle):

extern(C) private {
    pure @trusted @nogc nothrow pragma(mangle, "fakePureErrno") ref int pureErrno();
    ref int fakePureErrno() { return errno(); }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea.

@schveiguy
Copy link
Member

A question: I like the concept of changing the int * return to a ref int, but is that officially supported? I've used something similar in the past, with pipe, which takes an array of 2 ints in C, we prototyped it as ref int[2]. People complained that this might not be supported, since C does not support ref. Looking at the ABI page, I see that ref is simply passed the same as a pointer. It would be good to specify somewhere on that page that extern(C) functions that take or return ref are equivalent to C functions that take or return pointers of that type.

@WalterBright
Copy link
Member Author

I like the concept of changing the int * return to a ref int, but is that officially supported?

It's a bit hackish, and I'm a bit proud of myself for stumbling onto that as a solution to the errno problem. The ref is just syntactic sugar over *, and is true for C++ as well as D. I hesitate to say it is officially supported, but I cannot see any reason it would change. It's not something I'd suggest people start doing generally. The errno thing is a special case hack, even in C, and so I think a D hack to support it is in the spirit of things :-)

I also do not see any more convenient solution to errno, as you can see, there is no consistency whatsoever in how C compilers handle it, and betterC has to adapt to that.

@WalterBright
Copy link
Member Author

The ci/circleci failure appears to be spurious and unrelated.

@wilzbach
Copy link
Contributor

The ci/circleci failure appears to be spurious

Sadly that's a common problem for many CIs in the last months. The underlying cause is that the install script tries to fetch the dub binary from code.dlang.org, but fails due to its bad reliability/unavailability. However, since DUB is part of the official release since 2.072, this point of failure could easily be removed:

dlang/installer#218

@WalterBright WalterBright merged commit 7326110 into dlang:master Sep 29, 2017
@WalterBright WalterBright deleted the fix17829 branch September 29, 2017 20:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Bug Fix Include reference to corresponding bugzilla issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants