Help compiler deduce pure for moveEmplaceImpl

[JohanEngelen]

This fixes https://issues.dlang.org/show_bug.cgi?id=21202 .

This puts no extra requirements on moveEmplaceImpl. moveEmplaceImpl already has to be pure because moveEmplaceImpl is called by moveEmplace which is explicitly marked pure.

[dlang-bot]

Thanks for your pull request and interest in making D better, @JohanEngelen! 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
✓	21202	regression	[REG2.093] std.algorithm.mutation.moveEmplace cannot deduce purity and errors

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

To target stable perform these two steps:

1. Rebase your branch to upstream/stable:

git rebase --onto upstream/stable upstream/master

2. Change the base branch of your PR to stable

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 + phobos#7670"

[MoonlightSentinel]

This puts no extra requirements on moveEmplaceImpl. moveEmplaceImpl already has to be pure because moveEmplaceImpl is called by moveEmplace which is explicitly marked pure.

But breaks move which doesn't have this requirement and calls user-defined opAssign/opPostMove

[kinke]

I'm shocked to see this stuff is still in std.algorithm.mutation - hasn't this all been moved to core.lifetime?!

[wilzbach]

Yes, but we couldn't remove this as there was one compile-time check that pulled in the world (about 2k of additional templates to druntime)

[wilzbach]

Details: #6848

[edi33416]

Can you please add attributes for the unittest block?

[edi33416]

What stops the compiler from inferring the pureness of moveEmplaceImpl?

[edi33416]

But breaks move which doesn't have this requirement and calls user-defined opAssign/opPostMove

What if you put the function body in a pure lambda? Then the compiler should be able to infer the purity and the change won’t break move

[RazvanN7]

@JohanEngelen Why is this necessary? The compiler should be able to deduce the purity of that function.

[JohanEngelen]

@JohanEngelen Why is this necessary? The compiler should be able to deduce the purity of that function.

See https://issues.dlang.org/show_bug.cgi?id=21202

[JohanEngelen]

(based on some of the comments, I am no longer convinced this is the correct fix, but the issue still stands)

[RazvanN7]

@JohanEngelen It is bad practice to force an attribute on a template (especially one that potentially calls user-defined functions). This is more likely a compiler bug.

I'm going to close this PR and investigate this to get to the bottom of it.

I will close this PR as this is most likely not fixing the root cause. Please reopen if you have anything else to add.

[JohanEngelen]

@RazvanN7 Thanks for looking into fixing the compiler. This workaround is still needed at Weka.

[RazvanN7]

@JohanEngelen I have looked into it and it looks like a compiler bug: https://issues.dlang.org/show_bug.cgi?id=21850 .