Decrease template bloat for string functions

[n8sh]

Use const(E)[] instead of E[] when it is valid and doesn't change the return type.

[dlang-bot]

Thanks for your pull request, @n8sh!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	19364	enhancement	Decrease template bloat for string functions

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 + phobos#6743"

[burner]

I assumed you grep'ed for the usage of decodeImpl to make sure CastForDecode is used when needed?

[n8sh]

In effect yes. The only places it's not used are in unittests.

[burner]

Wouldn't it be better if decodeImpl would call CastForDecode, because currently we have to make sure that people remember calling CastForDecode if necessary.

[n8sh]

Can you suggest a good way to do that? I'm not enthusiastic about adding a wrapper function.

[burner]

sorry, strike my last comment. I misread the call side.

[jmdavis]

Have you done anything to measure these changes? Given the size and complexity of decodeImpl, it wouldn't surprise me at all if this does reduce template bloat, but you are actually adding template instantiations into the mix into order to avoid template instantiations, and the vast majority of string code is just string. So, for some programs at least, this is probably adding template bloat, though overall, it probably is reducing it. It also has a number of scope-related changes that seem orthogonal to the main point of the PR, which muddies things a bit.

Regardless, unless there's something that I'm missing here, I think that inout should be completely removed from the PR. It's my understanding that it has no impact on how many templates are instantiated, and it really doesn't provide any benefit, because the code is templated. It just further complicates the code.

[n8sh]

@jmdavis The purpose of this PR is not to improve compile times by reducing the absolute number of template uses, it's to improve the cache performance of the final executable by decreasing the number of distinct functions created by template instantiation. Look at the generated assembly for this code:

https://run.dlang.io/is/fMHThw

void foo(T)(ref T[] a)
{
    a = a[1 .. $];
}

void bar(T)(ref inout(T)[] a)
{
    a = a[1 .. $];
}

void main()
{
    int[] a = [1, 2, 3];
    const(int)[] b = [4, 5, 6];
    immutable(int)[] c = [7, 8, 9];
    foo(a);
    foo(b);
    foo(c);
    bar(a);
    bar(b);
    bar(c);
}

The generated code has 3 separate foo functions that are each called once, while there is 1 single bar function that is called 3 times.

[thewilsonator]

Yep, that went green.

[thewilsonator]

Since style is guaranteed to fail, feel free to abuse auto-merge to get priority to reduce this.

[thewilsonator]

Ooohh, or not. Why is style passing now?

[thewilsonator]

And now style fails, WTF?

[thewilsonator]

Its great that you found the source of the error, did you submit a bug for it?