Fix issue 21868 - conflation of return-ref and return-scope

[dkorpel]

Attempts to fix 21868 and more. The problem is that escape.d checks for return, infers return and suggests to add return without ever checking if it's actually the right return, e.g. Return-Ref vs. Return-Scope. In the bug report it happens from value->ref with &var, but it also goes wrong with ref->value with *var.

I added a test for all combinations of ref/non-ref return, and return/scope/ref parameters. I still have to check whether this works with constructors (which return by ref but not really or something), out, ref in foreach and auto ref.

Note: it still breaks if you assign the parameter to a local variable, but that's a separate issue (20245)

Since this is an accepts-invalid bug, it might break Druntime and Phobos again, we'll see.

[dlang-bot]

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	21868	normal	DIP1000 doesn't catch pointer to struct temporary

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#12665"

[dkorpel]

The failing tests say "Exit status: 2" but they don't say "Error:" anywhere. Why are they failing?

[MoonlightSentinel]

The failing tests say "Exit status: 2" but they don't say "Error:" anywhere. Why are they failing?

There is a compilation failure in the druntime tests. This probably occurs because your branch is not up-to-date and missing the recent CTFE improvements. The failure should vanish with a rebase (and become impossible after #12667 )

[dkorpel]

There is a compilation failure in the druntime tests.

Thank you.

Meanwhile, buildkite found:
https://github.com/atilaneves/automem/blob/8f61747f437b7624ad4b44085e9525fc143a9e6c/source/automem/vector.d#L209

struct Vector(E, Allocator = typeof(theAllocator)) if(isAllocator!Allocator) {
...
    E[] _elements;
...
    /// Access the ith element. Can throw RangeError.
    ref inout(E) opIndex(long i) scope return inout {
        if(i < 0 || i >= length)
            mixin(throwBoundsException);
        return _elements[i.toSizeT];
    }
...

source/automem/vector.d(212,25): Error: scope parameter `this` may not be returned
source/automem/vector.d(212,25):        note that `return` applies to `ref`, not the value

And:
https://github.com/symmetryinvestments/excel-d/blob/47173d1e83565550e79d6cb71cfb78175d9b4c4a/source/xlld/sdk/framework.d#L125

source/xlld/sdk/framework.d(125,68): Error: returning `&a` escapes a reference to parameter `a`
source/xlld/sdk/framework.d(125,68):        note that `return` applies to the value of `a`, not its address

    foreach(i, ref arg; args)
                ptrArgs[i] = (return scope ref const(XLOPER12) a) { return &a; }(arg);

Which are legit errors. The latter one doesn't look hard to fix since it's entirely internal, but the former one is a public api.
@atilaneves the current language doesn't allow a member function to return by ref with return-scope semantics instead of return-ref semantics. Do you have any ideas to fix this?

[WalterBright]

Attempts to fix 21868 and more.

This PR attempts to do too much. The bug fix should be its own PR, not mixed in with error message improvements.

[dkorpel]

This PR attempts to do too much. The bug fix should be its own PR, not mixed in with error message improvements.

True, note that this is still a draft. The idea is to first find the root of the problem and provide a full solution with thorough tests, and then split it up if it's too complex. The current thing blocking progress on this PR is atilaneves/automem#64

[dkorpel]

Superseded by #12829 and #12817