Remove variants of METHODtype tasty tag #8624
Conversation
bishabosha
added
the
area:tasty-format
bishabosha
force-pushed
the
tasty/single-methodtype-tag
branch
from
8881361
to
4088531
Compare
There was a problem hiding this comment.
I think we should strive to make the code here as lean as possible and the meaning as clear as possible. TreeUnpickler is a core piece of language infrastructure. It is already very long. So any line of code we add has to do something essential in the clearest possible way. That's why I was more picky than usual in my review. 😉
|
The aim of the refactoring is to be a simplification. If we end up with more lines than before, we should ask ourselves whether we missed that aim. |
bishabosha
force-pushed
the
tasty/single-methodtype-tag
branch
from
4088531
to
fde9672
Compare
| pt => registeringType(pt, paramReader.readParamTypes[PInfo](end)), | ||
| val (paramNames, mods) = nameReader.readParamNamesAndMods(end) | ||
| companionOp(mods)(paramNames.map(nameMap))( | ||
| pt => registeringType(pt, paramReader.readParamTypes[PInfo](paramNames.length)), |
There was a problem hiding this comment.
I was potentially concerned about traversing the paramNames to get a length versus comparing to an end address, but I guess it is insignificant compared to the logic to traverse types
There was a problem hiding this comment.
Yes, indeed. Generally, we assume that length is free since most lists are small.
bishabosha
force-pushed
the
tasty/single-methodtype-tag
branch
from
fde9672
to
0145620
Compare
|
Thank you for the very detailed review, I think it now seems a lot cleaner, and I did forget to change the grammar documentation |
bishabosha
force-pushed
the
tasty/single-methodtype-tag
branch
from
0145620
to
73209e0
Compare
bishabosha
force-pushed
the
tasty/single-methodtype-tag
branch
from
73209e0
to
c77f8e2
Compare
By swapping params to pickle types and then names, we can avoid a marker to separate names from flags. This also fixes the annoying fact that in TastyPrinter, param names were printed alongside the previous type printed, and not the associated parameter type