From e2af486434c7ff401c412d890b490b4ac11ab321 Mon Sep 17 00:00:00 2001
From: BigEpsilon <BigEpsilon@users.noreply.github.com>
Date: Sat, 28 Oct 2017 11:33:35 +0200
Subject: [PATCH] Add sections (type, var, let, const, using) support for
 reorder pragma (#6326)

---
 compiler/depends.nim       |   2 +-
 compiler/docgen.nim        |   4 +-
 compiler/importer.nim      |  74 +------
 compiler/modulepaths.nim   |  78 +++++++
 compiler/passes.nim        |   3 +-
 compiler/reorder.nim       | 426 +++++++++++++++++++++++++++++++++----
 compiler/rodwrite.nim      |   4 +-
 compiler/sem.nim           |   2 +-
 tests/modules/treorder.nim |  12 +-
 tests/pragmas/treorder.nim |  75 +++++++
 10 files changed, 551 insertions(+), 129 deletions(-)
 create mode 100644 compiler/modulepaths.nim
 create mode 100644 tests/pragmas/treorder.nim

diff --git a/compiler/depends.nim b/compiler/depends.nim
index e8c295a349..2b600c1dab 100644
--- a/compiler/depends.nim
+++ b/compiler/depends.nim
@@ -10,7 +10,7 @@
 # This module implements a dependency file generator.
 
 import
-  os, options, ast, astalgo, msgs, ropes, idents, passes, importer
+  os, options, ast, astalgo, msgs, ropes, idents, passes, modulepaths
 
 from modulegraphs import ModuleGraph
 
diff --git a/compiler/docgen.nim b/compiler/docgen.nim
index 8c50a4f1d4..7d32755b30 100644
--- a/compiler/docgen.nim
+++ b/compiler/docgen.nim
@@ -15,8 +15,8 @@ import
   ast, strutils, strtabs, options, msgs, os, ropes, idents,
   wordrecg, syntaxes, renderer, lexer, packages/docutils/rstast,
   packages/docutils/rst, packages/docutils/rstgen, times,
-  packages/docutils/highlite, importer, sempass2, json, xmltree, cgi,
-  typesrenderer, astalgo
+  packages/docutils/highlite, sempass2, json, xmltree, cgi,
+  typesrenderer, astalgo, modulepaths
 
 type
   TSections = array[TSymKind, Rope]
diff --git a/compiler/importer.nim b/compiler/importer.nim
index 07f42a1472..89ee00b9d4 100644
--- a/compiler/importer.nim
+++ b/compiler/importer.nim
@@ -11,83 +11,11 @@
 
 import
   intsets, strutils, os, ast, astalgo, msgs, options, idents, rodread, lookups,
-  semdata, passes, renderer, gorgeimpl
+  semdata, passes, renderer, modulepaths
 
 proc evalImport*(c: PContext, n: PNode): PNode
 proc evalFrom*(c: PContext, n: PNode): PNode
 
-proc lookupPackage(pkg, subdir: PNode): string =
-  let sub = if subdir != nil: renderTree(subdir, {renderNoComments}).replace(" ") else: ""
-  case pkg.kind
-  of nkStrLit, nkRStrLit, nkTripleStrLit:
-    result = scriptableImport(pkg.strVal, sub, pkg.info)
-  of nkIdent:
-    result = scriptableImport(pkg.ident.s, sub, pkg.info)
-  else:
-    localError(pkg.info, "package name must be an identifier or string literal")
-    result = ""
-
-proc getModuleName*(n: PNode): string =
-  # This returns a short relative module name without the nim extension
-  # e.g. like "system", "importer" or "somepath/module"
-  # The proc won't perform any checks that the path is actually valid
-  case n.kind
-  of nkStrLit, nkRStrLit, nkTripleStrLit:
-    try:
-      result = pathSubs(n.strVal, n.info.toFullPath().splitFile().dir)
-    except ValueError:
-      localError(n.info, "invalid path: " & n.strVal)
-      result = n.strVal
-  of nkIdent:
-    result = n.ident.s
-  of nkSym:
-    result = n.sym.name.s
-  of nkInfix:
-    let n0 = n[0]
-    let n1 = n[1]
-    if n0.kind == nkIdent and n0.ident.id == getIdent("as").id:
-      # XXX hack ahead:
-      n.kind = nkImportAs
-      n.sons[0] = n.sons[1]
-      n.sons[1] = n.sons[2]
-      n.sons.setLen(2)
-      return getModuleName(n.sons[0])
-    if n1.kind == nkPrefix and n1[0].kind == nkIdent and n1[0].ident.s == "$":
-      if n0.kind == nkIdent and n0.ident.s == "/":
-        result = lookupPackage(n1[1], n[2])
-      else:
-        localError(n.info, "only '/' supported with $package notation")
-        result = ""
-    else:
-      # hacky way to implement 'x / y /../ z':
-      result = getModuleName(n1)
-      result.add renderTree(n0, {renderNoComments})
-      result.add getModuleName(n[2])
-  of nkPrefix:
-    if n.sons[0].kind == nkIdent and n.sons[0].ident.s == "$":
-      result = lookupPackage(n[1], nil)
-    else:
-      # hacky way to implement 'x / y /../ z':
-      result = renderTree(n, {renderNoComments}).replace(" ")
-  of nkDotExpr:
-    result = renderTree(n, {renderNoComments}).replace(".", "/")
-  of nkImportAs:
-    result = getModuleName(n.sons[0])
-  else:
-    localError(n.info, errGenerated, "invalid module name: '$1'" % n.renderTree)
-    result = ""
-
-proc checkModuleName*(n: PNode; doLocalError=true): int32 =
-  # This returns the full canonical path for a given module import
-  let modulename = n.getModuleName
-  let fullPath = findModule(modulename, n.info.toFullPath)
-  if fullPath.len == 0:
-    if doLocalError:
-      localError(n.info, errCannotOpenFile, modulename)
-    result = InvalidFileIDX
-  else:
-    result = fullPath.fileInfoIdx
-
 proc importPureEnumField*(c: PContext; s: PSym) =
   var check = strTableGet(c.importTable.symbols, s.name)
   if check == nil:
diff --git a/compiler/modulepaths.nim b/compiler/modulepaths.nim
new file mode 100644
index 0000000000..6b9865677e
--- /dev/null
+++ b/compiler/modulepaths.nim
@@ -0,0 +1,78 @@
+#
+#    See the file "copying.txt", included in this
+#    distribution, for details about the copyright.
+#
+
+import ast, renderer, gorgeimpl, strutils, msgs, options, idents, ospaths
+
+proc lookupPackage(pkg, subdir: PNode): string =
+  let sub = if subdir != nil: renderTree(subdir, {renderNoComments}).replace(" ") else: ""
+  case pkg.kind
+  of nkStrLit, nkRStrLit, nkTripleStrLit:
+    result = scriptableImport(pkg.strVal, sub, pkg.info)
+  of nkIdent:
+    result = scriptableImport(pkg.ident.s, sub, pkg.info)
+  else:
+    localError(pkg.info, "package name must be an identifier or string literal")
+    result = ""
+
+proc getModuleName*(n: PNode): string =
+  # This returns a short relative module name without the nim extension
+  # e.g. like "system", "importer" or "somepath/module"
+  # The proc won't perform any checks that the path is actually valid
+  case n.kind
+  of nkStrLit, nkRStrLit, nkTripleStrLit:
+    try:
+      result = pathSubs(n.strVal, n.info.toFullPath().splitFile().dir)
+    except ValueError:
+      localError(n.info, "invalid path: " & n.strVal)
+      result = n.strVal
+  of nkIdent:
+    result = n.ident.s
+  of nkSym:
+    result = n.sym.name.s
+  of nkInfix:
+    let n0 = n[0]
+    let n1 = n[1]
+    if n0.kind == nkIdent and n0.ident.id == getIdent("as").id:
+      # XXX hack ahead:
+      n.kind = nkImportAs
+      n.sons[0] = n.sons[1]
+      n.sons[1] = n.sons[2]
+      n.sons.setLen(2)
+      return getModuleName(n.sons[0])
+    if n1.kind == nkPrefix and n1[0].kind == nkIdent and n1[0].ident.s == "$":
+      if n0.kind == nkIdent and n0.ident.s == "/":
+        result = lookupPackage(n1[1], n[2])
+      else:
+        localError(n.info, "only '/' supported with $package notation")
+        result = ""
+    else:
+      # hacky way to implement 'x / y /../ z':
+      result = getModuleName(n1)
+      result.add renderTree(n0, {renderNoComments})
+      result.add getModuleName(n[2])
+  of nkPrefix:
+    if n.sons[0].kind == nkIdent and n.sons[0].ident.s == "$":
+      result = lookupPackage(n[1], nil)
+    else:
+      # hacky way to implement 'x / y /../ z':
+      result = renderTree(n, {renderNoComments}).replace(" ")
+  of nkDotExpr:
+    result = renderTree(n, {renderNoComments}).replace(".", "/")
+  of nkImportAs:
+    result = getModuleName(n.sons[0])
+  else:
+    localError(n.info, errGenerated, "invalid module name: '$1'" % n.renderTree)
+    result = ""
+
+proc checkModuleName*(n: PNode; doLocalError=true): int32 =
+  # This returns the full canonical path for a given module import
+  let modulename = n.getModuleName
+  let fullPath = findModule(modulename, n.info.toFullPath)
+  if fullPath.len == 0:
+    if doLocalError:
+      localError(n.info, errCannotOpenFile, modulename)
+    result = InvalidFileIDX
+  else:
+    result = fullPath.fileInfoIdx
\ No newline at end of file
diff --git a/compiler/passes.nim b/compiler/passes.nim
index 6efd50385c..b84fe2f4d7 100644
--- a/compiler/passes.nim
+++ b/compiler/passes.nim
@@ -15,6 +15,7 @@ import
   condsyms, idents, renderer, types, extccomp, math, magicsys, nversion,
   nimsets, syntaxes, times, rodread, idgen, modulegraphs, reorder
 
+
 type
   TPassContext* = object of RootObj # the pass's context
     fromCache*: bool  # true if created by "openCached"
@@ -211,7 +212,7 @@ proc processModule*(graph: ModuleGraph; module: PSym, stream: PLLStream,
             if n.kind == nkEmpty: break
             sl.add n
           if sfReorder in module.flags:
-            sl = reorder sl
+            sl = reorder(graph, sl, module, cache)
           discard processTopLevelStmt(sl, a)
           break
         elif not processTopLevelStmt(n, a): break
diff --git a/compiler/reorder.nim b/compiler/reorder.nim
index a9ad1fd97d..1cf5a02177 100644
--- a/compiler/reorder.nim
+++ b/compiler/reorder.nim
@@ -1,9 +1,36 @@
 
-import intsets, tables, ast, idents, renderer
+import 
+  intsets, ast, idents, algorithm, renderer, parser, ospaths, strutils, 
+  sequtils, msgs, modulegraphs, syntaxes, options, modulepaths, tables
 
-const
-  nfTempMark = nfTransf
-  nfPermMark = nfNoRewrite
+type
+  DepN = ref object
+    pnode: PNode
+    id, idx, lowLink: int
+    onStack: bool
+    kids: seq[DepN]
+    hAQ, hIS, hB, hCmd: int
+    when not defined(release):
+      expls: seq[string]
+  DepG = seq[DepN]
+
+when not defined(release):
+  var idNames = newTable[int, string]()
+
+proc newDepN(id: int, pnode: PNode): DepN =
+  new(result)
+  result.id = id
+  result.pnode = pnode
+  result.idx = -1
+  result.lowLink = -1
+  result.onStack = false
+  result.kids = @[]
+  result.hAQ = -1
+  result.hIS = -1
+  result.hB = -1
+  result.hCmd = -1
+  when not defined(release):
+    result.expls = @[]
 
 proc accQuoted(n: PNode): PIdent =
   var id = ""
@@ -21,10 +48,19 @@ proc addDecl(n: PNode; declares: var IntSet) =
   of nkPragmaExpr: addDecl(n[0], declares)
   of nkIdent:
     declares.incl n.ident.id
+    when not defined(release):
+      idNames[n.ident.id] = n.ident.s
   of nkSym:
     declares.incl n.sym.name.id
+    when not defined(release):
+      idNames[n.sym.name.id] = n.sym.name.s
   of nkAccQuoted:
-    declares.incl accQuoted(n).id
+    let a = accQuoted(n)
+    declares.incl a.id
+    when not defined(release):
+      idNames[a.id] = a.s
+  of nkEnumFieldDef:
+    addDecl(n[0], declares)
   else: discard
 
 proc computeDeps(n: PNode, declares, uses: var IntSet; topLevel: bool) =
@@ -32,7 +68,7 @@ proc computeDeps(n: PNode, declares, uses: var IntSet; topLevel: bool) =
   template decl(n) =
     if topLevel: addDecl(n, declares)
   case n.kind
-  of procDefs:
+  of procDefs, nkMacroDef, nkTemplateDef:
     decl(n[0])
     for i in 1..bodyPos: deps(n[i])
   of nkLetSection, nkVarSection, nkUsingStmt:
@@ -44,43 +80,358 @@ proc computeDeps(n: PNode, declares, uses: var IntSet; topLevel: bool) =
     for a in n:
       if a.len >= 3:
         decl(a[0])
-        for i in 1..<a.len: deps(a[i])
+        for i in 1..<a.len:
+          if a[i].kind == nkEnumTy:
+            # declare enum members
+            for b in a[i]:
+              decl(b)
+          else:
+            deps(a[i])
+  of nkIdentDefs:
+    for i in 1..<n.len: # avoid members identifiers in object definition
+      deps(n[i])
   of nkIdent: uses.incl n.ident.id
   of nkSym: uses.incl n.sym.name.id
   of nkAccQuoted: uses.incl accQuoted(n).id
   of nkOpenSymChoice, nkClosedSymChoice:
     uses.incl n.sons[0].sym.name.id
-  of nkStmtList, nkStmtListExpr, nkWhenStmt, nkElifBranch, nkElse:
+  of nkStmtList, nkStmtListExpr, nkWhenStmt, nkElifBranch, nkElse, nkStaticStmt:
     for i in 0..<len(n): computeDeps(n[i], declares, uses, topLevel)
+  of nkPragma:
+    let a = n.sons[0]
+    if a.kind == nkExprColonExpr and a.sons[0].kind == nkIdent and
+       a.sons[0].ident.s == "pragma":
+        # user defined pragma
+        decl(a.sons[1])
+    else:
+      for i in 0..<safeLen(n): deps(n[i])
   else:
     for i in 0..<safeLen(n): deps(n[i])
 
-proc visit(i: int; all, res: PNode; deps: var seq[(IntSet, IntSet)]): bool =
-  let n = all[i]
-  if nfTempMark in n.flags:
-    # not a DAG!
+proc cleanPath(s: string): string =
+  # Here paths may have the form A / B or "A/B"
+  result = ""
+  for c in s:
+    if c != ' ' and c != '\"':
+      result.add c
+
+proc joinPath(parts: seq[string]): string =
+  let nb = parts.len
+  assert nb > 0
+  if nb == 1:
+    return parts[0]
+  result = parts[0] / parts[1]
+  for i in 2..<parts.len:
+    result = result / parts[i]
+
+proc getIncludePath(n: PNode, modulePath: string): string =
+  let istr = n.renderTree.cleanPath
+  let (pdir, _) = modulePath.splitPath
+  let p = istr.split('/').joinPath.addFileExt("nim")
+  result = pdir / p
+
+proc hasIncludes(n:PNode): bool =
+  for a in n:
+    if a.kind == nkIncludeStmt:
+      return true
+
+proc includeModule*(graph: ModuleGraph; s: PSym, fileIdx: int32;
+                    cache: IdentCache): PNode {.procvar.} =
+  result = syntaxes.parseFile(fileIdx, cache)
+  graph.addDep(s, fileIdx)
+  graph.addIncludeDep(s.position.int32, fileIdx)
+
+proc expandIncludes(graph: ModuleGraph, module: PSym, n: PNode, 
+                    modulePath: string, includedFiles: var IntSet,
+                    cache: IdentCache): PNode =
+  # Parses includes and injects them in the current tree
+  if not n.hasIncludes:
+    return n
+  result = newNodeI(nkStmtList, n.info)
+  for a in n:
+    if a.kind == nkIncludeStmt:
+      for i in 0..<a.len:
+        var f = checkModuleName(a.sons[i])
+        if f != InvalidFileIDX:
+          if containsOrIncl(includedFiles, f):
+            localError(a.info, errRecursiveDependencyX, f.toFilename)
+          else:
+            let nn = includeModule(graph, module, f, cache)
+            let nnn = expandIncludes(graph, module, nn, modulePath, 
+                                      includedFiles, cache)
+            excl(includedFiles, f)
+            for b in nnn:
+              result.add b
+    else:
+      result.add a
+
+proc splitSections(n: PNode): PNode =
+  # Split typeSections and ConstSections into
+  # sections that contain only one definition
+  assert n.kind == nkStmtList
+  result = newNodeI(nkStmtList, n.info)
+  for a in n:
+    if a.kind in {nkTypeSection, nkConstSection} and a.len > 1:
+      for b in a:
+        var s = newNode(a.kind)
+        s.info = b.info
+        s.add b
+        result.add s
+    else:
+      result.add a
+
+proc haveSameKind(dns: seq[DepN]): bool =
+  # Check if all the nodes in a strongly connected
+  # component have the same kind
+  result = true
+  let kind = dns[0].pnode.kind
+  for dn in dns:
+    if dn.pnode.kind != kind:
+      return false
+
+proc mergeSections(comps: seq[seq[DepN]], res: PNode) =
+  # Merges typeSections and ConstSections when they form
+  # a strong component (ex: circular type definition)
+  for c in comps:
+    assert c.len > 0
+    if c.len == 1:
+      res.add c[0].pnode
+    else:
+      let fstn = c[0].pnode
+      let kind = fstn.kind
+      # always return to the original order when we got circular dependencies
+      let cs = c.sortedByIt(it.id)
+      if kind in {nkTypeSection, nkConstSection} and haveSameKind(cs):
+        # Circular dependency between type or const sections, we just
+        # need to merge them
+        var sn = newNode(kind)
+        for dn in cs:
+          sn.add dn.pnode.sons[0]
+        res.add sn
+      else:
+          # Problematic circular dependency, we arrange the nodes into
+          # their original relative order and make sure to re-merge
+          # consecutive type and const sections
+          var wmsg = "Circular dependency detected. reorder pragma may not be able to" &
+            " reorder some nodes properely"
+          when not defined(release):
+            wmsg &= ":\n"
+            for i in 0..<cs.len-1:
+                for j in i..<cs.len:
+                  for ci in 0..<cs[i].kids.len:
+                    if cs[i].kids[ci].id == cs[j].id:
+                      wmsg &= "line " & $cs[i].pnode.info.line &
+                        " depends on line " & $cs[j].pnode.info.line &
+                        ": " & cs[i].expls[ci] & "\n"
+            for j in 0..<cs.len-1:
+                for ci in 0..<cs[^1].kids.len:
+                  if cs[^1].kids[ci].id == cs[j].id:
+                    wmsg &= "line " & $cs[^1].pnode.info.line &
+                      " depends on line " & $cs[j].pnode.info.line &
+                      ": " & cs[^1].expls[ci] & "\n"
+          message(cs[0].pnode.info, warnUser, wmsg)
+
+          var i = 0
+          while i < cs.len:
+            if cs[i].pnode.kind in {nkTypeSection, nkConstSection}:
+              let ckind = cs[i].pnode.kind
+              var sn = newNode(ckind)
+              sn.add cs[i].pnode[0]
+              inc i
+              while i < cs.len and cs[i].pnode.kind == ckind :
+                sn.add cs[i].pnode[0]
+                inc i
+              res.add sn
+            else:
+              res.add cs[i].pnode
+              inc i
+
+proc hasImportStmt(n: PNode): bool =
+  # Checks if the node is an import statement or
+  # i it contains one
+  case n.kind
+  of nkImportStmt, nkFromStmt, nkImportExceptStmt:
     return true
-  if nfPermMark notin n.flags:
-    incl n.flags, nfTempMark
-    var uses = deps[i][1]
-    for j in 0..<all.len:
-      if j != i:
-        let declares = deps[j][0]
+  of nkStmtList, nkStmtListExpr, nkWhenStmt, nkElifBranch, nkElse, nkStaticStmt:
+    for a in n:
+      if a.hasImportStmt:
+        return true
+  else:
+    result = false
+
+proc hasImportStmt(n: DepN): bool =
+  if n.hIS < 0:
+    n.hIS = ord(n.pnode.hasImportStmt)
+  result = bool(n.hIS)
+
+proc hasCommand(n: PNode): bool =
+  # Checks if the node is a command or a call
+  # or if it contains one
+  case n.kind
+  of nkCommand, nkCall:
+    result = true
+  of nkStmtList, nkStmtListExpr, nkWhenStmt, nkElifBranch, nkElse,
+      nkStaticStmt, nkLetSection, nkConstSection, nkVarSection,
+      nkIdentDefs:
+        for a in n:
+          if a.hasCommand:
+            return true
+  else:
+    return false
+
+proc hasCommand(n: DepN): bool =
+  if n.hCmd < 0:
+    n.hCmd = ord(n.pnode.hasCommand)
+  result = bool(n.hCmd)
+
+proc hasAccQuoted(n: PNode): bool =
+  if n.kind == nkAccQuoted:
+    return true
+  for a in n:
+    if hasAccQuoted(a):
+      return true
+
+const extandedProcDefs = procDefs + {nkMacroDef,  nkTemplateDef}
+
+proc hasAccQuotedDef(n: PNode): bool =
+  # Checks if the node is a function, macro, template ...
+  # with a quoted name or if it contains one
+  case n.kind
+  of extandedProcDefs:
+    result = n[0].hasAccQuoted
+  of nkStmtList, nkStmtListExpr, nkWhenStmt, nkElifBranch, nkElse, nkStaticStmt:
+    for a in n:
+      if a.hasAccQuotedDef:
+        return true
+  else:
+    result = false
+
+proc hasAccQuotedDef(n: DepN): bool =
+  if n.hAQ < 0:
+    n.hAQ = ord(n.pnode.hasAccQuotedDef)
+  result = bool(n.hAQ)
+
+proc hasBody(n: PNode): bool =
+  # Checks if the node is a function, macro, template ...
+  # with a body or if it contains one
+  case n.kind
+  of nkCommand, nkCall:
+    result = true
+  of extandedProcDefs:
+    result = n[^1].kind == nkStmtList
+  of nkStmtList, nkStmtListExpr, nkWhenStmt, nkElifBranch, nkElse, nkStaticStmt:
+    for a in n:
+      if a.hasBody:
+        return true
+  else:
+    result = false
+
+proc hasBody(n: DepN): bool =
+  if n.hB < 0:
+    n.hB = ord(n.pnode.hasBody)
+  result = bool(n.hB)
+
+proc intersects(s1, s2: IntSet): bool =
+  for a in s1:
+    if s2.contains(a):
+      return true
+
+proc buildGraph(n: PNode, deps: seq[(IntSet, IntSet)]): DepG =
+  # Build a dependency graph
+  result = newSeqOfCap[DepN](deps.len)
+  for i in 0..<deps.len:
+    result.add newDepN(i, n.sons[i])
+  for i in 0..<deps.len:
+    var ni = result[i]
+    let uses = deps[i][1]
+    let niHasBody = ni.hasBody
+    let niHasCmd = ni.hasCommand
+    for j in 0..<deps.len:
+      if i == j: continue
+      var nj = result[j]
+      let declares = deps[j][0]
+      if j < i and nj.hasCommand and niHasCmd:
+        # Preserve order for commands and calls
+        ni.kids.add nj
+        when not defined(release):
+          ni.expls.add "both have commands and one comes after the other"
+      elif j < i and nj.hasImportStmt:
+        # Every node that comes after an import statement must
+        # depend on that import
+        ni.kids.add nj
+        when not defined(release):
+          ni.expls.add "parent is, or contains, an import statement and child comes after it"
+      elif j < i and niHasBody and nj.hasAccQuotedDef:
+        # Every function, macro, template... with a body depends
+        # on precedent function declarations that have quoted names.
+        # That's because it is hard to detect the use of functions
+        # like "[]=", "[]", "or" ... in their bodies.
+        ni.kids.add nj
+        when not defined(release):
+          ni.expls.add "one declares a quoted identifier and the other has a body and comes after it"
+      elif j < i and niHasBody and not nj.hasBody and
+        intersects(deps[i][0], declares):
+          # Keep function declaration before function definition
+          ni.kids.add nj
+          when not defined(release):
+            for dep in deps[i][0]:
+              if dep in declares:
+                ni.expls.add "one declares \"" & idNames[dep] & "\" and the other defines it"
+      else:
         for d in declares:
           if uses.contains(d):
-            let oldLen = res.len
-            if visit(j, all, res, deps):
-              result = true
-              # rollback what we did, it turned out to be a dependency that caused
-              # trouble:
-              for k in oldLen..<res.len:
-                res.sons[k].flags = res.sons[k].flags - {nfPermMark, nfTempMark}
-              if oldLen != res.len: res.sons.setLen oldLen
-            break
-    n.flags = n.flags + {nfPermMark} - {nfTempMark}
-    res.add n
+            ni.kids.add nj
+            when not defined(release):
+              ni.expls.add "one declares \"" & idNames[d] & "\" and the other uses it"
 
-proc reorder*(n: PNode): PNode =
+proc strongConnect(v: var DepN, idx: var int, s: var seq[DepN],
+                   res: var seq[seq[DepN]]) =
+  # Recursive part of trajan's algorithm
+  v.idx = idx
+  v.lowLink = idx
+  inc idx
+  s.add v
+  v.onStack = true
+  for w in v.kids.mitems:
+    if w.idx < 0:
+      strongConnect(w, idx, s, res)
+      v.lowLink = min(v.lowLink, w.lowLink)
+    elif w.onStack:
+      v.lowLink = min(v.lowLink, w.idx)
+  if v.lowLink == v.idx:
+    var comp = newSeq[DepN]()
+    while true:
+      var w = s.pop
+      w.onStack = false
+      comp.add w
+      if w.id == v.id: break
+    res.add comp
+
+proc getStrongComponents(g: var DepG): seq[seq[DepN]] =
+  ## Tarjan's algorithm. Performs a topological sort
+  ## and detects strongly connected components.
+  result = newSeq[seq[DepN]]()
+  var s = newSeq[DepN]()
+  var idx = 0
+  for v in g.mitems:
+    if v.idx < 0:
+      strongConnect(v, idx, s, result)
+
+proc hasForbiddenPragma(n: PNode): bool =
+  # Checks if the tree node has some pragmas that do not
+  # play well with reordering, like the push/pop pragma
+  for a in n:
+    if a.kind == nkPragma and a[0].kind == nkIdent and
+        a[0].ident.s == "push":
+          return true
+
+proc reorder*(graph: ModuleGraph, n: PNode, module: PSym, cache: IdentCache): PNode =
+  if n.hasForbiddenPragma:
+    return n
+  var includedFiles = initIntSet()
+  let mpath = module.fileIdx.toFullPath
+  let n = expandIncludes(graph, module, n, mpath, 
+                          includedFiles, cache).splitSections
   result = newNodeI(nkStmtList, n.info)
   var deps = newSeq[(IntSet, IntSet)](n.len)
   for i in 0..<n.len:
@@ -88,15 +439,6 @@ proc reorder*(n: PNode): PNode =
     deps[i][1] = initIntSet()
     computeDeps(n[i], deps[i][0], deps[i][1], true)
 
-  for i in 0 .. n.len-1:
-    discard visit(i, n, result, deps)
-  for i in 0..<result.len:
-    result.sons[i].flags = result.sons[i].flags - {nfTempMark, nfPermMark}
-  when false:
-    # reverse the result:
-    let L = result.len-1
-    for i in 0 .. result.len div 2:
-      result.sons[i].flags = result.sons[i].flags - {nfTempMark, nfPermMark}
-      result.sons[L - i].flags = result.sons[L - i].flags - {nfTempMark, nfPermMark}
-      swap(result.sons[i], result.sons[L - i])
-  #echo result
+  var g = buildGraph(n, deps)
+  let comps = getStrongComponents(g)
+  mergeSections(comps, result)
diff --git a/compiler/rodwrite.nim b/compiler/rodwrite.nim
index 1bc136acf1..9aed33ec97 100644
--- a/compiler/rodwrite.nim
+++ b/compiler/rodwrite.nim
@@ -13,8 +13,8 @@
 
 import
   intsets, os, options, strutils, nversion, ast, astalgo, msgs, platform,
-  condsyms, ropes, idents, securehash, rodread, passes, importer, idgen,
-  rodutils
+  condsyms, ropes, idents, securehash, rodread, passes, idgen,
+  rodutils, modulepaths
 
 from modulegraphs import ModuleGraph
 
diff --git a/compiler/sem.nim b/compiler/sem.nim
index 5e98d7a65f..3ba2f93d58 100644
--- a/compiler/sem.nim
+++ b/compiler/sem.nim
@@ -12,7 +12,7 @@
 import
   ast, strutils, hashes, options, lexer, astalgo, trees, treetab,
   wordrecg, ropes, msgs, os, condsyms, idents, renderer, types, platform, math,
-  magicsys, parser, nversion, nimsets, semfold, importer,
+  magicsys, parser, nversion, nimsets, semfold, modulepaths, importer,
   procfind, lookups, rodread, pragmas, passes, semdata, semtypinst, sigmatch,
   intsets, transf, vmdef, vm, idgen, aliases, cgmeth, lambdalifting,
   evaltempl, patterns, parampatterns, sempass2, nimfix.pretty, semmacrosanity,
diff --git a/tests/modules/treorder.nim b/tests/modules/treorder.nim
index 25280c429e..8715e45482 100644
--- a/tests/modules/treorder.nim
+++ b/tests/modules/treorder.nim
@@ -9,7 +9,6 @@ defined
 {.reorder: on.}
 {.experimental.}
 
-{.push callconv: stdcall.}
 proc bar(x: T)
 
 proc foo() =
@@ -22,15 +21,16 @@ proc bar(x: T) =
   echo "works ", x
   foo(x)
 
+when defined(testdef):
+  proc whendep() = echo "defined"
+else:
+  proc whendep() = echo "undefined"
+
 foo()
 
 type
   T = int
 
-when defined(testdef):
-  proc whendep() = echo "defined"
-else:
-  proc whendep() = echo "undefined"
 
 when not declared(goo):
   proc goo(my, omy) = echo my
@@ -42,5 +42,3 @@ using
   my, omy: int
 
 goo(3, 4)
-
-{.pop.}
diff --git a/tests/pragmas/treorder.nim b/tests/pragmas/treorder.nim
new file mode 100644
index 0000000000..6a6bbff4db
--- /dev/null
+++ b/tests/pragmas/treorder.nim
@@ -0,0 +1,75 @@
+discard """
+output:'''0
+1
+2
+3'''
+"""
+
+import macros
+{.reorder: on .}
+
+echo foo(-1)
+echo callWithFoo(0)
+echo(CA+CD)
+echo useTypes(TA(x:TB(x:1)), 2)
+second(0)
+  
+template callWithFoo(arg: untyped): untyped =
+  foo(arg)
+  
+proc first(i: int): void
+
+proc second(i: int): void =
+  make(first)
+  first(i)
+
+proc useTypes(a: TA, d: TD): int =
+  result = a.x.x+d
+
+type
+  TDoubleCyclic = ref object
+    x: TCyclicA
+    y: TCyclicB
+
+type
+  TCyclicA = ref object
+    x: TDoubleCyclic
+  
+type
+  TCyclicB = ref object
+    x: TDoubleCyclic
+
+const
+  CA = 1
+  CB = CC
+
+type
+  TA = object
+    x: TB
+  TC = type(CC)
+  TD = type(CA)
+
+const
+  CC = 1
+  CD = CB
+
+type
+  TB = object
+    x: TC
+
+proc foo(x: int): int =
+  result = bar(x)
+
+proc bar(x: int): int =
+  result = x+1
+
+macro make(arg: untyped): untyped =
+  ss &= arg.repr
+  ss &= " "
+  discard
+
+proc first(i: int): void =
+  make(second)
+
+static:
+  var ss: string = ""
\ No newline at end of file