Skip over return parameters for return destination

[dkorpel]

Extension of #8504 in an attempt to make core.lifetime: move compatible with -dip1000.
Additional context: DIP1000: The return of 'Extend Return Scope Semantics'

The idea is: instead of only considering the first parameter as the return destination, skip over parameters already determined to be return. After all, in the signature of move:

void move(T)(ref return scope T source, ref scope T destination);

Making return parameter source its own return destination accomplishes nothing.
This could be further extended to skip over parameters without pointer types, e.g.:

void move(T)(ref return scope T p, int intermediate, ref scope T r);

But it deliberately does not do that to keep it a bit simpler.

@WalterBright @atilaneves

[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

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

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

[dkorpel]

Ping @WalterBright @atilaneves, it's a language change, so this needs approval from you. I'll add a changelog entry and spec PR if this gets the green light. If you want more information or time to ponder, please say so. Currently I'm not sure whether you're busy or simply overlooked this.

[atilaneves]

Let me confer with @WalterBright

[maxhaton]

I think we should be careful with changes in this direction - this seems better than what we have now, but (although they seem to be non grata in D) I prefer the redundancy of a proper annotation.

[dkorpel]

To be clear, the short term goal is to fix Issue 20150 - -dip1000 defeated by pure, which is blocked by Phobos and Druntime relying on the accepts-invalid bug to compile, so I've been working on removing the violations. Awaiting dlang/phobos#8123, this is the current list of remaining functions:

Druntime build: ✔️

Druntime unittest build:

src/core/internal/dassert.d(166): Warning: parameter `val` of `atomicLoad` is cheating `scope`
src/core/lifetime.d(1881): Warning: parameter `source` of `moveImpl` is cheating `scope`

Phobos build:

std/path.d(1495): Warning: parameter `segments` of `buildPath` is cheating `scope`
std/file.d(3058): Warning: parameter `path` of `dirName` is cheating `scope`
std/algorithm/mutation.d(1192): Warning: parameter `source` of `moveImpl` is cheating `scope`
std/path.d(2758): Warning: parameter `r` of `array` is cheating `scope`
std/internal/cstring.d(303): Warning: parameter `ptr` of `enforceRealloc` is cheating `scope`
std/algorithm/mutation.d(1192): Warning: parameter `source` of `moveImpl` is cheating `scope`
std/uni/package.d(1836): Warning: parameter `ptr` of `enforceRealloc` is cheating `scope`
std/algorithm/mutation.d(1192): Warning: parameter `source` of `moveImpl` is cheating `scope`

Phobos unittest build: TODO

As you can see, moveImpl is one of them for both Druntime and Phobos (they share the same code). How can we move forward?

1. let move be @system and remove @safe from the unittests
2. deprecate the current move, introduce a new move2 with swapped parameters
3. make a special case exception in the compiler for move
4. accept this PR
5. wait until a 'proper', long-term solution is implemented covering everyone's needs
6. ??? (insert your brilliant idea here!)

I think we should be careful with changes in this direction ... I prefer the redundancy of a proper annotation.

Me too, but the issue is long overdue for fixing. Considering what @atilaneves says about dip1000 bugs: #12578 (comment)

We need to fix all of them, post-haste

I think options 3 and 4 (or 6?) are the most viable, but I need an executive decision.

[WalterBright]

This needs an enhancement request bugzilla entry or a changelog.

[WalterBright]

My evaluation of the situation:

https://digitalmars.com/d/archives/digitalmars/D/DIP1000_The_return_of_Extend_Return_Scope_Semantics_350531.html#N350939