semantic: avoid runtime _d_arraysetlength call when .length = 0

[ljmf00]

Signed-off-by: Luís Ferreira contact@lsferreira.net

---

This simple code

void main()
{
    int[] a;
    a.length = 0;
}

Produces this code with a runtime call on semantic analysis:

void main()
{
	int[] a = null;
	_d_arraysetlengthT(a, 0LU);
	return 0;
}

The runtime call _d_arraysetlengthT does internally slicing if new_length <= current_length

if (newlength <= (*p).length)
{
    *p = (*p)[0 .. newlength];
    void* newdata = (*p).ptr;
    return newdata[0 .. newlength];
}

For 0 as new length, this expression will always be true, no matter what, so this should be a = a[0 .. 0] instead of _d_arraysetlengthT(a, 0LU)

I'm not quite sure if this optimization should be made on the semantic analysis but I assumed it because the ArrayLengthExpr assignment is converted to _d_arraysetlengthT there.

[dlang-bot]

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

[thewilsonator]

tests?

[ljmf00]

tests?

The behavior of .length = 0 is already being tested here https://github.com/dlang/dmd/blob/master/test/runnable/test28.d#L130 . I didn't found any test related to _d_arraysetlengthT call, so I assumed no furder tests needed.

I'm new to dmd compiler contributions, so, if there's any way to test the behavior of semantic analysis phase, could you guide me, if possible?

[thewilsonator]

That test that the behaviour has not changed, not that the implementation is different. You probably want a codegen test here, but those are not very nice to write.

@UplinkCoder , @rainers , anyone else, any better ideas?

[rainers]

@UplinkCoder , @rainers , anyone else, any better ideas?

How about verifying the output of compilation with -vcg-ast?

[ibuclaw]

Isn't this what POST_SCRIPT is for? Ensure there is no call to arraysetlength in the disassembled object file (though gdc and ldc is better at this ability in that they compile to assembly).

[ljmf00]

How about verifying the output of compilation with -vcg-ast?

I done a test case for that using POST_SCRIPT, can you review?

[ljmf00]

Maybe I'm missing something obvious, but the testcase runs locally with success.

[ljmf00]

I solved the issue, the test case was malformed because it was testing only 64bit literals.

[thewilsonator]

 ... fail_compilation/fail_arrayop3c.d -verrors=0 -o-  -fPIC ()
+ check_clean_git
+ git checkout test/compilable/issue17167.sh
+ rm -rf _generated
+ rm -f install.sh
+ rm -f test/compilable/vcg-ast.d.cg
+ make -f posix.mak check-clean-git
ERROR: Found the following residual temporary files.
ERROR: Temporary files should be stored in `test_results` or explicitly removed.
?? test/compilable/vcg-ast-arraylength.d.cg
posix.mak:69: recipe for target 'check-clean-git' failed
make: *** [check-clean-git] Error 1

[thewilsonator]

I have no idea why that isn't working.

[ljmf00]

It seems an heisenberg issue. I don't know either.

[WalterBright]

see my comment

review addressed.

[thewilsonator]

You might need to remove the generated file manually , see

dmd/.circleci/run.sh

Lines 151 to 152 in 33cde05

	# auto-removal of these files doesn't work on CirleCi
	rm -f test/compilable/vcg-ast.d.cg

[ljmf00]

You might need to remove the generated file manually , see

dmd/.circleci/run.sh

Lines 151 to 152 in 33cde05

	# auto-removal of these files doesn't work on CirleCi
	rm -f test/compilable/vcg-ast.d.cg

Done. I'll try to fix this later. I don't see a reason for this being hardcoded.

[thewilsonator]

could you rebase now that dlang/ci#436 is in. I think that should be it.

[thewilsonator]

restarted build kite, added auto merge.

[MoonlightSentinel]

This introduced a regression: https://issues.dlang.org/show_bug.cgi?id=21678

---

The problem is that the value of the expression changed from the new length (0) to the array slice.

Two possible solutions:

• revert this PR
• change the rewrite to yield 0, e.g.

a.length = 0;

(a = a[0 .. 0], 0) // Doesn't work for standalone assignments