[ntuple] ensure whole-class-rules are executed last

root-project · Feb 1, 2025 · d7b1704 · d7b1704
1 parent 05a76ae
commit d7b1704
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 21 deletions.
diff --git a/tree/ntuple/v7/src/RFieldMeta.cxx b/tree/ntuple/v7/src/RFieldMeta.cxx
@@ -171,32 +171,45 @@ std::vector<const ROOT::TSchemaRule *> ROOT::Experimental::RClassField::FindRule
       }
    }
 
-   // For the time being, we only support rules targeting transient members
-   auto targetsNonTransientMembers = [classp = fClass](const ROOT::TSchemaRule *rule) {
-      if (rule->GetTarget() == nullptr)
-         return false;
+   // Cleanup and sort rules
+   std::size_t nskip = 0; // skip whole-object-rules that were moved to the end of the rules vector
+   for (auto itr = rules.begin(); itr != rules.end() - nskip; ) {
+      const auto rule = *itr;
+
+      // Erase unknown rule types
+      if (rule->GetRuleType() != ROOT::TSchemaRule::kReadRule) {
+         R__LOG_WARNING(NTupleLog()) << "ignoring I/O customization rule with unsupported type: "
+                                     << rule->GetRuleType();
+         itr = rules.erase(itr);
+         continue;
+      }
+
+      // Rules targeting the entire object need to be executed at the end
+      if (rule->GetTarget() == nullptr) {
+         nskip++;
+         if (itr != rules.end() - nskip)
+            std::iter_swap(itr++, rules.end() - nskip);
+         continue;
+      }
+
+      // For the time being, we only support rules targeting transient members
+      bool hasPersistentTarget = false;
       for (auto target : ROOT::Detail::TRangeStaticCast<TObjString>(*rule->GetTarget())) {
-         const auto dataMember = classp->GetDataMember(target->GetString());
+         const auto dataMember = fClass->GetDataMember(target->GetString());
          if (!dataMember || dataMember->IsPersistent()) {
             R__LOG_WARNING(NTupleLog()) << "ignoring I/O customization rule with non-transient member: "
                                         << dataMember->GetName();
-            return true;
+            hasPersistentTarget = true;
+            break;
          }
       }
-      return false;
-   };
-   rules.erase(std::remove_if(rules.begin(), rules.end(), targetsNonTransientMembers), rules.end());
-
-   // Erase unknown rule types
-   auto hasUnknownType = [](const ROOT::TSchemaRule *rule) {
-      if (rule->GetRuleType() != ROOT::TSchemaRule::kReadRule) {
-         R__LOG_WARNING(NTupleLog()) << "ignoring I/O customization rule with unsupported type: "
-                                     << rule->GetRuleType();
-         return true;
+      if (hasPersistentTarget) {
+         itr = rules.erase(itr);
+         continue;
       }
-      return false;
-   };
-   rules.erase(std::remove_if(rules.begin(), rules.end(), hasUnknownType), rules.end());
+
+      ++itr;
+   }
 
    return rules;
 }

diff --git a/tree/ntuple/v7/test/CustomStruct.hxx b/tree/ntuple/v7/test/CustomStruct.hxx
@@ -204,6 +204,7 @@ struct StructWithTransientString {
 struct StructWithIORules : StructWithIORulesBase {
    StructWithTransientString s;
    float c = 0.0f; //! transient member
+   float cDerived = 0.0f; //! should become 2*c after rules for c applied
    float checksumA = 0.0f;   //! transient member, edited by checksum based rule
    float checksumB = 137.0f; //! transient member, skipped by checksum based rule due to checksum mismatch
 

diff --git a/tree/ntuple/v7/test/CustomStructLinkDef.h b/tree/ntuple/v7/test/CustomStructLinkDef.h
@@ -64,6 +64,10 @@
 #pragma read sourceClass = "StructWithTransientString" source = "char chars[4]" version = "[1-]" targetClass = \
    "StructWithTransientString" target = "str" include = "string" code = "{ str = std::string{onfile.chars, 4}; }"
 
+// Whole object rule (without target member) listed first but should be executed last
+#pragma read sourceClass = "StructWithIORules" version = "[1-]" targetClass = "StructWithIORules" \
+   source = "" target = "" code = "{ newObj->cDerived = 2 * newObj->c; }"
+
 #pragma read sourceClass = "StructWithIORules" source = "float a" version = "[1-]" targetClass = \
    "StructWithIORules" target = "c" code = "{ c = onfile.a + newObj->b; }"
 

diff --git a/tree/ntuple/v7/test/rfield_class.cxx b/tree/ntuple/v7/test/rfield_class.cxx
@@ -257,8 +257,9 @@ TEST(RNTuple, TClassReadRules)
       EXPECT_TRUE(0 == memcmp(c, viewKlass(i).s.chars, sizeof(c)));
 
       // The following values are set from a read rule; see CustomStructLinkDef.h
-      EXPECT_EQ(fi + 1.0f, viewKlass(i).b);
-      EXPECT_EQ(viewKlass(i).a + viewKlass(i).b, viewKlass(i).c);
+      EXPECT_FLOAT_EQ(fi + 1.0f, viewKlass(i).b);
+      EXPECT_FLOAT_EQ(viewKlass(i).a + viewKlass(i).b, viewKlass(i).c);
+      EXPECT_FLOAT_EQ(2 * (viewKlass(i).a + viewKlass(i).b), viewKlass(i).cDerived);
       EXPECT_STREQ("ROOT", viewKlass(i).s.str.c_str());
 
       // The following member is set by a checksum based rule