Bug 14925 – replaceInPlace fail compilation

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2015-08-16T08:49:06Z
Last change time
2020-03-21T03:56:35Z
Assigned to
No Owner
Creator
Florent GABRIEL

Comments

Comment #0 by tsalm — 2015-08-16T08:49:06Z
This code won't compile import std.array; import std.stdio; void main() { char[] a = "mon texte 1".dup; char[] b = "abc".dup; size_t x = 4; size_t y = 9; replaceInPlace( a, x , y, b ); writeln( a ); } Compilation fail with error : replaceInPlace.d(11): Error: std.array.replaceInPlace called with argument types (char[], uint, uint, char[]) matches both: /usr/include/dmd/phobos/std/array.d(2279): std.array.replaceInPlace!(char, char[]).replaceInPlace(ref char[] array, uint from, uint to, char[] stuff) and: /usr/include/dmd/phobos/std/array.d(2312): std.array.replaceInPlace!(char, char[]).replaceInPlace(ref char[] array, uint from, uint to, char[] stuff) Tested on DMD32 D Compiler v2.068.0-rc1
Comment #1 by schveiguy — 2015-08-17T12:27:24Z
Once again, the "specialized" nature of strings comes back to bite us. The call matches the second overload (which just calls replace, and overwrites the original slice), because char[] is an input range, and char[] "isSomeString" and char[] has element type of dchar. I think probably we need to verify in that branch of the if statement that T is immutable or const, or the element encoding type of the array is not T.
Comment #2 by schveiguy — 2015-08-17T14:13:33Z
Hm... I was wrong I guess (sort of). The *second* overload is the one that should be chosen. This means that even though the replace could be done directly, it will reallocate. This is because other pieces of the first overload cannot compile with narrow strings (namely, remove). So I just added a constraint to the first to prevent it from being called on narrow strings, and updated the second to work for all cases that don't match the first. I also found a bug where the first constraint would match 2 arrays where the second array wasn't identical, but the elements could be implicitly cast to the elements of the first. For example: auto a = [1L, 2, 3]; a.replaceInPlace(1, 2, [4,5,6]); Which fails to compile on 2.068
Comment #3 by schveiguy — 2015-08-17T14:18:50Z
Comment #4 by github-bugzilla — 2015-08-19T16:18:59Z
Commits pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/3f8b910e2c2834d2f87a543dc39692fe03a69c92 Fix issue 14925 -- merge overloads of replaceInPlace to avoid ambiguity with template instantiation. Also, preclude narrow strings and other invalid combinations (e.g. long[] and int[]) from being selected for optimized path. https://github.com/D-Programming-Language/phobos/commit/4d36627f363d9e23aae218578478a0c68da27278 Merge pull request #3561 from schveiguy/fix14925 Fix issue 14925 -- narrow strings should not match the first overload of replaceInPlace.
Comment #5 by github-bugzilla — 2015-08-20T12:07:29Z
Commits pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/ea811edab97021c9328dd2e81c49bba23c414b4d Supplemental fix for issue 14925 - remove redundant conditions https://github.com/D-Programming-Language/phobos/commit/de2df2bb04dfc4667ab0bac30425b9c14d6cb699 Merge pull request #3565 from 9rnsr/fix14925 Supplemental fix for issue 14925
Comment #6 by github-bugzilla — 2015-10-04T18:20:25Z