nvro don't touch cdecl types [backport: 1.6] (#19461)

* nvro don't touch cdecl types; fix #19342 again

Author: ringabout

compiler/ast.nim

@@ -501,7 +501,7 @@ type
     nfHasComment # node has a comment
 
   TNodeFlags* = set[TNodeFlag]
-  TTypeFlag* = enum   # keep below 32 for efficiency reasons (now: 43)
+  TTypeFlag* = enum   # keep below 32 for efficiency reasons (now: 45)
     tfVarargs,        # procedure has C styled varargs
                       # tyArray type represeting a varargs list
     tfNoSideEffect,   # procedure type does not allow side effects

compiler/ccgcalls.nim

@@ -76,7 +76,7 @@ proc fixupCall(p: BProc, le, ri: PNode, d: var TLoc,
   # getUniqueType() is too expensive here:
   var typ = skipTypes(ri[0].typ, abstractInst)
   if typ[0] != nil:
-    if isInvalidReturnType(p.config, typ[0]):
+    if isInvalidReturnType(p.config, typ):
       if params != nil: pl.add(~", ")
       # beware of 'result = p(result)'. We may need to allocate a temporary:
       if d.k in {locTemp, locNone} or not preventNrvo(p, le, ri):
@@ -439,7 +439,7 @@ proc genClosureCall(p: BProc, le, ri: PNode, d: var TLoc) =
   let rawProc = getClosureType(p.module, typ, clHalf)
   let canRaise = p.config.exc == excGoto and canRaiseDisp(p, ri[0])
   if typ[0] != nil:
-    if isInvalidReturnType(p.config, typ[0]):
+    if isInvalidReturnType(p.config, typ):
       if ri.len > 1: pl.add(~", ")
       # beware of 'result = p(result)'. We may need to allocate a temporary:
       if d.k in {locTemp, locNone} or not preventNrvo(p, le, ri):
@@ -737,7 +737,7 @@ proc genNamedParamCall(p: BProc, ri: PNode, d: var TLoc) =
     pl.add(~": ")
     pl.add(genArg(p, ri[i], param, ri))
   if typ[0] != nil:
-    if isInvalidReturnType(p.config, typ[0]):
+    if isInvalidReturnType(p.config, typ):
       if ri.len > 1: pl.add(~" ")
       # beware of 'result = p(result)'. We always allocate a temporary:
       if d.k in {locTemp, locNone}:

compiler/ccgstmts.nim

@@ -32,13 +32,20 @@ proc registerTraverseProc(p: BProc, v: PSym, traverseProc: Rope) =
       "$n\t#nimRegisterGlobalMarker($1);$n$n", [traverseProc])
 
 proc isAssignedImmediately(conf: ConfigRef; n: PNode): bool {.inline.} =
-  if n.kind == nkEmpty: return false
-  if isInvalidReturnType(conf, n.typ):
-    # var v = f()
-    # is transformed into: var v;  f(addr v)
-    # where 'f' **does not** initialize the result!
-    return false
-  result = true
+  if n.kind == nkEmpty:
+    result = false
+  elif n.kind in nkCallKinds and n[0] != nil and n[0].typ != nil and n[0].typ.skipTypes(abstractInst).kind == tyProc:
+    if isInvalidReturnType(conf, n[0].typ, true):
+      # var v = f()
+      # is transformed into: var v;  f(addr v)
+      # where 'f' **does not** initialize the result!
+      result = false
+    else:
+      result = true
+  elif isInvalidReturnType(conf, n.typ, false):
+    result = false
+  else:
+    result = true
 
 proc inExceptBlockLen(p: BProc): int =
   for x in p.nestedTryStmts:

compiler/ccgtypes.nim

@@ -215,12 +215,18 @@ proc isObjLackingTypeField(typ: PType): bool {.inline.} =
   result = (typ.kind == tyObject) and ((tfFinal in typ.flags) and
       (typ[0] == nil) or isPureObject(typ))
 
-proc isInvalidReturnType(conf: ConfigRef; rettype: PType): bool =
+proc isInvalidReturnType(conf: ConfigRef; typ: PType, isProc = true): bool =
   # Arrays and sets cannot be returned by a C procedure, because C is
   # such a poor programming language.
   # We exclude records with refs too. This enhances efficiency and
   # is necessary for proper code generation of assignments.
-  if rettype == nil or (tfByCopy notin rettype.flags and getSize(conf, rettype) > conf.target.floatSize*3):
+  var rettype = typ
+  var isAllowedCall = true
+  if isProc:
+    rettype = rettype[0]
+    isAllowedCall = typ.callConv in {ccClosure, ccInline, ccNimCall}
+  if rettype == nil or (isAllowedCall and
+                    getSize(conf, rettype) > conf.target.floatSize*3):
     result = true
   else:
     case mapType(conf, rettype, skResult)
@@ -257,11 +263,11 @@ proc addAbiCheck(m: BModule, t: PType, name: Rope) =
     # see `testCodegenABICheck` for example error message it generates
 
 
-proc fillResult(conf: ConfigRef; param: PNode) =
+proc fillResult(conf: ConfigRef; param: PNode, proctype: PType) =
   fillLoc(param.sym.loc, locParam, param, ~"Result",
           OnStack)
   let t = param.sym.typ
-  if mapReturnType(conf, t) != ctArray and isInvalidReturnType(conf, t):
+  if mapReturnType(conf, t) != ctArray and isInvalidReturnType(conf, proctype):
     incl(param.sym.loc.flags, lfIndirect)
     param.sym.loc.storage = OnUnknown
 
@@ -426,7 +432,7 @@ proc genProcParams(m: BModule, t: PType, rettype, params: var Rope,
                    check: var IntSet, declareEnvironment=true;
                    weakDep=false) =
   params = nil
-  if t[0] == nil or isInvalidReturnType(m.config, t[0]):
+  if t[0] == nil or isInvalidReturnType(m.config, t):
     rettype = ~"void"
   else:
     rettype = getTypeDescAux(m, t[0], check, skResult)
@@ -461,7 +467,7 @@ proc genProcParams(m: BModule, t: PType, rettype, params: var Rope,
       params.addf(", NI $1Len_$2", [param.loc.r, j.rope])
       inc(j)
       arr = arr[0].skipTypes({tySink})
-  if t[0] != nil and isInvalidReturnType(m.config, t[0]):
+  if t[0] != nil and isInvalidReturnType(m.config, t):
     var arr = t[0]
     if params != nil: params.add(", ")
     if mapReturnType(m.config, t[0]) != ctArray:

compiler/cgen.nim

@@ -1039,7 +1039,7 @@ proc genProcAux(m: BModule, prc: PSym) =
       internalError(m.config, prc.info, "proc has no result symbol")
     let resNode = prc.ast[resultPos]
     let res = resNode.sym # get result symbol
-    if not isInvalidReturnType(m.config, prc.typ[0]):
+    if not isInvalidReturnType(m.config, prc.typ):
       if sfNoInit in prc.flags: incl(res.flags, sfNoInit)
       if sfNoInit in prc.flags and p.module.compileToCpp and (let val = easyResultAsgn(procBody); val != nil):
         var decl = localVarDecl(p, resNode)
@@ -1053,7 +1053,7 @@ proc genProcAux(m: BModule, prc: PSym) =
         initLocalVar(p, res, immediateAsgn=false)
       returnStmt = ropecg(p.module, "\treturn $1;$n", [rdLoc(res.loc)])
     else:
-      fillResult(p.config, resNode)
+      fillResult(p.config, resNode, prc.typ)
       assignParam(p, res, prc.typ[0])
       # We simplify 'unsureAsgn(result, nil); unsureAsgn(result, x)'
       # to 'unsureAsgn(result, x)'

compiler/semstmts.nim

@@ -2137,6 +2137,7 @@ proc semProcAux(c: PContext, n: PNode, kind: TSymKind,
       incl(s.flags, sfWasForwarded)
     elif sfBorrow in s.flags: semBorrow(c, n, s)
   sideEffectsCheck(c, s)
+
   closeScope(c)           # close scope for parameters
   # c.currentScope = oldScope
   popOwner(c)

tests/objects/t19342_2.nim

@@ -0,0 +1,18 @@
+discard """
+  targets: "c cpp"
+"""
+
+{.compile: "m19342.c".}
+
+# bug #19342
+type
+  Node* {.byRef.} = object
+    data: array[25, cint]
+
+proc myproc(name: cint): Node {.importc: "hello", cdecl.}
+
+proc parse =
+  let node = myproc(10)
+  doAssert node.data[0] == 999
+
+parse()