[ntuple] fix type names with [U]Long64_t template args

tree/ntuple/inc/ROOT/RField.hxx

@@ -272,7 +272,7 @@ protected:
    void ReconcileOnDiskField(const RNTupleDescriptor &desc) final;
 
 public:
-   RStreamerField(std::string_view fieldName, std::string_view className, std::string_view typeAlias = "");
+   RStreamerField(std::string_view fieldName, std::string_view className);
    RStreamerField(RStreamerField &&other) = default;
    RStreamerField &operator=(RStreamerField &&other) = default;
    ~RStreamerField() final = default;

tree/ntuple/inc/ROOT/RField/RFieldProxiedCollection.hxx

@@ -255,7 +255,7 @@ template <typename T>
 class RField<T, typename std::enable_if<IsCollectionProxy<T>::value>::type> final : public RProxiedCollectionField {
 public:
    static std::string TypeName() { return ROOT::Internal::GetRenormalizedTypeName(typeid(T)); }
-   RField(std::string_view name) : RProxiedCollectionField(name, TypeName())
+   RField(std::string_view name) : RProxiedCollectionField(name, Internal::GetDemangledTypeName(typeid(T)))
    {
       static_assert(std::is_class<T>::value, "collection proxy unsupported for fundamental types");
    }

tree/ntuple/inc/ROOT/RFieldUtils.hxx

@@ -35,6 +35,14 @@ std::string GetRenormalizedTypeName(const std::string &metaNormalizedName);
 /// ensure that e.g. fundamental types are normalized to the type used by RNTuple (e.g. int -> std::int32_t).
 std::string GetRenormalizedTypeName(const std::type_info &ti);
 
+/// Checks if the meta normalized name is different from the RNTuple normalized name in a way that would cause
+/// the RNTuple normalized name to request different streamer info. This can happen, e.g., if the type name has
+/// Long64_t as a template parameter. In this case, RNTuple should use the meta normalized name as a type alias
+/// to ensure correct reconstruction of objects from disk.
+/// If the function returns true, renormalizedAlias contains the RNTuple normalized name that should be used as
+/// type alias.
+bool NeedsMetaNameAsAlias(const std::string &metaNormalizedName, std::string &renormalizedAlias);
+
 /// Applies all RNTuple type normalization rules except typedef resolution.
 std::string GetNormalizedUnresolvedTypeName(const std::string &origName);
 

tree/ntuple/src/RFieldMeta.cxx

@@ -158,6 +158,10 @@ ROOT::RClassField::RClassField(std::string_view fieldName, TClass *classp)
    if (!(fClass->ClassProperty() & kClassHasExplicitDtor))
       fTraits |= kTraitTriviallyDestructible;
 
+   std::string renormalizedAlias;
+   if (Internal::NeedsMetaNameAsAlias(classp->GetName(), renormalizedAlias))
+      fTypeAlias = renormalizedAlias;
+
    int i = 0;
    const auto *bases = fClass->GetListOfBases();
    assert(bases);
@@ -744,7 +748,7 @@ ROOT::RProxiedCollectionField::RProxiedCollectionField(std::string_view fieldNam
      fNWritten(0)
 {
    if (!classp->GetCollectionProxy())
-      throw RException(R__FAIL(std::string(GetTypeName()) + " has no associated collection proxy"));
+      throw RException(R__FAIL(std::string(classp->GetName()) + " has no associated collection proxy"));
    if (classp->Property() & kIsDefinedInStd) {
       static const std::vector<std::string> supportedStdTypes = {
          "std::set<", "std::unordered_set<", "std::multiset<", "std::unordered_multiset<",
@@ -760,6 +764,10 @@ ROOT::RProxiedCollectionField::RProxiedCollectionField(std::string_view fieldNam
          throw RException(R__FAIL(std::string(GetTypeName()) + " is not supported"));
    }
 
+   std::string renormalizedAlias;
+   if (Internal::NeedsMetaNameAsAlias(classp->GetName(), renormalizedAlias))
+      fTypeAlias = renormalizedAlias;
+
    fProxy.reset(classp->GetCollectionProxy()->Generate());
    fProperties = fProxy->GetProperties();
    fCollectionType = fProxy->GetCollectionType();
@@ -797,7 +805,7 @@ ROOT::RProxiedCollectionField::RProxiedCollectionField(std::string_view fieldNam
       case EDataType::kFloat_t: itemField = std::make_unique<RField<Float_t>>("_0"); break;
       case EDataType::kDouble_t: itemField = std::make_unique<RField<Double_t>>("_0"); break;
       case EDataType::kBool_t: itemField = std::make_unique<RField<Bool_t>>("_0"); break;
-      default: throw RException(R__FAIL("unsupported value type"));
+      default: throw RException(R__FAIL("unsupported value type: " + std::to_string(fProxy->GetType())));
       }
    }
 
@@ -1002,10 +1010,9 @@ class TBufferRecStreamer : public TBufferFile {
 
 } // anonymous namespace
 
-ROOT::RStreamerField::RStreamerField(std::string_view fieldName, std::string_view className, std::string_view typeAlias)
+ROOT::RStreamerField::RStreamerField(std::string_view fieldName, std::string_view className)
    : RStreamerField(fieldName, EnsureValidClass(className))
 {
-   fTypeAlias = typeAlias;
 }
 
 ROOT::RStreamerField::RStreamerField(std::string_view fieldName, TClass *classp)
@@ -1014,6 +1021,10 @@ ROOT::RStreamerField::RStreamerField(std::string_view fieldName, TClass *classp)
      fClass(classp),
      fIndex(0)
 {
+   std::string renormalizedAlias;
+   if (Internal::NeedsMetaNameAsAlias(classp->GetName(), renormalizedAlias))
+      fTypeAlias = renormalizedAlias;
+
    fTraits |= kTraitTypeChecksum;
    // For RClassField, we only check for explicit constructors and destructors and then recursively combine traits from
    // all member subfields. For RStreamerField, we treat the class as a black box and additionally need to check for
@@ -1026,7 +1037,7 @@ ROOT::RStreamerField::RStreamerField(std::string_view fieldName, TClass *classp)
 
 std::unique_ptr<ROOT::RFieldBase> ROOT::RStreamerField::CloneImpl(std::string_view newName) const
 {
-   return std::unique_ptr<RStreamerField>(new RStreamerField(newName, GetTypeName(), GetTypeAlias()));
+   return std::unique_ptr<RStreamerField>(new RStreamerField(newName, GetTypeName()));
 }
 
 std::size_t ROOT::RStreamerField::AppendImpl(const void *from)

tree/ntuple/src/RFieldUtils.cxx

@@ -62,7 +62,8 @@ const std::unordered_map<std::string_view, std::string_view> typeTranslationMap{
    {"ULong64_t", "unsigned long long"},
    {"uint64_t", "std::uint64_t"}};
 
-// Natively supported types drop the default template arguments and the CV qualifiers in template arguments.
+// Natively supported types drop the default template arguments and the CV qualifiers in template arguments;
+// the also keep Long64_t as template argument untouched.
 bool IsUserClass(const std::string &typeName)
 {
    return typeName.rfind("std::", 0) != 0 && typeName.rfind("ROOT::VecOps::RVec<", 0) != 0;
@@ -531,6 +532,39 @@ std::string ROOT::Internal::GetRenormalizedTypeName(const std::string &metaNorma
    return GetRenormalizedMetaTypeName(metaNormalizedName);
 }
 
+bool ROOT::Internal::NeedsMetaNameAsAlias(const std::string &metaNormalizedName, std::string &renormalizedAlias)
+{
+   const auto canonicalTypePrefix = ROOT::Internal::GetCanonicalTypePrefix(metaNormalizedName);
+   if (canonicalTypePrefix.find('<') == std::string::npos) {
+      // If there are no templates, the function is done.
+      return false;
+   }
+
+   bool result = false;
+   bool isTemplatedUserClass = IsUserClass(canonicalTypePrefix);
+   auto fnCheckLong64 = [&](const std::string &arg) -> std::string {
+      if ((arg == "Long64_t" || arg == "ULong64_t") && isTemplatedUserClass) {
+         result = true;
+         return arg;
+      }
+
+      std::string renormalizedArgAlias;
+      if (NeedsMetaNameAsAlias(arg, renormalizedArgAlias)) {
+         result = true;
+         return renormalizedArgAlias;
+      }
+
+      const auto renormalizedArg = GetRenormalizedMetaTypeName(arg);
+      isTemplatedUserClass = (renormalizedArg.find('<') == std::string::npos) && IsUserClass(renormalizedArg);
+      return renormalizedArg;
+   };
+
+   renormalizedAlias = canonicalTypePrefix;
+   NormalizeTemplateArguments(renormalizedAlias, 0 /* maxTemplateArgs */, fnCheckLong64);
+
+   return result;
+}
+
 std::string ROOT::Internal::GetNormalizedUnresolvedTypeName(const std::string &origName)
 {
    const TClassEdit::EModType modType = static_cast<TClassEdit::EModType>(

tree/ntuple/test/CustomStruct.hxx

@@ -100,14 +100,19 @@ struct alignas(std::uint64_t) TestEBO : public EmptyStruct {
 template <typename T>
 class EdmWrapper {
 public:
+   struct Inner {
+      T fX;
+   };
+
    bool fIsPresent = true;
    T fMember;
 };
 
 class EdmContainer {
 public:
+   using EdmWrapperLong64_t = EdmWrapper<long long>;
    // Used to test that the streamer info for fWrapper will use long long
-   EdmWrapper<long long> fWrapper;
+   EdmWrapperLong64_t fWrapper;
 };
 
 template <typename T>

tree/ntuple/test/CustomStructLinkDef.h

@@ -33,7 +33,7 @@
 #pragma link C++ class EdmWrapper<CustomStruct> +;
 #pragma link C++ class EdmHash < 1> + ;
 #pragma link C++ class EdmWrapper<long long>+;
-#pragma link C++ class EdmContainer;
+#pragma link C++ class EdmContainer+;
 
 #pragma link C++ class DataVector < int, double> + ;
 #pragma link C++ class DataVector < int, float> + ;
@@ -67,6 +67,7 @@
 #pragma link C++ class StructUsingCollectionProxy<CustomStruct> + ;
 #pragma link C++ class StructUsingCollectionProxy<StructUsingCollectionProxy<float>> + ;
 #pragma link C++ class StructUsingCollectionProxy<int> + ;
+#pragma link C++ class StructUsingCollectionProxy<Long64_t>+;
 
 #pragma link C++ class TrivialTraitsBase + ;
 #pragma link C++ class TrivialTraits + ;

tree/ntuple/test/SimpleCollectionProxy.hxx

@@ -72,6 +72,8 @@ public:
          return EDataType::kInt_t;
       if constexpr (std::is_same<typename CollectionT::ValueType, float>::value)
          return EDataType::kFloat_t;
+      if constexpr (std::is_same<typename CollectionT::ValueType, Long64_t>::value)
+         return EDataType::kLong64_t;
       return EDataType::kOther_t;
    }
 
@@ -122,8 +124,9 @@ template <>
 struct IsCollectionProxy<StructUsingCollectionProxy<float>> : std::true_type {
 };
 template <>
-struct IsCollectionProxy<StructUsingCollectionProxy<CustomStruct>> : std::true_type {
-};
+struct IsCollectionProxy<StructUsingCollectionProxy<Long64_t>> : std::true_type {};
+template <>
+struct IsCollectionProxy<StructUsingCollectionProxy<CustomStruct>> : std::true_type {};
 
 template <>
 struct IsCollectionProxy<StructUsingCollectionProxy<StructUsingCollectionProxy<float>>> : std::true_type {

tree/ntuple/test/ntuple_type_name.cxx

@@ -380,3 +380,19 @@ TEST(RNTuple, ContextDependentTypeNames)
       }
    }
 }
+
+TEST(RNTuple, NeedsMetaNameAsAlias)
+{
+   using ROOT::Internal::NeedsMetaNameAsAlias;
+
+   std::string renormalizedAlias;
+   EXPECT_FALSE(NeedsMetaNameAsAlias("bool", renormalizedAlias));
+   EXPECT_FALSE(NeedsMetaNameAsAlias("std::vector<long>", renormalizedAlias));
+   EXPECT_FALSE(NeedsMetaNameAsAlias("std::vector<Long64_t>", renormalizedAlias));
+   EXPECT_TRUE(NeedsMetaNameAsAlias("::MyClass<Long64_t>", renormalizedAlias));
+   EXPECT_EQ("MyClass<Long64_t>", renormalizedAlias);
+   EXPECT_TRUE(NeedsMetaNameAsAlias("MyClass<ULong64_t>", renormalizedAlias));
+   EXPECT_TRUE(NeedsMetaNameAsAlias("std::vector<MyClass<Long64_t> >", renormalizedAlias));
+   EXPECT_EQ("std::vector<MyClass<Long64_t>>", renormalizedAlias);
+   EXPECT_FALSE(NeedsMetaNameAsAlias("MyClass<ROOT::RVec<Long64_t>>", renormalizedAlias));
+}

tree/ntuple/test/rfield_class.cxx

@@ -1,4 +1,5 @@
 #include "ntuple_test.hxx"
+#include "SimpleCollectionProxy.hxx"
 
 #include <TMath.h>
 #include <TObject.h>
@@ -391,41 +392,56 @@ TEST(RNTuple, TClassMetaName)
    auto f4 = RFieldBase::Create("f", "EdmContainer").Unwrap();
    EXPECT_STREQ("EdmWrapper<Long64_t>",
                 static_cast<const ROOT::RClassField *>(f4->GetConstSubfields()[0])->GetClass()->GetName());
+
+   SimpleCollectionProxy<StructUsingCollectionProxy<Long64_t>> proxy;
+   auto cl = TClass::GetClass("StructUsingCollectionProxy<Long64_t>");
+   cl->CopyCollectionProxy(proxy);
+
+   auto f5 = std::make_unique<ROOT::RField<StructUsingCollectionProxy<Long64_t>>>("f");
+   EXPECT_TRUE(dynamic_cast<ROOT::RProxiedCollectionField *>(f5.get()));
+   EXPECT_EQ("StructUsingCollectionProxy<Long64_t>", f5->GetTypeAlias());
+
+   ROOT::RStreamerField f6("f", "EdmWrapper<long long>");
+   EXPECT_STREQ("EdmWrapper<Long64_t>", f6.GetClass()->GetName());
+   EXPECT_EQ("EdmWrapper<Long64_t>", f6.GetTypeAlias());
 }
 
 TEST(RNTuple, StreamerInfoRecords)
 {
-   // Every testee consists of the type stored on disk and the expected streamer info records
-   std::vector<std::pair<std::string, std::vector<std::string>>> testees{
-      {"float", {}},
-      {"std::vector<float>", {}},
-      {"std::pair<float, float>", {}},
-      {"std::map<int, float>", {}},
-      {"CustomStruct", {"CustomStruct"}},
-      {"std::vector<CustomStruct>", {"CustomStruct"}},
-      {"std::map<int, CustomStruct>", {"CustomStruct"}},
-      {"DerivedA", {"DerivedA", "CustomStruct"}},
-      {"std::pair<CustomStruct, DerivedA>", {"DerivedA", "CustomStruct"}},
-      {"EdmWrapper<long long>", {"EdmWrapper<Long64_t>"}},
-      {"TRotation", {"TRotation"}}};
+   // Every testee consists of the type stored on disk, the expected streamer info records, and the expected type alias
+   std::vector<std::tuple<std::string, std::vector<std::string>, std::string>> testees{
+      {"float", {}, ""},
+      {"std::vector<float>", {}, ""},
+      {"std::pair<float, float>", {}, ""},
+      {"std::map<int, float>", {}, ""},
+      {"CustomStruct", {"CustomStruct"}, ""},
+      {"std::vector<CustomStruct>", {"CustomStruct"}, ""},
+      {"std::map<int, CustomStruct>", {"CustomStruct"}, ""},
+      {"DerivedA", {"DerivedA", "CustomStruct"}, ""},
+      {"std::pair<CustomStruct, DerivedA>", {"DerivedA", "CustomStruct"}, ""},
+      {"EdmWrapper<long long>", {"EdmWrapper<Long64_t>"}, "EdmWrapper<Long64_t>"},
+      {"EdmContainer", {"EdmContainer", "EdmWrapper<Long64_t>"}, ""},
+      {"EdmWrapper<long long>::Inner", {"EdmWrapper<Long64_t>::Inner"}, "EdmWrapper<Long64_t>::Inner"},
+      {"EdmContainer::EdmWrapperLong64_t", {"EdmWrapper<Long64_t>"}, "EdmContainer::EdmWrapperLong64_t"},
+      {"TRotation", {"TRotation"}, ""}};
 
    for (const auto &t : testees) {
       FileRaii fileGuard("test_ntuple_streamer_info_records.root");
 
       {
          auto model = ROOT::RNTupleModel::Create();
-         if (t.first == "TRotation") {
-            model->AddField(std::make_unique<ROOT::RStreamerField>("f", t.first));
+         if (std::get<0>(t) == "TRotation") {
+            model->AddField(std::make_unique<ROOT::RStreamerField>("f", std::get<0>(t)));
          } else {
-            model->AddField(ROOT::RFieldBase::Create("f", t.first).Unwrap());
+            model->AddField(ROOT::RFieldBase::Create("f", std::get<0>(t)).Unwrap());
          }
          auto writer = ROOT::RNTupleWriter::Recreate(std::move(model), "ntpl", fileGuard.GetPath());
       }
 
       auto f = std::unique_ptr<TFile>(TFile::Open(fileGuard.GetPath().c_str()));
       ASSERT_TRUE(f && !f->IsZombie());
 
-      std::unordered_set<std::string> expectedInfos{t.second.begin(), t.second.end()};
+      std::unordered_set<std::string> expectedInfos{std::get<1>(t).begin(), std::get<1>(t).end()};
       expectedInfos.insert("ROOT::RNTuple");
       for (const auto info : TRangeDynCast<TVirtualStreamerInfo>(*f->GetStreamerInfoList())) {
          auto itr = expectedInfos.find(info->GetName());
@@ -436,5 +452,12 @@ TEST(RNTuple, StreamerInfoRecords)
          expectedInfos.erase(itr);
       }
       EXPECT_TRUE(expectedInfos.empty());
+
+      // Make sure we can reconstruct the fields
+      auto reader = RNTupleReader::Open("ntpl", fileGuard.GetPath());
+      EXPECT_EQ(std::get<2>(t), reader->GetModel().GetConstField("f").GetTypeAlias());
+      if (auto field = dynamic_cast<const ROOT::RClassField *>(&reader->GetModel().GetConstField("f"))) {
+         EXPECT_EQ(std::get<1>(t)[0], field->GetClass()->GetName());
+      }
    }
 }