Fix Bugzilla 24617 - array runtime erroneously copies flags from exis…

[ntrel]

…ting block

[dlang-bot]

Thanks for your pull request and interest in making D better, @ntrel! 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
✓	24617	minor	array runtime erroneously copies flags from existing block

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

[ntrel]

@schveiguy

the __arrayAlloc function copies the original bits instead of using the typeinfo to decide the new array bits if a BlkInfo has already been looked up

Sorry, I don't understand why allocating an array shouldn't always be appendable?

Options are to just always use the typeinfo, or to check for the appendable bit before using the old bits (but copying the other bits might be questionable as well).

Possibly NO_MOVE should be removed, although I thought D GC doesn't do moving anyway.

[schveiguy]

Sorry, I don't understand why allocating an array shouldn't always be appendable?

The problem is that in this case it is not marked appendable.

There are other problems with just using the bits. You are given a typeinfo with the thing that is to be allocated. If the bits don't match what the original typeinfo required, I don't see a reason to copy the old bits. I think the whole __arrayAlloc overload that uses the prior bits should be removed. The cost of constructing new flags is negligible.

An example:

struct S
{
   int *ptr;
   int[1] buf;
}

void main()
{
   auto s = new S; // s does not have NO_SCAN bit, because it contains pointers
   int[] arr = s.buf[];
   arr.length = 5; // new block should have NO_SCAN bit set, but bits are copied from the old block
}

[schveiguy]

Just discovered while working on the GC array code, that I added this "feature" back in 2014: https://github.com/dlang/druntime/pull/1075

While I get the point of it, I think the bits are too sticky. I think the use case for the original code is pretty sketchy.