Use CHAR_BIT instead of hard-coded literal 8 in generated C code

[pavel-kirienko]

It complicates the usage of Nunavut-generated code with those odd (DSP) platforms where CHAR_BIT != 8. See details at https://github.com/UAVCAN/libcanard/issues/184#issuecomment-1008287418

[TYuD]

It complicates the usage of Nunavut-generated code...

As the example with the nunavutCopyBits function showed, it does not complicate much. Although it is possible that difficulties may arise in other places of the program.

Nevertheless, do you intend to implement this feature or not?

For example, because of this feature, I don't consider uavcan for my systems.

[pavel-kirienko]

Neither I nor Scott is likely to work on this in the foreseeable future, but a pull request (from you or anybody else) would be welcome.

[TYuD]

I could take the C-code of the program. It seems C is not needed directly, but a python code generator. I don't use a python. Is it enough to fix only the C-code? Maybe I'll fix the C-code, and you'll make changes to Python?

[pavel-kirienko]

I don't foresee any changes to Python being necessary. Updating the C templates should be enough I think; specifically, look for all occurences of 8U:

• https://github.com/UAVCAN/nunavut/blob/main/src/nunavut/lang/c/support/serialization.j2
• https://github.com/UAVCAN/nunavut/blob/main/src/nunavut/lang/c/templates/serialization.j2
• https://github.com/UAVCAN/nunavut/blob/main/src/nunavut/lang/c/templates/deserialization.j2

[TYuD]

Templates of jinja2 I know much worse than python

[pavel-kirienko]

I recommend reading the Jinja docs here (you don't need to read everything, just the intro is enough): https://jinja.palletsprojects.com/en/3.0.x/templates/

[TYuD]

Is the following sequence possible:

1. write code in C;
2. check it with tests;
3. correct the jinja-templates based on the C-code;
4. use the templates to generate a new C-code;
5. check the generated C-code?

Are the tests of items 2) and 5) the same?

Are there these tests and where are they?

[pavel-kirienko]

Yes, but you may need to tweak the build/test workflow a little. The tests are located here: https://github.com/UAVCAN/nunavut/tree/main/verification/c/suite

The build recipe is here: https://github.com/UAVCAN/nunavut/blob/main/verification/CMakeLists.txt

Normally you would run the verification suite using this helper: https://github.com/UAVCAN/nunavut/blob/main/.github/verify.py

Please read the contributing guide to understand how to use all that: https://nunavut.readthedocs.io/en/latest/docs/dev.html

The tweak I referred to amounts to changing the verification build recipe such that it does not regenerate the support header on build (since you would be modifying it manually). I mean you can do that but should you really? Editing the template seems like a no-brainer, it's basically the same C code but with a slightly weirder syntax.

[TYuD]

Pavel, there is another problem. In some C compilers, the size of double is not necessarily 64 bits. Is it possible to use macros like FLOAT32, FLOAT64 instead of 'float' and 'double' keywords? Can You set these macros in the settings of the nunavut code generator?

[TYuD]

Pavel, I have update all functions in serialization.h (see: https://github.com/TYuD/hello_world/blob/master/serialization_2.h) and pass tests https://github.com/UAVCAN/nunavut/blob/main/verification/c/suite/test_support.c

Here is test output: ....\nunavut_qt\test_support.c:765:testNunavutCopyBits:PASS ....\nunavut_qt\test_support.c:766:testNunavutCopyBitsWithAlignedOffset:PASS ....\nunavut_qt\test_support.c:767:testNunavutCopyBitsWithUnalignedOffset:PASS ....\nunavut_qt\test_support.c:768:testNunavutSaturateBufferFragmentBitLength:PASS ....\nunavut_qt\test_support.c:769:testNunavutGetBits:PASS ....\nunavut_qt\test_support.c:770:testNunavutSetIxx_neg1:PASS ....\nunavut_qt\test_support.c:771:testNunavutSetIxx_neg255:PASS ....\nunavut_qt\test_support.c:772:testNunavutSetIxx_neg255_tooSmall:PASS ....\nunavut_qt\test_support.c:773:testNunavutSetIxx_bufferOverflow:PASS ....\nunavut_qt\test_support.c:774:testNunavutSetBit:PASS ....\nunavut_qt\test_support.c:775:testNunavutSetBit_bufferOverflow:PASS ....\nunavut_qt\test_support.c:776:testNunavutGetBit:PASS ....\nunavut_qt\test_support.c:777:testNunavutGetU8:PASS ....\nunavut_qt\test_support.c:778:testNunavutGetU8_tooSmall:PASS ....\nunavut_qt\test_support.c:779:testNunavutGetU16:PASS ....\nunavut_qt\test_support.c:780:testNunavutGetU16_tooSmall:PASS ....\nunavut_qt\test_support.c:781:testNunavutGetU32:PASS ....\nunavut_qt\test_support.c:782:testNunavutGetU32_tooSmall:PASS ....\nunavut_qt\test_support.c:278:testNunavutGetU64:FAIL: Unity 64-bit Support Disabled ....\nunavut_qt\test_support.c:284:testNunavutGetU64_tooSmall:FAIL: Unity 64-bit Support Disabled ....\nunavut_qt\test_support.c:785:testNunavutGetI8:PASS ....\nunavut_qt\test_support.c:786:testNunavutGetI8_tooSmall:PASS ....\nunavut_qt\test_support.c:787:testNunavutGetI8_tooSmallAndNegative:PASS ....\nunavut_qt\test_support.c:788:testNunavutGetI8_zeroDataLen:PASS ....\nunavut_qt\test_support.c:789:testNunavutGetI16:PASS ....\nunavut_qt\test_support.c:790:testNunavutGetI16_tooSmall:PASS ....\nunavut_qt\test_support.c:791:testNunavutGetI16_tooSmallAndNegative:PASS ....\nunavut_qt\test_support.c:792:testNunavutGetI16_zeroDataLen:PASS ....\nunavut_qt\test_support.c:793:testNunavutGetI32:PASS ....\nunavut_qt\test_support.c:794:testNunavutGetI32_tooSmall:PASS ....\nunavut_qt\test_support.c:795:testNunavutGetI32_tooSmallAndNegative:PASS ....\nunavut_qt\test_support.c:796:testNunavutGetI32_zeroDataLen:PASS ....\nunavut_qt\test_support.c:378:testNunavutGetI64:FAIL: Unity 64-bit Support Disabled ....\nunavut_qt\test_support.c:384:testNunavutGetI64_tooSmall:FAIL: Unity 64-bit Support Disabled ....\nunavut_qt\test_support.c:390:testNunavutGetI64_tooSmallAndNegative:FAIL: Unity 64-bit Support Disabled ....\nunavut_qt\test_support.c:396:testNunavutGetI64_zeroDataLen:FAIL: Unity 64-bit Support Disabled ....\nunavut_qt\test_support.c:801:testNunavutFloat16Pack:PASS ....\nunavut_qt\test_support.c:802:testNunavutFloat16Pack_NAN_cmath:PASS ....\nunavut_qt\test_support.c:803:testNunavutFloat16Pack_infinity:PASS ....\nunavut_qt\test_support.c:804:testNunavutFloat16Pack_zero:PASS ....\nunavut_qt\test_support.c:805:testNunavutFloat16Unpack:PASS ....\nunavut_qt\test_support.c:806:testNunavutFloat16PackUnpack:PASS ....\nunavut_qt\test_support.c:807:testNunavutFloat16PackUnpack_NAN:PASS ....\nunavut_qt\test_support.c:808:testNunavutFloat16Unpack_INFINITY:PASS ....\nunavut_qt\test_support.c:809:testNunavutSet16:PASS ....\nunavut_qt\test_support.c:810:testNunavutGet16:PASS ....\nunavut_qt\test_support.c:811:testNunavutSetF32:PASS ....\nunavut_qt\test_support.c:812:testNunavutGetF32:PASS ....\nunavut_qt\test_support.c:666:testNunavutGetF64:FAIL: Unity Double Precision Disabled ....\nunavut_qt\test_support.c:714:testNunavutSetF64:FAIL: Unity 64-bit Support Disabled

---

50 Tests 8 Failures 0 Ignored FAIL

There are 7 failures with description 'Unity 64-bit Support Disabled' and 1 failure 'Unity Double Precision Disabled'. Is it normal? How to enable 64-bit Support?

[pavel-kirienko]

Is it possible to use macros like FLOAT32, FLOAT64 instead of 'float' and 'double' keywords? Can You set these macros in the settings of the nunavut code generator?

That is not currently supported. If float is not IEEE754 binary32 compatible, OR if double is not IEEE754 binary64 compatible, you are guaranteed to get a compile-time error. The checks are done here:

https://github.com/UAVCAN/nunavut/blob/61232b31b82ba1b6a3b0bf82392975399a541137/src/nunavut/lang/c/support/serialization.j2#L86-L90

https://github.com/UAVCAN/nunavut/blob/61232b31b82ba1b6a3b0bf82392975399a541137/src/nunavut/lang/c/support/serialization.j2#L488-L651

We should perhaps define code-generation-time aliases for float32 and float64, similar to {{ typename_unsigned_length }} etc. @thirtytwobits would you support this?

There are 7 failures with description 'Unity 64-bit Support Disabled' and 1 failure 'Unity Double Precision Disabled'. Is it normal? How to enable 64-bit Support?

It is enabled already as you can see here: https://github.com/UAVCAN/nunavut/blob/61232b31b82ba1b6a3b0bf82392975399a541137/verification/CMakeLists.txt#L150 Are you sure you followed the instructions precisely? Did you change the unity submodule? The process name in your posted screenshot looks super suspicious, how can it be related to QtCreator?

N.B. it's always better to post CLI dumps as text rather than as images.

[TYuD]

Also I have much simplify functions nunavutGetIxx. Look at them please. Are they correct?

[TYuD]

N.B. it's always better to post CLI dumps as text rather than as images.

Sorry. It's my first activity with GitHub

[pavel-kirienko]

Also I have much simplify functions nunavutGetIxx. Look at look at them please. Are they correct?

Technically they are not because they rely on non-standard implementation-defined behavior: shifting a negative integer to the right is not guaranteed to produce the desired outcome (which is to extend the sign bit).

[TYuD]

Technically they are not because they rely on non-standard implementation-defined behavior: shifting a negative integer to the right is not guaranteed to produce the desired outcome (which is to extend the sign bit).

I was sure this trick is standard. It would be to make sure additionally

[pavel-kirienko]

For platforms that don't have small integer types, it should be safe to make small-size getters/setters mere aliases of their larger counterparts. E.g., nunavutGetU8 becomes an alias of nunavutGetU16.

I was sure this trick is standard.

See https://en.cppreference.com/w/c/language/operator_arithmetic and https://en.cppreference.com/w/cpp/language/operator_arithmetic. Actually, I should correct myself: the behavior is implementation-defined in C and undefined in C++.

[TYuD]

Are you sure you followed the instructions precisely?

I'm not following the instructions temporarily. I want to make sure that the solution exists without wasting time. Then I will study your technology stack. Am I distracting you a lot?

[pavel-kirienko]

I understand what you're saying but you are at a far greater risk of wasting time when you are not following the instructions. Seriously. Also, I'm not sure if anybody has attempted to develop Nunavut on Windows, so that's another possible source of issues.

[TYuD]

until C++20: For negative a, the value of a >> b is implementation-defined (in most implementations, this performs arithmetic right shift, so that the result remains negative).

since C++20: The value of a >> b is a/2b, rounded down (in other words, right shift on signed a is arithmetic right shift)

[pavel-kirienko]

Nunavut aims to support C++14 and C99, so C++20 is not that relevant.

[TYuD]

I'm not sure if anybody has attempted to develop Nunavut on Windows, so that's another possible source of issues.

I'm not going to be a nunavut developer. I just want a local feature for CHAR_BIT > 8 to appear in nunavut

[TYuD]

Nunavut aims to support C++14 and C99, so C++20 is not that relevant

It is possible with the help of conditional compilation (#if/#else) use a fast algorithm for those compilers that perform an arithmetic shift to the right. For the rest compilers - slow algorithm

[thirtytwobits]

We should perhaps define code-generation-time aliases for float32 and float64, similar to {{ typename_unsigned_length }}

agree. This is a good plan

[thirtytwobits]

Nunavut aims to support C++14 and C99, so C++20 is not that relevant.

I don't agree with this. C++17 is explicitly supported and C++20 implicitly (i.e. you should be able to use the C++17 headers with C++20)

[thirtytwobits]

@TYuD , thanks for the effort here. Yes, I don't have access to a windows PC so I've never tried to develop this project in that environment. Please let me know if there are issues.

[TYuD]

I do not know the rules of Github. Is our discussion a flood?

[TYuD]

Please let me know if there are issues.

Ok.

c/suite/test_support.c works Ok on Windows/Qt.

[TYuD]

That is not currently supported. If float is not IEEE754 binary32 compatible, OR if double is not IEEE754 binary64 compatible, you are guaranteed to get a compile-time error.

There is a nuance here. Both IEEE754 binary32 and IEEE754 binary64 standart can be supported by the compiler, so the protection static_assert(NUNAVUT_PLATFORM_IEEE754_DOUBLE) will pass. But! 'float' and 'double' may be the same. Namely IEEE754 binary32. IEEE754 binary64 may be realized as a 'long double'.

Is it possible to exclude float64 (in future) using the code generator options? This is highly desirable for a number of systems

[TYuD]

It is enabled already as you can see here: nunavut/verification/CMakeLists.txt Line 150 in 61232b3 add_definitions(-DUNITY_SUPPORT_64=1

I have added predefined macros UNITY_SUPPORT_64, UNITY_INCLUDE_FLOAT, UNITY_INCLUDE_DOUBLE. Now all tests passed:

50 Tests 0 Failures 0 Ignored OK

[pavel-kirienko]

There is a nuance here. Both IEEE754 binary32 and IEEE754 binary64 standart can be supported by the compiler, so the protection static_assert(NUNAVUT_PLATFORM_IEEE754_DOUBLE) will pass. But! 'float' and 'double' may be the same. Namely IEEE754 binary32. IEEE754 binary64 may be realized as a 'long double'.

It doesn't matter. If you look at the code you will see that the assertion checks will pass if and only if float is IEEE 754 binary32 and double is IEEE 754 binary64.

It is possible to make the code rely only on float (as in binary32), yes, but I suppose at the moment this would be considered a low-priority feature.