diff --git a/core/foundation/CMakeLists.txt b/core/foundation/CMakeLists.txt index 320a5db3bb2bf..63b93ac475843 100644 --- a/core/foundation/CMakeLists.txt +++ b/core/foundation/CMakeLists.txt @@ -14,7 +14,7 @@ set_property(TARGET Core APPEND PROPERTY DICT_HEADERS TClassEdit.h TError.h ThreadLocalStorage.h - ROOT/RAlignmentUtils.hxx + ROOT/BitUtils.hxx ROOT/RError.hxx ROOT/RLogger.hxx ROOT/RNotFn.hxx diff --git a/core/foundation/inc/ROOT/BitUtils.hxx b/core/foundation/inc/ROOT/BitUtils.hxx new file mode 100644 index 0000000000000..738e22b4b3519 --- /dev/null +++ b/core/foundation/inc/ROOT/BitUtils.hxx @@ -0,0 +1,154 @@ +// @(#)root/foundation: +// Author: Philippe Canal, April 2026 + +/************************************************************************* + * Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. * + * All rights reserved. * + * * + * For the licensing terms see $ROOTSYS/LICENSE. * + * For the list of contributors see $ROOTSYS/README/CREDITS. * + *************************************************************************/ + +#ifndef ROOT_BitUtils +#define ROOT_BitUtils + +#include +#include +#include +#include + +#ifdef _MSC_VER +#include // for _BitScan* +#endif + +namespace ROOT { +namespace Internal { + +/// Return true if \p align is a valid C++ alignment value: strictly positive +/// and a power of two. This is the set of values accepted by +/// `::operator new[](n, std::align_val_t(align))`. +inline constexpr bool IsValidAlignment(std::size_t align) noexcept +{ + return align > 0 && (align & (align - 1)) == 0; +} + +/// Round \p value up to the next multiple of \p align. +/// \p align must be a power of two (asserted at runtime in debug builds). +template +inline constexpr T AlignUp(T value, T align) noexcept +{ + assert(IsValidAlignment(static_cast(align))); // must be a power of two + return (value + align - 1) & ~(align - 1); +} + +/// Given an integer `x`, returns the number of leading 0-bits starting at the most significant bit position. +/// If `x` is 0, it returns the size of `x` in bits. +/// +/// Example: +/// +/// if x is a std::uint32_t with value 42 (0b0...0101010), then LeadingZeroes(x) == 26 +template +inline std::size_t LeadingZeroes(T x) +{ + constexpr std::size_t maxBits = sizeof(T) * 8; + static_assert(std::is_integral_v && (maxBits == 32 || maxBits == 64)); + + if (x == 0) + return maxBits; + +#ifdef _MSC_VER + unsigned long idx = 0; + [[maybe_unused]] unsigned char nonZero; + if constexpr (maxBits == 32) { + nonZero = _BitScanReverse(&idx, x); + } else { +#ifdef _WIN64 + // 64-bit machine + nonZero = _BitScanReverse64(&idx, x); +#else + // 32-bit machine + std::uint32_t low = (x & 0xFFFF'FFFF); + std::uint32_t high = (x >> 32) & 0xFFFF'FFFF; + unsigned long lowIdx, highIdx; + unsigned char lowNonZero = _BitScanReverse(&lowIdx, low); + unsigned char highNonZero = _BitScanReverse(&highIdx, high); + assert(lowNonZero | highNonZero); + if (high == 0) + idx = 63 - lowIdx; + else + idx = 31 - highIdx; + return static_cast(idx); + +#endif // _WIN64 + } + + assert(nonZero); + // NOTE: _BitScanReverse return the 0-based index of the leftmost non-zero bit. + // To convert it to the number of zeroes we need to "flip" it from [0, maxBits) to [maxBits, 0) + // (e.g. _BitScanReverse == 0 <=> LeadingZeroes == maxBits) + return static_cast(maxBits - 1 - idx); +#else + if constexpr (maxBits == 32) { + return static_cast(__builtin_clz(x)); + } else { + return static_cast(__builtin_clzl(x)); + } +#endif // _MSC_VER +} + +/// Given an integer `x`, returns the number of trailing 0-bits starting at the least significant bit position. +/// If `x` is 0, it returns the size of `x` in bits. +/// +/// Example: +/// +/// if x is a std::uint32_t with value 42 (0b0...0101010), then TrailingZeroes(x) == 1 +template +inline std::size_t TrailingZeroes(T x) +{ + constexpr std::size_t maxBits = sizeof(T) * 8; + static_assert(std::is_integral_v && (maxBits == 32 || maxBits == 64)); + + if (x == 0) + return maxBits; + +#ifdef _MSC_VER + unsigned long idx = 0; + [[maybe_unused]] unsigned char nonZero; + if constexpr (maxBits == 32) { + nonZero = _BitScanForward(&idx, x); + } else { +#ifdef _WIN64 + // 64-bit machine + nonZero = _BitScanForward64(&idx, x); +#else + // 32-bit machine + std::uint32_t low = (x & 0xFFFF'FFFF); + std::uint32_t high = (x >> 32) & 0xFFFF'FFFF; + unsigned long lowIdx, highIdx; + unsigned char lowNonZero = _BitScanForward(&lowIdx, low); + unsigned char highNonZero = _BitScanForward(&highIdx, high); + nonZero = lowNonZero | highNonZero; + if (low == 0) + idx = highIdx + 32; + else + idx = lowIdx; + +#endif // _WIN64 + } + assert(nonZero); + // Differently from LeadingZeroes, in this case the bit index returned by _BitScanForward is + // already equivalent to the number of trailing zeroes, so we don't need any transformation. + return static_cast(idx); +#else + if constexpr (maxBits == 32) { + return static_cast(__builtin_ctz(x)); + } else { + return static_cast(__builtin_ctzl(x)); + } +#endif // _MSC_VER + } + +} // namespace Internal +} // namespace ROOT + +#endif // ROOT_BitUtils diff --git a/core/foundation/inc/ROOT/RAlignmentUtils.hxx b/core/foundation/inc/ROOT/RAlignmentUtils.hxx deleted file mode 100644 index 422958d338793..0000000000000 --- a/core/foundation/inc/ROOT/RAlignmentUtils.hxx +++ /dev/null @@ -1,41 +0,0 @@ -// @(#)root/foundation: -// Author: Philippe Canal, April 2026 - -/************************************************************************* - * Copyright (C) 1995-2026, Rene Brun and Fons Rademakers. * - * All rights reserved. * - * * - * For the licensing terms see $ROOTSYS/LICENSE. * - * For the list of contributors see $ROOTSYS/README/CREDITS. * - *************************************************************************/ - -#ifndef ROOT_RAlignmentUtils -#define ROOT_RAlignmentUtils - -#include -#include - -namespace ROOT { -namespace Internal { - -/// Return true if \p align is a valid C++ alignment value: strictly positive -/// and a power of two. This is the set of values accepted by -/// `::operator new[](n, std::align_val_t(align))`. -inline constexpr bool IsValidAlignment(std::size_t align) noexcept -{ - return align > 0 && (align & (align - 1)) == 0; -} - -/// Round \p value up to the next multiple of \p align. -/// \p align must be a power of two (asserted at runtime in debug builds). -template -inline constexpr T AlignUp(T value, T align) noexcept -{ - assert(IsValidAlignment(static_cast(align))); // must be a power of two - return (value + align - 1) & ~(align - 1); -} - -} // namespace Internal -} // namespace ROOT - -#endif // ROOT_RAlignmentUtils diff --git a/core/foundation/test/CMakeLists.txt b/core/foundation/test/CMakeLists.txt index 0dba82f6289c8..0f19a713b0b5e 100644 --- a/core/foundation/test/CMakeLists.txt +++ b/core/foundation/test/CMakeLists.txt @@ -12,4 +12,5 @@ ROOT_ADD_GTEST(testException testException.cxx LIBRARIES Core GTest::gmock) ROOT_ADD_GTEST(testLogger testLogger.cxx LIBRARIES Core) ROOT_ADD_GTEST(testRRangeCast testRRangeCast.cxx LIBRARIES Core) ROOT_ADD_GTEST(testStringUtils testStringUtils.cxx LIBRARIES Core) +ROOT_ADD_GTEST(testBitUtils testBitUtils.cxx LIBRARIES Core) ROOT_ADD_GTEST(FoundationUtilsTests FoundationUtilsTests.cxx LIBRARIES Core INCLUDE_DIRS ../res) diff --git a/core/foundation/test/testBitUtils.cxx b/core/foundation/test/testBitUtils.cxx new file mode 100644 index 0000000000000..497efbf229ecf --- /dev/null +++ b/core/foundation/test/testBitUtils.cxx @@ -0,0 +1,45 @@ +#include "ROOT/BitUtils.hxx" + +#include "gtest/gtest.h" + +using namespace ROOT::Internal; + +TEST(BitUtils, LeadingZeroes32) +{ + EXPECT_EQ(LeadingZeroes(0), 32); + EXPECT_EQ(LeadingZeroes(~0), 0); + EXPECT_EQ(LeadingZeroes(0xF000'0000), 0); + EXPECT_EQ(LeadingZeroes(0x0000'F040), 16); + EXPECT_EQ(LeadingZeroes(0x0000'0003), 30); + EXPECT_EQ(LeadingZeroes(0x000F'F000), 12); +} + +TEST(BitUtils, LeadingZeroes64) +{ + EXPECT_EQ(LeadingZeroes(0ull), 64); + EXPECT_EQ(LeadingZeroes(~0ull), 0); + EXPECT_EQ(LeadingZeroes(0xF000'0000'0000'0000ull), 0); + EXPECT_EQ(LeadingZeroes(0x0000'F000'1000'1000ull), 16); + EXPECT_EQ(LeadingZeroes(0x0000'0000'0000'0003ull), 62); + EXPECT_EQ(LeadingZeroes(0x0000'000F'F000'0000ull), 28); +} + +TEST(BitUtils, TrailingZeroes32) +{ + EXPECT_EQ(TrailingZeroes(0), 32); + EXPECT_EQ(TrailingZeroes(~0), 0); + EXPECT_EQ(TrailingZeroes(0xF000'0000), 28); + EXPECT_EQ(TrailingZeroes(0x0000'F040), 6); + EXPECT_EQ(TrailingZeroes(0x0000'0003), 0); + EXPECT_EQ(TrailingZeroes(0x000F'F000), 12); +} + +TEST(BitUtils, TrailingZeroes64) +{ + EXPECT_EQ(TrailingZeroes(0ull), 64); + EXPECT_EQ(TrailingZeroes(~0ull), 0); + EXPECT_EQ(TrailingZeroes(0xF000'0000'0000'0000ull), 60); + EXPECT_EQ(TrailingZeroes(0x0000'F000'1000'1000ull), 12); + EXPECT_EQ(TrailingZeroes(0x0000'0000'0000'0003ull), 0); + EXPECT_EQ(TrailingZeroes(0x0000'000F'F000'0000ull), 28); +} diff --git a/core/meta/src/TClass.cxx b/core/meta/src/TClass.cxx index 6c6b13cbdae1d..37c2b3824ae2c 100644 --- a/core/meta/src/TClass.cxx +++ b/core/meta/src/TClass.cxx @@ -124,7 +124,7 @@ In order to access the name of a class within the ROOT type system, the method T #include "TClonesArray.h" #include "TRef.h" #include "TRefArray.h" -#include "ROOT/RAlignmentUtils.hxx" +#include "ROOT/BitUtils.hxx" using std::multimap, std::make_pair, std::string; diff --git a/core/meta/src/TStreamerElement.cxx b/core/meta/src/TStreamerElement.cxx index 305485e512748..71eaa795daca9 100644 --- a/core/meta/src/TStreamerElement.cxx +++ b/core/meta/src/TStreamerElement.cxx @@ -20,7 +20,7 @@ #include "TBaseClass.h" #include "TDataMember.h" #include "TDataType.h" -#include "ROOT/RAlignmentUtils.hxx" +#include "ROOT/BitUtils.hxx" #include "TEnum.h" #include "TRealData.h" #include "ThreadLocalStorage.h" diff --git a/core/metacling/src/TClingClassInfo.cxx b/core/metacling/src/TClingClassInfo.cxx index 61403c9f5fe39..cbba54e1b39d3 100644 --- a/core/metacling/src/TClingClassInfo.cxx +++ b/core/metacling/src/TClingClassInfo.cxx @@ -51,7 +51,7 @@ but the class metadata comes from the Clang C++ compiler, not CINT. #include "llvm/Support/Casting.h" #include "llvm/Support/raw_ostream.h" -#include "ROOT/RAlignmentUtils.hxx" +#include "ROOT/BitUtils.hxx" #include #include diff --git a/io/io/inc/TEmulatedCollectionProxy.h b/io/io/inc/TEmulatedCollectionProxy.h index eae8404029eac..97ee1ce90685a 100644 --- a/io/io/inc/TEmulatedCollectionProxy.h +++ b/io/io/inc/TEmulatedCollectionProxy.h @@ -12,7 +12,7 @@ #define ROOT_TEmulatedCollectionProxy #include "TGenCollectionProxy.h" -#include "ROOT/RAlignmentUtils.hxx" +#include "ROOT/BitUtils.hxx" #include #include diff --git a/io/io/inc/TGenCollectionProxy.h b/io/io/inc/TGenCollectionProxy.h index da0cced70c750..832d53e88de7f 100644 --- a/io/io/inc/TGenCollectionProxy.h +++ b/io/io/inc/TGenCollectionProxy.h @@ -17,7 +17,7 @@ #include "TCollectionProxyInfo.h" -#include "ROOT/RAlignmentUtils.hxx" +#include "ROOT/BitUtils.hxx" #include #include diff --git a/io/io/src/TStreamerInfo.cxx b/io/io/src/TStreamerInfo.cxx index 0b23b352d9d28..204c5844138f6 100644 --- a/io/io/src/TStreamerInfo.cxx +++ b/io/io/src/TStreamerInfo.cxx @@ -74,7 +74,7 @@ element type. #include "TVirtualMutex.h" #include "TStreamerInfoActions.h" -#include "ROOT/RAlignmentUtils.hxx" +#include "ROOT/BitUtils.hxx" #include #include diff --git a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx index a9298f2086ace..9d4426af547af 100644 --- a/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx +++ b/tree/ntuple/inc/ROOT/RNTupleDescriptor.hxx @@ -765,7 +765,18 @@ private: RNTupleDescriptor CloneSchema() const; public: - static constexpr unsigned int kFeatureFlagTest = 137; // Bit reserved for forward-compatibility testing + /// All known feature flags. + /// Note that the flag values represent the bit _index_, not the already-bitshifted integer. + enum EFeatureFlags { + // Insert new feature flags here, with contiguous values. If at any point a "hole" appears in the valid feature + // flags values, the check in RNTupleSerialize must be updated. + + // End of regular feature flags + kFeatureFlag_COUNT, + + /// Reserved for forward-compatibility testing + kFeatureFlag_Test = 137 + }; class RColumnDescriptorIterable; class RFieldDescriptorIterable; @@ -1736,6 +1747,8 @@ public: void SetVersionForWriting(); void SetNTuple(const std::string_view name, const std::string_view description); + /// Sets the `flag`-th bit of the feature flag to 1. + /// Note that `flag` itself is not a bitmask, just the bit index of the flag to enable. void SetFeature(unsigned int flag); void SetOnDiskHeaderXxHash3(std::uint64_t xxhash3) { fDescriptor.fOnDiskHeaderXxHash3 = xxhash3; } diff --git a/tree/ntuple/src/RColumnElement.hxx b/tree/ntuple/src/RColumnElement.hxx index a31bb2f172e03..99420e2cd0b17 100644 --- a/tree/ntuple/src/RColumnElement.hxx +++ b/tree/ntuple/src/RColumnElement.hxx @@ -7,6 +7,7 @@ // definitions are implementation details and should not be exposed to a public interface. #include +#include #include #include #include @@ -1000,36 +1001,6 @@ namespace Quantize { using Quantized_t = std::uint32_t; -[[maybe_unused]] inline std::size_t LeadingZeroes(std::uint32_t x) -{ - if (x == 0) - return 32; - -#ifdef _MSC_VER - unsigned long idx = 0; - if (_BitScanReverse(&idx, x)) - return static_cast(31 - idx); - return 32; -#else - return static_cast(__builtin_clz(x)); -#endif -} - -[[maybe_unused]] inline std::size_t TrailingZeroes(std::uint32_t x) -{ - if (x == 0) - return 32; - -#ifdef _MSC_VER - unsigned long idx = 0; - if (_BitScanForward(&idx, x)) - return static_cast(idx); - return 32; -#else - return static_cast(__builtin_ctz(x)); -#endif -} - /// Converts the array `src` of `count` floating point numbers into an array of their quantized representations. /// Each element of `src` is assumed to be in the inclusive range [min, max]. /// The quantized representation will consist of unsigned integers of at most `nQuantBits` (with `nQuantBits <= 8 * @@ -1065,7 +1036,7 @@ int QuantizeReals(Quantized_t *dst, const T *src, std::size_t count, double min, ByteSwapIfNecessary(q); // double-check we actually used at most `nQuantBits` - assert(outOfRange || LeadingZeroes(q) >= unusedBits); + assert(outOfRange || ROOT::Internal::LeadingZeroes(q) >= unusedBits); // we want to leave zeroes in the LSB, not the MSB, because we'll then drop the LSB // when bit packing. @@ -1095,7 +1066,7 @@ int UnquantizeReals(T *dst, const Quantized_t *src, std::size_t count, double mi for (std::size_t i = 0; i < count; ++i) { Quantized_t elem = src[i]; // Undo the LSB-preserving shift performed by QuantizeReals - assert(TrailingZeroes(elem) >= unusedBits); + assert(ROOT::Internal::TrailingZeroes(elem) >= unusedBits); elem >>= unusedBits; ByteSwapIfNecessary(elem); diff --git a/tree/ntuple/src/RNTupleDescriptor.cxx b/tree/ntuple/src/RNTupleDescriptor.cxx index b51af779c45a9..cd48ea0c9159a 100644 --- a/tree/ntuple/src/RNTupleDescriptor.cxx +++ b/tree/ntuple/src/RNTupleDescriptor.cxx @@ -1105,7 +1105,7 @@ void ROOT::Internal::RNTupleDescriptorBuilder::SetNTuple(const std::string_view void ROOT::Internal::RNTupleDescriptorBuilder::SetFeature(unsigned int flag) { - if (flag % 64 == 0) + if (flag > 0 && flag % 64 == 0) throw RException(R__FAIL("invalid feature flag: " + std::to_string(flag))); fDescriptor.fFeatureFlags.insert(flag); } diff --git a/tree/ntuple/src/RNTupleSerialize.cxx b/tree/ntuple/src/RNTupleSerialize.cxx index f5d9d04f1069a..5da2fa6a4f6b9 100644 --- a/tree/ntuple/src/RNTupleSerialize.cxx +++ b/tree/ntuple/src/RNTupleSerialize.cxx @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -1621,7 +1622,6 @@ ROOT::Internal::RNTupleSerializer::SerializeHeader(void *buffer, const ROOT::RNT void **where = (buffer == nullptr) ? &buffer : reinterpret_cast(&pos); pos += SerializeEnvelopePreamble(kEnvelopeTypeHeader, *where); - // So far we don't make use of feature flags if (auto res = SerializeFeatureFlags(desc.GetFeatureFlags(), *where)) { pos += res.Unwrap(); } else { @@ -1761,7 +1761,6 @@ ROOT::RResult ROOT::Internal::RNTupleSerializer::SerializeFooter( pos += SerializeEnvelopePreamble(kEnvelopeTypeFooter, *where); - // So far we don't make use of footer feature flags // NOTE: we currently serialize all feature flags in the footer, even those that were already written in the // header. This is fine, as they will be logically OR-ed together during deserialization. if (auto res = SerializeFeatureFlags(desc.GetFeatureFlags(), *where)) { @@ -1871,10 +1870,10 @@ static ROOT::RResult CheckFeatureFlags(const std::vector &f for (std::size_t i = 0; i < featureFlags.size(); ++i) { if (!featureFlags[i]) continue; - unsigned int bit = 0; - while (!(featureFlags[i] & (static_cast(1) << bit))) - bit++; - return R__FAIL("unsupported format feature: " + std::to_string(i * 64 + bit)); + // NOTE: this assumes all valid feature flags are consecutive, thus we can just check the highest one set. + unsigned int highestBitSet = 64 * i + (63 - ROOT::Internal::LeadingZeroes(featureFlags[i])); + if (highestBitSet >= ROOT::RNTupleDescriptor::kFeatureFlag_COUNT) + return R__FAIL("unsupported format feature: " + std::to_string(highestBitSet)); } return ROOT::RResult::Success(); } diff --git a/tree/ntuple/test/ntuple_compat.cxx b/tree/ntuple/test/ntuple_compat.cxx index 7b5abb7b5932d..6b2cd0a6a774e 100644 --- a/tree/ntuple/test/ntuple_compat.cxx +++ b/tree/ntuple/test/ntuple_compat.cxx @@ -29,14 +29,85 @@ TEST(RNTupleCompat, Epoch) } } -TEST(RNTupleCompat, FeatureFlag) +TEST(RNTupleCompat, FeatureFlagSupported) { - FileRaii fileGuard("test_ntuple_compat_feature_flag.root"); + // Write all known feature flags in the header and verify we can read the RNTuple correctly. + FileRaii fileGuard("test_ntuple_compat_feature_flag_supported.root"); RNTupleDescriptorBuilder descBuilder; descBuilder.SetVersionForWriting(); descBuilder.SetNTuple("ntpl", ""); - descBuilder.SetFeature(RNTupleDescriptor::kFeatureFlagTest); + for (unsigned int flag = 0; flag < RNTupleDescriptor::kFeatureFlag_COUNT; ++flag) + descBuilder.SetFeature(flag); + descBuilder.AddField(RFieldDescriptorBuilder::FromField(ROOT::RFieldZero()).FieldId(0).MakeDescriptor().Unwrap()); + ASSERT_TRUE(static_cast(descBuilder.EnsureValidDescriptor())); + + RNTupleWriteOptions options; + auto writer = RNTupleFileWriter::Recreate("ntpl", fileGuard.GetPath(), EContainerFormat::kTFile, options); + RNTupleSerializer serializer; + + auto ctx = serializer.SerializeHeader(nullptr, descBuilder.GetDescriptor()).Unwrap(); + auto buffer = std::make_unique(ctx.GetHeaderSize()); + ctx = serializer.SerializeHeader(buffer.get(), descBuilder.GetDescriptor()).Unwrap(); + writer->WriteNTupleHeader(buffer.get(), ctx.GetHeaderSize(), ctx.GetHeaderSize()); + + auto szFooter = serializer.SerializeFooter(nullptr, descBuilder.GetDescriptor(), ctx).Unwrap(); + buffer = std::make_unique(szFooter); + serializer.SerializeFooter(buffer.get(), descBuilder.GetDescriptor(), ctx); + writer->WriteNTupleFooter(buffer.get(), szFooter, szFooter); + + writer->Commit(); + // Call destructor to flush data to disk + writer = nullptr; + + auto pageSource = RPageSource::Create("ntpl", fileGuard.GetPath()); + EXPECT_NO_THROW(pageSource->Attach()); +} + +TEST(RNTupleCompat, FeatureFlagSupportedFooter) +{ + // Write all known feature flags in the footer and verify we can read the RNTuple correctly. + FileRaii fileGuard("test_ntuple_compat_feature_flag_supported_footer.root"); + + RNTupleDescriptorBuilder descBuilder; + descBuilder.SetVersionForWriting(); + descBuilder.SetNTuple("ntpl", ""); + descBuilder.AddField(RFieldDescriptorBuilder::FromField(ROOT::RFieldZero()).FieldId(0).MakeDescriptor().Unwrap()); + ASSERT_TRUE(static_cast(descBuilder.EnsureValidDescriptor())); + + RNTupleWriteOptions options; + auto writer = RNTupleFileWriter::Recreate("ntpl", fileGuard.GetPath(), EContainerFormat::kTFile, options); + RNTupleSerializer serializer; + + auto ctx = serializer.SerializeHeader(nullptr, descBuilder.GetDescriptor()).Unwrap(); + auto buffer = std::make_unique(ctx.GetHeaderSize()); + ctx = serializer.SerializeHeader(buffer.get(), descBuilder.GetDescriptor()).Unwrap(); + writer->WriteNTupleHeader(buffer.get(), ctx.GetHeaderSize(), ctx.GetHeaderSize()); + + for (unsigned int flag = 0; flag < RNTupleDescriptor::kFeatureFlag_COUNT; ++flag) + descBuilder.SetFeature(flag); + + auto szFooter = serializer.SerializeFooter(nullptr, descBuilder.GetDescriptor(), ctx).Unwrap(); + buffer = std::make_unique(szFooter); + serializer.SerializeFooter(buffer.get(), descBuilder.GetDescriptor(), ctx); + writer->WriteNTupleFooter(buffer.get(), szFooter, szFooter); + + writer->Commit(); + // Call destructor to flush data to disk + writer = nullptr; + + auto pageSource = RPageSource::Create("ntpl", fileGuard.GetPath()); + EXPECT_NO_THROW(pageSource->Attach()); +} + +TEST(RNTupleCompat, FeatureFlagUnsupported) +{ + FileRaii fileGuard("test_ntuple_compat_feature_flag_unsupported.root"); + + RNTupleDescriptorBuilder descBuilder; + descBuilder.SetVersionForWriting(); + descBuilder.SetNTuple("ntpl", ""); + descBuilder.SetFeature(RNTupleDescriptor::kFeatureFlag_Test); descBuilder.AddField(RFieldDescriptorBuilder::FromField(ROOT::RFieldZero()).FieldId(0).MakeDescriptor().Unwrap()); ASSERT_TRUE(static_cast(descBuilder.EnsureValidDescriptor())); @@ -67,9 +138,9 @@ TEST(RNTupleCompat, FeatureFlag) } } -TEST(RNTupleCompat, FeatureFlagInFooter) +TEST(RNTupleCompat, FeatureFlagUnsupportedInFooter) { - FileRaii fileGuard("test_ntuple_compat_feature_flag_footer.root"); + FileRaii fileGuard("test_ntuple_compat_feature_flag_unsupported_footer.root"); RNTupleDescriptorBuilder descBuilder; descBuilder.SetVersionForWriting(); @@ -87,7 +158,7 @@ TEST(RNTupleCompat, FeatureFlagInFooter) writer->WriteNTupleHeader(buffer.get(), ctx.GetHeaderSize(), ctx.GetHeaderSize()); // Write the feature flags in the footer - descBuilder.SetFeature(RNTupleDescriptor::kFeatureFlagTest); + descBuilder.SetFeature(RNTupleDescriptor::kFeatureFlag_Test); auto szFooter = serializer.SerializeFooter(nullptr, descBuilder.GetDescriptor(), ctx).Unwrap(); buffer = std::make_unique(szFooter); @@ -107,6 +178,49 @@ TEST(RNTupleCompat, FeatureFlagInFooter) } } +TEST(RNTupleCompat, FeatureFlagMixSupportedUnsupported) +{ + FileRaii fileGuard("test_ntuple_compat_feature_flag_mix_supported.root"); + + RNTupleDescriptorBuilder descBuilder; + descBuilder.SetVersionForWriting(); + descBuilder.SetNTuple("ntpl", ""); + for (unsigned int flag = 0; flag < RNTupleDescriptor::kFeatureFlag_COUNT; ++flag) + descBuilder.SetFeature(flag); + descBuilder.AddField(RFieldDescriptorBuilder::FromField(ROOT::RFieldZero()).FieldId(0).MakeDescriptor().Unwrap()); + ASSERT_TRUE(static_cast(descBuilder.EnsureValidDescriptor())); + + RNTupleWriteOptions options; + auto writer = RNTupleFileWriter::Recreate("ntpl", fileGuard.GetPath(), EContainerFormat::kTFile, options); + RNTupleSerializer serializer; + + auto ctx = serializer.SerializeHeader(nullptr, descBuilder.GetDescriptor()).Unwrap(); + auto buffer = std::make_unique(ctx.GetHeaderSize()); + ctx = serializer.SerializeHeader(buffer.get(), descBuilder.GetDescriptor()).Unwrap(); + writer->WriteNTupleHeader(buffer.get(), ctx.GetHeaderSize(), ctx.GetHeaderSize()); + + // This is unsupported! + descBuilder.SetFeature(RNTupleDescriptor::kFeatureFlag_COUNT); + + auto szFooter = serializer.SerializeFooter(nullptr, descBuilder.GetDescriptor(), ctx).Unwrap(); + buffer = std::make_unique(szFooter); + serializer.SerializeFooter(buffer.get(), descBuilder.GetDescriptor(), ctx); + writer->WriteNTupleFooter(buffer.get(), szFooter, szFooter); + + writer->Commit(); + // Call destructor to flush data to disk + writer = nullptr; + + auto pageSource = RPageSource::Create("ntpl", fileGuard.GetPath()); + try { + pageSource->Attach(); + FAIL() << "opening an RNTuple that uses an unsupported feature should fail"; + } catch (const ROOT::RException &err) { + EXPECT_THAT(err.what(), testing::HasSubstr("unsupported format feature: " + + std::to_string(RNTupleDescriptor::kFeatureFlag_COUNT))); + } +} + TEST(RNTupleCompat, FwdCompat_FutureNTupleAnchor) { using ROOT::RXTuple;