Bug #132335 for XS-Parse-Sublike: [PATCH] Build op trees that more closely resemble what core does

Hi Paul, Out of curiosity I was looking at the B::Concise and B::Deparse output for async subs, and noticed their signatures weren't recognised, and that empty ones had a stub op that normal subs didn't. After groveling around perly.y (and its history) for a bit, I came up with the attached patch, which makes subs with signatures constructed by 'func' from t/func.xs have identical optrees to regular subs, both on 5.30 and blead. - ilmari -- - Twitter seems more influential [than blogs] in the 'gets reported in the mainstream press' sense at least. - Matt McLeod - That'd be because the content of a tweet is easier to condense down to a mainstream media article. - Calle Dybedahl

From 9a0a3615b966206ced303edeebaa185b6cbca80c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org> Date: Mon, 13 Apr 2020 22:19:08 +0100 Subject: [PATCH] Build op trees that more closely resemble what core does This improves the deparseability of custom-built subs. - Eliminate stub op if we have a signature and an empty body - Append a nextstate at the end of the arg check blocks - On 5.31.5 and later, wrap the whole arg-checking sequence in a nulle-out ex-argcheck op. --- hax/parse_subsignature.c.inc | 10 ++++++++-- lib/XS/Parse/Sublike.xs | 25 ++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/hax/parse_subsignature.c.inc b/hax/parse_subsignature.c.inc index fa3274e..e42f2cb 100644 --- a/hax/parse_subsignature.c.inc +++ b/hax/parse_subsignature.c.inc @@ -205,10 +205,16 @@ endofelems: OP *checkop = newUNOP_AUX(OP_ARGCHECK, 0, NULL, aux); + checkop = op_prepend_elem(OP_LINESEQ, newSTATEOP(0, NULL, NULL), + op_prepend_elem(OP_LINESEQ, checkop, elems)); + + /* a nextstate at the end handles context correctly for an empty + * sub body */ + checkop = op_append_elem(OP_LINESEQ, checkop, newSTATEOP(0, NULL, NULL)); + LEAVE; - return op_prepend_elem(OP_LINESEQ, newSTATEOP(0, NULL, NULL), - op_prepend_elem(OP_LINESEQ, checkop, elems)); + return checkop; } #endif diff --git a/lib/XS/Parse/Sublike.xs b/lib/XS/Parse/Sublike.xs index 3d37ce7..40482cb 100644 --- a/lib/XS/Parse/Sublike.xs +++ b/lib/XS/Parse/Sublike.xs @@ -88,6 +88,21 @@ static int parse2(pTHX_ const struct XSParseSublikeHooks *hooksA, const struct X sigop = op_prepend_elem(OP_LINESEQ, newSTATEOP(0, NULL, NULL), newUNOP_AUX(OP_ARGCHECK, 0, NULL, aux)); + + /* a nextstate at the end handles context correctly for an empty + * sub body */ + sigop = op_append_elem(OP_LINESEQ, sigop, newSTATEOP(0, NULL, NULL)); + +#if HAVE_PERL_VERSION(5,31,5) + /* wrap the list of arg ops in a NULL aux op. This serves two + * purposes. First, it makes the arg list a separate subtree + * from the body of the sub, and secondly the null op may in + * future be upgraded to an OP_SIGNATURE when implemented. For + * now leave it as ex-argcheck + */ + sigop = newUNOP_AUX(OP_ARGCHECK, 0, sigop, NULL); + op_null(sigop); +#endif } else #endif @@ -111,8 +126,16 @@ static int parse2(pTHX_ const struct XSParseSublikeHooks *hooksA, const struct X SvREFCNT_inc(PL_compcv); #ifdef HAVE_PARSE_SUBSIGNATURE - if(sigop) + if(sigop) { + /* parse_block() returns an empy block as a stub op. + * no need to keep that if we we have a signature. + */ + if (ctx.body->op_type == OP_STUB) { + op_free(ctx.body); + ctx.body = NULL; + } ctx.body = op_append_list(OP_LINESEQ, sigop, ctx.body); + } #endif if(PL_parser->error_count) { -- 2.20.1