Nasty errors in CH32V003 header files

[mikeselectricstuff]

Just got caught by this - incorrectly defined bit definitions in the default header files in Mountriver Studio - bit positions are not actually wrong, but declared as the wrong type.
Most CH32V003 registers are only 16 bits, except the ones that aren't...
What is the point of assigning a type to constants like this anyway, especially if the compiler doesn't catch invalid usage like this?

[HwAoRrDk]

Yup. This is one of the errors I found myself a while back. I think I mentioned this recently in another thread pertaining to the CH32V003.

The latest version of the header on WCH's GitHub has this error fixed - all the FLASH_CTLR definitions with value >0xFFFF are now uint32_t:
https://github.com/openwch/ch32v003/blob/33b2af35325faf93928e2df84345080af96ac646/EVT/EXAM/SRC/Peripheral/inc/ch32v00x.h#L1157

It will be worth your while to update to the latest 'EVT' as there are many other corrections too. I couldn't say how that's done with MRS IDE though; I don't use it.

[mikeselectricstuff]

Any idea why anyone would bother assigning types for constants like these?

[HwAoRrDk]

I think it's primarily so that the compiler knows that these literal values are unsigned, so you don't get odd compiler warnings in various situations (because otherwise a numeric literal is a signed int). Sure, they could use U suffixes, but there's an argument to be made that using casts makes the numeric literal value clearer. As for the specific bit widths, I guess they're just trying to ensure the values are no bigger than the register width (even though they made a mistake there ).

Also, they're just copying the style of ST's HAL code, so you'd have to ask them for a definitive answer.

[SiliconWizard]

I've seen this kind of error signaled before, I think it was also for the CH32V003 SDK.

Since they are using stdint.h, as one should IMO (unless compliance to at least C99 is not possible), the much prefered way, and less cumbersome to write is using the constant macros that are also defined in stdint.h, and oddly enough, not well known.

So: ((uint32_t)0x00020000)
should be: UINT32_C(0x00020000) instead

[zilp]

I think it's primarily so that the compiler knows that these literal values are unsigned, so you don't get odd compiler warnings in various situations (because otherwise a numeric literal is a signed int).

It's just that it doesn't achieve that. A uint16_t will get promoted to a signed int by your average 32 bit compiler before any computation is done with it.

[brucehoult]

I think it's primarily so that the compiler knows that these literal values are unsigned, so you don't get odd compiler warnings in various situations (because otherwise a numeric literal is a signed int).

It's just that it doesn't achieve that. A uint16_t will get promoted to a signed int by your average 32 bit compiler before any computation is done with it.

.. by a STANDARDS CONFORMING compiler ...

[SiliconWizard]

I think it's primarily so that the compiler knows that these literal values are unsigned, so you don't get odd compiler warnings in various situations (because otherwise a numeric literal is a signed int). Sure, they could use U suffixes,

With C99 and stdint.h, there's not point in using L/U/UL suffixes if you're going to use stdint types, as I said above. Use the appropriate stdint macros for defining constants. As I also said, weirdly, it looks like less than 1% of C developers actually know these macros, while they've been there for a long time.

[SiliconWizard]

I think it's primarily so that the compiler knows that these literal values are unsigned, so you don't get odd compiler warnings in various situations (because otherwise a numeric literal is a signed int).

It's just that it doesn't achieve that. A uint16_t will get promoted to a signed int by your average 32 bit compiler before any computation is done with it.

It does achieve that if you directly use the constant, such as in an assignment or argument passing, by itself. That's the most you can do when defining constants anyway. So this is not a reason not to "cast" the constant to a specific type. As I said (3rd time), there are stdint macros for this anyway.

That being said, yes, when used in any arithmetic expression, integers will get promoted to int implicitly by C, which is one of its most horrific features IMVHO (very humble and devoted, just not to offense anyone, you never know). It has made generations of C developers make stupid mistakes with what looked like the most innocuous expressions. Anyway. (Yeah as soon as you don't want strict typing, that's almost inevitably what you get.)

In any expression which would combine several constants, such as a OR in the OP's example, you'd still need to cast the whole expression to make things perfectly warning-free with even the most aggressive warning options.

So that would be: FLASH->CTLR = (uint32_t)(FLASH_CTLR_STRT | FLASH_CTLR_PAGE_ER);

If you'd expect or'ing two uint32_t integers to naturally yield a uint32_t integer, then try avoiding C. Really!

Some (a lot?) developers don't care much though and don't cast much of anything unless tortured to do so, and live with the warnings or disable as much of them as possible. Not that I suggest doing that, but, your pick.

[brucehoult]

If you'd expect or'ing two uint32_t integers to naturally yield a uint32_t integer, then try avoiding C. Really!

Uhh .. no.

If you give any binary operator two uint32_t then the operation will be done using unsigned arithmetic (when that makes a difference e.g. division or right shift) and the result will also be uint32_t.  Also if one operand is uint32_t and the other operand is a smaller or equal type (whether signed or unsigned).

Code: [Select]

#include <stdio.h>

void main(){
    short a = -1;
    unsigned int b = 10;
    printf("%d\n", a/b);
}

The result is 429496729 because a was sign extended to 32 bits, then made unsigned to match b and unsigned division is done. You get the same result if a is int.

If you removed the "unsigned" from b then the result is -1/10 = 0.

[zilp]

It does achieve that if you directly use the constant, such as in an assignment or argument passing, by itself. That's the most you can do when defining constants anyway. So this is not a reason not to "cast" the constant to a specific type. As I said (3rd time), there are stdint macros for this anyway.

You can just use a 'U' suffix to avoid suprises from integer promotions, as that would give you a type that is unsigned, at least as wide as int (thus no promotion) and that is guaranteed to be able to represent the value (assuming the value can be represented in some unsigned type, of course). Which is in contrast to the stdint macros, which in principle can give you a type that's narrower than int.

[zilp]

If you give any binary operator two uint32_t then the operation will be done using unsigned arithmetic (when that makes a difference e.g. division or right shift) and the result will also be uint32_t.  Also if one operand is uint32_t and the other operand is a smaller or equal type (whether signed or unsigned).

Only if int is 32 bits (or smaller). So, should probably be true with anything compiling for ch32v003, but is not really guaranteed as far as C is concerned.

[brucehoult]

If you give any binary operator two uint32_t then the operation will be done using unsigned arithmetic (when that makes a difference e.g. division or right shift) and the result will also be uint32_t.  Also if one operand is uint32_t and the other operand is a smaller or equal type (whether signed or unsigned).

Only if int is 32 bits (or smaller). So, should probably be true with anything compiling for ch32v003, but is not really guaranteed as far as C is concerned.

The topic here is specifically the CH32V003 where, like all RISC-V and Arm, int is 32 bits.

If you have some weird machine where int is not 32 bits then substitute whatever width your int is where I said uint32_t.  Or put "unsigned int" but that's more typing.