Skip to content

Commit

Permalink
fix instantiation errors with generic =deepCopy (#895)
Browse files Browse the repository at this point in the history
## Summary

Fix "unresolved generic parameter" error occurring when instantiating
generic `=deepCopy` implementations that create locals of the
instantiated type the generic operator is instantiated for.

## Details

A generic `=deepCopy` operator attached to a generic type is
instantiated when an instance of the generic type is created. At the
end of instantiating body of the operator, the second semantic pass
(`sempass2`) lifts the type-bound operators for types used in the body.

If the type the generic `=deepCopy` operator is instantiated for is used
in the body, this means that the other type-bound operators are also
lifted already. However, if one of the other type-bound operators is
generic, this leads to `liftdestructors.inst` being called, which then
reports an `rsemUnresolvedGenericParameter` error, as `typeInst` is nil.

The reason for `typeInst` being 'nil' at this point is that
`handleGenericInvocation`, from where the generic `=deepCopy` is
instantiated, only assigns the `typeInst` field *after* the hook
is instantiated.

The `typeInst` field for an instantiation is now set *before*
instantiating the generic `=deepCopy` operator, fixing the issue. In
addition, `typeInst` not being set when trying to instantiate a generic
type-bound operator is changed to an internal error (`unreachable`)
and the `rsemUnresolvedGenericParameter` report is removed.

---------

Co-authored-by: Saem Ghani <saemghani+github@gmail.com>
  • Loading branch information
zerbina and saem authored Sep 12, 2023
1 parent 228e8dd commit f6e6de0
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 11 deletions.
1 change: 0 additions & 1 deletion compiler/ast/report_enums.nim
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,6 @@ type
rsemRawTypeMismatch

rsemCannotConvertTypes
rsemUnresolvedGenericParameter
rsemCannotCreateFlowVarOfType
rsemTypeNotAllowed

Expand Down
3 changes: 0 additions & 3 deletions compiler/front/cli_reporter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -2009,9 +2009,6 @@ proc reportBody*(conf: ConfigRef, r: SemReport): string =
of rsemOnOrOffExpected:
result = "'on' or 'off' expected"

of rsemUnresolvedGenericParameter:
result = "unresolved generic parameter"

of rsemRawTypeMismatch:
result = "type mismatch"

Expand Down
5 changes: 4 additions & 1 deletion compiler/sem/liftdestructors.nim
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import
msgs
],
compiler/utils/[
idioms
],
compiler/sem/[
semdata,
Expand Down Expand Up @@ -976,7 +977,9 @@ proc inst(g: ModuleGraph; c: PContext; t: PType; kind: TTypeAttachedOp; idgen: I
patchBody(g, c, opInst.ast, info, a.idgen)
setAttachedOp(g, idgen.module, t, kind, opInst)
else:
localReport(g.config, info, reportSem(rsemUnresolvedGenericParameter))
# the type's associated ``tyGenericInst`` type was not set properly.
# This hints at an issue in the ``semtypinst`` module.
unreachable()

proc createTypeBoundOps(g: ModuleGraph; c: PContext; orig: PType; info: TLineInfo;
idgen: IdGenerator) =
Expand Down
14 changes: 8 additions & 6 deletions compiler/sem/semtypinst.nim
Original file line number Diff line number Diff line change
Expand Up @@ -494,12 +494,6 @@ proc handleGenericInvocation(cl: var TReplTypeVars, t: PType): PType =
rawAddSon(result, newbody)
checkPartialConstructedType(cl.c.config, cl.info, newbody)
if true:
let dc = cl.c.graph.getAttachedOp(newbody, attachedDeepCopy)
if dc != nil and sfFromGeneric notin dc.flags:
# 'deepCopy' needs to be instantiated for
# generics *when the type is constructed*:
cl.c.graph.setAttachedOp(cl.c.module.position, newbody, attachedDeepCopy,
cl.c.instTypeBoundOp(cl.c, dc, result, cl.info, attachedDeepCopy, 1))
if newbody.typeInst == nil:
# doAssert newbody.typeInst == nil
newbody.typeInst = result
Expand All @@ -512,6 +506,14 @@ proc handleGenericInvocation(cl: var TReplTypeVars, t: PType): PType =
discard
else:
newbody.lastSon.typeInst = result

let dc = cl.c.graph.getAttachedOp(newbody, attachedDeepCopy)
if dc != nil and sfFromGeneric notin dc.flags:
# 'deepCopy' needs to be instantiated for generics *when the type is
# constructed* but *after* the type's `typeInst` field is set:
cl.c.graph.setAttachedOp(cl.c.module.position, newbody, attachedDeepCopy,
cl.c.instTypeBoundOp(cl.c, dc, result, cl.info, attachedDeepCopy, 1))

# DESTROY: adding object|opt for opt[topttree.Tree]
# sigmatch: Formal opt[=destroy.T] real opt[topttree.Tree]
# adding myseq for myseq[system.int]
Expand Down
26 changes: 26 additions & 0 deletions tests/system/tdeepcopy.nim
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,29 @@ doAssert(sizeof(PtrTable) == 2*sizeof(int)+sizeof(pointer)*2*100)

main()
echo "ok"

block generic_deep_copy_using_instantiated_for_type:
# when a generic ``=deepCopy`` implementation used the type instance it
# is instatatied for in its body, an "unresolved generic parameter"
# error would occur, when the generic type also had other generic type-bound
# operators attached
type Generic[T] = object
x: T

var numDestroy {.global.} = 0

proc `=destroy`[T](x: var Generic[T]) =
inc numDestroy

proc `=deepCopy`[T](x: ref Generic[T]): ref Generic[T] =
var v = Generic[T]() # <- forces the generic `=destroy` operator to be
# instantiated
result = x
# `v` is destroyed here, incrementing `numDestroy` by one

let v = new(Generic[int]) # <- the deep-copy operator is instantiated here

# make sure the operator really works:
let other = deepCopy(v)
doAssert v == other
doAssert numDestroy == 1

0 comments on commit f6e6de0

Please sign in to comment.