Expand hoisted default params in sem (#15270)

* Expand hoisted default params in sem Introduce ast.newTree{I,IT} Add test for default params in procs * Cleanup * Simplify hoist transformation and expand test
2025-12-28 17:04:41 +00:00 · 2020-09-05 22:01:47 +02:00
parent 70d6238756
commit 35ff17410f
12 changed files with 178 additions and 123 deletions
--- a/compiler/ast.nim
+++ b/compiler/ast.nim
@@ -312,10 +312,6 @@ const
    # the compiler will avoid printing such names
    # in user messages.

-  sfHoisted* = sfForward
-    # an expression was hoised to an anonymous variable.
-    # the flag is applied to the var/let symbol
-
  sfNoForward* = sfRegister
    # forward declarations are not required (per module)
  sfReorder* = sfForward
@@ -1121,12 +1117,49 @@ proc newNode*(kind: TNodeKind): PNode =
      writeStackTrace()
    inc gNodeId

+proc newNodeI*(kind: TNodeKind, info: TLineInfo): PNode =
+  result = PNode(kind: kind, info: info)
+  when defined(useNodeIds):
+    result.id = gNodeId
+    if result.id == nodeIdToDebug:
+      echo "KIND ", result.kind
+      writeStackTrace()
+    inc gNodeId
+
+proc newNodeI*(kind: TNodeKind, info: TLineInfo, children: int): PNode =
+  result = PNode(kind: kind, info: info)
+  if children > 0:
+    newSeq(result.sons, children)
+  when defined(useNodeIds):
+    result.id = gNodeId
+    if result.id == nodeIdToDebug:
+      echo "KIND ", result.kind
+      writeStackTrace()
+    inc gNodeId
+
+proc newNodeIT*(kind: TNodeKind, info: TLineInfo, typ: PType): PNode =
+  result = newNode(kind)
+  result.info = info
+  result.typ = typ
+
 proc newTree*(kind: TNodeKind; children: varargs[PNode]): PNode =
  result = newNode(kind)
  if children.len > 0:
    result.info = children[0].info
  result.sons = @children

+proc newTreeI*(kind: TNodeKind; info: TLineInfo; children: varargs[PNode]): PNode =
+  result = newNodeI(kind, info)
+  if children.len > 0:
+    result.info = children[0].info
+  result.sons = @children
+
+proc newTreeIT*(kind: TNodeKind; info: TLineInfo; typ: PType; children: varargs[PNode]): PNode =
+  result = newNodeIT(kind, info, typ)
+  if children.len > 0:
+    result.info = children[0].info
+  result.sons = @children
+
 template previouslyInferred*(t: PType): PType =
  if t.sons.len > 1: t.lastSon else: nil

@@ -1227,43 +1260,6 @@ proc newSymNode*(sym: PSym, info: TLineInfo): PNode =
  result.typ = sym.typ
  result.info = info

-proc newNodeI*(kind: TNodeKind, info: TLineInfo): PNode =
-  result = PNode(kind: kind, info: info)
-  when defined(useNodeIds):
-    result.id = gNodeId
-    if result.id == nodeIdToDebug:
-      echo "KIND ", result.kind
-      writeStackTrace()
-    inc gNodeId
-
-proc newNodeI*(kind: TNodeKind, info: TLineInfo, children: int): PNode =
-  result = PNode(kind: kind, info: info)
-  if children > 0:
-    newSeq(result.sons, children)
-  when defined(useNodeIds):
-    result.id = gNodeId
-    if result.id == nodeIdToDebug:
-      echo "KIND ", result.kind
-      writeStackTrace()
-    inc gNodeId
-
-proc newNode*(kind: TNodeKind, info: TLineInfo, sons: TNodeSeq = @[],
-              typ: PType = nil): PNode =
-  # XXX use shallowCopy here for ownership transfer:
-  result = PNode(kind: kind, info: info, typ: typ)
-  result.sons = sons
-  when defined(useNodeIds):
-    result.id = gNodeId
-    if result.id == nodeIdToDebug:
-      echo "KIND ", result.kind
-      writeStackTrace()
-    inc gNodeId
-
-proc newNodeIT*(kind: TNodeKind, info: TLineInfo, typ: PType): PNode =
-  result = newNode(kind)
-  result.info = info
-  result.typ = typ
-
 proc newIntNode*(kind: TNodeKind, intVal: BiggestInt): PNode =
  result = newNode(kind)
  result.intVal = intVal
--- a/compiler/docgen.nim
+++ b/compiler/docgen.nim
@@ -992,8 +992,8 @@ proc documentEffect(cache: IdentCache; n, x: PNode, effectType: TSpecialWord, id
      # set the type so that the following analysis doesn't screw up:
      effects[i].typ = real[i].typ

-    result = newNode(nkExprColonExpr, n.info, @[
-      newIdentNode(getIdent(cache, specialWords[effectType]), n.info), effects])
+    result = newTreeI(nkExprColonExpr, n.info, 
+      newIdentNode(getIdent(cache, specialWords[effectType]), n.info), effects)

 proc documentWriteEffect(cache: IdentCache; n: PNode; flag: TSymFlag; pragmaName: string): PNode =
  let s = n[namePos].sym
@@ -1005,8 +1005,8 @@ proc documentWriteEffect(cache: IdentCache; n: PNode; flag: TSymFlag; pragmaName
      effects.add params[i]

  if effects.len > 0:
-    result = newNode(nkExprColonExpr, n.info, @[
-      newIdentNode(getIdent(cache, pragmaName), n.info), effects])
+    result = newTreeI(nkExprColonExpr, n.info, 
+      newIdentNode(getIdent(cache, pragmaName), n.info), effects)

 proc documentRaises*(cache: IdentCache; n: PNode) =
  if n[namePos].kind != nkSym: return
--- a/compiler/lowerings.nim
+++ b/compiler/lowerings.nim
@@ -369,14 +369,3 @@ proc genLen*(g: ModuleGraph; n: PNode): PNode =
    result[0] = newSymNode(getSysMagic(g, n.info, "len", mLengthSeq))
    result[1] = n

-proc hoistExpr*(varSection, expr: PNode, name: PIdent, owner: PSym): PSym =
-  result = newSym(skLet, name, owner, varSection.info, owner.options)
-  result.flags.incl sfHoisted
-  result.typ = expr.typ
-
-  var varDef = newNodeI(nkIdentDefs, varSection.info, 3)
-  varDef[0] = newSymNode(result)
-  varDef[1] = newNodeI(nkEmpty, varSection.info)
-  varDef[2] = expr
-
-  varSection.add varDef
--- a/compiler/parser.nim
+++ b/compiler/parser.nim
@@ -1444,7 +1444,7 @@ proc parseExprStmt(p: var Parser): PNode =
        result.add(commandParam(p, isFirstParam, pmNormal))
        if p.tok.tokType != tkComma: break
    elif p.tok.indent < 0 and isExprStart(p):
-      result = newNode(nkCommand, a.info, @[a])
+      result = newTreeI(nkCommand, a.info, a)
      while true:
        result.add(commandParam(p, isFirstParam, pmNormal))
        if p.tok.tokType != tkComma: break
--- a/compiler/pragmas.nim
+++ b/compiler/pragmas.nim
@@ -532,7 +532,7 @@ proc processLink(c: PContext, n: PNode) =
 proc semAsmOrEmit*(con: PContext, n: PNode, marker: char): PNode =
  case n[1].kind
  of nkStrLit, nkRStrLit, nkTripleStrLit:
-    result = newNode(if n.kind == nkAsmStmt: nkAsmStmt else: nkArgList, n.info)
+    result = newNodeI(if n.kind == nkAsmStmt: nkAsmStmt else: nkArgList, n.info)
    var str = n[1].strVal
    if str == "":
      localError(con.config, n.info, "empty 'asm' statement")
@@ -563,7 +563,7 @@ proc semAsmOrEmit*(con: PContext, n: PNode, marker: char): PNode =
      a = c + 1
  else:
    illFormedAstLocal(n, con.config)
-    result = newNode(nkAsmStmt, n.info)
+    result = newNodeI(nkAsmStmt, n.info)

 proc pragmaEmit(c: PContext, n: PNode) =
  if n.kind notin nkPragmaCallKinds or n.len != 2:
@@ -626,7 +626,7 @@ proc processPragma(c: PContext, n: PNode, i: int) =
    invalidPragma(c, n)

  var userPragma = newSym(skTemplate, it[1].ident, nil, it.info, c.config.options)
-  userPragma.ast = newNode(nkPragma, n.info, n.sons[i+1..^1])
+  userPragma.ast = newTreeI(nkPragma, n.info, n.sons[i+1..^1])
  strTableAdd(c.userPragmas, userPragma)

 proc pragmaRaisesOrTags(c: PContext, n: PNode) =
--- a/compiler/semdata.nim
+++ b/compiler/semdata.nim
@@ -381,8 +381,7 @@ proc makeNotType*(c: PContext, t1: PType): PType =
  result.flags.incl tfHasMeta

 proc nMinusOne(c: PContext; n: PNode): PNode =
-  result = newNode(nkCall, n.info, @[
-    newSymNode(getSysMagic(c.graph, n.info, "pred", mPred)), n])
+  result = newTreeI(nkCall, n.info, newSymNode(getSysMagic(c.graph, n.info, "pred", mPred)), n)

 # Remember to fix the procs below this one when you make changes!
 proc makeRangeWithStaticExpr*(c: PContext, n: PNode): PType =
@@ -391,9 +390,8 @@ proc makeRangeWithStaticExpr*(c: PContext, n: PNode): PType =
  result.sons = @[intType]
  if n.typ != nil and n.typ.n == nil:
    result.flags.incl tfUnresolved
-  result.n = newNode(nkRange, n.info, @[
-    newIntTypeNode(0, intType),
-    makeStaticExpr(c, nMinusOne(c, n))])
+  result.n = newTreeI(nkRange, n.info, newIntTypeNode(0, intType),
+    makeStaticExpr(c, nMinusOne(c, n)))

 template rangeHasUnresolvedStatic*(t: PType): bool =
  tfUnresolved in t.flags
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -1556,10 +1556,9 @@ proc propertyWriteAccess(c: PContext, n, nOrig, a: PNode): PNode =
  # this is ugly. XXX Semantic checking should use the ``nfSem`` flag for
  # nodes?
  let aOrig = nOrig[0]
-  result = newNode(nkCall, n.info, sons = @[setterId, a[0],
-                                            semExprWithType(c, n[1])])
+  result = newTreeI(nkCall, n.info, setterId, a[0], semExprWithType(c, n[1]))
  result.flags.incl nfDotSetter
-  let orig = newNode(nkCall, n.info, sons = @[setterId, aOrig[0], nOrig[1]])
+  let orig = newTreeI(nkCall, n.info, setterId, aOrig[0], nOrig[1])
  result = semOverloadedCallAnalyseEffects(c, result, orig, {})

  if result != nil:
@@ -2041,7 +2040,7 @@ proc semQuoteAst(c: PContext, n: PNode): PNode =
    dummyTemplate[paramsPos].add getSysSym(c.graph, n.info, "untyped").newSymNode # return type
    ids.add getSysSym(c.graph, n.info, "untyped").newSymNode # params type
    ids.add c.graph.emptyNode # no default value
-    dummyTemplate[paramsPos].add newNode(nkIdentDefs, n.info, ids)
+    dummyTemplate[paramsPos].add newTreeI(nkIdentDefs, n.info, ids)

  var tmpl = semTemplateDef(c, dummyTemplate)
  quotes[0] = tmpl[namePos]
@@ -2053,10 +2052,10 @@ proc semQuoteAst(c: PContext, n: PNode): PNode =
                    newIdentNode(getIdent(c.cache, "newIdentNode"), n.info)
                  else:
                    identNodeSym.newSymNode
-  quotes[1] = newNode(nkCall, n.info, @[identNode, newStrNode(nkStrLit, "result")])
-  result = newNode(nkCall, n.info, @[
+  quotes[1] = newTreeI(nkCall, n.info, identNode, newStrNode(nkStrLit, "result"))
+  result = newTreeI(nkCall, n.info, 
     createMagic(c.graph, "getAst", mExpandToAst).newSymNode,
-    newNode(nkCall, n.info, quotes)])
+     newTreeI(nkCall, n.info, quotes))
  result = semExpandToAst(c, result)

 proc tryExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
@@ -2579,6 +2578,43 @@ proc shouldBeBracketExpr(n: PNode): bool =
          n[0] = be
          return true

+proc hoistParamsUsedInDefault(c: PContext, call, letSection, defExpr: var PNode) =
+  # This takes care of complicated signatures such as:
+  # proc foo(a: int, b = a)
+  # proc bar(a: int, b: int, c = a + b)
+  #
+  # The recursion may confuse you. It performs two duties:
+  #
+  # 1) extracting all referenced params from default expressions
+  #    into a let section preceding the call
+  #
+  # 2) replacing the "references" within the default expression
+  #    with these extracted skLet symbols.
+  #
+  # The first duty is carried out directly in the code here, while the second
+  # duty is activated by returning a non-nil value. The caller is responsible
+  # for replacing the input to the function with the returned non-nil value.
+  # (which is the hoisted symbol)
+  if defExpr.kind == nkSym and defExpr.sym.kind == skParam and defExpr.sym.owner == call[0].sym:
+    let paramPos = defExpr.sym.position + 1
+
+    if call[paramPos].kind != nkSym:
+      let hoistedVarSym = newSym(skLet, getIdent(c.graph.cache, genPrefix), c.p.owner, letSection.info, c.p.owner.options)
+      hoistedVarSym.typ = call[paramPos].typ
+
+      letSection.add newTreeI(nkIdentDefs, letSection.info,
+        newSymNode(hoistedVarSym),
+        newNodeI(nkEmpty, letSection.info),
+        call[paramPos])
+
+      call[paramPos] = newSymNode(hoistedVarSym) # Refer the original arg to its hoisted sym
+
+    # arg we refer to is a sym, wether introduced by hoisting or not doesn't matter, we simply reuse it
+    defExpr = call[paramPos]
+  else:
+    for i in 0..<defExpr.safeLen:
+      hoistParamsUsedInDefault(c, call, letSection, defExpr[i])
+
 proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
  when defined(nimCompilerStackraceHints):
    setFrameMsg c.config$n.info & " " & $n.kind
@@ -2707,6 +2743,15 @@ proc semExpr(c: PContext, n: PNode, flags: TExprFlags = {}): PNode =
      result = semDirectOp(c, n, flags)
    else:
      result = semIndirectOp(c, n, flags)
+
+    if nfDefaultRefsParam in result.flags:
+      result = result.copyTree #XXX: Figure out what causes default param nodes to be shared.. (sigmatch bug?)
+      # We've found a default value that references another param.
+      # See the notes in `hoistParamsUsedInDefault` for more details.
+      var hoistedParams = newNodeI(nkLetSection, result.info)
+      for i in 1..<result.len:
+        hoistParamsUsedInDefault(c, result, hoistedParams, result[i])
+      result = newTreeIT(nkStmtListExpr, result.info, result.typ, hoistedParams, result)
  of nkWhen:
    if efWantStmt in flags:
      result = semWhen(c, n, true)
--- a/compiler/sempass2.nim
+++ b/compiler/sempass2.nim
@@ -1281,7 +1281,7 @@ proc trackStmt*(c: PContext; module: PSym; n: PNode, isTopLevel: bool) =
                nkTypeSection, nkConverterDef, nkMethodDef, nkIteratorDef}:
    return
  let g = c.graph
-  var effects = newNode(nkEffectList, n.info)
+  var effects = newNodeI(nkEffectList, n.info)
  var t: TEffects
  initEffects(g, effects, module, t, c)
  t.isTopLevel = isTopLevel
--- a/compiler/semtypinst.nim
+++ b/compiler/semtypinst.nim
@@ -207,7 +207,7 @@ proc replaceTypeVarsN(cl: var TReplTypeVars, n: PNode; start=0): PNode =
    result.sym = replaceTypeVarsS(cl, n.sym)
    if result.sym.typ.kind == tyVoid:
      # don't add the 'void' field
-      result = newNode(nkRecList, n.info)
+      result = newNodeI(nkRecList, n.info)
  of nkRecWhen:
    var branch: PNode = nil              # the branch to take
    for i in 0..<n.len:
--- a/compiler/transf.nim
+++ b/compiler/transf.nim
@@ -886,43 +886,6 @@ proc commonOptimizations*(g: ModuleGraph; c: PSym, n: PNode): PNode =
    else:
      result = n

-proc hoistParamsUsedInDefault(c: PTransf, call, letSection, defExpr: PNode): PNode =
-  # This takes care of complicated signatures such as:
-  # proc foo(a: int, b = a)
-  # proc bar(a: int, b: int, c = a + b)
-  #
-  # The recursion may confuse you. It performs two duties:
-  #
-  # 1) extracting all referenced params from default expressions
-  #    into a let section preceding the call
-  #
-  # 2) replacing the "references" within the default expression
-  #    with these extracted skLet symbols.
-  #
-  # The first duty is carried out directly in the code here, while the second
-  # duty is activated by returning a non-nil value. The caller is responsible
-  # for replacing the input to the function with the returned non-nil value.
-  # (which is the hoisted symbol)
-  if defExpr.kind == nkSym:
-    if defExpr.sym.kind == skParam and defExpr.sym.owner == call[0].sym:
-      let paramPos = defExpr.sym.position + 1
-
-      if call[paramPos].kind == nkSym and sfHoisted in call[paramPos].sym.flags:
-        # Already hoisted, we still need to return it in order to replace the
-        # placeholder expression in the default value.
-        return call[paramPos]
-
-      let hoistedVarSym = hoistExpr(letSection,
-                                    call[paramPos],
-                                    getIdent(c.graph.cache, genPrefix),
-                                    c.transCon.owner).newSymNode
-      call[paramPos] = hoistedVarSym
-      return hoistedVarSym
-  else:
-    for i in 0..<defExpr.safeLen:
-      let hoisted = hoistParamsUsedInDefault(c, call, letSection, defExpr[i])
-      if hoisted != nil: defExpr[i] = hoisted
-
 proc transform(c: PTransf, n: PNode): PNode =
  when false:
    var oldDeferAnchor: PNode
@@ -989,16 +952,6 @@ proc transform(c: PTransf, n: PNode): PNode =
  of nkBreakStmt: result = transformBreak(c, n)
  of nkCallKinds:
    result = transformCall(c, n)
-    var call = result
-    if nfDefaultRefsParam in call.flags:
-      # We've found a default value that references another param.
-      # See the notes in `hoistParamsUsedInDefault` for more details.
-      var hoistedParams = newNodeI(nkLetSection, call.info, 0)
-      for i in 1..<call.len:
-        let hoisted = hoistParamsUsedInDefault(c, call, hoistedParams, call[i])
-        if hoisted != nil: call[i] = hoisted
-      result = newTree(nkStmtListExpr, hoistedParams, call)
-      result.typ = call.typ
  of nkAddr, nkHiddenAddr:
    result = transformAddrDeref(c, n, nkDerefExpr, nkHiddenDeref)
  of nkDerefExpr, nkHiddenDeref:
--- a/compiler/vmdeps.nim
+++ b/compiler/vmdeps.nim
@@ -19,8 +19,7 @@ proc opSlurp*(file: string, info: TLineInfo, module: PSym; conf: ConfigRef): str
    # we produce a fake include statement for every slurped filename, so that
    # the module dependencies are accurate:
    discard conf.fileInfoIdx(AbsoluteFile filename)
-    appendToModule(module, newNode(nkIncludeStmt, info, @[
-      newStrNode(nkStrLit, filename)]))
+    appendToModule(module, newTreeI(nkIncludeStmt, info, newStrNode(nkStrLit, filename)))
  except IOError:
    localError(conf, info, "cannot open file: " & file)
    result = ""
--- a/tests/defaultprocparam/tdefaultprocparam.nim
+++ b/tests/defaultprocparam/tdefaultprocparam.nim
@@ -1,8 +1,83 @@
 discard """
 output: '''
 hi
+hi
+topLevel|topLevel|
+topLevel2|topLevel2|
+inProc|inProc|
+inProc2|inProc2|
+topLevel|9
+topLevel2|10
+inProc|7
+inProc2|8
+must have been the wind..
+I'm there
+must have been the wind..
+I'm there
+symbol'a'symbol'a'
+symbol'b'symbol'b'
+symbol'a'symbol'b'
+symbol'a'9
+symbol'b'9
+symbol'a'0
 '''
 """
 import mdefaultprocparam

 p()
+
+proc testP =
+  p()
+
+testP()
+
+proc p2(s: string, count = s): string = s & count
+
+proc testP2 =
+  echo p2 """inProc|"""
+  echo p2 """inProc2|"""
+
+echo p2 """topLevel|"""
+echo p2 """topLevel2|"""
+
+testP2()
+
+import macros
+macro dTT(a: typed) = echo a.treeRepr
+
+proc p3(s: string, count = len(s)): string = s & $count
+
+proc testP3 =
+  echo p3 """inProc|"""
+  echo p3 """inProc2|"""
+
+echo p3 """topLevel|"""
+echo p3 """topLevel2|"""
+
+testP3()
+
+proc cut(s: string, c = len(s)): string =
+  s[0..<s.len-c]
+
+echo "must have been the wind.." & cut "I'm gone"
+echo cut("I'm gone", 4) & "there"
+
+proc testCut =
+  echo "must have been the wind.." & cut "I'm gone"
+  echo cut("I'm gone", 4) & "there"
+
+testCut()
+
+var a = "symbol'a'"
+var b = "symbol'b'"
+
+block:
+  echo p2(a)
+block:
+  echo p2(b)
+block:
+  echo p2(a, b)
+block:
+  echo p3(a)
+  echo p3(b)
+  echo p3(a, 0)