Handle empty subroutine signatures in parse_subsignature() (fixes #17689)
#23679
Conversation
leonerd
force-pushed
the
gh17689
branch
2 times, most recently
from
8b361d4 to
455f15b
Compare
ext/XS-APItest/APItest.xs
Outdated
| case OP_NULL: | ||
| break; | ||
| default: | ||
| fprintf(stderr, "TODO: examine kid %p (optype=%s)\n", kid, PL_op_name[kid->op_type]); |
There was a problem hiding this comment.
(This may be slightly off-topic, but since that fprintf stderr line is exposed here, I might as well ask this.)
Do we really need this line printing a TODO reminder that we are apparently doing nothing about? If you sometimes run make test_harness 1>/dev/null (as I do, when I'm only interested in the overall result), then this is one of the few remaining statements that appear in your terminal via STDERR.
(The line appears to have entered core in 996b0cb back in 2019.)
There was a problem hiding this comment.
Yeah that's a fair question. It appears I added this back in that commit, and it's probably not that necessary any more. May be best just to remove it actually.
This was useful during early development of the test but it's too fragile and prints far too easily at other times, we might as well remove it. Perl#23679 (comment)
leonerd
force-pushed
the
gh17689
branch
2 times, most recently
from
9648f78 to
5114cc7
Compare
ext/XS-APItest/APItest.xs
Outdated
| retop = op_append_list(OP_LIST, retop, newSVOP(OP_CONST, 0, | ||
| /* newSVpvf("nextstate:%s:%d", CopFILE(cCOPx(kid)), cCOPx(kid)->cop_line))); */ | ||
| newSVpvf("nextstate:%u", (unsigned int)cCOPx(kid)->cop_line))); | ||
| newSVpvf("nextstate:%" UVuf, (UV)CopLINE(cCOPx(kid))))); |
There was a problem hiding this comment.
You can use LINE_Tf for the format instead of UVuf and omit the cast.
There was a problem hiding this comment.
Ahah, I had a look for one of those but managed to miss it. Will update.
|
Looks fine otherwise. |
This was useful during early development of the test but it's too fragile and prints far too easily at other times, we might as well remove it. #23679 (comment)
3 participants
(As per comment in code), perly.y can't cope with an empty subsignature when recursing directly into a grammar node, because the optionality was already handled in the regular grammar at a higher level than this. Instead, we must detect the empty signature by noticing an immediately-following close parenthesis, and side-step the grammar to perform the appropriate action steps manually.
Also tidies up some noisy warnings from XS-APItest at test time.