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
We need to go with a specific action that hold the sub sequence (sequence for a base class) rather than insert of the action directly into the main sequence as (currently) the splicing of the action sequences needed to implemenent the split streaming in TTree relies on the order/index of the elements
Test Results 18 files 18 suites 4d 15h 40m 1s ⏱️ Results for commit 54ae470. ♻️ 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. |
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.