From bc264618f532013e8816a824fb41a6b1cfbee712 Mon Sep 17 00:00:00 2001 From: Araq Date: Mon, 16 Mar 2015 22:39:48 +0100 Subject: [PATCH] fixes #2257 --- compiler/ast.nim | 8 +++- compiler/sempass2.nim | 59 +++++++++++++-------------- tests/parallel/tarray_of_channels.nim | 26 ++++++++++++ tests/parallel/tgc_unsafe.nim | 32 +++++++++++++++ 4 files changed, 93 insertions(+), 32 deletions(-) create mode 100644 tests/parallel/tarray_of_channels.nim create mode 100644 tests/parallel/tgc_unsafe.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index 0c9f4bffc4..274a49b527 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -1335,8 +1335,12 @@ proc propagateToOwner*(owner, elem: PType) = if elem.isMetaType: owner.flags.incl tfHasMeta - if owner.kind != tyProc: - if elem.isGCedMem or tfHasGCedMem in elem.flags: + if owner.kind notin {tyProc, tyGenericInst, tyGenericBody, + tyGenericInvocation}: + let elemB = elem.skipTypes({tyGenericInst}) + if elemB.isGCedMem or tfHasGCedMem in elemB.flags: + # for simplicity, we propagate this flag even to generics. We then + # ensure this doesn't bite us in sempass2. owner.flags.incl tfHasGCedMem proc rawAddSon*(father, son: PType) = diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index 14644a8d69..b8820d85c6 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -8,12 +8,12 @@ # import - intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, trees, + intsets, ast, astalgo, msgs, renderer, magicsys, types, idents, trees, wordrecg, strutils, options, guards # Second semantic checking pass over the AST. Necessary because the old # way had some inherent problems. Performs: -# +# # * effect+exception tracking # * "usage before definition" checking # * checks for invalid usages of compiletime magics (not implemented) @@ -23,7 +23,7 @@ import # Predefined effects: # io, time (time dependent), gc (performs GC'ed allocation), exceptions, # side effect (accesses global), store (stores into *type*), -# store_unknown (performs some store) --> store(any)|store(x) +# store_unknown (performs some store) --> store(any)|store(x) # load (loads from *type*), recursive (recursive call), unsafe, # endless (has endless loops), --> user effects are defined over *patterns* # --> a TR macro can annotate the proc with user defined annotations @@ -31,31 +31,31 @@ import # Load&Store analysis is performed on *paths*. A path is an access like # obj.x.y[i].z; splitting paths up causes some problems: -# +# # var x = obj.x # var z = x.y[i].z # # Alias analysis is affected by this too! A good solution is *type splitting*: -# T becomes T1 and T2 if it's known that T1 and T2 can't alias. -# +# T becomes T1 and T2 if it's known that T1 and T2 can't alias. +# # An aliasing problem and a race condition are effectively the same problem. # Type based alias analysis is nice but not sufficient; especially splitting # an array and filling it in parallel should be supported but is not easily # done: It essentially requires a built-in 'indexSplit' operation and dependent # typing. - + # ------------------------ exception and tag tracking ------------------------- discard """ exception tracking: - + a() # raises 'x', 'e' try: b() # raises 'e' except e: # must not undo 'e' here; hrm c() - + --> we need a stack of scopes for this analysis # XXX enhance the algorithm to care about 'dirty' expressions: @@ -209,8 +209,7 @@ proc useVar(a: PEffects, n: PNode) = a.init.add s.id if {sfGlobal, sfThread} * s.flags == {sfGlobal} and s.kind in {skVar, skLet}: if s.guard != nil: guardGlobal(a, n, s.guard) - if (tfHasGCedMem in s.typ.flags or s.typ.isGCedMem) and - tfGcSafe notin s.typ.flags: + if (tfHasGCedMem in s.typ.flags or s.typ.isGCedMem): if warnGcUnsafe in gNotes: warnAboutGcUnsafe(n) markGcUnsafe(a) @@ -321,7 +320,7 @@ proc trackTryStmt(tracked: PEffects, n: PNode) = dec tracked.inTryStmt for i in oldState.. = toCover: tracked.init.add id # else we can't merge as it is not exhaustive setLen(tracked.guards, oldFacts) - + proc trackBlock(tracked: PEffects, n: PNode) = if n.kind in {nkStmtList, nkStmtListExpr}: var oldState = -1 @@ -782,7 +781,7 @@ proc checkMethodEffects*(disp, branch: PSym) = checkRaisesSpec(tagsSpec, actual.sons[tagEffects], "can have an unlisted effect: ", hints=off, subtypeRelation) if sfThread in disp.flags and notGcSafe(branch.typ): - localError(branch.info, "base method is GC-safe, but '$1' is not" % + localError(branch.info, "base method is GC-safe, but '$1' is not" % branch.name.s) if branch.typ.lockLevel > disp.typ.lockLevel: when true: @@ -814,14 +813,14 @@ proc initEffects(effects: PNode; s: PSym; t: var TEffects) = newSeq(effects.sons, effectListLen) effects.sons[exceptionEffects] = newNodeI(nkArgList, s.info) effects.sons[tagEffects] = newNodeI(nkArgList, s.info) - + t.exc = effects.sons[exceptionEffects] t.tags = effects.sons[tagEffects] t.owner = s t.init = @[] t.guards = @[] t.locked = @[] - + proc trackProc*(s: PSym, body: PNode) = var effects = s.typ.n.sons[0] internalAssert effects.kind == nkEffectList diff --git a/tests/parallel/tarray_of_channels.nim b/tests/parallel/tarray_of_channels.nim new file mode 100644 index 0000000000..11b5234017 --- /dev/null +++ b/tests/parallel/tarray_of_channels.nim @@ -0,0 +1,26 @@ +# bug #2257 +import threadpool + +type StringChannel = TChannel[string] +var channels: array[1..3, StringChannel] + +type + MyObject[T] = object + x: T + +var global: MyObject[string] +var globalB: MyObject[float] + +proc consumer(ix : int) {.thread.} = + echo channels[ix].recv() ###### not GC-safe: 'channels' + echo globalB + +proc main = + for ix in 1..3: channels[ix].open() + for ix in 1..3: spawn consumer(ix) + for ix in 1..3: channels[ix].send("test") + sync() + for ix in 1..3: channels[ix].close() + +when isMainModule: + main() diff --git a/tests/parallel/tgc_unsafe.nim b/tests/parallel/tgc_unsafe.nim new file mode 100644 index 0000000000..6548bbec81 --- /dev/null +++ b/tests/parallel/tgc_unsafe.nim @@ -0,0 +1,32 @@ +discard """ + errormsg: "'consumer' is not GC-safe" + line: 19 +""" + +# bug #2257 +import threadpool + +type StringChannel = TChannel[string] +var channels: array[1..3, StringChannel] + +type + MyObject[T] = object + x: T + +var global: MyObject[string] +var globalB: MyObject[float] + +proc consumer(ix : int) {.thread.} = + echo channels[ix].recv() ###### not GC-safe: 'channels' + echo global + echo globalB + +proc main = + for ix in 1..3: channels[ix].open() + for ix in 1..3: spawn consumer(ix) + for ix in 1..3: channels[ix].send("test") + sync() + for ix in 1..3: channels[ix].close() + +when isMainModule: + main()