diff options
| -rw-r--r-- | compiler/ast.nim | 1 | ||||
| -rw-r--r-- | compiler/msgs.nim | 2 | ||||
| -rw-r--r-- | compiler/sempass2.nim | 48 | ||||
| -rw-r--r-- | tests/parallel/tgc_unsafe2.nim | 39 | ||||
| -rw-r--r-- | todo.txt | 1 |
5 files changed, 81 insertions, 10 deletions
diff --git a/compiler/ast.nim b/compiler/ast.nim index 427d12643..6e09916fe 100644 --- a/compiler/ast.nim +++ b/compiler/ast.nim @@ -728,6 +728,7 @@ type typScope*: PScope of routineKinds: procInstCache*: seq[PInstantiation] + gcUnsafetyReason*: PSym # for better error messages wrt gcsafe #scope*: PScope # the scope where the proc was defined of skModule: # modules keep track of the generic symbols they use from other modules. diff --git a/compiler/msgs.nim b/compiler/msgs.nim index 22ef9dc30..d9db1f670 100644 --- a/compiler/msgs.nim +++ b/compiler/msgs.nim @@ -387,7 +387,7 @@ const warnProveField: "cannot prove that field '$1' is accessible [ProveField]", warnProveIndex: "cannot prove index '$1' is valid [ProveIndex]", warnGcUnsafe: "not GC-safe: '$1' [GcUnsafe]", - warnGcUnsafe2: "cannot prove '$1' is GC-safe. Does not compile with --threads:on.", + warnGcUnsafe2: "$1", warnUninit: "'$1' might not have been initialized [Uninit]", warnGcMem: "'$1' uses GC'ed memory [GcMem]", warnDestructor: "usage of a type with a destructor in a non destructible context. This will become a compile time error in the future. [Destructor]", diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim index d49945c1c..6928dbaf4 100644 --- a/compiler/sempass2.nim +++ b/compiler/sempass2.nim @@ -194,8 +194,38 @@ proc warnAboutGcUnsafe(n: PNode) = #assert false message(n.info, warnGcUnsafe, renderTree(n)) -template markGcUnsafe(a: PEffects) = +proc markGcUnsafe(a: PEffects; reason: PSym) = a.gcUnsafe = true + if a.owner.kind in routineKinds: a.owner.gcUnsafetyReason = reason + +proc markGcUnsafe(a: PEffects; reason: PNode) = + a.gcUnsafe = true + if a.owner.kind in routineKinds: + if reason.kind == nkSym: + a.owner.gcUnsafetyReason = reason.sym + else: + a.owner.gcUnsafetyReason = newSym(skUnknown, getIdent("<unknown>"), + a.owner, reason.info) + +proc listGcUnsafety(s: PSym; onlyWarning: bool) = + let u = s.gcUnsafetyReason + if u != nil: + let msgKind = if onlyWarning: warnGcUnsafe2 else: errGenerated + if u.kind in {skLet, skVar}: + message(s.info, msgKind, + ("'$#' is not GC-safe as it accesses '$#'" & + " which is a global using GC'ed memory") % [s.name.s, u.name.s]) + elif u.kind in routineKinds: + # recursive call *always* produces only a warning so the full error + # message is printed: + listGcUnsafety(u, true) + message(s.info, msgKind, + "'$#' is not GC-safe as it calls '$#'" % + [s.name.s, u.name.s]) + else: + internalAssert u.kind == skUnknown + message(u.info, msgKind, + "'$#' is not GC-safe as it performs an indirect call here" % s.name.s) proc useVar(a: PEffects, n: PNode) = let s = n.sym @@ -211,7 +241,7 @@ proc useVar(a: PEffects, n: PNode) = if s.guard != nil: guardGlobal(a, n, s.guard) if (tfHasGCedMem in s.typ.flags or s.typ.isGCedMem): #if warnGcUnsafe in gNotes: warnAboutGcUnsafe(n) - markGcUnsafe(a) + markGcUnsafe(a, s) type TIntersection = seq[tuple[id, count: int]] # a simple count table @@ -450,7 +480,7 @@ proc propagateEffects(tracked: PEffects, n: PNode, s: PSym) = if notGcSafe(s.typ) and sfImportc notin s.flags: if warnGcUnsafe in gNotes: warnAboutGcUnsafe(n) - markGcUnsafe(tracked) + markGcUnsafe(tracked, s) mergeLockLevels(tracked, n, s.getLockLevel) proc notNilCheck(tracked: PEffects, n: PNode, paramType: PType) = @@ -504,13 +534,13 @@ proc trackOperand(tracked: PEffects, n: PNode, paramType: PType) = # assume GcUnsafe unless in its type; 'forward' does not matter: if notGcSafe(op) and not isOwnedProcVar(a, tracked.owner): if warnGcUnsafe in gNotes: warnAboutGcUnsafe(n) - markGcUnsafe(tracked) + markGcUnsafe(tracked, a) else: mergeEffects(tracked, effectList.sons[exceptionEffects], n) mergeTags(tracked, effectList.sons[tagEffects], n) if notGcSafe(op): if warnGcUnsafe in gNotes: warnAboutGcUnsafe(n) - markGcUnsafe(tracked) + markGcUnsafe(tracked, a) notNilCheck(tracked, n, paramType) proc breaksBlock(n: PNode): bool = @@ -658,7 +688,7 @@ proc track(tracked: PEffects, n: PNode) = # and it's not a recursive call: if not (a.kind == nkSym and a.sym == tracked.owner): warnAboutGcUnsafe(n) - markGcUnsafe(tracked) + markGcUnsafe(tracked, a) for i in 1 .. <len(n): trackOperand(tracked, n.sons[i], paramType(op, i)) if a.kind == nkSym and a.sym.magic in {mNew, mNewFinalize, mNewSeq}: # may not look like an assignment, but it is: @@ -853,9 +883,11 @@ proc trackProc*(s: PSym, body: PNode) = if sfThread in s.flags and t.gcUnsafe: if optThreads in gGlobalOptions and optThreadAnalysis in gGlobalOptions: - localError(s.info, "'$1' is not GC-safe" % s.name.s) + #localError(s.info, "'$1' is not GC-safe" % s.name.s) + listGcUnsafety(s, onlyWarning=false) else: - localError(s.info, warnGcUnsafe2, s.name.s) + listGcUnsafety(s, onlyWarning=true) + #localError(s.info, warnGcUnsafe2, s.name.s) if not t.gcUnsafe: s.typ.flags.incl tfGcSafe if s.typ.lockLevel == UnspecifiedLockLevel: diff --git a/tests/parallel/tgc_unsafe2.nim b/tests/parallel/tgc_unsafe2.nim new file mode 100644 index 000000000..ec4605fe9 --- /dev/null +++ b/tests/parallel/tgc_unsafe2.nim @@ -0,0 +1,39 @@ +discard """ + line: 28 + nimout: '''tgc_unsafe2.nim(22, 5) Warning: 'trick' is not GC-safe as it accesses 'global' which is a global using GC'ed memory +tgc_unsafe2.nim(26, 5) Warning: 'track' is not GC-safe as it calls 'trick' +tgc_unsafe2.nim(28, 5) Error: 'consumer' is not GC-safe as it calls 'track' +''' + errormsg: "'consumer' is not GC-safe as it calls 'track'" +""" + +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 trick(ix: int) = + echo global.x + echo channels[ix].recv() + +proc track(ix: int) = trick(ix) + +proc consumer(ix: int) {.thread.} = + track(ix) + +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/todo.txt b/todo.txt index fc62385a8..c10406979 100644 --- a/todo.txt +++ b/todo.txt @@ -1,7 +1,6 @@ version 0.10.4 ============== -- improve GC-unsafety warnings - make 'nil' work for 'add' and 'len' - overloading of '=' |