[NFC][ntuple] add clarifying comment in RVariantField #15721
Conversation
Test Results 11 files 11 suites 2d 7h 6m 41s ⏱️ Results for commit 0cf959e. |
| @@ -3281,6 +3281,8 @@ ROOT::Experimental::RVariantField::RVariantField(std::string_view fieldName, | |||
| fVariantOffset = dm->GetOffset(); | |||
|
|
|||
| const auto tagSize = GetVariantTagSize(); | |||
| // To calculate the padding, we use the tag size as a substitute for the tag alignment, i.e. the next line | |||
There was a problem hiding this comment.
If I understand correctly, we are making the assumption that for the tagtype alignment is (less or) equal to the size. The fact that the test passed on the CI shows that this assumption works on the platform we are testing on (are we testing Power10?) but it is iffy. Similarly the implementation of GetVariantTagSize has similar assumption (it even assumes that the tagtype is independent of the template arguments (I could imagine that to save space, an implementation would choose to use char for small number of argument and int for larger number of arguments or even possible a size_t (since it is the return type of index).
Do we already have a test that probes those assumptions (i.e. check with memcmp that the 1 is in the expect place for a few variant types? If/When we do, we should refer here to the tests (which should also have trivial check of size == alignment for the type we think is being used.
Follow-up of https://github.com/root-project/root/pull/15582/files#r1622902513