Skip to content

Fix the concat assign case of Issue 20321 in a frontend solution#10539

Closed
SSoulaimane wants to merge 9 commits intodlang:masterfrom
SSoulaimane:copy1
Closed

Fix the concat assign case of Issue 20321 in a frontend solution#10539
SSoulaimane wants to merge 9 commits intodlang:masterfrom
SSoulaimane:copy1

Conversation

@SSoulaimane
Copy link
Member

@SSoulaimane SSoulaimane commented Nov 5, 2019

See discussion on #10510.

Do more lowering in the front-end that reduces the need to create temporaries in the glue layer.
This PR only does the concatenation assignment for arrays, but more cases could be potentially solved in the same way.
When the glue layer decides to copy or move it doesn't call the copy constructor or the postblit since only the semantic phase knows how elaborate copy semantics works.

@SSoulaimane SSoulaimane force-pushed the copy1 branch 21 times, most recently from e0a8351 to edd93f9 Compare November 9, 2019 14:27
@SSoulaimane SSoulaimane changed the title Fix some cases of Issue 20321 in a frontend solution Fix the concat assign case of Issue 20321 in a frontend solution Nov 10, 2019
@SSoulaimane
Copy link
Member Author

buildkite failures are fixed by this DUB PR dlang/dub#1796.

@PetarKirov
Copy link
Member

@SSoulaimane although I agree with the change to dub, it looks worrying that this PR causes change in behavior. Do you know what now causes DMD to "print some stuff in between calls of pragma(msg)"?

@SSoulaimane
Copy link
Member Author

SSoulaimane commented Nov 11, 2019

Do you know what now causes DMD to "print some stuff in between calls of pragma(msg)"?

Yes, the first commit of this pull request moves druntime calls which are issued for array element concatenation assignment ~= from the glue layer to the front-end, so now every time there is element concatenation assignment it instantiates a template https://github.com/dlang/druntime/blob/cd1a3e06b912a801877a3104c5e57498970822ed/src/core/internal/array/appending.d#L16-L19. the template imports the module core.internal.array.utils, and subsequently DMD prints import core.internal.array.utils ... in between the pragma(msg) calls, because one of the pragmas invokes a ctfe function which does a ~= array operation.

@PetarKirov
Copy link
Member

PetarKirov commented Nov 12, 2019

The part that actually confuses me is this one:

DMD prints import core.internal.array.utils ... in between the pragma(msg)

That shouldn't happen, unless dmd is invoked with -v. Is this what dub is doing?

@SSoulaimane
Copy link
Member Author

@dlang-bot
Copy link
Contributor

dlang-bot commented Nov 20, 2019

Thanks for your pull request, @SSoulaimane!

Bugzilla references

Auto-close Bugzilla Severity Description
20321 blocker Uncontrollable blitting should be preventable for proper elaborate copy semantics

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

Solves one of the cases of issue 20321 which wasn't straight forward to solve in the backend since elablorate copy semantics are currently understood by the semantic phase only.

See issues 17448 and 20321.

The problem is solved by avoiding `doCopyOrMove()` which only partially helps the backend with elaborate copy.

This goes hand in hand with the effort being spent on templatizing druntime, this benefit from that effort actually.
Some cases in the test suite started failing after the lowering of concat element assign, particularly `test9023()` from `runnable/interpret.d` which combines array operations with associative arrays.
The instantiation of `_d_arrayappendTImpl` template complains that `__c_wchar_t` is forward referenced because the declaration had no body.

Reproducible with:
```
enum T : int;

void main()
{
    T[] a;
    _d_arrayappendTImpl!(T[])(a, 1);
}
```
preserves the behaviour of test file `compilable/vgc2.d`.
`static foreach` generates a struct type called `Tuple` which is meant for compile time use only, thus it never ends in the object file. We can't pass this type to a druntime function which uses typeinfo.
Index is not decremented which causes bounds checking error.
Assignment to a ref variable is really assignment to where it points to, which is known to the compiler when the variable is initialized.

Reference initialization happens only for foreach parameters and internally made temporaries as far as I know, since `ref` cannot be used for regular variables.
@RazvanN7
Copy link
Contributor

RazvanN7 commented Jul 5, 2022

This has been superseded by: #13495

@RazvanN7 RazvanN7 closed this Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants