Fix #1544: Allow long signatures in names #1588
Conversation
Fixes scala#1544 by making the length field use 1 or 2 bytes, depending on the number of parameters in a signature.
odersky
changed the title
Blaisorblade
left a comment
Blaisorblade
left a comment
There was a problem hiding this comment.
Apart from the assertion this looks reasonable. (I can't reasonably review NameBuffer though).
| val length = currentAddr.index - lengthAddr.index - 1 | ||
| assert(length < 128) | ||
| putNat(lengthAddr, length, 1) | ||
| putNat(lengthAddr, length, lengthWidth) |
There was a problem hiding this comment.
My only question is: what happens when the (altered) assertion fails? There can't be bounds-checking, so assert(length < (1 << (7 * lengthWidth))) would be good. I assume you're sure this will never happen, and that you're currently right, that's why I propose an assert, which would trigger if NameBuffer gets out-of-sync and lengthWidth is 1 when it shouldn't. Or simply if somebody forgets to specify lengthWidth 😉.
There was a problem hiding this comment.
There's already an assertion in NameBuffer that we don't overflow, and which even comes with a comprehensible error message. So we are good.
Blaisorblade
left a comment
Blaisorblade
left a comment
There was a problem hiding this comment.
Ah, the assertion in TastyBuffer.putNat—you're right, thanks!
2 participants
Fixes #1544 by making the length field use 1 or 2 bytes,
depending on the number of parameters in a signature.
Review by @Blaisorblade ?