fixes 'gcsafe'

8 files changed, 56 insertions, 22 deletions

diff --git a/compiler/astalgo.nim b/compiler/astalgo.nim
index dbf13f764..3431aaf41 100644
--- a/compiler/astalgo.nim
+++ b/compiler/astalgo.nim
@@ -124,6 +124,17 @@ proc skipConv*(n: PNode): PNode =
     result = n.sons[1]
   else: result = n
 
+proc skipConvAndClosure*(n: PNode): PNode =
+  result = n
+  while true:
+    case result.kind
+    of nkObjUpConv, nkObjDownConv, nkChckRange, nkChckRangeF, nkChckRange64,
+       nkClosure:
+      result = result.sons[0]
+    of nkHiddenStdConv, nkHiddenSubConv, nkConv:
+      result = result.sons[1]
+    else: break
+
 proc skipConvTakeType*(n: PNode): PNode =
   result = n.skipConv
   result.typ = n.typ
diff --git a/compiler/options.nim b/compiler/options.nim
index 58a340d21..02719cacc 100644
--- a/compiler/options.nim
+++ b/compiler/options.nim
@@ -95,7 +95,7 @@ var
                          optBoundsCheck, optOverflowCheck, optAssert, optWarns, 
                          optHints, optStackTrace, optLineTrace,
                          optPatterns, optNilCheck}
-  gGlobalOptions*: TGlobalOptions = {}
+  gGlobalOptions*: TGlobalOptions = {optThreadAnalysis}
   gExitcode*: int8
   gCmd*: TCommands = cmdNone  # the command
   gSelectedGC* = gcRefc       # the selected GC
diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim
index c8ce5e787..acc2425f1 100644
--- a/compiler/sempass2.nim
+++ b/compiler/sempass2.nim
@@ -115,7 +115,7 @@ proc useVar(a: PEffects, n: PNode) =
       a.addUse(copyNode(n))
     if (tfHasGCedMem in s.typ.flags or s.typ.isGCedMem) and 
         tfGcSafe notin s.typ.flags:
-      message(n.info, warnGcUnsafe, renderTree(n))
+      if warnGcUnsafe in gNotes: message(n.info, warnGcUnsafe, renderTree(n))
       a.gcUnsafe = true
 
 type
@@ -332,7 +332,7 @@ proc propagateEffects(tracked: PEffects, n: PNode, s: PSym) =
   mergeTags(tracked, tagSpec, n)
 
   if notGcSafe(s.typ) and sfImportc notin s.flags:
-    message(n.info, warnGcUnsafe, renderTree(n))
+    if warnGcUnsafe in gNotes: message(n.info, warnGcUnsafe, renderTree(n))
     tracked.gcUnsafe = true
 
   when trackGlobals:
@@ -358,7 +358,7 @@ proc notNilCheck(tracked: PEffects, n: PNode, paramType: PType) =
     of impYes: discard
 
 proc trackOperand(tracked: PEffects, n: PNode, paramType: PType) =
-  let op = n.typ
+  let op = skipConvAndClosure(n).typ
   if op != nil and op.kind == tyProc and n.kind != nkNilLit:
     internalAssert op.n.sons[0].kind == nkEffectList
     var effectList = op.n.sons[0]
@@ -367,21 +367,24 @@ proc trackOperand(tracked: PEffects, n: PNode, paramType: PType) =
       propagateEffects(tracked, n, s.sym)
     elif effectList.len == 0:
       if isForwardedProc(n):
+        # we have no explicit effects but it's a forward declaration and so it's
+        # stated there are no additional effects, so simply propagate them:
         propagateEffects(tracked, n, n.sym)
       else:
+        # we have no explicit effects so assume the worst:
         addEffect(tracked, createRaise(n))
         addTag(tracked, createTag(n))
         when trackGlobals: addUse(tracked, createAnyGlobal(n))
-      # assume GcUnsafe unless in its type:
-      if notGcSafe(op): 
-        message(n.info, warnGcUnsafe, renderTree(n))
+      # assume GcUnsafe unless in its type; 'forward' does not matter:
+      if notGcSafe(op):
+        if warnGcUnsafe in gNotes: message(n.info, warnGcUnsafe, renderTree(n))
         tracked.gcUnsafe = true
     else:
       mergeEffects(tracked, effectList.sons[exceptionEffects], n)
       mergeTags(tracked, effectList.sons[tagEffects], n)
       when trackGlobals: mergeUses(tracked, effectList.sons[usesEffects], n)
       if notGcSafe(op):
-        message(n.info, warnGcUnsafe, renderTree(n))
+        if warnGcUnsafe in gNotes: message(n.info, warnGcUnsafe, renderTree(n))
         tracked.gcUnsafe = true
   notNilCheck(tracked, n, paramType)
 
@@ -510,9 +513,6 @@ proc track(tracked: PEffects, n: PNode) =
     if op != nil and op.kind == tyProc and op.n.sons[0].kind == nkEffectList:
       if a.kind == nkSym and a.sym == tracked.owner:
         tracked.isRecursive = true
-      elif notGcSafe(op) and not importedFromC(a):
-        message(n.info, warnGcUnsafe, renderTree(n))
-        tracked.gcUnsafe = true
       var effectList = op.n.sons[0]
       if a.kind == nkSym and a.sym.kind == skMethod:
         propagateEffects(tracked, n, a.sym)
@@ -528,6 +528,11 @@ proc track(tracked: PEffects, n: PNode) =
         mergeEffects(tracked, effectList.sons[exceptionEffects], n)
         mergeTags(tracked, effectList.sons[tagEffects], n)
         when trackGlobals: mergeUses(tracked, effectList.sons[usesEffects], n)
+        if notGcSafe(op) and not importedFromC(a):
+          # and it's not a recursive call:
+          if not (a.kind == nkSym and a.sym == tracked.owner):
+            message(n.info, warnGcUnsafe, renderTree(n))
+            tracked.gcUnsafe = true
     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:
@@ -713,8 +718,8 @@ proc trackProc*(s: PSym, body: PNode) =
       effects.sons[usesEffects] = usesSpec
   if optThreadAnalysis in gGlobalOptions:
     if sfThread in s.flags and t.gcUnsafe:
-      localError(s.info, warnGcUnsafe2, s.name.s)
-      #localError(s.info, "'$1' is not GC-safe" % s.name.s)
+      #localError(s.info, warnGcUnsafe2, s.name.s)
+      localError(s.info, "'$1' is not GC-safe" % s.name.s)
     if not t.gcUnsafe: s.typ.flags.incl tfGcSafe
 
 proc trackTopLevelStmt*(module: PSym; n: PNode) =
diff --git a/lib/system.nim b/lib/system.nim
index 03275b8fa..66eda5741 100644
--- a/lib/system.nim
+++ b/lib/system.nim
@@ -1997,16 +1997,16 @@ when not defined(nimrodVM) and hostOS != "standalone":
     ## returns an informative string about the GC's activity. This may be useful
     ## for tweaking.
     
-  proc GC_ref*[T](x: ref T) {.magic: "GCref".}
-  proc GC_ref*[T](x: seq[T]) {.magic: "GCref".}
-  proc GC_ref*(x: string) {.magic: "GCref".}
+  proc GC_ref*[T](x: ref T) {.magic: "GCref", gcsafe.}
+  proc GC_ref*[T](x: seq[T]) {.magic: "GCref", gcsafe.}
+  proc GC_ref*(x: string) {.magic: "GCref", gcsafe.}
     ## marks the object `x` as referenced, so that it will not be freed until
     ## it is unmarked via `GC_unref`. If called n-times for the same object `x`,
     ## n calls to `GC_unref` are needed to unmark `x`. 
     
-  proc GC_unref*[T](x: ref T) {.magic: "GCunref".}
-  proc GC_unref*[T](x: seq[T]) {.magic: "GCunref".}
-  proc GC_unref*(x: string) {.magic: "GCunref".}
+  proc GC_unref*[T](x: ref T) {.magic: "GCunref", gcsafe.}
+  proc GC_unref*[T](x: seq[T]) {.magic: "GCunref", gcsafe.}
+  proc GC_unref*(x: string) {.magic: "GCunref", gcsafe.}
     ## see the documentation of `GC_ref`.
 
 template accumulateResult*(iter: expr) =
diff --git a/tests/effects/teffects1.nim b/tests/effects/teffects1.nim
index 0014cff46..f73230ff9 100644
--- a/tests/effects/teffects1.nim
+++ b/tests/effects/teffects1.nim
@@ -1,5 +1,5 @@
 discard """
-  line: 1913
+  line: 2136
   file: "system.nim"
   errormsg: "can raise an unlisted exception: ref EIO"
 """
diff --git a/tests/effects/tgcsafe.nim b/tests/effects/tgcsafe.nim
new file mode 100644
index 000000000..87388238a
--- /dev/null
+++ b/tests/effects/tgcsafe.nim
@@ -0,0 +1,16 @@
+discard """
+  line: 15
+  errormsg: "'mainUnsafe' is not GC-safe"
+"""
+
+proc mymap(x: proc ()) =
+  x()
+
+var
+  myglob: string
+
+proc mainSafe() {.gcsafe.} =
+  mymap(proc () = echo "foo")
+
+proc mainUnsafe() {.gcsafe.} =
+  mymap(proc () = myglob = "bar"; echo "foo", myglob)
diff --git a/todo.txt b/todo.txt
index 05426d71d..d8085226d 100644
--- a/todo.txt
+++ b/todo.txt
@@ -2,13 +2,11 @@ version 0.9.6
 =============
 
 - scopes are still broken for generic instantiation!
-- start experimental branch
 - implicit deref for parameter matching
 
 Concurrency
 -----------
 
-- 'gcsafe' inference needs to be fixed
 - 'deepCopy' needs to be instantiated for
   generics *when the type is constructed*
 - test 'deepCopy'
diff --git a/web/news.txt b/web/news.txt
index b194d5503..2d7086be6 100644
--- a/web/news.txt
+++ b/web/news.txt
@@ -26,6 +26,10 @@ News
   - **system.defined has been split into system.defined and system.declared**.
     You have to use ``--symbol`` to declare new conditional symbols that can be
     set via ``--define``.
+  - ``--threadanalysis:on`` is now the default. To make your program compile
+    you can disable it but this is only a temporary solution as this option
+    will disappear soon!
+  
 
   Language Additions
   ------------------