TProtoClass: do not complain innards of transients. #16202
Merged
Conversation
This file contains hidden or 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
Test Results 18 files 18 suites 4d 1h 30m 43s ⏱️ Results for commit bea2dd7. ♻️ This comment has been updated with latest results. |
dpiparo
changed the title
dpiparo
reviewed
Aug 9, 2024
Member
Author
|
The 2 failures are pre-exisiting (Mac-Beta due to XCode upgrade and TMapFile random behavior). |
dpiparo
requested changes
Aug 15, 2024
Member
dpiparo
left a comment
dpiparo
left a comment
There was a problem hiding this comment.
This PR is very good, thanks for preparing it. I highlighted some opportunities for improvement as well as clarification in form of comments. Addressing all of them should not take much.
Do we have test coverage for the change? For example a test where before there was output that after the PR disappear?
Do not complain about missing information about data members that directly or indirectly within a transient members of the top level class in GetRealData. This solves the underlying problem from root-project#15733
We do want to record as transient a real data member that is inside a nested object that is itself marked as transient. This is particular useful to know when to warn about missing information and when not. Previously only the data member explicitly marked as transient were marked as transient in the RealData. Now we also mark the member nested inside a transient member. This will prevent spurrious warning about missing information about the inner content of a transient data member. However we can not rely on when the `isTransient` parameter to `TBuildRealData::Inspect` is set to true because it is set both in the case where the object is really nested and the real data is being added to the current class (what we want) and in the case where we want to setup a related class but the real data will not be added to the current class (eg. collection or pointer to a class).
When/if a TClass for a std::tuple is first loaded in forward declared state and then the interpreter information is loaded, we now properly replace the ClassInfo and StreamerInfo
Contributor
|
@pcanal, the failures are real. What is the status of this PR? |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In
GetRealData, do not complain about missing information about data members that are directly or indirectly within a transient member of the top level class.This solves the underlying problem from #15733