Skip to content

{.noinit.} ignored in for loop -> bad codegen for non-movable types #14118

Closed
@mratsim

Description

@mratsim

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.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions