Skip to content

Commit

Permalink
fix C compilation errors with closures and nested types (#1397)
Browse files Browse the repository at this point in the history
## Summary

Fix a bug with hook synthesis that led to C compiler errors when using
some closure procedures or `ref`s of non-top-level `object` type.

## Details

* `sighashes` now uses the symbol ID as the object representation for
  anonymous object/enum types and object/enum types not defined at the
  top-level
* this means that `hashType(x) != hashType(y)` when
  `sameType(x, y) == false`, for object and enum types
* as a consequence, unique hook procedures are used for `ref T` types
  that used the aforementioned types for `T`
* RTTI, which is assigned to types based on type hashes, also uses
  unique instances per object type now

Multiple same-shaped anonymous/non-top-level object types are *not*
merged into a single one by the C code generator, which due to all
sharing the same set of hook procedures, resulted in C compiler errors
due to implicit conversions between incompatible pointer types.

---------

Co-authored-by: Saem Ghani <[email protected]>
  • Loading branch information
zerbina and saem authored Aug 4, 2024
1 parent ec713a1 commit f5d8ce9
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 22 deletions.
34 changes: 12 additions & 22 deletions compiler/sem/sighashes.nim
Original file line number Diff line number Diff line change
Expand Up @@ -164,28 +164,18 @@ proc hashType(c: var MD5Context, t: PType; flags: set[ConsiderFlag]) =
else:
c.hashSym(t.sym)

var symWithFlags: PSym
template hasFlag(sym): bool =
let ret = {sfAnon, sfGenSym} * sym.flags != {}
if ret: symWithFlags = sym
ret
if hasFlag(t.sym) or (t.kind == tyObject and t.owner.kind == skType and t.owner.typ.kind == tyRef and hasFlag(t.owner)):
# for `PFoo:ObjectType`, arising from `type PFoo = ref object`
# Generated object names can be identical, so we need to
# disambiguate furthermore by hashing the field types and names.
if t.n.len > 0:
let oldFlags = symWithFlags.flags
# Hack to prevent endless recursion
# xxx instead, use a hash table to indicate we've already visited a type, which
# would also be more efficient.
symWithFlags.flags.excl {sfAnon, sfGenSym}
hashTree(c, t.n, flags + {CoHashTypeInsideNode})
symWithFlags.flags = oldFlags
else:
# The object has no fields: we _must_ add something here in order to
# make the hash different from the one we produce by hashing only the
# type name.
c &= ".empty"
if t.sym.flags * {sfAnon, sfGenSym} != {} or
(t.kind == tyObject and t.owner.kind == skType and
tfRefsAnonObj in t.owner.typ.flags):
# one or more of the following are true for the type:
# * it's anonymous
# * it's defined not in the top-level scope
# * it's the object type from a ``ref object`` type construction
# The only property that uniquely identifies the type in this case is
# the symbol ID, so we use that. **This means that the hash produced
# for such types is dependent on the type's surroundings**
c &= "."
c &= $t.sym.id
else:
c &= t.id
if t.len > 0 and t[0] != nil:
Expand Down
21 changes: 21 additions & 0 deletions tests/lang_objects/destructor/tseparate_hooks1.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
discard """
description: '''
Ensure that separate hooks are created for ``ref T`` types where T are non-
top-level object types sharing the exact same name and shape
'''
targets: "c js vm"
"""

# XXX: this currently relies on the backend C compiler complaining. Eventually,
# the test should inspect the MIR output and make sure two different
# destroy hooks are used

block:
type Nested = object

var a = (ref Nested)()

block:
type Nested = object

var b = (ref Nested)()
31 changes: 31 additions & 0 deletions tests/lang_objects/destructor/tseparate_hooks2.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
discard """
description: '''
Ensure that separate hooks are created for ``ref T`` types where T are non-
top-level object types sharing the exact same name and shape
'''
targets: "c js vm"
"""

# XXX: this currently relies on the backend C compiler complaining. Eventually,
# the test should inspect the MIR output and make sure two different
# destroy hooks are used

# for this test, both procedures must:
# * share the same user-provided name
# * create an anonymous environment object with the exact same shape and field
# names

proc outer(x: int) =
var x = 1
proc inner() =
x = 2
inner()

proc outer(x: float) =
var x = 1
proc inner() =
x = 2
inner()

outer(1)
outer(1.0)

0 comments on commit f5d8ce9

Please sign in to comment.