Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.

Make _d_arrayctor weakly pure#3587

Merged
dlang-bot merged 4 commits intodlang:masterfrom
teodutu:make-_d_arrayctor-weakly-pure
Dec 3, 2021
Merged

Make _d_arrayctor weakly pure#3587
dlang-bot merged 4 commits intodlang:masterfrom
teodutu:make-_d_arrayctor-weakly-pure

Conversation

@teodutu
Copy link
Copy Markdown
Member

@teodutu teodutu commented Oct 12, 2021

This PR changes _d_arrayctor from a pure function to a weakly pure function.
This change is required because the removal of the _d_arrayctor hook implemented in this PR: dlang/dmd#13116, lowers constructions such as the one below to calls to _d_arrayctor:

struct S {};
const S[2] b;
const S[2] a = b;

The above code is lowered to _d_arrayctor(a, b). For const and immutable arguments, the function is strongly pure, and since its return value is ignored, the compiler might choose to remove the call altogether.

In order to avoid this behaviour, _d_arrayctor has to be made weakly pure, which is what the added 3rd parameter does.

@dlang-bot
Copy link
Copy Markdown
Contributor

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

@teodutu teodutu force-pushed the make-_d_arrayctor-weakly-pure branch from 1e9b037 to 2eaed68 Compare October 13, 2021 13:59
@PetarKirov
Copy link
Copy Markdown
Member

@teodutu @RazvanN7 have you considered casting away const/immutable of the argument to inside the frontend, before this function is called?

void _d_arrayctor(T)(return scope T[] to, scope const(T)[] from);

struct S {};
const S[2] b;

const S[2] a = b; // gets lowered to:

const S[2] a = void;
_d_arrayctor!S(cast(S[]) a, b);

Essentially, T should only be instantiated with mutable types.

Also, why does _d_arrayctor need to return anything but unit (void), given that the result written to to?

@teodutu
Copy link
Copy Markdown
Member Author

teodutu commented Oct 14, 2021

@teodutu @RazvanN7 have you considered casting away const/immutable of the argument to inside the frontend, before this function is called?

Not yet, but this might work. Thanks!

Also, why does _d_arrayctor need to return anything but unit (void), given that the result written to to?

This is necessary for constructors where you might do something like this:

struct S
{
    int[3] a, b;
    this (ref int[3] c)
    {
        a = b = c;
        // this would be lowered to: _d_arrayctor(a, _d_arrayctor(b, c))
    }
}

@RazvanN7
Copy link
Copy Markdown
Contributor

RazvanN7 commented Oct 14, 2021

@PetarKirov But isn't that going to make the call to _d_arrayCtor not possible in @safe code?

@RazvanN7
Copy link
Copy Markdown
Contributor

@PetarKirov see also: https://forum.dlang.org/post/hglpwuvihkfxqcgbwmjb@forum.dlang.org

@dlang-bot dlang-bot added Needs Rebase needs a `git rebase` performed Needs Work labels Nov 9, 2021
Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@teodutu teodutu force-pushed the make-_d_arrayctor-weakly-pure branch from 2eaed68 to 254c1e2 Compare December 2, 2021 20:18
@dlang-bot dlang-bot removed Needs Work Needs Rebase needs a `git rebase` performed labels Dec 2, 2021
@teodutu teodutu force-pushed the make-_d_arrayctor-weakly-pure branch from 254c1e2 to 1a82c3e Compare December 2, 2021 20:20
teodutu added a commit to teodutu/dmd that referenced this pull request Dec 2, 2021
druntime PR: dlang/druntime#3587

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
teodutu added a commit to teodutu/dmd that referenced this pull request Dec 2, 2021
druntime PR: dlang/druntime#3587

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
This undoes the changes brought by dlang#3611

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@teodutu teodutu force-pushed the make-_d_arrayctor-weakly-pure branch from 1a82c3e to 18e0799 Compare December 2, 2021 20:40
teodutu added a commit to teodutu/dmd that referenced this pull request Dec 2, 2021
druntime PR: dlang/druntime#3587

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
teodutu added a commit to teodutu/dmd that referenced this pull request Dec 2, 2021
druntime PR: dlang/druntime#3587

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
@RazvanN7
Copy link
Copy Markdown
Contributor

RazvanN7 commented Dec 3, 2021

Please follow-up on deleting the @trusted on the function.

@dlang-bot dlang-bot merged commit 4f6a249 into dlang:master Dec 3, 2021
@teodutu teodutu deleted the make-_d_arrayctor-weakly-pure branch December 3, 2021 19:01
teodutu added a commit to teodutu/dmd that referenced this pull request Dec 3, 2021
druntime PR: dlang/druntime#3587

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>
RazvanN7 pushed a commit to dlang/dmd that referenced this pull request Dec 4, 2021
* Lower array construction to _d_array{,set}ctor

Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>

* Remove backend code for _d_array{,set}ctor

Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>

* Intercept and handle _d_array{,set}ctor for CTFE

Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>

* Merge similar branches

* Fix failing test errors

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

* Update frontend.h with symbols for new templates

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

* Fix _d_array functions position in frontend.h

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

* _d_arrayctor: Update CTFE for weakly pure hook

druntime PR: dlang/druntime#3587

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

Co-authored-by: Dan Printzell <xwildn00bx@gmail.com>
kinke pushed a commit to ldc-developers/dmd-testsuite that referenced this pull request Dec 19, 2021
* Lower array construction to _d_array{,set}ctor

Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>

* Remove backend code for _d_array{,set}ctor

Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>

* Intercept and handle _d_array{,set}ctor for CTFE

Signed-off-by: Dan Printzell <xwildn00bx@gmail.com>

* Merge similar branches

* Fix failing test errors

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

* Update frontend.h with symbols for new templates

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

* Fix _d_array functions position in frontend.h

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

* _d_arrayctor: Update CTFE for weakly pure hook

druntime PR: dlang/druntime#3587

Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

Co-authored-by: Dan Printzell <xwildn00bx@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants