fix Issue 5305 - intrinsic functions have @safe stripped of them in r…

[WalterBright]

…elease mode

Seems to work, adding a test case to ensure it.

[dlang-bot]

Fix	Bugzilla	Description
✓	5305	Cannot take pointer to intrinsic function

[andralex]

Auto-merge toggled on

[JackStouffer]

I seem to recall @yebblies was adamant that Phobos shouldn't be imported in DMD tests

[yebblies]

Yes.

[yebblies]

Auto-merge toggled off

[WalterBright]

This is a compiler test, not a library test, and as such a test for it properly belongs in the compiler suite.

[mathias-lang-sociomantic]

Shouldn't // REQUIRE_ARGS: -release be used here ?

[WalterBright]

I don't see why.

[mathias-lang-sociomantic]

Given std.math just forward to core.math, couldn't you provide a module that does the same ?

[WalterBright]

The intrinsic is directly associated, by the compiler, to std.math. Not something else. That's what needs to be tested.

@yebblies is generally right, because when porting dmd to a new platform it is very convenient to get it to pass the compiler tests before attempting to compile/debug Phobos. This is why this particular test is in its own file - it can be easily skipped when porting. This case is an exception.

[mathias-lang-sociomantic]

The issue title is confusing. I read it again and I'm going to close the issue as duplicate of 4541.

For reference:
You mentioned that the issue was actually taking a pointer to an intrinsic, however the title was left unchanged. Out of the 2 solutions you mentioned, one of them was partially implemented by @yebblies in dlang/phobos#3599 (with tests). Now the underlying issue is still present - taking the pointer of an intrinsic doesn't work -. It's just that the test code has shifted to:

import core.math;
void map(real function(real) f) { }
void main() { map(&sqrt); }

So I suggest to close this P.R. as well.

[WalterBright]

The test case is to verify that taking the address of sqrt in std.math works. This test case is valid, and should be pulled to resolve the issue. 4541 is something else.

[mathias-lang-sociomantic]

@WalterBright : The intrinsics are defined in core.math.

In DMD we have a duplicate definition, one for core.math and one for std.math. The duplication was introduced in #20 to allow for this very change, but was obviously forgotten for a couple of years.

Thanks to the Phobos P.R. I mentioned earlier, a symbol with the same mangling as the std.math intrinsic is now present in Phobos.
Just nm over your libphobos:

nm /usr/lib/x86_64-linux-gnu/libphobos2.a | grep D3std4math3sinFNaNbNiNfeZe
0000000000000000 T _D3std4math3sinFNaNbNiNfeZe

[andralex]

Yah, it's reasonable to import std here - it's the sync between the compiler and std that's being tested (rare).

[andralex]

Auto-merge toggled on

[mathias-lang-sociomantic]

Yah, it's reasonable to import std here - it's the sync between the compiler and std that's being tested (rare).

Did you read the previous comments ?

[yebblies]

What the hell Andrei?

This is not a compiler issue - it is a phobos issue. Phobos did not provide a body for the function, so its address couldn't be taken. The argument that this should be in the dmd test suite and import phobos is a load of garbage.

[WalterBright]

Phobos and the compiler need to work together to make this work.

[yebblies]

You should have a look at what I changed to make this work. Nothing to do with the compiler's handling of intrinsics.

[WalterBright]

The point of this test is to ensure that the fix, whatever it was, remains working.

[yebblies]

It is testing that a phobos function can be used in a certain way - the test belongs in phobos. From the compiler's perspective this is just a normal function.