Fix regression 13393: cartesianProduct + joiner = runtime assertion failure

[quickfur]

Fixes: https://issues.dlang.org/show_bug.cgi?id=13393

Caused by .save failing to save entire range state (it's missing .empty), causing inconsistency in .saved objects.

[Safety0ff]

LGTM, though the test can be reduced to:

assert(!cartesianProduct([0],[0],[0]).save.empty);

[quickfur]

Good idea, done.

[monarchdodra]

Often, save is implemented in terms of copy + saved range overwrite, eg:

             Result copy = this;
             foreach (i, _; RR)
             {
                 copy.ranges[i] = ranges[i].save;
                 copy.current[i] = current[i].save;
             }

It adds a some extra dead assignments, but the compiler should take care of these. It is more robust, since anything not explicitly processed is still implicitly copied. The overall code is more "generic" in terms of copy paste too.

The fact that the variable was named copy and not ret makes me wonder if that wasn't the original intent?

At this point, it's mostly style, so whatever you think is best.

That said, I'm not a huge fan of the foreach (i, r; ranges) declaration, which arbitrarily "binds" one of the 4 indexed range to a (by-copy) variable. I think the code I posted above is better.

But whatever. Nitpicks. I'll merge when you say it's good to go, or in 24h, whichever comes first.

[quickfur]

Agreed, but I was aiming for the "minimal change to fix the bug".

[monarchdodra]

OK, works for me.

[monarchdodra]

Auto-merge toggled on