Issue 12780 - Multiplying integer array by scalar double fails

[9rnsr]

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

Support upcasting slice elements in array operations.

1. The tweak in the buildArrayIdent() retains backward compatibility.

   a) If no element upcasting exists in array operation, the generated function name is not changed.

   double_res[] = double_val * double_arr[];
   // _arrayExpSliceMulSliceAssign_d

   b) If upcasting slice elements is necessary, it will be encoded by the postfix "Of" + element-type-deco

   double_res[] = double_val * int_arr[];
   // _arrayExpSlice'Ofi'MulSliceAssign_d

2. Even if the whole array operation requires double element, the sub array operations can be processed by int.

   double_res[] = (int_val ^ int_arr[]) * double_val;
   // typeof(int_val ^ int_arr[]) == int[]

[9rnsr]

The issue is on the boundary of an enhancement request and rejects-valid bug.

[yebblies]

Will this work now? It seems similar but I can't see a test for it.

int[] a = [0, 0, 0];
short[] b = [1, 2, 3];
a[] = b[];
assert(a == [1, 2, 3]);

[9rnsr]

@yebblies No, this PR does not accept it.

[yebblies]

Why not? Isn't it equivalent to int[] = 1 * short[], which I assume is supported?

[9rnsr]

The enhanced upcasting affects only in the right side array operation of an assignment.

Therefore, following code yet doesn't work.

short[] a = [1,2,3];
int[] r = [0,0,0];
r[] = 1 * a[];    // cannot implicitly convert expression (a[] * cast(short)1) of type short[] to int[]

---

I think at least it should go another enhancement, because currently built-in element-wise assignment has a little weird behavior.

static int postblit, dtor, assign;
struct S {
    this(this) { postblit++; }
    ~this() { dtor++; }
    ref S opAssign(S rhs) { assign++; return this; }
}
void main() {
    S[] s1 = [S(), S()];
    S[] s2 = [S(), S()];
    assert(postblit == 0 && dtor == 0 && assign == 0);
    s1[] = s2[];
    assert(postblit == 2 && dtor == 2 && assign == 0);
}

Each elements is assigned by "copy, swap, and destroy" steps and user-defined opAssign is just ignored.

If we support int[] = short[], I think it would mean that applying "normal assignment rule" in element-wise assignment. Therefore, we should also consider opAssign.

So I don't want to support it in this PR. Big PR will be left for a long time even if it will introduce more consistent behavior.

[yebblies]

It's really weird that this works:

{
    int s = 1;
    int[] a;
    short[] b;
    a[] = s * b[];
}

But this doesn't:

{
    int[] a;
    short[] b;
    a[] = 1 * b[];
}

But I'm ok with it not being fixed in this pull. I think ideally array operations should always compile if the equivalent hand-written loop would compile.

Each elements is assigned by "copy, swap, and destroy" steps and user-defined opAssign is just ignored.

Isn't that a valid way to expand it? I thought the two were functionally equivalent and picking one is meant to be an optimization choice.

If upcasting slice elements is necessary, it will be encoded by the postfix "Of" + element-type-deco

Is it possible to have conflicts if you have a struct called eg "OfAddMul" or something?

[9rnsr]

Isn't that a valid way to expand it? I thought the two were functionally equivalent and picking one is meant to be an optimization choice.

If a[] = a[]; is really equivalent with a[0] = b[0], a[1] = b[1], ..., should each assignments consider opAssign? I don't think it could be called an optimization.

Is it possible to have conflicts if you have a struct called eg "OfAddMul" or something?

Never. If the AddMul struct declared in mod module, it will be "OfS3mod6AddMul" so any conflicts does not occur..

[yebblies]

If a[] = a[]; is really equivalent with a[0] = b[0], a[1] = b[1], ..., should each assignments consider opAssign? I don't think it could be called an optimization.

I'm not sure, but you're probably right. At least for primitive types it's fairly straightforward.

Never. If the AddMul struct declared in mod module, it will be "OfS3mod6AddMul" so any conflicts does not occur..

Ok good.

[yebblies]

Auto-merge toggled on

[MartinNowak]

It wasn't the best idea in retrospect to support vector operations with different element sizes/counts. This cannot be handled by some simple CVTDQ2PS and needs juggling of multiple intermediate values/register to adapt the different element counts.

[MartinNowak]

I think ideally array operations should always compile if the equivalent hand-written loop would compile.

Array operations already behave differently, e.g. with regards to integer promotion

dmd/src/ddmd/arrayop.d

Line 471 in 200bef3

ex2 = new CastExp(Loc(), ex2, e.e1.type.nextOf());

.