consistently disallow rebinding type-bound ops (#891)

## Summary

Consistently disallow providing a generic type-bound operator
implementation for generic types where one of its instances had the
operator implicitly lifted already.

This is how it already worked for parametric nominal types where the
parameter is not used in the body (see `tinvalid_rebind.nim`). The
design of type-bound operators at the language-level is still sorted
out, and until that's done, it's better to have their behaviour be
consistent.

## Details

Rebinding for parametric nominal types where none of the parameters is
used in the type's body was only disallowed because of a side-effect of
how types are currently instantiated: if they don't contain generic
parameters, the `PType` is not copied, meaning that the underlying
`tyObject` for a `type Obj[T] = object` is the exact same for
both the original type and all its instances.

Therefore, when an operator is implicitly lifted, it is, effectively,
bound to the operator slot of all instances plus that of the generic
type, thus blocking binding a custom generic implementation. If the
`PType` is copied during the instantiation process (as is the case for
all parametric nominal types where at least one parameter is used in
the type's body), only the slot of the instance is assigned to.

In order to make the behaviour consistent, when producing the symbol for
an operator during lifting (`produceSym`), it is checked whether the
lifted-for nominal type is coming from a generic (indicated by the
`tfFromGeneric` flag), and if it is, a pseudo symbol is assigned to the
originating-from `PType`. The originating-from type is stored as the
last position of a `tyGenericBody`, which is itself stored in the first
position of a `tyGenericInst` -- the `tyGenericInst` for an instantiated
type is accessible via `TType.typeInst`.

Parametric nominal types where none of the parameters are used in the
type's body don't have the `tfFromGeneric` flag set, and they thus
keep working as they did previously.

The pseudo-operator blocks the operator slot of the generic type,
preventing a custom implementation from being bound *explicitly*. So
that the pseudo-operator is not copied to a new instance during generic
instantiation (which would disable the implicit operator lifting),
`copyTypeProps` looks for the `sfAnon` flag (which is only included for
pseudo-operators) on the to-be-copied operators, and if the flag is set,
doesn't copy the operator.

The tests for operator binding are expanded to make sure that explicitly
binding a generic operator implementation keeps working when a custom
implementation was already *explicitly* bound to specific instantiation
of the type.

compiler/modules/modulegraphs.nim

@@ -416,7 +416,9 @@ proc hasDisabledAsgn*(g: ModuleGraph; t: PType): bool =
 proc copyTypeProps*(g: ModuleGraph; module: int; dest, src: PType) =
   for k in low(TTypeAttachedOp)..high(TTypeAttachedOp):
     let op = getAttachedOp(g, src, k)
-    if op != nil:
+    # the ``sfAnon`` flag signals that the operator is a pseudo-operator, and
+    # those are not inherited/copied
+    if op != nil and sfAnon notin op.flags:
       setAttachedOp(g, module, dest, k, op)
 
 proc loadCompilerProc*(g: ModuleGraph; name: string): PSym =

compiler/sem/liftdestructors.nim

@@ -820,6 +820,31 @@ proc fillBody(c: var TLiftCtx; t: PType; body, x, y: PNode) =
      tyGenericInst, tyAlias, tySink:
     fillBody(c, lastSon(t), body, x, y)
 
+proc bindPseudoOp(g: ModuleGraph, c: PContext, idgen: IdGenerator,
+                  kind: TTypeAttachedOp, typ: PType, info: TLineInfo) =
+  ## If the `kind` slot is not alread filled, assigns a pseudo-operator to the
+  ## slot of `typ`'s originating-from generic type.
+  assert tfFromGeneric in typ.flags
+  if c == nil or kind == attachedDeepCopy:
+    # without a ``PContext``, we cannot create a pseudo-op, but it's also not
+    # necessary, as a ``PContext`` is only missing for types created outside of
+    # semantic analysis.
+    # ``=deepCopy`` operators are currently not implicitly lifted like the
+    # others, so we don't block the generic type's slot
+    return
+
+  # attach to the generic type, not to the ``tyGenericBody``
+  let generic = typ.typeInst[0].lastSon
+  if getAttachedOp(g, generic, kind) == nil:
+    # no custom operator is bound to the `kind` slot for the generic type;
+    # block it
+    let
+      name = g.cache.getIdent(AttachedOpToStr[kind])
+      op   = newSym(skProc, name, nextSymId c.idgen, typ.owner, info)
+    # mark the symbol as anonymous, making it possible to later detect it
+    op.flags.incl sfAnon
+    setAttachedOp(g, c.idgen.module, generic, kind, op)
+
 proc produceSymDistinctType(g: ModuleGraph; c: PContext; typ: PType;
                             kind: TTypeAttachedOp; info: TLineInfo;
                             idgen: IdGenerator): PSym =
@@ -830,6 +855,10 @@ proc produceSymDistinctType(g: ModuleGraph; c: PContext; typ: PType;
   result = getAttachedOp(g, baseType, kind)
   setAttachedOp(g, idgen.module, typ, kind, result)
 
+  if tfFromGeneric in typ.flags:
+    # block the generic type's operator slot
+    bindPseudoOp(g, c, idgen, kind, typ, info)
+
 proc symPrototype(g: ModuleGraph; typ: PType; owner: PSym; kind: TTypeAttachedOp;
               info: TLineInfo; idgen: IdGenerator): PSym =
 
@@ -880,6 +909,11 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
   if result == nil:
     result = symPrototype(g, typ, typ.owner, kind, info, idgen)
 
+  if typ.kind in {tyObject, tyEnum} and tfFromGeneric in typ.flags:
+    # for nominal types (``tyDistinct`` is handled separately), block
+    # the operator slot of the generic type
+    bindPseudoOp(g, c, idgen, kind, typ, info)
+
   var a = TLiftCtx(info: info, g: g, kind: kind, c: c, asgnForType: typ, idgen: idgen,
                    fn: result)
 

tests/lang_objects/destructor/tinvalid_rebind.nim

@@ -1,8 +1,6 @@
 discard """
-joinable: false
-cmd: "nim check $file"
-errormsg: "cannot bind another '=destroy' to: Foo; previous declaration was constructed here implicitly: tinvalid_rebind.nim(12, 7)"
-line: 14
+errormsg: "cannot bind another '=destroy' to: Foo; previous declaration was constructed here implicitly: tinvalid_rebind.nim(10, 7)"
+line: 12
 """
 
 type

tests/lang_objects/destructor/tinvalid_rebind_2.nim

@@ -0,0 +1,17 @@
+discard """
+errormsg: "cannot bind another '=destroy' to: Foo; previous declaration was constructed here implicitly: tinvalid_rebind_2.nim(14, 7)"
+line: 16
+"""
+
+# compared to ``tinvalid_rebind.nim``, the ``Foo`` type here uses the generic
+# parameter in its body
+
+type
+  Foo[T] = object
+    x: T
+
+proc main =
+  var f: Foo[int]
+
+proc `=destroy`[T](f: var Foo[T]) =
+  discard

tests/lang_objects/destructor/tvalid_rebind.nim

@@ -0,0 +1,26 @@
+discard """
+  targets: "c js vm"
+  description: '''
+    Ensure that it's possible to explicitly bind a type-bound operator to
+    both the generic type and a specific instance thereof
+  '''
+  knownIssue: "No copy of the type is created for ``Phantom[int]``"
+"""
+
+type
+  Phantom[T] = object
+    ## A type where the generic parameter is not used in the body
+
+proc `=destroy`(x: var Phantom[int]) =
+  discard
+
+# explicitly binding an operator to the generic type works and doesn't
+# affect the one bound to the specific instance
+
+proc `=destroy`[T](x: var Phantom[T]) =
+  doAssert false
+
+proc test() =
+  var x = Phantom[int]()
+
+test()

tests/lang_objects/destructor/tvalid_rebind_2.nim

@@ -0,0 +1,26 @@
+discard """
+  targets: "c js vm"
+  description: '''
+    Ensure that it's possible to explicitly bind a type-bound operator to
+    both the generic type and a specific instance thereof
+  '''
+"""
+
+type
+  Object[T] = object
+    ## A type where the generic parameter *is* used in the body
+    x: T
+
+proc `=destroy`(x: var Object[int]) =
+  discard
+
+# explicitly binding an operator to the generic type works and doesn't
+# affect the one bound to the specific instance
+
+proc `=destroy`[T](x: var Object[T]) =
+  doAssert false
+
+proc test() =
+  var a = Object[int]()
+
+test()