platform.h: fixed clang-analyzer-core.BitwiseShift warnings and adjusted types of *_bit members

[firewave]

No description provided.

[firewave]

The original CSA warnings:

/home/runner/work/cppcheck/cppcheck/lib/platform.h:50:22: error: Right operand is negative in left shift [clang-analyzer-core.BitwiseShift,-warnings-as-errors]
   50 |         return -(1LL << (bit-1));
      |                      ^
/home/runner/work/cppcheck/cppcheck/lib/platform.cpp:447:29: note: Assuming 'cppstd' is < CPP11
  447 |     return getLimitsDefines(cppstd >= Standards::cppstd_t::CPP11);
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/cppcheck/cppcheck/lib/platform.cpp:447:12: note: Calling 'Platform::getLimitsDefines'
  447 |     return getLimitsDefines(cppstd >= Standards::cppstd_t::CPP11);
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/cppcheck/cppcheck/lib/platform.cpp:308:25: note: Calling 'Platform::min_value'
  308 |     s += std::to_string(min_value(char_bit));
      |                         ^~~~~~~~~~~~~~~~~~~
/home/runner/work/cppcheck/cppcheck/lib/platform.h:48:13: note: 'bit' is < 64
   48 |         if (bit >= 64)
      |             ^~~
/home/runner/work/cppcheck/cppcheck/lib/platform.h:48:9: note: Taking false branch
   48 |         if (bit >= 64)
      |         ^
/home/runner/work/cppcheck/cppcheck/lib/platform.h:50:22: note: The result of left shift is undefined because the right operand is negative
   50 |         return -(1LL << (bit-1));
      |                  ~~~~^~~~~~~~~~

/home/runner/work/cppcheck/cppcheck/lib/platform.h:56:21: error: Left shift by '2147483647' overflows the capacity of 'long long' [clang-analyzer-core.BitwiseShift,-warnings-as-errors]
   56 |         return (1LL << (bit-1)) - 1LL;
      |                     ^
/home/runner/work/cppcheck/cppcheck/lib/platform.cpp:447:29: note: Assuming 'cppstd' is < CPP11
  447 |     return getLimitsDefines(cppstd >= Standards::cppstd_t::CPP11);
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/cppcheck/cppcheck/lib/platform.cpp:447:12: note: Calling 'Platform::getLimitsDefines'
  447 |     return getLimitsDefines(cppstd >= Standards::cppstd_t::CPP11);
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/cppcheck/cppcheck/lib/platform.cpp:312:25: note: Calling 'Platform::max_value'
  312 |     s += std::to_string(max_value(char_bit+1));
      |                         ^~~~~~~~~~~~~~~~~~~~~
/home/runner/work/cppcheck/cppcheck/lib/platform.h:54:13: note: Assuming 'bit' is < 64
   54 |         if (bit >= 64)
      |             ^~~~~~~~~
/home/runner/work/cppcheck/cppcheck/lib/platform.h:54:9: note: Taking false branch
   54 |         if (bit >= 64)
      |         ^
/home/runner/work/cppcheck/cppcheck/lib/platform.h:56:21: note: The result of left shift is undefined because the right operand '2147483647' is not smaller than 64, the capacity of 'long long'
   56 |         return (1LL << (bit-1)) - 1LL;
      |                 ~~~~^~~~~~~~~~

I filed https://trac.cppcheck.net/ticket/13600 about detecting it ourselves.

The redundant code could be extracted into a helper function. I will take a look at that with fixing the TODO in a follow-up.

[firewave]

uint8_t is treated as a character with stream insertion operators. This caused invalid dump files to be written.

First, we should not write these files raw but using TinyXML2.

Second, I wonder if it would make sense to have a check which detects the usage of uint8_t in stream insertion operators. If you want it to a be an actual character you should probably use {unsigned|signed} char or char8_t.

[danmar]

Second, I wonder if it would make sense to have a check which detects the usage of uint8_t in stream insertion operators. If you want it to a be an actual character you should probably use {unsigned|signed} char or char8_t.

Spontanously that checker sounds reasonable to me.

[danmar]

The original CSA warnings:

If char_bit will always be a positive integer then that clang warning is technically a false positive.

We should ensure that char_bit is not assigned 0 when a platform xml file is loaded. I have the feeling that could have bad effects in other places also.

[danmar]

after your changes this would not work well the day we switch to 128-bit bigints.. however I guess there are plenty of other code that will not work perfectly that day.

[firewave]

This actually causes a compilation failure when using the 128-bit int. That's how I came across this.

[firewave]

If char_bit will always be a positive integer then that clang warning is technically a false positive.

The type does not guarantee that.

We should ensure that char_bit is not assigned 0 when a platform xml file is loaded. I have the feeling that could have bad effects in other places also.

That's why I added the asserts. That should actually be handled during the loading of the platform. The platform loading might lack most of the error handling - at least it was at some point and I did not check if I added that yet.