Allow _d_arrayappendT to call impure functions

[teodutu]

The compiler will lower a ~= b to an expression containing a call to _d_arrayappendT. The hook calls copyEmplace, which might be impure. This would have made _d_arrayappendT impure as well.

As a ~= b is allowed in pure contexts, _d_arrayappendT needs to be pure as well. However, the current stop-gap implementations of _d_arrayappend{cTX,T} use mixins and still call the old _d_arrayappendcTX hook, which sometimes falsely make them impure. This is why the hook itself has to be declared pure.

But when _d_arrayappendT has to call an impure postblit or copy ctor, for example, the impurity of the latter conflicts with the hooks purity. For this reason, this PR fakes the purity of _d_arrayappendT while allowing the functions it calls be impure. to be pure regardless of the purity of copyEmplace, by an indirect call and a forced cast. This is required by dlang/dmd#13495.

[dlang-bot]

Thanks for your pull request and interest in making D better, @teodutu! 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 + druntime#3662"

[dkorpel]

Does this mean that currently a pure function can append an object which has an impure copy constructor to an array? Because that sounds like a bug.

[ibuclaw]

Casting away impurity is the wrong approach.

[teodutu]

Does this mean that currently a pure function can append an object which has an impure copy constructor to an array? Because that sounds like a bug.

No. The compiler still checks the purity of a ~= b. This PR only affects the lowering. Previously, the same lowering was performed by e2ir.d, so it skipped semantic analysis.

[dkorpel]

This PR only affects the lowering.

Then why does it need to fake purity, does copyEmplace fail to infer pure when it should?

[teodutu]

@ibuclaw @dkorpel, I've updated the PR's description in the hope of better clarifying why it's needed. I expressed myself poorly that it's about copyEmplace. It's about the fact that _d_arrayappendT has to be declared pure, which conflicts with the fact that sometimes copyEmplace rightly is impure.

Either way, this is a hack for a stop-gap solution, as the current templated hook still ends up calling the old _d_arrayappendcTX. Both _d_arraycTX and _d_arrayappend will be rewritten in the future and made independent of the old hook.

[RazvanN7]

@ibuclaw @dkorpel It all boils down to the fact that we want to allow a ~= b in pure contexts. The old hook is impure, however, because the lowering is performed in e2ir, purity checks would have been bypassed. Right now, we are transitioning to templates and the call is performed during semantic, therefore we cannot lie anymore. The cleanest solution would be to make copyEmplace pure, however, that is the topic of a different PR. Right now, we want to get the hook in and then we can look at cleaning it up. I, repeat, we are not changing anything now, but are using as @teodutu mentioned a stop gap solution.

[schveiguy]

If current code is using an invalid assumption by the compiler, it's best to fix that sooner rather than later.

If we are going to allow purity violations, we must do so in a way that alerts the user of the error, and then we will deprecate and remove it.

Unfortunately, it looks like you can't overload a function solely based on purity (which would make this easy). In this case, the compiler needs to help it by calling different hooks based on the context. Is this possible?

Specifically, we need to print a deprecation message (along with where the function is called) in the case where:

1. The context requires a pure function call
2. The append operation would not be inferred pure

[ibuclaw]

Unfortunately, it looks like you can't overload a function solely based on purity (which would make this easy). In this case, the compiler needs to help it by calling different hooks based on the context. Is this possible?

Yes, it is possible for the compiler to call a different hook based on the purity of the current function context. I agree, that a pure implementation should be done instead that lives alongside the old impure one. Having a grep through the rest of core.lifetime, it's unfortunate that this style of subverting purity has made it's way into other hooks that have been "templatized".

[RazvanN7]

@teodutu is this still needed?

[teodutu]

@RazvanN7 Most likely not. _d_arrayappendT is working fine now and giving up on the template wrapper _d_arrayappendTImpl (#3805) has also eliminated any purity-related problems.