assert() cannot be used in expression context

[dhalbert]

@kamtom480: a question for you:

The implementation of assert() in spresense comes from nuttx. assert() is defined as ASSERT() when NDEBUG is not defined. See https://github.com/sonydevworld/spresense-nuttx/blob/new-master/include/assert.h:

#define ASSERT(f)        do { if (!(f)) PANIC(); } while (0)

This version of assert() cannot be used in an expression context, such as a comma expression. It can only be used in statement context.

We are merging recent changes from upstream MicroPython into CircuitPython, and there are several places where MicroPython uses assert() in a comma expression here:
https://github.com/micropython/micropython/blob/05cb1406ad1b421a238faf763e19f4119f5f6bb2/py/obj.h#L921-L926

#define mp_type_assert_not_bool_int_str_nonetype(t) (                                     \
    MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_bool), assert((t) != &mp_type_bool),         \
    MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_int), assert((t) != &mp_type_int),           \
    MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_str), assert((t) != &mp_type_str),           \
    MP_STATIC_ASSERT_NOT_MSC((t) != &mp_type_NoneType), assert((t) != &mp_type_NoneType), \
    1)

This doesn't work with the statement-only do ... while(0) version of assert() above, and so we can't build the spresense port in in our merge branch right now.

man 3 assert says assert() has the signature void assert(scalar expression);, though it might be implemented as a macro.

The other versions of assert() in the other CircuitPython and MicroPython can be used in expressions.

I am not sure how to fix this: the particular macro above is tricky and hard to rewrite with the nuttx assert. The nuttx assert could be modified, but that is a change in that submodule. Or a new version of assert() could be introduced to substitute for the nuttx version.

What do you think? Thanks.

We don't have a merge PR yet, but @tannewt and I are working in https://github.com/dhalbert/circuitpython/tree/v1.20-merge

[kamtom480]

@dhalbert Thank you for letting me know. I'm currently checking if we can change the assert in NuttX to also work in CircuitPython. But this is not a quick solution. This will take time.

However, in the meantime I noticed that the Spresense SDK is built with the NDEBUG flag: https://github.com/sonydevworld/spresense-exported-sdk/blob/c12296c5ffbd0779e630a653c76a78558b463271/nuttx/.config#L58

But the configuration was not used in CircuitPython. Below is a patch that fixes this:

diff --git a/ports/cxd56/Makefile b/ports/cxd56/Makefile
index 238edb96e4..81b1b171e3 100644
--- a/ports/cxd56/Makefile
+++ b/ports/cxd56/Makefile
@@ -23,6 +23,7 @@
 # THE SOFTWARE.
 
 include ../../py/circuitpy_mkenv.mk
+include ./spresense-exported-sdk/nuttx/.config
 
 CROSS_COMPILE = arm-none-eabi-
 
@@ -91,6 +92,10 @@ CFLAGS += \
 	-fdata-sections \
 	-Wall \
 
+ifeq ($(CONFIG_NDEBUG),y)
+    CFLAGS += -DNDEBUG
+endif
+
 OPTIMIZATION_FLAGS ?= -O2 -fno-inline-functions
 
 # option to override compiler optimization level, set in boards/$(BOARD)/mpconfigboard.mk

At this point you should be able to build CircuitPython without any problem with Spresense. However, when the NDEBUG flag is enabled, assert will do nothing.

Maybe CircuitPython could have its own assert implementation? What do you think about this?

[dhalbert]

Thanks for your comments. Right now we simply removed the troublesome assert, and that may be all we need to do. In the long run perhaps we could talk to the nuttx people about the issue. It does seem to be a rare problem, though. We will also see about NDEBUG, as you mentioned.