Add support for enums with non-default size #17009
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This enables the StreamerInfoActions to enable the shortcuts that are possible in that case.
The version recorded 'here' is the version of the TStreamerInfo class not the user class
Even without schema evolution, the representation of a collection of enums on file is a `vector<int>` so `TStreamerElement::fType` and `TStreamerElement::fClassObject` should point to `vector<int>` while `TStreamerElement::fNewType` and `TStreamerElement::fNewClass` should point to the current in memory representaition `actualCollectionType< actualEnum >` that will know/remember what is the actual enum in memory representation.
Combine the code with the one use for the text actions
Combine the code with the one use for the text actions
To use be for case where the read and write implementation differs only slightly
To be used for function template that can be used for more than one actual looper (usually in conjunction with the LoopOverCollection template
Allow to share code with the collection loopers.
This required the addition of a concept that can be used to reduce boiler plate duplication. We now have an example where a generic function (Read/WriteStreamerLoop) is used with action with different signature by leveraging template arguments packs and a using statement declaration within the *Looper helper structs
Test Results 18 files 18 suites 4d 7h 39m 15s ⏱️ For more details on these failures, see this check. Results for commit 86262f0. ♻️ This comment has been updated with latest results. |
Using the new bit kBitsIsValid. This allows to both store the validity in the rootpcm file and also to execute IsValid on a TEnum read from a rootpcm without doing a Clang lookup
I.e. take into account the non-default size of an enum declaration
We split the reprensentation of a collection of enums between in memory representation and onfile representation (always `vector<int>`). Similarly for data member that enum types we set the in memory type (`NewType`) to be correct and keep the onfile type (`fType`) as it was (`Int_t`)
pcanal
force-pushed
the
master-non-defaults-enums
branch
from
a9b72fd
to
54ae470
Compare
pcanal
changed the title
|
@ktf This ready for further testing and will be merged once the reviews are done. |
|
Thanks. I will need a bit to digest what we actually need to do for this. I also assume you are going to backport this to 6-32? |
|
It is an unusually large/significant change to backport but if you need it in v6.32/34, we can indeed backport it. |
pcanal
force-pushed
the
master-non-defaults-enums
branch
from
4f1f306
to
0a54631
Compare
This automatically set the in-memory type (newType) of the TStreamerElement to the correct value. We just need to set the onfile type (fType/old type) to be Int_t
pcanal
force-pushed
the
master-non-defaults-enums
branch
from
0a54631
to
86262f0
Compare
|
Let me check how it goes with this and the HEAD on our ARM box and then we can discuss the backporting. @dpiparo can you remind me the timeline for v6-36? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR Is chained (includes all commit from) #16995.
The companion roottest PR is root-project/roottest#1226
It also requires an increase in the TStreamerInfo class version number (to mark which files/StreamerInfo have weird data for non-default size enum and which files are correct).
This fixes #16312. However files produced without this PR and with enum with a non default size will have data that is incorrectly saved but potential recoverable. The way the data is written will dependent on the size of the enum at the time of writing. Since this size is not recorded in the file, we can not have a general solution to read those corrupted files. However when that enum size used at write time is known, the techniques described here and here can be used to recover those weird files.
If reading both the corrupted files (using the above mentioned work-around) and the new files (correctly written with this PR) the recommendation is to increase the version number of the classes that contain an enum with non-default size so that the rules or custom streamer can distinguish the old and new files.