Update alignment to handle terminator alignment #1601
Conversation
- Add support for including terminator MTA and length in `endingAlignmentApprox` calculations. - Introduce `terminatorMTAApprox` and `terminatorLengthApprox` for reusable terminator alignment logic. - Add new test cases (`test_sep_alignment_5`, `test_sep_alignment_6`) to validate terminator alignment in sequences. - Update comments DAFFODIL-3057
olabusayoT
requested review from
jadams-tresys,
mbeckerle and
stevedlawrence
stevedlawrence
left a comment
stevedlawrence
left a comment
There was a problem hiding this comment.
+1 Looks good, just minor cleanup/corrections
| LengthMultipleOf(t.knownEncodingAlignmentInBits) | ||
| case _ => LengthMultipleOf(0) |
There was a problem hiding this comment.
I think these want to be AlignmentMultipleOf since they represent an alignment and not a length. I imagine the math ends up the same. Though maybe the default case then becomes AlignmentMultipleOf(1)?
| } | ||
|
|
||
| private lazy val terminatorMTAApprox = | ||
| this match { |
There was a problem hiding this comment.
Can this match case be simplified to an if-statement?
| case t if t.hasTerminator => | ||
| getEncodingLengthApprox(t) | ||
| case _ => LengthMultipleOf(0) | ||
| } |
There was a problem hiding this comment.
Same here, can this be simplified to an if-statement?
| + trailingSkipApprox | ||
| ) | ||
| res | ||
| } else |
There was a problem hiding this comment.
Instead of adding the same three fields to a bunch of places in the various conditions can we do something like
val contentEndAlignment = ... //existing logic
val res =
contentEndAlignment
+ terminatorMTAAPprox
+ terminatorLengthApprox
+ trailingSkipApproxI'm not sure if contentEndAlignment is the right name, but something along those lines?
- MTA are alignments not lengths - simplify pattern matching to if statement - refactor duplicate addition of terminator + trailing skip DAFFODIL-2295
|
Subsumed by #1604 |
2 participants
endingAlignmentApproxcalculations.terminatorMTAApproxandterminatorLengthApproxfor reusable terminator alignment logic.test_sep_alignment_5,test_sep_alignment_6) to validate terminator alignment in sequences.DAFFODIL-3057