Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix C compilation errors with closures and nested types #1397

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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):
Comment on lines +167 to +169
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the top level check wouldn't it make sense to check if the owning symbol is not an skModule, or would that be a problem for generic instantiations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing whether the owner is an skModule symbol wouldn't be able to detect the Nested type from tseparate_hooks1.nim (where the owner is a module symbol).

sem marks such types' symbols with the sfGenSym at the moment, so that they can be detected here. The only proper solution to this mess is getting rid of sighashes, really.

# 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)
Loading