Closed
Description
In some situation, in particular in a for loop, the compiler may ignore the {.noInit.}
pragma.
This is problematic for non-copyable non-movable type, for example a shared atomic counter.
This can be caught at compile-time by making =sink
an error:
import std/atomics
type AtomicCounter* = object
count: Atomic[int]
proc `=`*(dst: var AtomicCounter, src: AtomicCounter) {.error: "An atomic counter cannot be copied.".}
proc `=sink`*(dst: var AtomicCounter, src: AtomicCounter) {.error: "An atomic counter cannot be moved.".}
proc `=destroy`*(sb: var AtomicCounter) {.inline.}=
discard
proc initialize*(ac: var AtomicCounter) {.inline.} =
ac.count.store(0, moRelaxed)
proc inc*(p: ptr AtomicCounter) =
discard p.count.fetchAdd(1, moRelaxed)
var currentCounter: ptr AtomicCounter
template scopedCounter*(body: untyped): untyped =
bind currentCounter
block:
let suspended = currentCounter
var a {.noInit.}: AtomicCounter
a.initialize()
currentCounter = addr(a)
body
currentCounter = suspended
proc foo() =
scopedCounter:
echo "Counting ..."
proc bar() =
for i in 0 .. 2:
scopedCounter:
echo "Counting ..."
foo()
bar() # Does not compile, Nim tries to insert `=sink` inside a for loop
Error: '=sink' is not available for type <AtomicCounter>; routine: bar
In the C codegen we can see the eqsink
import std/atomics
type AtomicCounter* = object
count: Atomic[int]
proc `=`*(dst: var AtomicCounter, src: AtomicCounter) {.error: "An atomic counter cannot be copied.".}
when false:
proc `=sink`*(dst: var AtomicCounter, src: AtomicCounter) {.error: "An atomic counter cannot be moved.".}
else:
proc `=sink`*(dst: var AtomicCounter, src: AtomicCounter) {.inline.} =
system.`=sink`(dst.count, src.count)
proc `=destroy`*(sb: var AtomicCounter) {.inline.}=
discard
proc initialize*(ac: var AtomicCounter) {.inline.} =
ac.count.store(0, moRelaxed)
proc inc*(p: ptr AtomicCounter) =
discard p.count.fetchAdd(1, moRelaxed)
var currentCounter: ptr AtomicCounter
template scopedCounter*(body: untyped): untyped =
bind currentCounter
block:
let suspended = currentCounter
var a {.noInit.}: AtomicCounter
a.initialize()
currentCounter = addr(a)
body
currentCounter = suspended
proc foo() =
scopedCounter:
echo "Counting ..."
proc bar() =
for i in 0 .. 2:
scopedCounter:
echo "Counting ..."
foo()
bar() # eqsink in C codegen
foo function
N_LIB_PRIVATE N_NIMCALL(void, foo__JzbnjO5RDbS420V3wzhF6Q)(void) {
tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q aX60gensym5240276_;
TSafePoint TM__9aQ9cEDplzy5RH1NH07Dxehw_2;
pushSafePoint(&TM__9aQ9cEDplzy5RH1NH07Dxehw_2);
TM__9aQ9cEDplzy5RH1NH07Dxehw_2.status = setjmp(TM__9aQ9cEDplzy5RH1NH07Dxehw_2.context);
if (TM__9aQ9cEDplzy5RH1NH07Dxehw_2.status == 0) {
{
tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q* suspendedX60gensym5240275_;
suspendedX60gensym5240275_ = currentCounter__uTp0tKFcbHqRY13s6w9cfig;
initialize__5UeDB2gx8kATVzHYGeWFRAnoinit_custom_destructors((&aX60gensym5240276_));
currentCounter__uTp0tKFcbHqRY13s6w9cfig = (&aX60gensym5240276_);
echoBinSafe(TM__9aQ9cEDplzy5RH1NH07Dxehw_3, 1);
currentCounter__uTp0tKFcbHqRY13s6w9cfig = suspendedX60gensym5240275_;
}
popSafePoint();
}
else {
popSafePoint();
}
{
eqdestroy___DjicWcWwV2o4NWG9aKCOq3gnoinit_custom_destructors((&aX60gensym5240276_));
if (TM__9aQ9cEDplzy5RH1NH07Dxehw_2.status != 0) nimLeaveFinally();
}
if (TM__9aQ9cEDplzy5RH1NH07Dxehw_2.status != 0) reraiseException();
}
bar function
N_LIB_PRIVATE N_NIMCALL(void, bar__JzbnjO5RDbS420V3wzhF6Q_2)(void) {
tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q aX60gensym5245035_;
TSafePoint TM__9aQ9cEDplzy5RH1NH07Dxehw_5;
pushSafePoint(&TM__9aQ9cEDplzy5RH1NH07Dxehw_5);
TM__9aQ9cEDplzy5RH1NH07Dxehw_5.status = setjmp(TM__9aQ9cEDplzy5RH1NH07Dxehw_5.context);
if (TM__9aQ9cEDplzy5RH1NH07Dxehw_5.status == 0) {
{
NI i;
NI res;
i = (NI)0;
res = ((NI) 0);
{
while (1) {
NI TM__9aQ9cEDplzy5RH1NH07Dxehw_6;
if (!(res <= ((NI) 2))) goto LA4;
i = res;
{
tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q* suspendedX60gensym5245034_;
tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q T6_;
suspendedX60gensym5245034_ = currentCounter__uTp0tKFcbHqRY13s6w9cfig;
nimZeroMem((void*)(&T6_), sizeof(tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q));
eqsink___f9azodprU2vXmRJy7KoSVUAnoinit_custom_destructors((&aX60gensym5245035_), T6_);
initialize__5UeDB2gx8kATVzHYGeWFRAnoinit_custom_destructors((&aX60gensym5245035_));
currentCounter__uTp0tKFcbHqRY13s6w9cfig = (&aX60gensym5245035_);
echoBinSafe(TM__9aQ9cEDplzy5RH1NH07Dxehw_3, 1);
currentCounter__uTp0tKFcbHqRY13s6w9cfig = suspendedX60gensym5245034_;
}
if (nimAddInt(res, ((NI) 1), &TM__9aQ9cEDplzy5RH1NH07Dxehw_6)) { raiseOverflow(); };
res = (NI)(TM__9aQ9cEDplzy5RH1NH07Dxehw_6);
} LA4: ;
}
}
popSafePoint();
}
else {
popSafePoint();
}
{
eqdestroy___DjicWcWwV2o4NWG9aKCOq3gnoinit_custom_destructors((&aX60gensym5245035_));
if (TM__9aQ9cEDplzy5RH1NH07Dxehw_5.status != 0) nimLeaveFinally();
}
if (TM__9aQ9cEDplzy5RH1NH07Dxehw_5.status != 0) reraiseException();
}
More importantly, this will avoid bad codegen in C++ that only happen for bar
but not for foo
........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp: In function ‘void eqsink___f9azodprU2vXmRJy7KoSVUAnoinit_custom_destructors(tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q&, tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q)’:
........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp:126:18: error: use of deleted function ‘std::atomic<long int>& std::atomic<long int>::operator=(const std::atomic<long int>&)’
126 | dst.count = src.count;
| ^~~~~
In file included from ........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp:10:
/usr/include/c++/9.3.0/atomic:802:15: note: declared here
802 | atomic& operator=(const atomic&) = delete;
| ^~~~~~~~
........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp: In function ‘void bar__JzbnjO5RDbS420V3wzhF6Q_2()’:
........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp:150:88: error: use of deleted function ‘tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q::tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q(const tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q&)’
150 | eqsink___f9azodprU2vXmRJy7KoSVUAnoinit_custom_destructors(aX60gensym5215035_, T8_);
| ^
........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp:38:8: note: ‘tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q::tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q(const tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q&)’ is implicitly deleted because the default definition would be ill-formed:
38 | struct tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp:38:8: error: use of deleted function ‘std::atomic<long int>::atomic(const std::atomic<long int>&)’
In file included from ........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp:10:
/usr/include/c++/9.3.0/atomic:801:7: note: declared here
801 | atomic(const atomic&) = delete;
| ^~~~~~
........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp:125:184: note: initializing argument 2 of ‘void eqsink___f9azodprU2vXmRJy7KoSVUAnoinit_custom_destructors(tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q&, tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q)’
125 | static N_INLINE(void, eqsink___f9azodprU2vXmRJy7KoSVUAnoinit_custom_destructors)(tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q& dst, tyObject_AtomicCounter__Yz7FoDTBlGPGeC6wQ9aoO7Q src) {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~
Error: execution of an external compiler program 'g++ -c -w -w -fpermissive -pthread -O3 -fno-strict-aliasing -fno-ident -std=gnu++14 -funsigned-char -I/home/beta/.choosenim/toolchains/nim-1.2.0/lib -I......... -o ........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp.o ........./nimcache/atcounter/@mnoinit_custom_destructors.nim.cpp' failed with exit code: 1
This may be related to my C++ codegen woes in #13093 which I have no workaround for at the moment.