perl5
perl5 copied to clipboard
toke.c: S_intuit_more: Cleanup
"This is terrifying code"
At $work there was a story about hardware that collectively handled perhaps billions of phone calls a day. The interrupt handler was long spaghetti code which had a single comment /* Subtle */. Everyone was afraid to touch it, instead creating workarounds for its bugs. Eventually Management decided to spend the money to rewrite it.
I think this is similar to a portion of S_intuit_more in toke.c, as expressed in #16478. I have spent some hours going through the code, commenting it and adding white space to make it more legible. In the process, I discovered some wasted effort, and some missing things. This PR should make it less terrifying for people to work on
I did not analyze why the particular weight factors were chosen. And #16478 indicates this doesn't work as well as claimed
I'm a bit concerned that this isn't just cleanup, but also makes a variety of functional changes. Can we list all those in one place (perhaps in the ticket), so that we have the chance to more carefully consider which patterns will be affected by them, and thus what sort of breakage this might lead to in the wild?
I share this concern, and I think the functional changes should be a separate follow-on PR from the cleanup, so that we can confirm that the cleanup didn't break anything before we merge the changes.
Also, some tests demonstrating the changed behaviour would be nice.
Yes the cleanup should be split from any changes
But now that I understand it better, I see how brittle it is, and that some tests pass just because the arbitrarily chosen numbers happened to work.
One thing that I see missing is context. If this is called immediately when parsing $a[...]
it would be helpful for this routine to know that there exists a @a
which would lead the initial bias more towards a subscript
I'd like to know why we aren't getting more reports of failures from the field
The original pull request is now replaced by other commits which don't change the current behavior