[ntuple] RClassField: add support to load/store class hierarchies#8552
Conversation
|
Starting build on |
|
Build failed on mac11.0/cxx17. Errors:
|
|
Build failed on ROOT-debian10-i386/cxx14. Errors:
|
|
Build failed on windows10/cxx14. |
jalopezg-git
force-pushed
the
ntuple-tclass-hierarchy
branch
from
7666f9b to
0f23836
Compare
|
Starting build on |
jalopezg-git
force-pushed
the
ntuple-tclass-hierarchy
branch
from
0f23836 to
bc8e03b
Compare
|
Starting build on |
|
Build failed on ROOT-debian10-i386/cxx14. Failing tests: |
jblomer
left a comment
jblomer
left a comment
There was a problem hiding this comment.
Very nice! Let's add a few more tests to that:
- Test skipping non-persistent members
- Test the most complicated (artificial) example for a class hierarchy, e.g. diamond inheritance and members that have types of the inherited classes
- Test a class with a non-trivial constructor (we probably have to think about construction more carefully)
- Trying to read/write polymorphic classes: perhaps we can detect with TClass if we have a derived class in our hands. In which case... I guess we want to warn? Or silently slice?
|
Starting build on |
|
Build failed on ROOT-debian10-i386/cxx14. Errors:
|
|
Build failed on ROOT-performance-centos8-multicore/default. Errors:
|
|
Build failed on windows10/cxx14. Errors:
|
|
Build failed on mac11.0/cxx17. Errors:
|
|
Build failed on ROOT-ubuntu16/nortcxxmod. Errors:
|
|
Build failed on mac1014/python3. Errors:
|
jalopezg-git
force-pushed
the
ntuple-tclass-hierarchy
branch
from
ef435b4 to
97c7b5d
Compare
|
Starting build on |
|
Build failed on mac11.0/cxx17. Failing tests: |
Improve support of storage of arbitrary types via TClass by handling also members inherited from base classes. A subfield named `:XXX` is created for each inherited XXX class. As per a discussion in a previous I/O meeting, always use `CaptureValue()` in `ReadGlobalImpl()` (so that reading a new entry is roughly equal to copy assignment).
… classes Use `Detail::RFieldBase::Create()` instead, as base classes might not be handled by `RClassField`, e.g. we might have `std::vector<T>` as a base class.
…hierarchies via TClass.
Test storage of types that inherit from a templated base class, e.g.
```
template <class T>
struct Foobar : public std::vector<T> {
// ...
};
```
jalopezg-git
force-pushed
the
ntuple-tclass-hierarchy
branch
from
97c7b5d to
10a3e0b
Compare
|
Starting build on |
Done. See check in test
Done. Also, I have added a test for types inheriting from a templated class, based on the mails exchanged with Marcin Nowak.
Currently, we throw -see test |
|
Build failed on mac11.0/cxx17. Failing tests: |
|
|
||
| for (auto baseClass : ROOT::Detail::TRangeStaticCast<TBaseClass>(*fClass->GetListOfBases())) { | ||
| TClass *c = baseClass->GetClassPointer(); | ||
| auto subField = Detail::RFieldBase::Create(std::string(kPrefixInherited) + c->GetName(), |
There was a problem hiding this comment.
I think for the moment it's fine to use the type name as field name. Together with the v1 serialization, I'd like to go through the RField classes and change the use of type names altogether to generic names (e.g. R_0, R_1, etc.). This way, field names are valid C++ identifiers (except for the : of base classes).
4 participants
This pull-request extends the support in
RClassFieldto load/store arbitrary types by correctly handling inheritance. See below for the list of changes.Changes or fixes:
:XXXis created for each inheritedXXXclass; additional fields are recursively generated for data members/base classes.//!, are not stored.This PR closes issue #7856.