array self-assignment gives wrong results

[lscrd]

When assigning globally an array, the generated code may be wrong.

Example

var a = [1, 2, 3]
a = [0, a[0], a[1]]
echo a

proc p() =
  var b = [1, 2, 3]
  b = [0, b[0], b[1]]
  echo b
p()

Current Output

[0, 0, 0]
[0, 0, 0]

Expected Output

[0, 1, 2]
[0, 1, 2]

Possible Solution

Assigning the elements individually in the right order (ugly).

Additional Information

Occurs for global and local variables.

$ nim -v
Nim Compiler Version 1.4.4 [Linux: amd64]

Issue exists also in development version.

EDIT: changed "a" to "b" in the second assignment.

[timotheecour]

@lscrd can you update the example?
I think you mean this instead: (change: b = [0, b[0], b[1]])

var a = [1, 2, 3]
a = [0, a[0], a[1]]
echo a

proc p() =
  var b = [1, 2, 3]
  b = [0, b[0], b[1]]
  echo b
p()

Issue exists also in development version.

update example by showing git hash; development version is a moving target

(i've tested this in c8dda86)

[lscrd]

Thanks for your comment. I changed the name of the local variable from "a" to "b" to avoid confusion and forgot to change it in the array.

Better title too.

[timotheecour]

it's not clear how this can be fixed without introducing a performance penalty involving making a temporary copy, eg:

var a = [10,11]
a = [a[1], a[0]]

with temporary copy:

var a = [10,11]
var tmp = [a[1], a[0]]
a = tmp

furthermore, I'm not sure how to reliably detect cases where a temporary copy is needed, eg:

var a = [10,11]
proc fn0(): auto = a[0]
proc fn1(): auto = a[1]
a = [fn1(), fn0()]

thougts?

[lscrd]

Yes, there is no way to detect reliably the possibility to avoid a copy. In fact, it seems that there is only two alternatives:

– keep the current behavior and add in the documentation a sentence stating that the compiler may do the assignment element by element and so the result may be undefined in some cases;
– use a temporary copy; an optimization may avoid the copy if the value is read only (constant, literal, element of a read-only array or tuple, etc.).

As far as I remember, the issue has already been encountered with tuples and has been solved by assigning the fields in temporary variables, then assigning these variables to the tuple fields. I expected the C compiler to be able to remove some useless copies but, looking at the assembly, is seems that it is not the case.

[Araq]

We'll use alias analysis for this just like we do for object constructors. The compiler can emit a warning when it needs to introduce an expensive copy. It should do that for objects too.

[timotheecour]

similar to #18099; refs #18099 (comment)