fix Issue 23682 - dip1000 problem with return by ref

[WalterBright]

No description provided.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	23682	normal	dip1000 problem with return by ref

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14869"

[WalterBright]

I'm not quite sure this fix is right, so I'll look into it further.

[WalterBright]

@RazvanN7 it's failing compiling druntime because that gets compiled before the tests/compilable test suite is run. So now I'm doing another reduction job through a thicket of templates. Sigh.

[WalterBright]

@RazvanN7 I fixed the problem compiling druntime. But the very next thing it does is try to compile Phobos. And then it fails, and I'm borked again by the order in which the test suite is run.

Every single check attempts to compile Phobos before running any tests.

[WalterBright]

This is failing in std.uni, probably the most template heavy piece of code. I can't make much sense of it. It uses auto so much it's really hard to figure out what types are being used. Anyway, being the pragmatic sort, I'm going to put this fix behind a -preview switch. It's likely to break user code, too, that relied on the old behavior. Hopefully this will get things through the test suite.

[WalterBright]

oops pushed the wrong button

[ibuclaw]

Every single check attempts to compile Phobos before running any tests.

You can't run any runnable tests without a runtime library to link. You can't run any compilable LINK tests without a runtime library either (though they if any should be standalone programs).

[WalterBright]

It should link with the previous version.

[dkorpel]

This is the wrong fix.

Fixing a rejects-valid bug shouldn't break druntime and Phobos.

The function returning by ref doesn't mean return scope parameters return a reference to the input variable, that would always be a dangling stack pointer.

The real issue is that assigning a ref to a non-ref implicitly dereferences a pointer, but this only manifests in the glue layer, so it's invisible to escape.d.

Without redesigning dmd so the dereferencing of ref values appears in the front-end AST, here's a possible fix:
#14871

[WalterBright]

****** FAIL release64 std.random
core.exception.AssertError@std/random.d(3957): Assertion failure

A random failure in std.random. I wish somehow we could create repeatable tests.

fixed the problems

[WalterBright]

Now this new failure on Mac OS:

core.exception.AssertError@hospital.d(35): Assertion failure

what's going on here?

[WalterBright]

And our old friend, the socket failure, reappears:

core.exception.AssertError@std/socket.d(805): unittest failure

[WalterBright]

dlang/phobos#8681

[WalterBright]

trying again to see if we avoid heisenbugs

[WalterBright]

And the heisenbugs go away! Ready to merge.

[WalterBright]

@dkorpel ping

[dkorpel]

Once again, this fix is overfitted to the single example in the one bugzilla issue, but I can fix this in my PR to solve the issue proper.

[dkorpel]

This line is pointless

[dkorpel]

Ref_ReturnScope should not be treated different to ReturnScope

[WalterBright]

they are different because a ReturnScope can be returned as a Ref

[WalterBright]

The trouble is a ref return from a struct constructor is different from a regular ref return. It's an inconsistency in the language, but I don't think there's much we can do about it.