[ishaaaan17]

summary of changes

fixes #21274.

when an RNTuple collection proxy field (for example std::set or std::map) was created for a type that has no dictionary, ROOT hit an assertion failure in TGenCollectionProxy.cxx:401 (R__ASSERT(e->IsValid())), or failed while inspecting the collection proxy.

this change:

1. checks classp->HasDictionary() in the constructor of RProxiedCollectionField, before the collection proxy is generated, and throws a ROOT::RException with the name of the missing type ("<type> has no dictionary entry available").
2. adds a unit test in tree/ntuple/test/ntuple_types.cxx under TEST(RNTuple, TVirtualCollectionProxy). it checks that a collection type without a loaded dictionary (std::map<double, double>) throws a ROOT::RException with this error message.

checklist

• code style checked with .clang-format
• unit test added
• follows the minimal diff guidelines (12 lines added, 0 deleted)

ai disclosure

Assisted-by: antigravity:gemini. i used an ai assistant to follow the collection proxy generation call stack in RProxiedCollectionField and TGenCollectionProxy. i checked the dictionary lookup against TClass::HasDictionary(), wrote the unit test and the exception check, and checked the formatting with .clang-format.

[jblomer]

In principle looks good to me.

[pcanal]

This condition in itself might be too strict. In particular for the case of std::pair, we only need to have its 2 argument to have a dictionary (or be a pair).

Related, if emulated collection proxy are supported here, the underlying line type would not have a dictionary.

[pcanal]

Correcting myself, the test is not on the containee but on the container. It has the side effect (intentional or not) of prevent the use of emulated collection proxy.

[ishaaaan17]

thanks for the note @pcanal!

yes, it is intentional that RProxiedCollectionField does not allow emulated collection proxies. RProxiedCollectionField depends on compiled iterator function pointers (proxy->GetFunctionCreateIterators(), etc., in RCollectionIterableOnce::GetIteratorFuncs, line 1322), and these are not available for emulated collection proxies.

for on-disk collections that have no compiled dictionary, RNTuple reads them as untyped emulated vector fields (CreateEmulatedVectorField) and not through RProxiedCollectionField.

[pcanal]

iterator function pointers ... and these are not available for emulated collection proxies.

Note that this appears to be inaccurate:

root [0] auto proxy = TClass::GetClass("vector<TObject*>")->GetCollectionProxy()
(TVirtualCollectionProxy *) 0x703a78600
root [1] proxy->GetProperties() & TVirtualCollectionProxy::kIsEmulated
(int) 8
root [2] proxy->GetFunctionCreateIterators()
(TVirtualCollectionProxy::CreateIterators_t) Function @0x108a37508

[ishaaaan17]

thank you @pcanal for pointing this out and checking it! you are right, emulated collection proxies (TEmulatedCollectionProxy) do implement GetFunctionCreateIterators() and give working iterator function pointers.

this is actually why the earlier HasDictionary() check broke TEST(RNTupleEmulated, CollectionProxy). since emulated proxies do have valid iterators, rejecting types based on the container dictionary was too strict.

in the latest commit (a7666f1), i removed the HasDictionary() check completely. the check is now directly in RCollectionIterableOnce::GetIteratorFuncs, where it replaces the low-level R__ASSERT with a ROOT::RException, and it is thrown only when the iterator function pointers are really missing (if (!ifuncs.fCreateIterators || !ifuncs.fDeleteTwoIterators || !ifuncs.fNext)). emulated proxies with valid iterators keep working as before.

[github-actions]

Test Results

    24 files      24 suites   3d 11h 14m 40s ⏱️
 3 873 tests  3 869 ✅ 0 💤  4 ❌
76 342 runs  76 260 ✅ 0 💤 82 ❌

For more details on these failures, see this check.

Results for commit 721fe94.

[ishaaaan17]

following up on this @pcanal, you were right.

the CI showed that TEST(RNTupleEmulated, CollectionProxy) and rfield_class failed, because custom and emulated proxies (like StructWithCollectionProxyForEmuTest) are created in memory through RProxiedCollectionField without a container dictionary, but they have their own valid iterator function pointers.

so i changed the fix to check what we really depend on. instead of checking !classp->HasDictionary() on the container, i replaced the low-level R__ASSERT in GetIteratorFuncs (line 1322) with a RException that has a clear message, which is thrown when the iterator function pointers are missing.

this catches missing compiled iterators and does not affect emulated or custom proxies. the tests in ntuple-emulated, ntuple-field-name and rfield-class now pass again.

this catches missing compiled iterators and does not affect emulated or custom proxies.

@jblomer could you please trigger or approve the CI workflows for the updated commit? thanks!

[ishaaaan17]

hi @jblomer,

i checked the CI failures on gtest-tree-ntuple-ntuple-types. the failure came from the test assertion i had added (EXPECT_THROW(ROOT::RFieldBase::Create("noDictMap", "std::map<double, double>").Unwrap(), ROOT::RException);). in ROOT, std::map<double, double> already has a compiled dictionary and collection proxy, so it worked and did not throw.

i have removed that assertion, so now only the exception check in RFieldMeta.cxx is left. all the other 3,872 tests in the suite had already passed.

could you please re-trigger the CI workflows when you get time? thank you!

[jblomer]

Would still be nice to have a test for this. You can perhaps check the evolution tests and their forking mechanism for a blueprint.

[ishaaaan17]

@jblomer added the test, TEST(RNTupleEmulated, MissingCollectionProxyIterator) in tree/ntuple/test/ntuple_emulated.cxx. i used ExecInFork, like the other emulated collection proxy tests.

it defines a proxied collection type with no iterator functions and checks that RFieldBase::Create throws a ROOT::RException with "has no compiled iterator functions available". thanks!