Bug 10543 – std.algorithm.map incorrectly uses source range length for narrow strings
Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P2
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2013-07-04T09:30:00Z
Last change time
2013-07-06T02:25:20Z
Keywords
accepts-invalid, pull
Assigned to
monarchdodra
Creator
peter.alexander.au
Comments
Comment #0 by peter.alexander.au — 2013-07-04T09:30:32Z
void main()
{
import std.algorithm;
import std.stdio;
string s = "こんにちは世界";
auto m = s.map!(a => 1);
writeln(m, ", ", m.length);
}
Gives:
[1, 1, 1, 1, 1, 1, 1], 21
Clearly the reported length (21) is wrong, it should be 7.
Comment #1 by peter.alexander.au — 2013-07-04T09:31:54Z
(In reply to comment #0)
> Clearly the reported length (21) is wrong, it should be 7.
Scratch that. length shouldn't be available at all for narrow strings as it is unobtainable in constant time.
Comment #2 by bearophile_hugs — 2013-07-04T10:41:41Z
Where's the bug here?
Comment #3 by peter.alexander.au — 2013-07-04T10:43:22Z
(In reply to comment #2)
> Where's the bug here?
It says the length of [1, 1, 1, 1, 1, 1, 1] is 21. It isn't. It's 7.
Comment #4 by mailnew4ster — 2013-07-04T10:47:52Z
21 is the number of UTF-8 code units.
Using dstring produces 7.
Comment #5 by peter.alexander.au — 2013-07-04T10:54:42Z
(In reply to comment #4)
> 21 is the number of UTF-8 code units.
> Using dstring produces 7.
I am not requesting the length of the string, I am requesting the length of the map over the string. As ranges, strings are ranges of code points, not code units, so the number of elements in the *map* (confirmed by the output) is seven, i.e. that is the number of times you can safely call popFront on the map.
I'm struggling to understand the confusion.
m is a range of seven integers
m.length is 21
walkLength(m) is also 21
This is completely broken.
I fully understand that narrow strings use length to report code units, not code points, but m is not a string, so that distinction does not apply.
Comment #6 by monarchdodra — 2013-07-04T12:51:16Z
(In reply to comment #5)
> I'm struggling to understand the confusion.
The problem is perfectly clear.
Are you already correcting this? If not, I'll take care of it.
I remember having seen this in one of my first pulls correcting map, but got confused about the fact there is an *explicit* test to do things wrong... Time to correct it for good I guess.
Comment #7 by peter.alexander.au — 2013-07-04T13:15:10Z
(In reply to comment #6)
> (In reply to comment #5)
> > I'm struggling to understand the confusion.
>
> The problem is perfectly clear.
>
> Are you already correcting this? If not, I'll take care of it.
>
> I remember having seen this in one of my first pulls correcting map, but got
> confused about the fact there is an *explicit* test to do things wrong... Time
> to correct it for good I guess.
I'm not fixing it right now, go ahead.
I only noticed it when I looked at the source. No idea why it's explicitly coded to be wrong...