better effects handling for callbacks

4 files changed, 63 insertions, 6 deletions

diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim
index f43820fa7..9213bd48d 100644
--- a/compiler/sempass2.nim
+++ b/compiler/sempass2.nim
@@ -174,7 +174,10 @@ proc trackTryStmt(tracked: PEffects, n: PNode) =
   tracked.bottom = oldBottom
 
 proc isIndirectCall(n: PNode): bool =
-  result = n.kind != nkSym or n.sym.kind notin routineKinds
+  # we don't count f(...) as an indirect call if 'f' is an parameter.
+  # Instead we track expressions of type tyProc too. See the manual for
+  # details:
+  result = n.kind != nkSym or n.sym.kind notin (routineKinds+{skParam})
 
 proc isForwardedProc(n: PNode): bool =
   result = n.kind == nkSym and sfForward in n.sym.flags
@@ -236,6 +239,24 @@ proc propagateEffects(tracked: PEffects, n: PNode, s: PSym) =
   let tagSpec = effectSpec(pragma, wTags)
   mergeTags(tracked, tagSpec, n)
 
+proc trackOperand(tracked: PEffects, n: PNode) =
+  let op = 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]
+    let s = n.skipConv
+    if s.kind == nkSym and s.sym.kind in routineKinds:
+      propagateEffects(tracked, n, s.sym)
+    elif effectList.len == 0:
+      if isForwardedProc(n):
+        propagateEffects(tracked, n, n.sym)
+      else:
+        addEffect(tracked, createRaise(n))
+        addTag(tracked, createTag(n))
+    else:
+      mergeEffects(tracked, effectList.sons[exceptionEffects], n)
+      mergeTags(tracked, effectList.sons[tagEffects], n)
+
 proc track(tracked: PEffects, n: PNode) =
   case n.kind
   of nkRaiseStmt:
@@ -259,6 +280,7 @@ proc track(tracked: PEffects, n: PNode) =
       else:
         mergeEffects(tracked, effectList.sons[exceptionEffects], n)
         mergeTags(tracked, effectList.sons[tagEffects], n)
+    for i in 1 .. <len(n): trackOperand(tracked, n.sons[i])
   of nkTryStmt:
     trackTryStmt(tracked, n)
     return
diff --git a/doc/manual.txt b/doc/manual.txt
index 132f6d038..22aa22266 100644
--- a/doc/manual.txt
+++ b/doc/manual.txt
@@ -2909,15 +2909,37 @@ possibly raised exceptions; the algorithm operates on ``p``'s call graph:
 1. Every indirect call via some proc type ``T`` is assumed to
    raise ``system.E_Base`` (the base type of the exception hierarchy) and
    thus any exception unless ``T`` has an explicit ``raises`` list.
-2. Every call to a proc ``q`` which has an unknown body (due to a forward 
+   However if the call is of the form ``f(...)`` where ``f`` is a parameter
+   is ignored. Rule 2 compensates for this case.
+2. Every expression of some proc type wihtin a call that is not a call 
+   itself (and not nil) is assumed to be called indirectly somehow and thus 
+   its raises list is added to ``p``'s raises list.
+3. Every call to a proc ``q`` which has an unknown body (due to a forward 
    declaration or an ``importc`` pragma) is assumed to 
    raise ``system.E_Base`` unless ``q`` has an explicit ``raises`` list.
-3. Every call to a method ``m`` is assumed to 
+4. Every call to a method ``m`` is assumed to 
    raise ``system.E_Base`` unless ``m`` has an explicit ``raises`` list.
-4. For every other call the analysis can determine an exact ``raises`` list.
-5. For determining a ``raises`` list, the ``raise`` and ``try`` statements 
+5. For every other call the analysis can determine an exact ``raises`` list.
+6. For determining a ``raises`` list, the ``raise`` and ``try`` statements 
    of ``p`` are taken into consideration.
 
+Rules 1-2 ensure the following works: 
+
+.. code-block:: nimrod
+  proc noRaise(x: proc()) {.raises: [].} =
+    # unknown call that might raise anything, but valid:
+    x()
+    
+  proc doRaise() {.raises: [EIO].} =
+    raise newException(EIO, "IO")
+  
+  proc use() =
+    noRaise(doRaise)
+    # Here the compiler inferes that EIO can be raised.
+
+So in many cases a callback does not cause the compiler to be overly
+conservative in its effect analysis.
+
 
 Tag tracking
 ------------
diff --git a/tests/compile/teffects1.nim b/tests/compile/teffects1.nim
index 49af28469..54200f2c3 100644
--- a/tests/compile/teffects1.nim
+++ b/tests/compile/teffects1.nim
@@ -15,3 +15,17 @@ createMenuItem(s, "Go to definition...",
           echo("blah")
 )
 
+
+proc noRaise(x: proc()) {.raises: [].} =
+  # unknown call that might raise anything, but valid:
+  x()
+  
+proc doRaise() {.raises: [EIO].} =
+  raise newException(EIO, "IO")
+
+proc use*() =
+  noRaise(doRaise)
+  # Here the compiler inferes that EIO can be raised.
+
+
+use()
diff --git a/todo.txt b/todo.txt
index 764ba1e13..2f1105df4 100644
--- a/todo.txt
+++ b/todo.txt
@@ -28,7 +28,6 @@ version 0.9.4
 =============
 
 - macros as type pragmas
-- effect propagation for callbacks
 - provide tool/API to track leaks/object counts
 - hybrid GC
 - use big blocks in the allocator