I feel as if I'm posting something along the exact same lines, for the 1000th time, but I guess it's because I still cannot make it work as I want to.
So... the question is - theoretically - very simple.
We have this piece of code:
doSomethingWith(x)
Now this x is a bit particular. First, it's a ref object. Then it has two field: a bool one (let's call it b), b) and a seq one (let's call it a).
Based on this b field, we want to do something slightly different.
Let's say:
if x.b:
doSomethingWith(x.a)
else:
doSomethingWith(butFirstPreprocess(x.a))
This works fine - let's keep that.
Now, the problem is that this particular check is going to appear 300 times in the code (ok, the doSomethingWith part may change, but the rest not so much). So, I'm thinking... templates!
So, I'm doing something like this:
template getOrPreprocess(z: myobj): untyped =
if z.b:
z.a
else:
butFirstPreprocess(z.a)
Which "works" as well. The quotes are because suddenly, even in the first case where all I want is z.a value, NOT a copy, again I come across this eqcopy thing in the code...
And no, I'm not saying the template didn't work as it's supposed to. I guess it did. Copy-pasting the code did pretty much the same:
doSomethingWith((if z.b: z.a else: butFirstPreprocess(z.a))
Yes, that copies z.a no-matter-what.
So... yes, of course I can fill my source code with countless copies of this if-else thing as in the 2nd example. The question is how can I solve this specific example, without bloating the source code that much?
It's more usual to do the oposite with templates:
template getOrPreprocess(z: myobj, op: untyped) =
if z.b:
op(z.a)
else:
op(butFirstPreprocess(z.a))
getOrPreprocess(x, doSomethingWith)
btw why exactly butFirstPreprocess makes a copy? I don't get it.
I get your point, totally.
The problem is that this op part, may be more than just... one proc call.
btw why exactly butFirstPreprocess makes a copy? I don't get it.
I wish I did.
Basically, let's take this (actual code shown):
push newInteger((if x.cleaned: x.a else: cleanedBlockImpl(x.a)).len)
The if x.cleaned part is what we're talking about here.
The resulting C code is:
tySequence__7OdtgriBtO6Ssh4FL1Hwew colontmpD_;
tySequence__7OdtgriBtO6Ssh4FL1Hwew colontmpD__2;
tySequence__7OdtgriBtO6Ssh4FL1Hwew T30_;
NI T36_;
tyObject_ValuecolonObjectType___i8zh8z6r9coyOmdltuF3inw* T37_;
colontmpD_.len = 0; colontmpD_.p = NIM_NIL;
colontmpD__2.len = 0; colontmpD__2.p = NIM_NIL;
// obj = default(typeof(obj))
// push newInteger((if x.cleaned: x.a else: cleanedBlockImpl(x.a)).len)
// push newInteger((if x.cleaned: x.a else: cleanedBlockImpl(x.a)).len)
T30_.len = 0; T30_.p = NIM_NIL;
// push newInteger((if x.cleaned: x.a else: cleanedBlockImpl(x.a)).len)
{
if (!(*x)._kind_19.cleaned) goto LA33_;
// obj = default(typeof(obj))
// obj = default(typeof(obj))
eqcopy___vmZvaluesZvalue_486((&colontmpD_), (*x)._kind_19.a);
if (NIM_UNLIKELY(*nimErr_)) goto LA1_;
T30_ = colontmpD_;
}
goto LA31_;
LA33_: ;
{
// push newInteger((if x.cleaned: x.a else: cleanedBlockImpl(x.a)).len)
// push newInteger((if x.cleaned: x.a else: cleanedBlockImpl(x.a)).len)
colontmpD__2 = cleanedBlockImpl__vmZvaluesZvalue_5436((*x)._kind_19.a);
T30_ = colontmpD__2;
}
LA31_: ;
T36_ = T30_.len;
// push newInteger((if x.cleaned: x.a else: cleanedBlockImpl(x.a)).len)
T37_ = NIM_NIL;
T37_ = newInteger__vmZvaluesZvalue_1260(T36_);
// obj = default(typeof(obj))
eqsink___vmZvaluesZvalue_543(&Stack__vmZstack_9.p->data[SP__vmZstack_13], T37_);
The copy is the eqcopy___vmZvaluesZvalue_486((&colontmpD_), (*x)._kind_19.a); line, where it practically copies x.a to a temp variable, instead of just using it directly.
"Trivial" you would say. Not quite. This particular line, in just one case, means a performance drop by close to 20% (obviously, depending on the case). Now, imagine that this thing happens constantly...
And ofc, as I said, I could just go on and write it exactly as I want. But I thought there should be a smoother way, especially given all these template/macro capabilities...
The truth is I haven't looked into it, nor do I know how they work exactly. I cannot find anything in the documentation.
Thanks for the suggestion in any case! :)
Ultra-minimal example that highlights the issues with using if .. else as a pseudo-ternary operator:
type
SomeObj = ref object
s: string
when isMainModule:
var ob = SomeObj(s:"Hello world")
var a = 2
echo (if a == 2: ob.s else: "4")
As straightforward as it seems, I would expect it to simply echo ob.s when a==2; but it doesn't. It copies ob.s to a temp var and then attempts the echo.
Compiled with `nim r -d:danger --mm:orc`:
#define NIM_INTBITS 64
#include "nimbase.h"
#undef LANGUAGE_C
#undef MIPSEB
#undef MIPSEL
#undef PPC
#undef R3000
#undef R4000
#undef i386
#undef linux
#undef mips
#undef near
#undef far
#undef powerpc
#undef unix
#define nimfr_(x, y)
#define nimln_(x, y)
typedef struct tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg;
typedef struct NimStrPayload NimStrPayload;
typedef struct NimStringV2 NimStringV2;
typedef struct TNimTypeV2 TNimTypeV2;
typedef struct tyObject_RefHeader__Gi7WQzlT1ZRToh9a2ueYb4A tyObject_RefHeader__Gi7WQzlT1ZRToh9a2ueYb4A;
struct NimStrPayload {
NI cap;
NIM_CHAR data[SEQ_DECL_SIZE];
};
struct NimStringV2 {
NI len;
NimStrPayload* p;
};
struct tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg {
NimStringV2 s;
};
typedef NimStringV2 tyArray__nHXaesL0DJZHyVS07ARPRA[1];
struct tyObject_RefHeader__Gi7WQzlT1ZRToh9a2ueYb4A {
NI rc;
NI rootIdx;
};
struct TNimTypeV2 {
void* destructor;
NI size;
NI align;
NCSTRING name;
void* traceImpl;
void* typeInfoV1;
NI flags;
};
N_LIB_PRIVATE N_NIMCALL(void*, nimNewObjUninit)(NI size, NI alignment);
N_LIB_PRIVATE N_NIMCALL(void, eqcopy___system_3266)(NimStringV2* dest, NimStringV2 src);
N_LIB_PRIVATE N_NIMCALL(void, echoBinSafe)(NimStringV2* args, NI argsLen_0);
N_LIB_PRIVATE N_NIMCALL(void, eqdestroy___system_3263)(NimStringV2* dest);
static N_INLINE(NIM_BOOL*, nimErrorFlag)(void);
N_LIB_PRIVATE N_NIMCALL(void, nimTestErrorFlag)(void);
N_LIB_PRIVATE N_NIMCALL(void, eqdestroy___testtern_19)(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg** dest);
static N_INLINE(NIM_BOOL, nimDecRefIsLastCyclicStatic)(void* p, TNimTypeV2* desc);
static N_INLINE(NI, minuspercent___system_716)(NI x, NI y);
N_LIB_PRIVATE N_NOINLINE(void, rememberCycle__system_3164)(NIM_BOOL isDestroyAction, tyObject_RefHeader__Gi7WQzlT1ZRToh9a2ueYb4A* s, TNimTypeV2* desc);
N_LIB_PRIVATE N_NIMCALL(void, eqdestroy___testtern_6)(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg* dest);
N_LIB_PRIVATE N_NIMCALL(void, nimRawDispose)(void* p, NI alignment);
static N_INLINE(void, initStackBottomWith)(void* locals);
N_LIB_PRIVATE N_NIMCALL(void, systemInit000)(void);
N_LIB_PRIVATE N_NIMCALL(void, testternDatInit000)(void);
N_LIB_PRIVATE N_NIMCALL(void, NimMainModule)(void);
static const struct {
NI cap; NIM_CHAR data[11+1];
} TM__p4sU0W5e8vSyxyRZxOVEZg_2 = { 11 | NIM_STRLIT_FLAG, "Hello world" };
static const NimStringV2 TM__p4sU0W5e8vSyxyRZxOVEZg_3 = {11, (NimStrPayload*)&TM__p4sU0W5e8vSyxyRZxOVEZg_2};
static const struct {
NI cap; NIM_CHAR data[1+1];
} TM__p4sU0W5e8vSyxyRZxOVEZg_4 = { 1 | NIM_STRLIT_FLAG, "4" };
static const NimStringV2 TM__p4sU0W5e8vSyxyRZxOVEZg_5 = {1, (NimStrPayload*)&TM__p4sU0W5e8vSyxyRZxOVEZg_4};
N_LIB_PRIVATE TNimTypeV2 NTIv2__L0d6eYijvbPCxxp6TULlkg_;
N_LIB_PRIVATE tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg* ob__testtern_4;
N_LIB_PRIVATE NI a__testtern_5 = ((NI) 2);
extern NIM_BOOL nimInErrorMode__system_4237;
static N_INLINE(NIM_BOOL*, nimErrorFlag)(void) {
NIM_BOOL* result;
result = (NIM_BOOL*)0;
result = (&nimInErrorMode__system_4237);
return result;
}
static N_INLINE(NI, minuspercent___system_716)(NI x, NI y) {
NI result;
result = (NI)0;
result = ((NI) ((NU)((NU64)(((NU) (x))) - (NU64)(((NU) (y))))));
return result;
}
static N_INLINE(NIM_BOOL, nimDecRefIsLastCyclicStatic)(void* p, TNimTypeV2* desc) {
NIM_BOOL result;
NIM_BOOL* nimErr_;
{nimErr_ = nimErrorFlag();
result = (NIM_BOOL)0;
{
tyObject_RefHeader__Gi7WQzlT1ZRToh9a2ueYb4A* cell;
NI T5_;
if (!!((p == NIM_NIL))) goto LA3_;
T5_ = (NI)0;
T5_ = minuspercent___system_716(((NI) (ptrdiff_t) (p)), ((NI) 16));
cell = ((tyObject_RefHeader__Gi7WQzlT1ZRToh9a2ueYb4A*) (T5_));
{
if (!((NI)((*cell).rc & ((NI) -16)) == ((NI) 0))) goto LA8_;
result = NIM_TRUE;
}
goto LA6_;
LA8_: ;
{
(*cell).rc -= ((NI) 16);
}
LA6_: ;
rememberCycle__system_3164(result, cell, desc);
if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_;
}
LA3_: ;
}BeforeRet_: ;
return result;
}
N_LIB_PRIVATE N_NIMCALL(void, eqdestroy___testtern_6)(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg* dest) {
eqdestroy___system_3263((&(*dest).s));
}
N_LIB_PRIVATE N_NIMCALL(void, eqdestroy___testtern_19)(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg** dest) {
{
NIM_BOOL T3_;
T3_ = (NIM_BOOL)0;
T3_ = nimDecRefIsLastCyclicStatic((*dest), (&NTIv2__L0d6eYijvbPCxxp6TULlkg_));
if (!T3_) goto LA4_;
eqdestroy___testtern_6((*dest));
nimRawDispose((*dest), ((NI) 8));
}
LA4_: ;
}
static N_INLINE(void, initStackBottomWith)(void* locals) {
}
N_LIB_PRIVATE void PreMainInner(void) {
}
N_LIB_PRIVATE int cmdCount;
N_LIB_PRIVATE char** cmdLine;
N_LIB_PRIVATE char** gEnv;
N_LIB_PRIVATE void PreMain(void) {
void (*volatile inner)(void);
inner = PreMainInner;
systemInit000();
testternDatInit000();
(*inner)();
}
N_LIB_PRIVATE N_CDECL(void, NimMainInner)(void) {
NimMainModule();
}
N_CDECL(void, NimMain)(void) {
void (*volatile inner)(void);
PreMain();
inner = NimMainInner;
initStackBottomWith((void *)&inner);
(*inner)();
}
int main(int argc, char** args, char** env) {
cmdLine = args;
cmdCount = argc;
gEnv = env;
NimMain();
return nim_program_result;
}
N_LIB_PRIVATE N_NIMCALL(void, NimMainModule)(void) {
{
NimStringV2 colontmpD_;
NimStringV2 colontmpD__2;
tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg* T2_;
tyArray__nHXaesL0DJZHyVS07ARPRA T3_;
NIM_BOOL* nimErr_;
nimErr_ = nimErrorFlag();
colontmpD_.len = 0; colontmpD_.p = NIM_NIL;
colontmpD__2.len = 0; colontmpD__2.p = NIM_NIL;
T2_ = NIM_NIL;
T2_ = (tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg*) nimNewObjUninit(sizeof(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg), NIM_ALIGNOF(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg));
(*T2_).s = TM__p4sU0W5e8vSyxyRZxOVEZg_3;
ob__testtern_4 = T2_;
{
if (!(a__testtern_5 == ((NI) 2))) goto LA6_;
eqcopy___system_3266((&colontmpD_), (*ob__testtern_4).s);
T3_[0] = colontmpD_;
}
goto LA4_;
LA6_: ;
{
colontmpD__2 = TM__p4sU0W5e8vSyxyRZxOVEZg_5;
T3_[0] = colontmpD__2;
}
LA4_: ;
echoBinSafe(T3_, 1);
{
LA1_:;
}
{
eqdestroy___system_3263((&colontmpD__2));
eqdestroy___system_3263((&colontmpD_));
}
if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_;
eqdestroy___testtern_19(&ob__testtern_4);
BeforeRet_: ;
nimTestErrorFlag();
}
}
N_LIB_PRIVATE N_NIMCALL(void, testternDatInit000)(void) {
NTIv2__L0d6eYijvbPCxxp6TULlkg_.destructor = (void*)eqdestroy___testtern_6; NTIv2__L0d6eYijvbPCxxp6TULlkg_.size = sizeof(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg); NTIv2__L0d6eYijvbPCxxp6TULlkg_.align = NIM_ALIGNOF(tyObject_SomeObjcolonObjectType___L0d6eYijvbPCxxp6TULlkg); NTIv2__L0d6eYijvbPCxxp6TULlkg_.name = "|unknown.testtern.SomeObj:ObjectType|";
; NTIv2__L0d6eYijvbPCxxp6TULlkg_.traceImpl = (void*)NIM_NIL; NTIv2__L0d6eYijvbPCxxp6TULlkg_.flags = 0;}
Here is what you can do to eliminate the copy:
proc getOrPreprocess(z: myobj): lent seq[T] {.inline.} =
if z.b:
z.a
else:
butFirstPreprocess(z.a)
If you don't change the length of the sequence (add, remove, setLen, etc) then declare the doSomethingWith proc's working on var openarray s as @shirleyquirk suggested.
type
Foo = object
x: string
proc `=copy`(dest: var Foo; src: Foo) =
# Echo some message when Foo is copied
echo src.x, " is copied"
dest = Foo(x: src.x)
proc someProc(x: openArray[Foo]) =
echo x
proc someVarProc(x: var openArray[Foo]) =
echo x
var
s = @[Foo(x: "s")]
t = @[Foo(x: "t")]
# This doesn't copy s
echo s
# if expression copies s
#echo if true: s else: t
# Just passing seq to openArray parameter doesn't copy.
someProc(s)
# Adding if expression makes copy
#someProc(if true: s else: t)
# You cannot pass if expression to var parameter:
# first type mismatch at position: 1
# required type for x: var openArray[Foo]
# but expression 'if true: s else: t' is immutable, not 'var'
#someVarProc(if true: s else: t)
# Adding `toOpenArray` to each seq prevents copy
someProc(if true: s.toOpenArray(0, s.high) else: t.toOpenArray(0, t.high))
# read s and t so that they are not moved.
echo s
echo t@planetis Adding lent return type cause compile error if return value is a seq created by a proc.
type
Foo = object
x: string
proc `=copy`(dest: var Foo; src: Foo) =
# Echo some message when Foo is copied
echo src.x, " is copied"
dest = Foo(x: src.x)
proc butFirstPreprocess(s: seq[Foo]): seq[Foo] =
s & @[Foo(x: "preprocess")]
when false:
proc getOrPreprocess(s: seq[Foo]): lent seq[Foo] {.inline.} =
# Compile Error: expression has no address
if true:
s
else:
butFirstPreprocess(s)
let s = @[Foo(x: "s")]
# echo getOrPreprocess(s)
proc butSecondPreprocess(s: seq[Foo]): lent seq[Foo] {.inline.} =
s
proc getOrPreprocess2(s: seq[Foo]): lent seq[Foo] {.inline.} =
# Error: expression has no address
if true:
s
else:
butSecondPreprocess(s)
var s2 = @[Foo(x: "s2")]
echo getOrPreprocess2(s2)This could be refactored to:
proc getOrPreprocess(s: seq[Foo]): lent seq[Foo] {.inline.} =
# Compile Error: expression has no address
if not true:
butFirstPreprocess(s)
s
Thanks a lot for the input, all of you!
The problem with @planetis's approach is this lent, as @demotomohiro also mentioned.
I have managed to resolve the issue with a rather weird approach, but it definitely does what I want it to:
macro ensureCleaned*(name: untyped): untyped =
let cleanName = ident("clean" & ($name).capitalizeAscii())
let cleanedBlock = ident("cleanedBlockTmp" & ($name).capitalizeAscii())
when not defined(NOERRORLINES):
result = quote do:
var `cleanedBlock`: ValueArray
let `cleanName` {.cursor.} = (
if `name`.dirty:
`cleanedBlock` = cleanedBlockImpl(`name`.a)
`cleanedBlock`
else:
`name`.a
)
else:
result = quote do:
let `cleanName` {.cursor.} = `name`.a
The actual code resides here: https://github.com/arturo-lang/arturo/blob/master/src/vm/values/clean.nim#L57-L72
And here's an example of it in use: https://github.com/arturo-lang/arturo/blob/master/src/library/Collections.nim#L1299-L1301