Fix unexpected type casting issues

[casperlamboo]

Description

We encountered some issues when compiling for a different operating system where size_t is 32bit rather then 64bit. this causes the following simple example

void main() {
  size_t x = 1;
  int64_t y = -x;
  std::cout << y << std::endl;
}

to unexpectedly output 4294967295 rather then -1.

Note: this PR is based on https://github.com/Ultimaker/CuraEngine/tree/NP-327_emscripten_communication; merge PR #2131 before this one.

Type of change

Prevent unexpected integer overflows by more explicitly casting.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

yes

Test Configuration:

• Operating System: MacOS 13.3

Checklist:

• My code follows the style guidelines of this project as described in UltiMaker Meta
• I have read the Contribution guide
• I have commented my code, particularly in hard-to-understand areas
• I have uploaded any files required to test this change

NP-343

[jellespijker]

The changes make sense imo. But I would rather have a more robust solution in place. Using Concepts to guard against size differences. This is just a quick example put together. Use it as inspiration. Check it out on Compiler Explorer, just uncomment the idx_no_compile variable to see it in action. https://godbolt.org/z/GsK7Wc569

constexpr everything ;-)

#include <type_traits>
#include <concepts>
#include <cstdint>

template<typename T, typename U>
concept SameOrLargerSize = requires {
  // Check for arithmetic types
  requires std::is_arithmetic_v<T> && std::is_arithmetic_v<U>;

  // Ensure the size of U is at least the same as T
  requires sizeof(T) >= sizeof(U);

  // Additional checks for integral and floating-point types
  // To handle signed/unsigned comparisons
  requires (!std::is_integral_v<T> || !std::is_integral_v<U>) || 
            (std::is_signed_v<T> == std::is_signed_v<U>);

  // To prevent comparing integral and floating-point types directly
  requires (!std::is_integral_v<T> || !std::is_floating_point_v<U>) &&
            (!std::is_floating_point_v<T> || !std::is_integral_v<U>);
};

// Helper concept to check for integral types that are the same size or larger
template<typename T, typename U>
concept SameOrLargeSizeInteger = SameOrLargerSize<T, U>  && std::is_integral_v<T>;

// Helper concept to check for floating-point types that are the same size or larger
template<typename T, typename U>
concept SameOrLargeSizeFloatingPoint = SameOrLargerSize<T, U>  && std::is_floating_point_v<T>;

struct LayerIndex
{
    using value_type = std::int32_t;
    value_type value{};

    // Constructor for floating-point types that are the same size or larger
    template<typename U>
        requires SameOrLargeSizeFloatingPoint<value_type, U>
    constexpr explicit LayerIndex(const U val) noexcept
        : value{ static_cast<value_type>(val) }
    {
    }

    // Constructor for integral types that are the same size or larger
    template<typename U>
        requires SameOrLargeSizeInteger<value_type, U>
    constexpr LayerIndex(const U val) noexcept
        : value{ static_cast<value_type>(val) }
    {
    }

    constexpr LayerIndex operator-(const LayerIndex& other) const noexcept
    {
        return LayerIndex(value - other.value); // Explicitly construct the result
    }
};

int main() {
    constexpr std::size_t bigger { 20 };
    constexpr std::int8_t smaller { 5 };
    constexpr LayerIndex idx(20);
    constexpr LayerIndex idx_compile(smaller);
    // const LayerIndex idx_no_compile(bigger); // This will still fail due to size mismatch
    constexpr auto comp_idx = idx - idx_compile;
    return static_cast<int>(comp_idx.value);
}

When un-commenting the idx_no_compile you would get the following error

[casperlamboo]

@jellespijker in the example I shared above the unexpected behaviour occurs when casting a smaller to a larger type due to the sign flipping. This is a different issue then you're trying solve with the stated example.

[jellespijker]

  size_t x = 1;
  int64_t y = -x;

The example was intended as inspiration and a quick showcase. Maybe the example below is more suited and recognizable.
https://godbolt.org/z/ocEs5aW71

#include <type_traits>
#include <concepts>
#include <cstdint>

template<typename T, typename U>
concept SafeConversion = requires {
    // Check for arithmetic types
    requires std::is_arithmetic_v<T> && std::is_arithmetic_v<U>;

    // Ensure the size of T is at least the same as U to avoid truncation
    requires sizeof(T) >= sizeof(U);

    // Handle signed/unsigned conversions to prevent unintended behavior
    requires (!std::is_integral_v<T> || !std::is_integral_v<U>) || 
             (std::is_signed_v<T> == std::is_signed_v<U>) ||
             (std::is_unsigned_v<T> && std::is_signed_v<U> && sizeof(T) > sizeof(U)); // Allow if unsigned T is larger

    // Prevent direct comparisons between integral and floating-point types
    requires (!std::is_integral_v<T> || !std::is_floating_point_v<U>) &&
             (!std::is_floating_point_v<T> || !std::is_integral_v<U>);
};

int main() {
    std::size_t x = 1;
    std::int64_t y = -x;
    static_assert(SafeConversion<decltype(x), decltype(y)>);
    return y;
}

Resulting in:

<source>:26:19: error: static assertion failed
   26 |     static_assert(SafeConversion<decltype(x), decltype(y)>);
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<source>:26:19: note: constraints not satisfied
<source>:6:9:   required by the constraints of 'template<class T, class U> concept SafeConversion'
<source>:6:26:   in requirements  [with T = long unsigned int; U = long int]
<source>:14:14: note: nested requirement '((((! is_integral_v<T>) || (! is_integral_v<U>)) || (is_signed_v<T> == is_signed_v<U>)) || ((is_unsigned_v<T> && is_signed_v<U>) && (sizeof (T) > sizeof (U))))' is not satisfied
   14 |     requires (!std::is_integral_v<T> || !std::is_integral_v<U>) ||
      |     ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   15 |              (std::is_signed_v<T> == std::is_signed_v<U>) ||
      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   16 |              (std::is_unsigned_v<T> && std::is_signed_v<U> && sizeof(T) > sizeof(U)); // Allow if unsigned T is larger

[wawanbreton]

@jellespijker You really have a concepts-based solution for every situation 😛

In the present situation, this would indeed generate a proper error instead of a warning, but it doesn't actually fix the issue. What we discussed briefly with @casperlamboo would be to have our own types for everything (which we already do in the engine, partially), and proper conversion operators between them, so that you can have a compact, readable and error/warning-free code.

However we agreed for now to just fix this issue locally, and possibly add a C&C ticket to discuss this.

[wawanbreton]

One minor suggestion, otherwise looks good

[ToyboxZach]

I've been trying to compile this in WASM, and I believe I found one more place that needs to be cast:

Raft::LayerType Raft::getLayerType(LayerIndex layer_index)
{
    const auto airgap = Raft::getFillerLayerCount();
    const auto interface_layers = Raft::getInterfaceLayers();
    const auto surface_layers = Raft::getSurfaceLayers();

    if (layer_index < -airgap - surface_layers - interface_layers)

On my architecture this ends up saying layer_index < Really big number.