fix constant expressions sometimes having the wrong value (#1198)

## Summary

Fix a critical bug with the constant-data table, which led to, in rare
cases, constant aggregate construction expressions evaluating to the
wrong value at run-time. All backends were affected.

## Details

The `MirNode` comparison procedure of `DataTable` didn't take all
relevant fields into account, meaning that unequal `mnkObjConstr`,
`mnkConstr`, and `mnkField` were treated as equal. For example, the
comparison would result in 'true' for `Obj(a: 1)` and `Obj(b: 1)`.

This problem was hidden by the hashing procedure properly considering
all relevant `MirNode` fields; only when there were hash or bucket
collisions did the equality problem surface. All fields are taken into
account by `datatables.==` now, and the used case statement is made
exhaustive in order to prevent similar issues in the future.

Since a language-level test would be rather contrived and brittle (a
hash/bucket collision is required), a unit test for the comparison
used by `datatables` is added.

compiler/mir/datatables.nim

@@ -85,9 +85,14 @@ proc cmp(a, b: ConstrTree): bool =
       exprStructuralEquivalent(a.lit, b.lit)
     of mnkProc:
       a.prc == b.prc
-    else:
-      # all other nodes are equal when their kind is the same
-      true
+    of mnkConstr, mnkObjConstr:
+      a.len == b.len
+    of mnkField:
+      a.field.id == b.field.id
+    of mnkArg, mnkEnd:
+      true # same node kind -> equal nodes
+    of AllNodeKinds - ConstrTreeNodes:
+      unreachable(a.kind)
 
   if not a[0].typ.sameBackendType(b[0].typ) or a.len != b.len:
     # the (backend-)type is different -> not the same constant expressions

tests/compiler/tdatatables.nim

@@ -0,0 +1,87 @@
+discard """
+  description: "Tests for the compiler/mir/datatables module"
+  targets: native
+"""
+
+import compiler/ast/ast
+include compiler/mir/datatables
+
+# some placeholder types to assing to the nodes. For object types, a different
+# ID means that it's a different type
+let
+  t1 = PType(itemId: ItemId(item: 1), kind: tyObject, sons: @[PType nil])
+  t2 = PType(itemId: ItemId(item: 2), kind: tyObject, sons: @[PType nil])
+  t3 = PType(itemId: ItemId(item: 3), kind: tyObject, sons: @[PType nil])
+  field1 = PSym(itemId: ItemId(item: 1))
+  field2 = PSym(itemId: ItemId(item: 2))
+
+# node constructor
+template node(k: MirNodeKind, t: PType, field, val: untyped): MirNode =
+  MirNode(kind: k, typ: t, field: val)
+template node(k: MirNodeKind, field, val: untyped): MirNode =
+  MirNode(kind: k, field: val)
+template node(k: MirNodeKind): MirNode =
+  MirNode(kind: k)
+template literal(val: PNode): MirNode =
+  MirNode(kind: mnkLiteral, lit: val)
+
+block tree_equality:
+  # the type is only relevant for the head of the tree (the first node)
+
+  # setup a list of structurally valid and unique (in terms of equality) trees
+  let trees = @[
+    # --- literals
+    @[node(mnkLiteral, t1, lit, newIntNode(nkIntLit, 0))],
+    @[node(mnkLiteral, t2, lit, newIntNode(nkIntLit, 0))],
+    @[node(mnkLiteral, t1, lit, newStrNode(nkStrLit, ""))],
+    @[node(mnkLiteral, t1, lit, newStrNode(nkStrLit, "a"))],
+    @[node(mnkLiteral, t1, lit, newFloatNode(nkFloatLit, 0.0))],
+    # 0.0 and -0.0 are different float values
+    # FIXME: doesn't work yet
+    #@[node(mnkLiteral, t1, lit, newFloatNode(nkFloatLit, -0.0))],
+
+    # --- ordered aggregates
+    @[node(mnkConstr, t1, len, 0), node(mnkEnd)],
+    @[node(mnkConstr, t2, len, 0), node(mnkEnd)],
+    @[node(mnkConstr, t1, len, 1),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)),
+      node(mnkEnd)],
+    @[node(mnkConstr, t1, len, 2),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+      node(mnkEnd)],
+
+    # --- aggregates with fields
+    @[node(mnkObjConstr, t1, len, 0), node(mnkEnd)],
+    @[node(mnkObjConstr, t2, len, 0), node(mnkEnd)],
+    @[node(mnkObjConstr, t1, len, 1),
+        node(mnkField, field, field1),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+      node(mnkEnd)],
+    # same field value, different field:
+    @[node(mnkObjConstr, t1, len, 1),
+        node(mnkField, field, field2),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+      node(mnkEnd)],
+    @[node(mnkObjConstr, t1, len, 1),
+        node(mnkField, field, field1),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+        node(mnkField, field, field2),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+      node(mnkEnd)],
+    # swapped fields
+    @[node(mnkObjConstr, t1, len, 1),
+        node(mnkField, field, field2),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+        node(mnkField, field, field1),
+        node(mnkArg), literal(newIntNode(nkIntLit, 0)), node(mnkEnd),
+      node(mnkEnd)]
+  ]
+
+  # compare all trees with each other
+  for i in 0..<trees.len:
+    doAssert cmp(trees[i], trees[i]) # tree must be equal to itself
+    for j in (i+1)..<trees.len:
+      if cmp(trees[i], trees[j]):
+        echo "compared equal, but shouldn't: ", i, " vs. ", j
+        doAssert false