From a64f5349a02842df7d4f13fefd58244b011da217 Mon Sep 17 00:00:00 2001 From: Petr Pucil Date: Sun, 30 Mar 2025 17:07:43 +0200 Subject: [PATCH 1/3] GH Actions: add macOS 14 --- .github/workflows/build.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c7b0798..98daa60 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -39,6 +39,33 @@ jobs: -DCMAKE_CXX_INCLUDE_WHAT_YOU_USE='include-what-you-use;-Xiwyu;--verbose=3' - name: Run tests run: .build/run-unittest --valgrind + macos: + runs-on: macos-14 + strategy: + fail-fast: false + matrix: + cpp-standard: + - '98' + - '11' + - '20' + steps: + - uses: actions/checkout@v4 + - name: Install GoogleTest + env: + CPP_STANDARD: ${{ matrix.cpp-standard }} + run: .build/install-gtest "$CPP_STANDARD" + - name: Build + env: + CPP_STANDARD: ${{ matrix.cpp-standard }} + # This tells the C++ compiler to produce debugging info that Valgrind needs to report line numbers. + # See also https://valgrind.org/docs/manual/manual-core.html#manual-core.started + CMAKE_BUILD_TYPE: Debug + run: | + .build/build \ + -DCMAKE_BUILD_TYPE="$CMAKE_BUILD_TYPE" \ + -DCMAKE_CXX_STANDARD="$CPP_STANDARD" -DCMAKE_CXX_STANDARD_REQUIRED=ON -DCMAKE_CXX_EXTENSIONS=OFF + - name: Run tests + run: .build/run-unittest # Builds and runs tests on Ubuntu 7.10. # It ships with: From cf85fea4df3b0e383a959cb455c6b87a13eddec5 Mon Sep 17 00:00:00 2001 From: Petr Pucil Date: Sun, 30 Mar 2025 17:24:37 +0200 Subject: [PATCH 2/3] unittest.cpp: disable GB2312 `EILSEQ` test on macOS as well --- tests/unittest.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/unittest.cpp b/tests/unittest.cpp index f2954bc..60b04dc 100644 --- a/tests/unittest.cpp +++ b/tests/unittest.cpp @@ -578,15 +578,17 @@ TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_gb2312_too_short) } } -#if defined(__FreeBSD__) || defined(__NetBSD__) +#if defined(__FreeBSD__) || defined(__NetBSD__) || defined(__APPLE__) TEST(KaitaiStreamTest, DISABLED_bytes_to_str_invalid_seq_gb2312_two_bytes) #else TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_gb2312_two_bytes) #endif { - // 0xB0 0x30 is illegal sequence in GB2312: 0xB0 must be followed by [0xA1..0xFE]. - // However, some iconv engines, namely CITRUS integrated with modern FreeBSD (10+) and NetBSD, - // are not considering this as error and thus not returning EILSEQ. + // 0xB0 0x30 is illegal sequence in GB2312: 0xB0 must be followed by [0xA1..0xFE]. However, + // some iconv engines, namely CITRUS integrated with modern FreeBSD (10+) and NetBSD, are + // not considering this an error and thus not returning EILSEQ. Iconv preinstalled in the + // GitHub Actions `macos-14` runner image does not consider this an error either. + // try { std::string res = kaitai::kstream::bytes_to_str("\xb0\x30", "GB2312"); FAIL() << "Expected illegal_seq_in_encoding exception"; From e95f7d951a737e39957dcca05f8f21778d862309 Mon Sep 17 00:00:00 2001 From: Petr Pucil Date: Sun, 30 Mar 2025 20:31:32 +0200 Subject: [PATCH 3/3] to_string(): define `long long` overloads even in C++98 mode Since `uint64_t` is an alias for `unsigned long long` on macOS, I think this is the only way (except for templates that we used until https://github.com/kaitai-io/kaitai_struct_cpp_stl_runtime/pull/76) to ensure that `to_string()` supports an `uint64_t` argument. `long long` is technically not in C++98, but if we detect that the `LLONG_MAX` macro is defined, we should be able to use it. And as I mentioned in `Common.cmake`, I don't think we currently support any compiler without `long long` support, given that we already require `int64_t`/`uint64_t`, which are also only part of C++11 and not C++98. --- Common.cmake | 7 +++++++ gcc4.mk | 6 ++++++ kaitai/kaitaistream.h | 15 +++++++++++---- tests/unittest.cpp | 16 ---------------- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/Common.cmake b/Common.cmake index 3e4f2ec..4d4a175 100644 --- a/Common.cmake +++ b/Common.cmake @@ -15,6 +15,13 @@ target_compile_options(${PROJECT_NAME} PRIVATE $<$:/W4 /WX> $<$>: -Wall -Wextra -Wpedantic -Werror + + # We're using the `long long` type intentionally. Although it's not part of C++98, in + # practice it is usually supported even by ancient compilers with very limited C++11 + # support. And we already unconditionally require `uint64_t`, so it would be strange if + # the compiler supported `uint64_t` and not `long long`. + -Wno-error=long-long + # See : # # > Level 1: (...) it has very few false negatives. However, it has many false positives. diff --git a/gcc4.mk b/gcc4.mk index 661f9ac..b6b177b 100644 --- a/gcc4.mk +++ b/gcc4.mk @@ -13,6 +13,12 @@ OBJS := \ CXXFLAGS := -std=c++98 -Wall -Wextra -pedantic +# `-pedantic` enables `-Wlong-long`, which for some reason turns into +# `error: ISO C++ does not support 'long long'`, even though we don't use `-Werror`. Since +# we're using `long long` intentionally, we suppress this error using `-Wno-long-long` +# (see https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Warning-Options.html#index-Wlong_002dlong-305). +CXXFLAGS += -Wno-long-long + # NOTE: the meaning of `` values in `-Wstrict-aliasing=` is different in GCC 4.1.2 # than in recent versions of GCC. According to # https://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Warning-Options.html#index-Wstrict_002daliasing-246, diff --git a/kaitai/kaitaistream.h b/kaitai/kaitaistream.h index 625de2d..1d6bcc3 100644 --- a/kaitai/kaitaistream.h +++ b/kaitai/kaitaistream.h @@ -13,6 +13,7 @@ #include // std::streamsize, forward declaration of std::istream // IWYU pragma: keep #include // std::size_t +#include // LLONG_MAX, ULLONG_MAX #include // std::istringstream // IWYU pragma: keep #include // std::string @@ -252,8 +253,11 @@ class kstream { return to_string_signed(val); } -// The `long long` type is only available since C++11, so we use it only in C++11 mode. -#ifdef KAITAI_STREAM_H_CPP11_SUPPORT +// The `long long` type is technically available only since C++11. It is usually +// supported even by ancient compilers that have very limited C++11 support, but we can +// still check if the `LLONG_MAX` macro is defined (it seems very unlikely that it is not, +// though). +#ifdef LLONG_MAX /** * Converts given integer `val` to a decimal string representation. * Should be used in place of `std::to_string(long long)` (which is available only @@ -282,8 +286,11 @@ class kstream { return to_string_unsigned(val); } -// The `unsigned long long` type is only available since C++11, so we use it only in C++11 mode. -#ifdef KAITAI_STREAM_H_CPP11_SUPPORT +// The `unsigned long long` type is technically available only since C++11. It is usually +// supported even by ancient compilers that have very limited C++11 support, but we can +// still check if the `LLONG_MAX` macro is defined (it seems very unlikely that it is not, +// though). +#ifdef ULLONG_MAX /** * Converts given integer `val` to a decimal string representation. * Should be used in place of `std::to_string(unsigned long long)` (which is available only diff --git a/tests/unittest.cpp b/tests/unittest.cpp index 60b04dc..3a79694 100644 --- a/tests/unittest.cpp +++ b/tests/unittest.cpp @@ -154,8 +154,6 @@ TEST(KaitaiStreamTest, to_string_long) #pragma warning(pop) #endif -// The `long long` type is only available since C++11, so we use it only in C++11 mode. -#ifdef KAITAI_STREAM_H_CPP11_SUPPORT TEST(KaitaiStreamTest, to_string_unsigned_long_long) { EXPECT_EQ(kaitai::kstream::to_string(std::numeric_limits::min()), "0"); @@ -167,20 +165,6 @@ TEST(KaitaiStreamTest, to_string_long_long) EXPECT_EQ(kaitai::kstream::to_string(std::numeric_limits::min()), "-9223372036854775808"); EXPECT_EQ(kaitai::kstream::to_string(std::numeric_limits::max()), "9223372036854775807"); } -#else -// Make sure we still support 64-bit integers. -TEST(KaitaiStreamTest, to_string_uint64) -{ - EXPECT_EQ(kaitai::kstream::to_string(std::numeric_limits::min()), "0"); - EXPECT_EQ(kaitai::kstream::to_string(std::numeric_limits::max()), "18446744073709551615"); -} - -TEST(KaitaiStreamTest, to_string_int64) -{ - EXPECT_EQ(kaitai::kstream::to_string(std::numeric_limits::min()), "-9223372036854775808"); - EXPECT_EQ(kaitai::kstream::to_string(std::numeric_limits::max()), "9223372036854775807"); -} -#endif TEST(KaitaiStreamTest, string_to_int) {