Fix padding of compound types #581
Conversation
| (((member_size) >= (current_size)) ? (((member_size) - (current_size)) % (member_size)) \ | ||
| : ((((member_size) - (((current_size) - (member_size)) % (member_size)))) % (member_size))) |
There was a problem hiding this comment.
I don't know if I'm able to explain it 🤪
Codecov Report
@@ Coverage Diff @@
## master #581 +/- ##
=========================================
Coverage ? 80.38%
=========================================
Files ? 66
Lines ? 3539
Branches ? 0
=========================================
Hits ? 2845
Misses ? 694
Partials ? 0 Continue to review full report at Codecov.
|
| (((member_size) >= (current_size)) \ | ||
| ? (((member_size) - (current_size)) % (member_size)) \ | ||
| : ((((member_size) - (((current_size) - (member_size)) % (member_size)))) % \ | ||
| (member_size))) |
There was a problem hiding this comment.
My brain can't handle these parenthesis! Worse than Lisp… do we really need all the moduli?
There was a problem hiding this comment.
In fact, it seems that yes.
What we want is:
(member_size-current_size) % member_size
if member_size is smaller than current-size it does not matter we will got (in math) a positive modulo.
But in C++ it gives a negative one (more over we use size_t).
So we compute the opposite modulo:
m = (current_size-member_size) % member_size
After we only need:
member_size - m
But if m == 0, the result should be 0 and not member_size so, a new modulo.
Example:
member_size = 8
current_size = 16
(current_size - member_size) % member_size = 0
member_size - 0 = 8
8 % 8 = 0
There was a problem hiding this comment.
I think this would benefit from a clear description of what it's trying to achieve. Examples of question are:
- align to which boundary?
- what is
current_sizewhat ismember_size(which one corresponds to the 'element` in the comment)?
Then, since none of us seem particularly confident about our integer arithmetic, it might be good to check all of this in unit-tests, covering all corner cases.
There was a problem hiding this comment.
Thank you for the rundown, @alkino 🙏
| (((member_size) >= (current_size)) \ | ||
| ? (((member_size) - (current_size)) % (member_size)) \ | ||
| : ((((member_size) - (((current_size) - (member_size)) % (member_size)))) % \ | ||
| (member_size))) |
There was a problem hiding this comment.
Thank you for the rundown, @alkino 🙏
It seems that it can fix everything and staying > 0 in the same time.