Translate _d_arraysetlengthT to template

[Vild]

druntime PR: dlang/druntime#2656

The exp.e1.type.toBasetype().nextOf().isZeroInit check is now inside of the runtime hook.

[dlang-bot]

Thanks for your pull request and interest in making D better, @Vild! 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 fetch digger
dub run digger -- build "master + dmd#10106"

[JinShil]

@Vild The druntime PR is in, so I guess that automatically makes this our first merge candidate. Please rebase, or do whatever is necessary to make this pass this test suite. cc @jpf91

[jpf91]

BTW, maybe the code above can somehow be merged into verifyHookExists?

Expression id = verifyHookExist(exp.loc, *sc, hook, "resizing arrays", Id.object);
if (!id)
    return setError();

[jpf91]

Maybe rename it though and also provide a way to get the hook without error reporting:

Expression id = findRuntimeHook(... bool printError);

[Vild]

I don't think this PR is the correct place to change the the implementation of verifyHookExist.
It is already used for the already implemented hook lowerings, so I think it would be better to change it in another PR.

[Vild]

This patch doesn't compile DCD currently because it uses -inline, so my dinterpret.d hack gets bypassed when the hook is inlined.

[JinShil]

my dinterpret.d hack gets bypassed when the hook is inlined.

What, specifically, is the problem that the dinterpret hack is trying to avoid? Can an if (__ctfe) branch be added to the runtime lowering to achieve the same thing?

[Vild]

So the 'hack' is basically turning the _d_arraysetlengthT(arr, newLength) call back to arr.length = newLength, so it can run the correct dinterpret.d code path to handle the resize correctly.

The problem now was that I didn't test that the code worked with -inline, which caused a problem in my dinterpret.d hack because I use the visit(CallExp) function to be able to rewrite it back to arr.length=newLen.

I got it working by adding pragma(inline, false) to the runtime hooks, so now I will always be able to find the CallExp with the call to the hook.

But sadly now another problem arose, error: undefined reference to '_D2rt5array8capacity__T23_d_arraysetlengthTTraceHTAaTaZQBgFNaNbNeAyaiQeMNkKQzmZm', when compiling DCD with -profile=gc. So for some reason not all the template instantiations are exported.

[Vild]

It currently fails during the profilegc test:

./generated/linux/release/64/profilegc ./generated/linux/release/64/myprofilegc.log
diff myprofilegc.log.linux.64.exp ./generated/linux/release/64/myprofilegc.log
5,6d4
<              48	              1	float[] D main src/profilegc.d:42
<              48	              1	int[] D main src/profilegc.d:41

I cannot reproduce this locally, so I'm not sure what to do.

[JinShil]

I cannot reproduce this locally

Try testing both debug and release builds.

[Vild]

Try testing both debug and release builds.

That was the problem, I only tested it with debug and it got triggered only on release builds (-release -O).
The call to accumulate gets culled away because it is pure and the return value isn't used (it's void): https://github.com/dlang/druntime/blob/master/src/rt/array/capacity.d#L287
So I guess I need to make sure that nothing gets culled inside of a tracehook, or somehow change the hook so it isn't pure.

[thewilsonator]

Is this good to go?

[Vild]

Is this good to go?

Not fully yet, I need to fix the *Trace detection for the ctfe intercept

[Vild]

@thewilsonator It is done now :)

[Vild]

Any more changes needed?

[JinShil]

The workarounds bug me, but they are isolated and I'm not able to come up with anything better.