[ntuple] fix type name normalization for C style arrays

Author: jblomer

tree/ntuple/inc/ROOT/RFieldUtils.hxx

@@ -20,6 +20,8 @@ namespace Internal {
 /// Applies RNTuple specific type name normalization rules (see specs) that help the string parsing in
 /// RFieldBase::Create(). The normalization of templated types does not include full normalization of the
 /// template arguments (hence "Prefix").
+/// Furthermore, if the type is a C-style array, rules are applied to the base type and the C style array
+/// is then mapped to an std::array.
 std::string GetCanonicalTypePrefix(const std::string &typeName);
 
 /// Given a type name normalized by ROOT meta, renormalize it for RNTuple. E.g., insert std::prefix.
@@ -48,13 +50,6 @@ enum class ERNTupleSerializationMode {
 
 ERNTupleSerializationMode GetRNTupleSerializationMode(TClass *cl);
 
-/// Parse a type name of the form `T[n][m]...` and return the base type `T` and a vector that contains,
-/// in order, the declared size for each dimension, e.g. for `unsigned char[1][2][3]` it returns the tuple
-/// `{"unsigned char", {1, 2, 3}}`. Extra whitespace in `typeName` should be removed before calling this function.
-///
-/// If `typeName` is not an array type, it returns a tuple `{T, {}}`. On error, it returns a default-constructed tuple.
-std::tuple<std::string, std::vector<std::size_t>> ParseArrayType(const std::string &typeName);
-
 /// Used in RFieldBase::Create() in order to get the comma-separated list of template types
 /// E.g., gets {"int", "std::variant<double,int>"} from "int,std::variant<double,int>".
 /// If maxArgs > 0, stop tokenizing after the given number of tokens are found. Used to strip

tree/ntuple/src/RFieldBase.cxx

@@ -292,7 +292,6 @@ ROOT::RFieldBase::Create(const std::string &fieldName, const std::string &typeNa
                          const ROOT::RCreateFieldOptions &options, const ROOT::RNTupleDescriptor *desc,
                          ROOT::DescriptorId_t fieldId)
 {
-   using ROOT::Internal::ParseArrayType;
    using ROOT::Internal::ParseUIntTypeToken;
    using ROOT::Internal::TokenizeTypeList;
 
@@ -331,15 +330,6 @@ ROOT::RFieldBase::Create(const std::string &fieldName, const std::string &typeNa
    // try-catch block to intercept any exception that may be thrown by Unwrap() so that this
    // function never throws but returns RResult::Error instead.
    try {
-      if (auto [arrayBaseType, arraySizes] = ParseArrayType(resolvedType); !arraySizes.empty()) {
-         std::unique_ptr<RFieldBase> arrayField = Create("_0", arrayBaseType, options, desc, fieldId).Unwrap();
-         for (int i = arraySizes.size() - 1; i >= 0; --i) {
-            arrayField =
-               std::make_unique<RArrayField>((i == 0) ? fieldName : "_0", std::move(arrayField), arraySizes[i]);
-         }
-         return arrayField;
-      }
-
       if (resolvedType == "bool") {
          result = std::make_unique<RField<bool>>(fieldName);
       } else if (resolvedType == "char") {

tree/ntuple/src/RFieldUtils.cxx

@@ -66,6 +66,50 @@ bool IsUserClass(const std::string &typeName)
    return typeName.rfind("std::", 0) != 0 && typeName.rfind("ROOT::VecOps::RVec<", 0) != 0;
 }
 
+/// Parse a type name of the form `T[n][m]...` and return the base type `T` and a vector that contains,
+/// in order, the declared size for each dimension, e.g. for `unsigned char[1][2][3]` it returns the tuple
+/// `{"unsigned char", {1, 2, 3}}`. Extra whitespace in `typeName` should be removed before calling this function.
+///
+/// If `typeName` is not an array type, it returns a tuple `{T, {}}`. On error, it returns a default-constructed tuple.
+std::tuple<std::string, std::vector<std::size_t>> ParseArrayType(const std::string &typeName)
+{
+   std::vector<std::size_t> sizeVec;
+
+   // Only parse outer array definition, i.e. the right `]` should be at the end of the type name
+   std::string prefix{typeName};
+   while (prefix.back() == ']') {
+      auto posRBrace = prefix.size() - 1;
+      auto posLBrace = prefix.rfind('[', posRBrace);
+      if (posLBrace == std::string_view::npos) {
+         throw ROOT::RException(R__FAIL(std::string("invalid array type: ") + typeName));
+      }
+      if (posRBrace - posLBrace <= 1) {
+         throw ROOT::RException(R__FAIL(std::string("invalid array type: ") + typeName));
+      }
+
+      const std::size_t size =
+         ROOT::Internal::ParseUIntTypeToken(prefix.substr(posLBrace + 1, posRBrace - posLBrace - 1));
+      if (size == 0) {
+         throw ROOT::RException(R__FAIL(std::string("invalid array size: ") + typeName));
+      }
+
+      sizeVec.insert(sizeVec.begin(), size);
+      prefix.resize(posLBrace);
+   }
+   return std::make_tuple(prefix, sizeVec);
+}
+
+/// Assembles a (nested) std::array<> based type based on the dimensions retrieved from ParseArrayType(). Returns
+/// baseType if there are no dimensions.
+std::string GetStandardArrayType(const std::string &baseType, const std::vector<std::size_t> &dimensions)
+{
+   std::string typeName = baseType;
+   for (auto i = dimensions.rbegin(), iEnd = dimensions.rend(); i != iEnd; ++i) {
+      typeName = "std::array<" + typeName + "," + std::to_string(*i) + ">";
+   }
+   return typeName;
+}
+
 // Recursively normalizes a template argument using the regular type name normalizer F as a helper.
 template <typename F>
 std::string GetNormalizedTemplateArg(const std::string &arg, bool keepQualifier, F fnTypeNormalizer)
@@ -85,7 +129,7 @@ std::string GetNormalizedTemplateArg(const std::string &arg, bool keepQualifier,
    // strips the qualifier.
    // Demangled names may have the CV qualifiers suffixed and not prefixed (but const always before volatile).
    // Note that in the latter case, we may have the CV qualifiers before array brackets, e.g. `int const[2]`.
-   const auto [base, _] = ROOT::Internal::ParseArrayType(arg);
+   const auto [base, _] = ParseArrayType(arg);
    if (base.rfind("const ", 0) == 0 || base.rfind("volatile const ", 0) == 0 ||
        base.find(" const", base.length() - 6) != std::string::npos ||
        base.find(" const volatile", base.length() - 15) != std::string::npos) {
@@ -336,7 +380,7 @@ void NormalizeTemplateArguments(std::string &templatedTypeName, int maxTemplateA
 // Given a type name normalized by ROOT Meta, return the type name normalized according to the RNTuple rules.
 std::string GetRenormalizedMetaTypeName(const std::string &metaNormalizedName)
 {
-   const std::string canonicalTypePrefix{ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName)};
+   const auto canonicalTypePrefix = ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName);
    // RNTuple resolves Double32_t for the normalized type name but keeps Double32_t for the type alias
    // (also in template parameters)
    if (canonicalTypePrefix == "Double32_t")
@@ -357,14 +401,16 @@ std::string GetRenormalizedMetaTypeName(const std::string &metaNormalizedName)
 // RNTuple rules.
 std::string GetRenormalizedDemangledTypeName(const std::string &demangledName, bool renormalizeStdString)
 {
-   std::string canonicalTypePrefix{demangledName};
+   std::string tn{demangledName};
+   RemoveSpaceBefore(tn, '[');
+   auto [canonicalTypePrefix, dimensions] = ParseArrayType(tn);
    RemoveCVQualifiers(canonicalTypePrefix);
    RemoveLeadingKeyword(canonicalTypePrefix);
    MapIntegerType(canonicalTypePrefix);
 
    if (canonicalTypePrefix.find('<') == std::string::npos) {
       // If there are no templates, the function is done.
-      return canonicalTypePrefix;
+      return GetStandardArrayType(canonicalTypePrefix, dimensions);
    }
    RemoveSpaceBefore(canonicalTypePrefix, '>');
    RemoveSpaceAfter(canonicalTypePrefix, ',');
@@ -396,15 +442,21 @@ std::string GetRenormalizedDemangledTypeName(const std::string &demangledName, b
       RenormalizeStdString(normName);
    }
 
-   return normName;
+   return GetStandardArrayType(normName, dimensions);
 }
 
 } // namespace
 
 std::string ROOT::Internal::GetCanonicalTypePrefix(const std::string &typeName)
 {
-   // Remove outer cv qualifiers
-   std::string canonicalType{TClassEdit::CleanType(typeName.c_str(), /*mode=*/1)};
+   // Remove outer cv qualifiers and extra white spaces
+   const std::string cleanedType = TClassEdit::CleanType(typeName.c_str(), /*mode=*/1);
+
+   // Can happen when called from RFieldBase::Create() and is caught there
+   if (cleanedType.empty())
+      return "";
+
+   auto [canonicalType, dimensions] = ParseArrayType(cleanedType);
 
    RemoveLeadingKeyword(canonicalType);
    if (canonicalType.substr(0, 2) == "::") {
@@ -456,7 +508,7 @@ std::string ROOT::Internal::GetCanonicalTypePrefix(const std::string &typeName)
 
    MapIntegerType(canonicalType);
 
-   return canonicalType;
+   return GetStandardArrayType(canonicalType, dimensions);
 }
 
 std::string ROOT::Internal::GetRenormalizedTypeName(const std::type_info &ti)
@@ -474,9 +526,9 @@ std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &o
    const TClassEdit::EModType modType = static_cast<TClassEdit::EModType>(
       TClassEdit::kDropStlDefault | TClassEdit::kDropComparator | TClassEdit::kDropHash);
    TClassEdit::TSplitType splitname(origName.c_str(), modType);
-   std::string canonicalTypePrefix;
-   splitname.ShortType(canonicalTypePrefix, modType);
-   canonicalTypePrefix = GetCanonicalTypePrefix(canonicalTypePrefix);
+   std::string shortType;
+   splitname.ShortType(shortType, modType);
+   const auto canonicalTypePrefix = GetCanonicalTypePrefix(shortType);
 
    if (canonicalTypePrefix.find('<') == std::string::npos) {
       // If there are no templates, the function is done.
@@ -620,30 +672,6 @@ ROOT::Internal::ERNTupleSerializationMode ROOT::Internal::GetRNTupleSerializatio
    }
 }
 
-std::tuple<std::string, std::vector<std::size_t>> ROOT::Internal::ParseArrayType(const std::string &typeName)
-{
-   std::vector<std::size_t> sizeVec;
-
-   // Only parse outer array definition, i.e. the right `]` should be at the end of the type name
-   std::string prefix{typeName};
-   while (prefix.back() == ']') {
-      auto posRBrace = prefix.size() - 1;
-      auto posLBrace = prefix.rfind('[', posRBrace);
-      if (posLBrace == std::string_view::npos) {
-         throw RException(R__FAIL(std::string("invalid array type: ") + typeName));
-      }
-
-      const std::size_t size = ParseUIntTypeToken(prefix.substr(posLBrace + 1, posRBrace - posLBrace - 1));
-      if (size == 0) {
-         throw RException(R__FAIL(std::string("invalid array size: ") + typeName));
-      }
-
-      sizeVec.insert(sizeVec.begin(), size);
-      prefix.resize(posLBrace);
-   }
-   return std::make_tuple(prefix, sizeVec);
-}
-
 std::vector<std::string> ROOT::Internal::TokenizeTypeList(std::string_view templateType, std::size_t maxArgs)
 {
    std::vector<std::string> result;

tree/ntuple/test/ntuple_type_name.cxx

@@ -37,6 +37,14 @@ TEST(RNTuple, TypeNameNormalizationByName)
    EXPECT_EQ("std::uint32_t", RFieldBase::Create("f", "SG::sgkey_t").Unwrap()->GetTypeName());
    EXPECT_EQ("SG::sgkey_t", RFieldBase::Create("f", "SG::sgkey_t").Unwrap()->GetTypeAlias());
 
+   EXPECT_EQ("std::array<std::array<std::uint32_t,3>,2>",
+             RFieldBase::Create("f", "SG::sgkey_t[2][3]").Unwrap()->GetTypeName());
+   EXPECT_EQ("std::array<SG::sgkey_t,2>", RFieldBase::Create("f", "SG::sgkey_t[2]").Unwrap()->GetTypeAlias());
+   EXPECT_EQ("std::vector<std::array<double,2>>",
+             RFieldBase::Create("f", "vector<Double32_t[2]>").Unwrap()->GetTypeName());
+   EXPECT_EQ("std::vector<std::array<Double32_t,2>>",
+             RFieldBase::Create("f", "vector<Double32_t[2]>").Unwrap()->GetTypeAlias());
+
    const std::string innerCV = "class InnerCV<const int, const volatile int, volatile const int, volatile int>";
    const std::string normInnerCV =
       "InnerCV<const std::int32_t,const volatile std::int32_t,const volatile std::int32_t,volatile std::int32_t>";
@@ -84,6 +92,11 @@ TEST(RNTuple, TypeNameNormalizationById)
    EXPECT_EQ("std::unique_ptr<std::int32_t>", ROOT::Internal::GetRenormalizedTypeName(typeid(std::unique_ptr<int>)));
    EXPECT_EQ("std::optional<std::int32_t>", ROOT::Internal::GetRenormalizedTypeName(typeid(std::optional<int>)));
 
+   EXPECT_EQ("std::array<std::array<std::uint32_t,3>,2>",
+             ROOT::Internal::GetRenormalizedTypeName(typeid(SG::sgkey_t[2][3])));
+   EXPECT_EQ("std::vector<std::array<double,2>>",
+             ROOT::Internal::GetRenormalizedTypeName(typeid(std::vector<Double32_t[2]>)));
+
    EXPECT_EQ("std::set<std::int32_t>", ROOT::Internal::GetRenormalizedTypeName(typeid(std::set<int>)));
    EXPECT_EQ("std::unordered_set<std::int32_t>",
              ROOT::Internal::GetRenormalizedTypeName(typeid(std::unordered_set<int>)));