Skip to content

Commit

Permalink
fix(typeops): do not copy non-existent type fields (nim-works#832)
Browse files Browse the repository at this point in the history
## Summary

During liftdestructors, do not produce hidden type field copies for
types without such fields, preventing invalid code from being generated.

## Details

`liftdestructors` produces symbols as it proceeds via `produceSym`. This
procedure not only produces a symbol, but ensures it's initialized,
type operations ("destructors") are attached, and necessary ops called.
One such operation is copying of the hidden type field for objects types
that have them.

This latter opertation was guarded by `isObjLackingTypeField` that
expects any type passed to it be unwrapped (i.e. removing `tyDistinct`,
`tyAlias`, etc). That wasn't the case and so `liftdestructors` was
producing copy operations for hidden type fields that didn't exist. Now
the type passed to `isObjLackingTypeField` is first unwrapped.

A test, `ttypeops_misc`, has also been added for this (contributed by
@zerbina).

---------

Co-authored-by: zerbina <100542850+zerbina@users.noreply.github.com>
  • Loading branch information
saem and zerbina authored Aug 8, 2023
1 parent 2290a4e commit 48a07fb
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 6 deletions.
14 changes: 8 additions & 6 deletions compiler/sem/liftdestructors.nim
Original file line number Diff line number Diff line change
Expand Up @@ -904,11 +904,13 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
result.ast[bodyPos].add newOpCall(a, getAttachedOp(g, typ, attachedDestructor), d[0])
result.ast[bodyPos].add newAsgnStmt(d, src)
else:
var tk: TTypeKind
if g.config.selectedGC in {gcArc, gcOrc}:
tk = skipTypes(typ, {tyOrdinal, tyRange, tyInferred, tyGenericInst, tyStatic, tyAlias, tySink}).kind
else:
tk = tyNone # no special casing for strings and seqs
let (tk, baseTyp) =
if g.config.selectedGC in {gcArc, gcOrc}:
let t = skipTypes(typ, {tyOrdinal, tyRange, tyInferred, tyGenericInst,
tyStatic, tyAlias, tySink})
(t.kind, t)
else:
(tyNone, typ) # no special casing for strings and seqs
case tk
of tySequence:
fillSeqOp(a, typ, result.ast[bodyPos], d, src)
Expand All @@ -917,7 +919,7 @@ proc produceSym(g: ModuleGraph; c: PContext; typ: PType; kind: TTypeAttachedOp;
else:
fillBody(a, typ, result.ast[bodyPos], d, src)
if tk == tyObject and a.kind in {attachedAsgn, attachedSink, attachedDeepCopy} and
not isObjLackingTypeField(typ):
not isObjLackingTypeField(baseTyp):
# bug #19205: Do not forget to also copy the hidden type field:
genTypeFieldCopy(a, typ, result.ast[bodyPos], d, src)

Expand Down
24 changes: 24 additions & 0 deletions tests/lang_objects/destructor/ttypeops_misc.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
discard """
"""

block ensure_lifting_is_not_confused_by_type_wrappers:
## previously this led to a code gen bug where a non-existent hidden type
## field was being copied. the various wrappers alias/distinct weren't being
## skipped when testing to see if the underlying type was an object.
type WithHooks = object

proc `=destroy`(x: var WithHooks) =
# force `WithHooks` to require lifetime hooks
discard

type
Object = object
x: WithHooks # make sure that `Object` gets a synthesized copy hook
Alias = Object
Distinct = distinct Alias

proc test(y: Distinct) =
var x = y
discard (addr x) # prevent `x` being turned into a cursor

test(default(Distinct))

0 comments on commit 48a07fb

Please sign in to comment.