From 0c3892c3c707e75520d6c8b5a65a0eaf8c15fcd4 Mon Sep 17 00:00:00 2001 From: flywind Date: Fri, 28 Jan 2022 16:53:42 +0800 Subject: [PATCH] nvro don't touch cdecl types [backport: 1.6] (#19461) * nvro don't touch cdecl types; fix #19342 again --- compiler/ast.nim | 2 +- compiler/ccgcalls.nim | 6 +++--- compiler/ccgstmts.nim | 21 ++++++++++++++------- compiler/ccgtypes.nim | 18 ++++++++++++------ compiler/cgen.nim | 4 ++-- compiler/semstmts.nim | 1 + tests/objects/t19342_2.nim | 18 ++++++++++++++++++ 7 files changed, 51 insertions(+), 19 deletions(-) create mode 100644 tests/objects/t19342_2.nim diff --git a/compiler/ast.nim b/compiler/ast.nim index 4286f940f138..ad5ec12f530f 100644 --- a/compiler/ast.nim +++ b/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 diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim index fe0516f9a584..197728ccfc7b 100644 --- a/compiler/ccgcalls.nim +++ b/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}: diff --git a/compiler/ccgstmts.nim b/compiler/ccgstmts.nim index 7fecc1475291..58a6dd015c0a 100644 --- a/compiler/ccgstmts.nim +++ b/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: diff --git a/compiler/ccgtypes.nim b/compiler/ccgtypes.nim index df1874b26870..f010b25c906b 100644 --- a/compiler/ccgtypes.nim +++ b/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: diff --git a/compiler/cgen.nim b/compiler/cgen.nim index 246b8c0e9eae..10b4b55d0f2c 100644 --- a/compiler/cgen.nim +++ b/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)' diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim index 33e304ab2264..c2b385c80d58 100644 --- a/compiler/semstmts.nim +++ b/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) diff --git a/tests/objects/t19342_2.nim b/tests/objects/t19342_2.nim new file mode 100644 index 000000000000..6f6d0f2b3d31 --- /dev/null +++ b/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() \ No newline at end of file