This used to print true-false, now prints true-true. Is this a bug?


import std.stdio, std.algorithm;
void main() {
    auto x = "abcd";
    writeln(isSorted(x));
    auto y = "acbd";
    writeln(isSorted(y));
}


Maybe this line:

https://github.com/D-Programming-Language/phobos/blob/master/std/algorithm.d#L9237

        for (; !ahead.empty; ahead.popFront(), ++i)

Should be:

        for (; !ahead.empty; ahead.popFront(), r.popFront(), ++i)

Bye,
bearophile

On Wednesday, February 06, 2013 01:33:29 bearophile wrote:
> This used to print true-false, now prints true-true. Is this a bug?
> 
> 
> import std.stdio, std.algorithm;
> void main() {
>      auto x = "abcd";
>      writeln(isSorted(x));
>      auto y = "acbd";
>      writeln(isSorted(y));
> }
> 
> 
> Maybe this line:
> 
> https://github.com/D-Programming-Language/phobos/blob/master/std/algorithm.d #L9237
> 
>          for (; !ahead.empty; ahead.popFront(), ++i)
> 
> Should be:
> 
>          for (; !ahead.empty; ahead.popFront(), r.popFront(), ++i)

That definitely looks like a bug. "acbd" _isn't_ sorted.

- Jonathan M Davis

Have a GitHub account, bearophile? Just hit Edit at the top of that link you gave, make the change, and click "Proposal File Change" and it'll roll up a pull request. Should take less than a minute. I use this for almost every minor pull requests I do. The autotester takes care of the rest.

Looks like line 9221 is essentially the same code so you may want to change that line too.

BA


On Tue, Feb 5, 2013 at 5:33 PM, bearophile <bearophileHUGS@lycos.com> wrote:

> This used to print true-false, now prints true-true. Is this a bug?
>
>
> import std.stdio, std.algorithm;
> void main() {
>     auto x = "abcd";
>     writeln(isSorted(x));
>     auto y = "acbd";
>     writeln(isSorted(y));
> }
>
>
> Maybe this line:
>
> https://github.com/D-**Programming-Language/phobos/** blob/master/std/algorithm.d#**L9237<https://github.com/D-Programming-Language/phobos/blob/master/std/algorithm.d#L9237>
>
>         for (; !ahead.empty; ahead.popFront(), ++i)
>
> Should be:
>
>         for (; !ahead.empty; ahead.popFront(), r.popFront(), ++i)
>
> Bye,
> bearophile
>

Jonathan M Davis:

> That definitely looks like a bug. "acbd" _isn't_ sorted.

OK. Thank you.

http://d.puremagic.com/issues/show_bug.cgi?id=9457

--------------------

Brad Anderson:

> Have a GitHub account, bearophile?

Not yet, but probably I will have one.


> Just hit Edit at the top of
> that link you gave, make the change, and click "Proposal File
> Change" and it'll roll up a pull request. Should take less than a
> minute. I use this for almost every minor pull requests I do. The
> autotester takes care of the rest.

Sounds quick & easy. I will remember this. Thank you.


> Looks like line 9221 is essentially the same code so you may want
> to change that line too.

Added in the bug report.

Bye,
bearophile

On 6-2-2013 1:33, bearophile wrote:

>
> Maybe this line:
>
> https://github.com/D-Programming-Language/phobos/blob/master/std/algorithm.d#L9237
>
>          for (; !ahead.empty; ahead.popFront(), ++i)
>
> Should be:
>
>          for (; !ahead.empty; ahead.popFront(), r.popFront(), ++i)
>

Thought that line looked fishy. But I don't understand half the code in Phobos so I
figured it must be me...

Brad Anderson:

> Just hit Edit at the top of
> that link you gave, make the change, and click "Proposal File
> Change" and it'll roll up a pull request. Should take less than a minute.

Plus naturally 15-30 minutes to write down the missing unittests, that weren't written, so they didn't catch a so basic case as isSorted(string).


> The autotester takes care of the rest.

The autotester is not enough. You also need some of your brain.

Bye,
bearophile