FYI We used to replace these macros but they were removed a while ago and need to be re-added.

FYI We used to replace these macros but they were removed a while ago and need to be re-added.

@firewave we need to keep UINT32_C in some way for misra. For instance the code UINT32_C(10UL) is non-compliant according to misra 7.5. The code 10UL does not violate 7.5.

The Misra checker for rule 7.5 could in theory use the "raw tokens" instead but that means that violations in headers are not detected and there could also be false negatives when macros is used.

So for the sake of Misra I would suggest that we don't replace UINT32_C. Or do you think there is some better replacement that will still mean we can detect misra violations properly?

@firewave we need to keep UINT32_C in some way for misra. For instance the code UINT32_C(10UL) is non-compliant according to misra 7.5. The code 10UL does not violate 7.5.

Isn't that what the macro information is for? This seems to be similar to #6559.

I am saying that if we would add something like this:

 <define name="UINT32_C(VALUE)" value="VALUE"/>

Then the dumpfile will not contain the necessary information. If the code is UINT32_C(10UL) then that would be preprocessed to 10UL which is compliant according to 7.5 and Token::macroName will not say UINT32_C anywhere because no token comes from that macro.

Right because those are just replacements and are not treated as macros. Maybe that should be changed.

But as you noted in the past UINT32_C needs to be implemented via the platform so in that case we should have the information, right?

But as you noted in the past UINT32_C needs to be implemented via the platform so in that case we should have the information, right?

I am not sure how to implement that.

How about this define:

  <define name="UINT32_C(V)" value="(uint32_t)(V)"/>

If the code says UINT32_C(10UL) then the preprocessor output will be (uint32_t)(10UL) and for the "(" token the Token::macroName will be "UINT32_C"..

I am not sure how to implement that.

It should no different from the existing defines we already generate.

But these should be flagged as non-internal ones. That is a feature we need to implement in simplecpp first. And it is probably not important.

Shouldn’t the macro simply be added to C99_STDLIB_IDENTIFIERS under stdint.h? At least this side claims it is since C99 https://en.cppreference.com/w/c/types/integer.

Especially as INTMAX_C and UINTMAX_C are already in this list and also listed on cppreference.com as macros.

Then Misra will not complain as 17.3 checks isStdLibId and it will then return that this is a known std lib macro.

And actually checking https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf Appendix B.17 as referenced in the code there might be a missunderstanding (bug) in the first place.

The list C99_STDLIB_IDENTIFIERS mentions 'INTN_C', 'UINTN_C',, similar as it is listed in the Standard, but N in this case means 8,16,32
and i only see this iteration done for the literal types uint8_t ... with this:

STDINT_TYPES = ['%s%d_t' % (n, v) for n, v in itertools.product(
        ['int', 'uint', 'int_least', 'uint_least', 'int_fast', 'uint_fast'],
        [8, 16, 32, 64])]

I added a PR which updates the list of std library identifiers. #7325

which should fix this issue in the way all other cases where handled before. And is based on the C standard automatically (meaning C89 where these are not present get an error above C99 we don’t get the error).

Shouldn’t the macro simply be added to C99_STDLIB_IDENTIFIERS under stdint.h? At least this side claims it is since C99 https://en.cppreference.com/w/c/types/integer.

That sounds better to me.

However one more detail is that Cppcheck now does not manage to determine the value of expression UINT32_C(0) properly. I am not sure how to deal with this. Ideally we would like to warn for this code:

uint32_t x = 1000 / UINT32_C(0);

The cast macro I showed earlier would solve this problem but I am afraid that the cast macro I showed will not be able to distinguish C89/C99/etc..

I added a PR which updates the list of std library identifiers. #7325
which should fix this issue in the way all other cases where handled before. And is based on the C standard automatically (meaning C89 where these are not present get an error above C99 we don’t get the error).

Thanks!

However one more detail is that Cppcheck now does not manage to determine the value of expression UINT32_C(0) properly. I am not sure how to deal with this. Ideally we would like to warn for this code:

uint32_t x = 1000 / UINT32_C(0);

The cast macro I showed earlier would solve this problem but I am afraid that the cast macro I showed will not be able to distinguish C89/C99/etc..

I am not sure if this is in line with your goals but C89 does not have the UINT32_C macro so C89 code with this Makro in it shouldn’t compile. I am not sure if the harm would be so big with C89? Especially as you could argue for Misra all define/makros which are caught by this rule are false positives as they are not a function. Putting them into the StdLibIds is only necessary because they are not resolved by the preprocessor in the first place.

But I would need to double check what happens in the dump file exactly for defines to maybe add other suggestions.

The cast macro I showed earlier would solve this problem but I am afraid that the cast macro I showed will not be able to distinguish C89/C99/etc..

The problem is not specific to the UINT32_C Makros in general all Makros from the STD Lib or in general external libraries which we don’t have in our include path would be handled like this.
A solution might be to add the std library path of the compile to the include path.
In default case an idea might be to have a set of std library files in cppcheck itself so the Makro lines which need to be added to the dump file are not hand tailored but from real code.

But in that case we would defiantly need this: https://trac.cppcheck.net/ticket/12359
So that all the std libraries are not checked for any errors but only the defines function prototypes and so on are added. Not sure how big the performance hit would be. To extract all the necessary info without doing any checks on the files. But in general that feature would be very nice, especially needed in embedded development where you have processor specific HALs.

I think the actual problem is https://trac.cppcheck.net/ticket/13341.