[ntuple] some cleanups for TestFutureColumn

root-project · Oct 1, 2024 · 432c428 · 432c428
1 parent 2be84f3
commit 432c428
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 15 deletions.
diff --git a/tree/ntuple/v7/inc/ROOT/RColumnElementBase.hxx b/tree/ntuple/v7/inc/ROOT/RColumnElementBase.hxx
@@ -135,10 +135,11 @@ enum class EColumnCppType {
    kMax
 };
 
-inline constexpr EColumnCppType kTestFutureColumn = static_cast<EColumnCppType>((int)EColumnCppType::kMax + 1);
+inline constexpr EColumnCppType kTestFutureColumn =
+   static_cast<EColumnCppType>(std::numeric_limits<std::underlying_type_t<EColumnCppType>>::max() - 1);
 
 struct RTestFutureColumn {
-   int dummy;
+   std::uint32_t dummy;
 };
 
 std::unique_ptr<RColumnElementBase> GenerateColumnElement(EColumnCppType cppType, EColumnType colType);

diff --git a/tree/ntuple/v7/src/RColumnElement.cxx b/tree/ntuple/v7/src/RColumnElement.cxx
@@ -58,8 +58,10 @@ ROOT::Experimental::Internal::RColumnElementBase::GetValidBitRange(EColumnType t
    case EColumnType::kSplitUInt16: return std::make_pair(16, 16);
    case EColumnType::kReal32Trunc: return std::make_pair(10, 31);
    case EColumnType::kReal32Quant: return std::make_pair(1, 32);
-   case kTestFutureType: return std::make_pair(32, 32);
-   default: assert(false);
+   default:
+      if (type == kTestFutureType)
+         return std::make_pair(32, 32);
+      assert(false);
    }
    // never here
    return std::make_pair(0, 0);
@@ -97,8 +99,10 @@ std::string ROOT::Experimental::Internal::RColumnElementBase::GetTypeName(EColum
    case EColumnType::kSplitUInt16: return "SplitUInt16";
    case EColumnType::kReal32Trunc: return "Real32Trunc";
    case EColumnType::kReal32Quant: return "Real32Quant";
-   case kTestFutureType: return "TestFutureType";
-   default: return "UNKNOWN";
+   default:
+      if (type == kTestFutureType)
+         return "TestFutureType";
+      return "UNKNOWN";
    }
 }
 
@@ -139,8 +143,10 @@ ROOT::Experimental::Internal::RColumnElementBase::Generate<void>(EColumnType typ
    case EColumnType::kSplitUInt16: return std::make_unique<RColumnElement<std::uint16_t, EColumnType::kSplitUInt16>>();
    case EColumnType::kReal32Trunc: return std::make_unique<RColumnElement<float, EColumnType::kReal32Trunc>>();
    case EColumnType::kReal32Quant: return std::make_unique<RColumnElement<float, EColumnType::kReal32Quant>>();
-   case kTestFutureType: return std::make_unique<RColumnElement<Internal::RTestFutureColumn, kTestFutureType>>();
-   default: assert(false);
+   default:
+      if (type == kTestFutureType)
+         return std::make_unique<RColumnElement<Internal::RTestFutureColumn, kTestFutureType>>();
+      assert(false);
    }
    // never here
    return nullptr;
@@ -165,8 +171,10 @@ ROOT::Experimental::Internal::GenerateColumnElement(EColumnCppType cppType, ECol
    case EColumnCppType::kDouble: return GenerateColumnElementInternal<double>(type);
    case EColumnCppType::kClusterSize: return GenerateColumnElementInternal<ClusterSize_t>(type);
    case EColumnCppType::kColumnSwitch: return GenerateColumnElementInternal<RColumnSwitch>(type);
-   case kTestFutureColumn: return GenerateColumnElementInternal<RTestFutureColumn>(type);
-   default: R__ASSERT(!"Invalid column cpp type");
+   default:
+      if (cppType == kTestFutureColumn)
+         return GenerateColumnElementInternal<RTestFutureColumn>(type);
+      R__ASSERT(!"Invalid column cpp type");
    }
    // never here
    return nullptr;

diff --git a/tree/ntuple/v7/src/RColumnElement.hxx b/tree/ntuple/v7/src/RColumnElement.hxx
@@ -268,7 +268,8 @@ using ROOT::Experimental::EColumnType;
 using ROOT::Experimental::Internal::RColumnElementBase;
 
 // testing value for an unknown future column type
-inline constexpr EColumnType kTestFutureType = static_cast<EColumnType>(int(EColumnType::kMax) + 1);
+inline constexpr EColumnType kTestFutureType =
+   static_cast<EColumnType>(std::numeric_limits<std::underlying_type_t<EColumnType>>::max() - 1);
 
 template <typename CppT, EColumnType>
 class RColumnElement;
@@ -306,8 +307,10 @@ std::unique_ptr<RColumnElementBase> GenerateColumnElementInternal(EColumnType ty
    case EColumnType::kSplitUInt16: return std::make_unique<RColumnElement<CppT, EColumnType::kSplitUInt16>>();
    case EColumnType::kReal32Trunc: return std::make_unique<RColumnElement<CppT, EColumnType::kReal32Trunc>>();
    case EColumnType::kReal32Quant: return std::make_unique<RColumnElement<CppT, EColumnType::kReal32Quant>>();
-   case kTestFutureType: return std::make_unique<RColumnElement<CppT, kTestFutureType>>();
-   default: R__ASSERT(false);
+   default:
+      if (type == kTestFutureType)
+         return std::make_unique<RColumnElement<CppT, kTestFutureType>>();
+      R__ASSERT(false);
    }
    // never here
    return nullptr;

diff --git a/tree/ntuple/v7/src/RNTupleSerialize.cxx b/tree/ntuple/v7/src/RNTupleSerialize.cxx
@@ -711,8 +711,10 @@ ROOT::Experimental::Internal::RNTupleSerializer::SerializeColumnType(ROOT::Exper
    case EColumnType::kSplitUInt16: return SerializeUInt16(0x15, buffer);
    case EColumnType::kReal32Trunc: return SerializeUInt16(0x1D, buffer);
    case EColumnType::kReal32Quant: return SerializeUInt16(0x1E, buffer);
-   case kTestFutureType: return SerializeUInt16(0x99, buffer);
-   default: throw RException(R__FAIL("ROOT bug: unexpected column type"));
+   default:
+      if (type == kTestFutureType)
+         return SerializeUInt16(0x99, buffer);
+      throw RException(R__FAIL("ROOT bug: unexpected column type"));
    }
 }
 

diff --git a/tree/ntuple/v7/test/ntuple_serialize.cxx b/tree/ntuple/v7/test/ntuple_serialize.cxx
@@ -65,6 +65,7 @@ TEST(RNTuple, SerializeColumnType)
    RNTupleSerializer::SerializeUInt16(5000, buffer);
    auto res = RNTupleSerializer::DeserializeColumnType(buffer, type);
    EXPECT_TRUE(bool(res));
+   EXPECT_EQ(type, EColumnType::kUnknown);
 
    for (int i = 1; i < static_cast<int>(EColumnType::kMax); ++i) {
       RNTupleSerializer::SerializeColumnType(static_cast<EColumnType>(i), buffer);