Deal with multi schema layout for a pair (caused enum underlying type)#10132
Deal with multi schema layout for a pair (caused enum underlying type)#10132pcanal merged 12 commits intoroot-project:masterfrom
Conversation
|
Starting build on |
In TGenCollectionProxy's init, hen needing to replace the TClass for std::pair by a 'better' version, make sure to import all the existing StreamerInfo. Also, make TClass::ForceReload public so that TGenCollectionProxy can use it.
If we don't have a dictionary for a std::pair and do not have (in the call to TClass::Init) the hints necessary to build it correctly (offset and size), then look for a related collection (STL associated container) that would induce the generation of the std::pair's TClass (it's collection proxy has the necessary information).
Because enums can change underlying type (especially since gathering the enum underlying info was inconsistent between TStreamerInfo::GenerateInfoForPair and TDataMember::Init, the schema layout for a std::pair *can* change ....
1635627 to
b556649
Compare
|
Starting build on |
|
Build failed on ROOT-debian10-i386/cxx14. Failing tests: |
|
Build failed on ROOT-performance-centos8-multicore/default. Failing tests: |
|
Build failed on ROOT-ubuntu16/nortcxxmod. Failing tests: |
|
Build failed on mac11/cxx17. Failing tests: |
|
Build failed on mac1015/python3. Failing tests: |
|
Build failed on windows10/cxx14. Failing tests: |
|
Build failed on ROOT-ubuntu2004/soversion. Failing tests: |
|
Starting build on |
|
Build failed on ROOT-performance-centos8-multicore/default. Warnings:
Failing tests: |
|
Build failed on mac11/cxx17. Failing tests: |
This follow the changes in root-project/root#10132
|
@phsft-bot build |
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/soversion. Warnings:
|
|
Build failed on ROOT-ubuntu16/nortcxxmod. Warnings:
|
|
Build failed on ROOT-performance-centos8-multicore/default. Warnings:
|
|
Build failed on ROOT-debian10-i386/cxx14. Warnings:
|
|
Build failed on mac1015/python3. Failing tests: |
Axel-Naumann
left a comment
There was a problem hiding this comment.
Do we have a test for this? And requesting my review you get a couple of questions for free :-)
| // In this case we initialised this TClass instance starting from the fwd declared state | ||
| // and we know we have no dictionary: no need to warn | ||
| ::Warning("TClass::Init", "no dictionary for class %s is available", fName.Data()); | ||
| const bool ispairbase = TClassEdit::IsStdPairBase(fName.Data()) && !IsFromRootCling(); |
There was a problem hiding this comment.
It is not immediately obvious to me why this needs && !IsFromRootCling(). Sorry. Maybe add a comment?
There was a problem hiding this comment.
This is explained on line 3079+. Basically in rootcling we should not interfere with explicit dictionary request.
| // We are in the case of an 'emulated' class. | ||
|
|
||
| if (fOnFileClassVersion >= 2) { | ||
| if (fOnFileClassVersion >= 2 && !isStdPair) { |
There was a problem hiding this comment.
What is fOnFileClassVersion for an existing pair?
There was a problem hiding this comment.
In the failing case it was 2.
Make sure that an old StreamerInfo transfered from an emulated TClass into a compiled or synthetic TClass is properly reset (correct TStreamerElement offfset and correct size).
cbaeedf to
9678405
Compare
|
Starting build on |
|
Build failed on windows10/cxx14. Failing tests:
|
|
Starting build on |
This follow the changes in root-project/root#10132
This fixes #10131.
The core issue is that TDataMember::Init and TStreamerInfo::GenerateInfoForPair were not consistent. TDataMember::Init was ignoring the underlying type of an enum while the newer TStreamerInfo::GenerateInfoForPair was taking it in consideration. In the reported case, it meant that some of the pair's TStreamerInfo recorded the type as being 'signed intwhile other was recording the type asunsigned int`.
In addition the whole infrastructure assumed (but only in "some/most" places) that the TStreamerInfo for a std::pair could never change and the infrastructure was also inconsistent on knowing whether the schema layout for std::pair is version of non-versioned.
NOTE: The last commit is might cause the user classes to require a version incrementing when using enums ... (i.e. this commit might be removed) ... it was indeed removing ... in addition changing the stored type of the enum changes the schema layout but does not (sometimes?) change (yet?) the checksum so it leads to incorrect reading of data ....