Address clang signed-unsigned warnings

[alt-graph]

This MR fixes a few warnings generated by clang 18 about implicit conversions between signed and unsigned values. The changes are straightforward (I hope).

[Finii]

These look all good.

Was a bit astonished that hex_string() can not work backwards and that is not mentioned anywhere ;)

BUT ... Is this a draft?

There are still so many warnings:

../include/gul17/join_split.h:193:68: warning: implicit conversion changes signedness: 'difference_type' (aka 'long') to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/join_split.h:198:64: warning: implicit conversion changes signedness: 'difference_type' (aka 'long') to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:1009:62: warning: implicit conversion changes signedness: 'size_type' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:1020:62: warning: implicit conversion changes signedness: 'size_type' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:1124:39: warning: implicit conversion changes signedness: 'size_type' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:1171:35: warning: implicit conversion changes signedness: 'size_type' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:1291:51: warning: implicit conversion changes signedness: 'size_type' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:1292:51: warning: implicit conversion changes signedness: 'const size_type' (aka 'const unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:1301:58: warning: implicit conversion changes signedness: 'unsigned long const' to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:694:26: warning: implicit conversion changes signedness: 'difference_type' (aka 'long') to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:703:70: warning: implicit conversion changes signedness: 'difference_type' (aka 'long') to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:711:70: warning: implicit conversion changes signedness: 'difference_type' (aka 'long') to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:732:26: warning: implicit conversion changes signedness: 'difference_type' (aka 'long') to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:741:70: warning: implicit conversion changes signedness: 'difference_type' (aka 'long') to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:749:34: warning: implicit conversion changes signedness: 'size_type' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/SlidingBuffer.h:997:62: warning: implicit conversion changes signedness: 'size_type' (aka 'unsigned long') to 'difference_type' (aka 'long') [-Wsign-conversion]
../include/gul17/statistics.h:645:20: warning: implicit conversion changes signedness: 'typename iterator_traits<_Deque_iterator<StatisticsElement<double, unsigned int>, const StatisticsElement<double, unsigned int> &, const StatisticsElement<double, unsigned int> *>>::difference_type' (aka 'long') to 'std::size_t' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/statistics.h:645:20: warning: implicit conversion changes signedness: 'typename iterator_traits<_Deque_iterator<StatisticsElement<double, unsigned int>, StatisticsElement<double, unsigned int> &, StatisticsElement<double, unsigned int> *>>::difference_type' (aka 'long') to 'std::size_t' (aka 'unsigned long') [-Wsign-conversion]
../include/gul17/statistics.h:645:20: warning: implicit conversion changes signedness: 'typename iterator_traits<__normal_iterator<double *, vector<double>>>::difference_type' (aka 'long') to 'std::size_t' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:1347:23: warning: implicit conversion changes signedness: 'unsigned int' to 'value_type' (aka 'int') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:1365:23: warning: implicit conversion changes signedness: 'unsigned int' to 'value_type' (aka 'int') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:1395:23: warning: implicit conversion changes signedness: 'unsigned int' to 'value_type' (aka 'int') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:1422:23: warning: implicit conversion changes signedness: 'unsigned int' to 'value_type' (aka 'int') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:1505:36: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:1529:23: warning: implicit conversion changes signedness: 'unsigned int' to 'value_type' (aka 'int') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:240:33: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:241:27: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:252:34: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:259:37: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:916:33: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SlidingBuffer.cc:917:27: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SmallVector.cc:1201:46: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SmallVector.cc:1203:53: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:1209:21: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SmallVector.cc:1209:32: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:1255:39: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SmallVector.cc:1266:21: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_SmallVector.cc:1266:32: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:1382:25: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:1413:26: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:1429:26: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:328:25: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:352:26: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_SmallVector.cc:363:26: warning: implicit conversion changes signedness: 'int' to 'SizeType' (aka 'unsigned int') [-Wsign-conversion]
../tests/test_ThreadPool.cc:554:24: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]
../tests/test_ThreadPool.cc:592:24: warning: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Wsign-conversion]

On doocsdev24 with clang 18.1.3

[Finii]

Btw, I finally removed -Wconversion from our standard builds, because there are too many cases that are perfectly ok and require readability-reducing static_casts galore 😬

Iirc there were discussions about these warnings on cppcon, with my takeaway being: Nice and should be done from time to time but not usable for always-on-silent-builds. 🤷‍♀️

Especially bad is HLC machine and its dependents, resulting in this always to be halfway able to compile:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wconversion"
#pragma GCC diagnostic ignored "-Wfloat-conversion"
#include <hlc_machine.h>
#pragma GCC diagnostic pop
#include ... the rest ...

[alt-graph]

BUT ... Is this a draft?

There are still so many warnings:

Well, it was not meant to be complete. I just wanted to address some warnings that I stumbled over while working with the code.

Btw, I finally removed -Wconversion from our standard builds, because there are too many cases that are perfectly ok and require readability-reducing static_casts galore 😬

Iirc there were discussions about these warnings on cppcon, with my takeaway being: Nice and should be done from time to time but not usable for always-on-silent-builds. 🤷‍♀️

I hear you. Maybe we should revisit our defaults for the main libraries. For GUL17 we might want to keep -Wconversion on, just because it is our equivalent to a standard library and we should be extra-pedantic about edge cases. For clientlib etc. it is clearly overkill.

[Finii]

👍

For clientlib etc. it is clearly overkill.

Well, it would be good if the header files are -Wconversion clean at least. I'm not talking about compiling the libraries, just using the libraries with their header files.

Where we have, along the lines of

In file included from /export/doocs/lib/include/doocs/timing_pattern.h:36,
                 from /export/doocs/lib/include/hlc/machine/TimingInfo.h:31,
                 from /export/doocs/lib/include/hlc_machine.h:190:
/export/doocs/lib/include/doocs/TimingWord.h: In static member function ‘static doocs::ChargeRange doocs::ChargeRange::find_for_charge(float)’:
/export/doocs/lib/include/doocs/TimingWord.h:82:45: warning: conversion from ‘int’ to ‘doocs::ChargeRange::BaseType’ {aka ‘unsigned char’} may change value [-Wconversion]
   82 |             if (charge_nC < get_upper_limit(i))

Hmm, it's not even a template, why is the implementation in the header? Whatever ;-) Just one example.

class ChargeRange
{
public:
    using BaseType = uint8_t;

...

    static ChargeRange find_for_charge(float charge_nC)
    {
        if (charge_nC <= 0.0)
            return ChargeRange(0);

        for (int i = 1; i < 16; i++)
            if (charge_nC < get_upper_limit(i))    // <=---- this line
                return ChargeRange(i);             // <=---- same problem here

        return ChargeRange(0);
    }

...

    /// Return the upper charge limit in nC.   
    constexpr static float get_upper_limit(BaseType code) noexcept
    {
        switch (code)
        {
        case 0:
            return 0;
        case 1:

TimingWord.h: get_upper_limit() etc take BaseType aka uint8 but the loop is int