further improvements for thread analysis

compiler/passes.nim

@@ -13,9 +13,9 @@
 import 
   strutils, lists, options, ast, astalgo, llstream, msgs, platform, os, 
   condsyms, idents, renderer, types, extccomp, math, magicsys, nversion, 
-  nimsets, syntaxes, times, rodread
+  nimsets, syntaxes, times, rodread, semthreads
 
-type 
+type
   TPassContext* = object of TObject # the pass's context
   PPassContext* = ref TPassContext
   TPass* = tuple[
@@ -60,8 +60,8 @@ proc astNeeded*(s: PSym): bool =
       ({sfCompilerProc, sfCompileTime} * s.flags == {}) and
       (s.typ.callConv != ccInline) and 
       (s.ast.sons[genericParamsPos].kind == nkEmpty): 
-    result = false
-  else: 
+    result = semthreads.needsGlobalAnalysis()
+  else:
     result = true
   
 const 

compiler/sem.nim

@@ -199,6 +199,10 @@ proc myProcess(context: PPassContext, n: PNode): PNode =
       RecoverContext(c)
       result = ast.emptyNode
   
+proc checkThreads(c: PContext) =
+  for i in 0 .. c.threadEntries.len-1:
+    semthreads.AnalyseThread(c.threadEntries[i])
+  
 proc myClose(context: PPassContext, n: PNode): PNode = 
   var c = PContext(context)
   closeScope(c.tab)           # close module's scope
@@ -208,6 +212,7 @@ proc myClose(context: PPassContext, n: PNode): PNode =
   else: 
     InternalError(n.info, "n is not nil") #result := n;
   addCodeForGenerics(c, result)
+  checkThreads(c)
   popOwner()
   popProcCon(c)
 

compiler/semdata.nim

@@ -40,6 +40,7 @@ type
     generics*: PNode          # a list of the things to compile; list of
                               # nkExprEqExpr nodes which contain the
                               # generic symbol and the instantiated symbol
+    threadEntries*: PNode     # list of thread entries to check
     lastGenericIdx*: int      # used for the generics stack
     tab*: TSymTab             # each module has its own symbol table
     AmbiguousSymbols*: TIntSet # ids of all ambiguous symbols (cannot
@@ -123,6 +124,7 @@ proc newContext(module: PSym, nimfile: string): PContext =
   append(result.optionStack, newOptionEntry())
   result.module = module
   result.generics = newNode(nkStmtList)
+  result.threadEntries = newNode(nkStmtList)
   result.converters = @[]
   result.filename = nimfile
   IntSetInit(result.includedFiles)

compiler/semexprs.nim

@@ -568,11 +568,8 @@ proc semMagic(c: PContext, n: PNode, s: PSym, flags: TExprFlags): PNode =
   of mEcho: result = semEcho(c, setMs(n, s))
   of mCreateThread: 
     result = semDirectOp(c, n, flags)
-    if gGlobalOptions * {optThreads, optThreadAnalysis} == 
-                        {optThreads, optThreadAnalysis}:
-      # XXX This analysis should be done as late as possible
-      # (forward references!)
-      semthreads.AnalyseThread(result)
+    if semthreads.needsGlobalAnalysis():
+      c.threadEntries.add(result)
   else: result = semDirectOp(c, n, flags)
   
 proc isTypeExpr(n: PNode): bool = 

compiler/semthreads.nim

@@ -171,6 +171,8 @@ proc analyseCall(c: PProcCtx, n: PNode): TThreadOwner =
       newCtx.mapping[formal.id] = call.args[i-1]
     pushInfoContext(n.info)
     result = analyse(newCtx, prc.ast.sons[codePos])
+    if prc.ast.sons[codePos].kind == nkEmpty:
+      Message(n.info, warnAnalysisLoophole, renderTree(n))
     if prc.typ.sons[0] != nil:
       if prc.ast.len > resultPos:
         result = newCtx.mapping[prc.ast.sons[resultPos].sym.id]
@@ -221,14 +223,17 @@ template aggregateOwner(result, ana: expr) =
     if result == toNil: result = a
     else: localError(n.info, errDifferentHeaps)
 
+proc analyseArgs(c: PProcCtx, n: PNode, start = 1) =
+  for i in start..n.len-1: discard analyse(c, n[i])
+
 proc analyseOp(c: PProcCtx, n: PNode): TThreadOwner =
   if n[0].kind != nkSym or n[0].sym.kind != skProc:
     Message(n.info, warnAnalysisLoophole, renderTree(n))
     result = toNil
   else:
     var prc = n[0].sym
-    # XXX create thread!?
     case prc.magic
+    of mNone: result = analyseCall(c, n)
     of mNew, mNewFinalize, mNewSeq, mSetLengthStr, mSetLengthSeq,
         mAppendSeqElem, mReset, mAppendStrCh, mAppendStrStr:
       writeAccess(c, n[1], toMine)
@@ -244,10 +249,14 @@ proc analyseOp(c: PProcCtx, n: PNode): TThreadOwner =
         mConTArr, mConTT, mSlice, 
         mRepr, mArrToSeq, mCopyStr, mCopyStrLast, 
         mNewString, mNewStringOfCap:
-      # XXX no check for effects in the arguments?
+      analyseArgs(c, n)
       result = toMine
     else:
-      result = analyseCall(c, n)
+      # don't recurse, but check args; NOTE: This is essential that
+      # ``mCreateThread`` is handled here to avoid the recursion
+      analyseArgs(c, n)
+      if prc.typ.sons[0] == nil: result = toVoid
+      else: result = toNil
 
 proc analyse(c: PProcCtx, n: PNode): TThreadOwner =
   case n.kind
@@ -325,8 +334,7 @@ proc analyse(c: PProcCtx, n: PNode): TThreadOwner =
       result = toVoid
   else: InternalError(n.info, "analysis not implemented for: " & $n.kind)
 
-proc AnalyseThread*(threadCreation: PNode) =
-  var n = threadCreation
+proc analyseThreadCreationCall(n: PNode) =
   # thread proc is second param of ``createThread``:
   if n[2].kind != nkSym or n[2].sym.kind != skProc:
     Message(n.info, warnAnalysisLoophole, renderTree(n))
@@ -337,3 +345,11 @@ proc AnalyseThread*(threadCreation: PNode) =
   c.mapping[formal.id] = toTheirs # thread receives foreign data!
   discard analyse(c, prc.ast.sons[codePos])
 
+proc needsGlobalAnalysis*: bool =
+  result = gGlobalOptions * {optThreads, optThreadAnalysis} == 
+                            {optThreads, optThreadAnalysis}
+
+proc AnalyseThread*(threadCreation: PNode) =
+  if needsGlobalAnalysis():
+    analyseThreadCreationCall(threadCreation)
+

tests/reject/tthreadanalysis2.nim

@@ -1,7 +1,8 @@
 discard """
   file: "tthreadanalysis2.nim"
-  line: 44
+  line: 45
   errormsg: "possible inconsistency of thread local heaps"
+  cmd: "nimrod cc --hints:on --threads:on $# $#"
 """
 
 import os

todo.txt

@@ -1,6 +1,4 @@
 * codegen for threadvars
-* clean up thread analysis: fix remaining XXX; thread analysis as a separate
-  pass
 * implement message passing built-ins
 
 * add --deadlock_prevention:on|off switch? timeout for locks?