Skip to content

Conversation

@leonerd
Copy link
Contributor

@leonerd leonerd commented Sep 4, 2024

When working with nontrivially-shaped custom ops, such as ones based on UNOP_AUX with interesting op_aux arrays, it is often useful to be able to peek into the contents of these op structures with op_dump(). Perl's core dumper cannot know the contents of these aux arrays, but by defining a helper function in the module that provides the custom op, help can be achieved.

A helper function, opdump_printf is also provided that acts as a printf()-alike function for outputting lines of content. The various internal arguments to it (level, bar, file) are bundled up into an opaque structure, so as to achieve a modicum of abstraction away from the specific internals on how dump.c happens to work.

@leonerd
Copy link
Contributor Author

leonerd commented Sep 4, 2024

As an example of it in use:

The following function is added to my .xs file:

static void opdump_argelems_named(pTHX_ const OP *o, void *ctx)
{
  UNOP_AUX_item *aux = cUNOP_AUXo->op_aux;

  UV n_params = aux[1].uv;
  opdump_printf(ctx, "ARGIX = %" UVuf "\n", aux[0].uv);
  opdump_printf(ctx, "PARAMS = (%" UVuf ")\n", n_params);
  for(U32 parami = 0; parami < n_params; parami++) {
    UNOP_AUX_item *paramdef = aux + 2 + 4 * parami;

    UV flags = paramdef[NAMEDPARAMaux_FLAGS].uv;
    SV *flagstr = newSVpvn("", 0);
    SAVEFREESV(flagstr);
    if(flags & NAMEDPARAMf_UTF8) sv_catpvf(flagstr, "%sUTF8", SvCUR(flagstr)?", ":"");
    if(flags & NAMEDPARAMf_REQUIRED) sv_catpvf(flagstr, "%sREQUIRED", SvCUR(flagstr)?", ":"");

    opdump_printf(ctx, "  [%d] = {.name=\"%s\", .padix=%u, .flags=(%s)}\n",
        parami,
        paramdef[NAMEDPARAMaux_NAMEPV].pv,
        (unsigned int)paramdef[NAMEDPARAMaux_PADIX].uv,
        SvPV_nolen(flagstr));
  }
}

and added to the XOP structure by:

  XopENTRY_set(&xop_argelems_named, xop_dump, &opdump_argelems_named);

When printed with op_dump(o), it extends the output:

5    +--argelems_named UNOP_AUX(0x55e0f1555af0) ===> [0x0]
         FLAGS = (UNKNOWN,SLABBED)
         ARGIX = 0
         PARAMS = (2)
           [0] = {.name="y", .padix=2, .flags=(REQUIRED)}
           [1] = {.name="x", .padix=1, .flags=(REQUIRED)}

Without this facility, only the top two lines would have been output, resulting in far less useful debug information.

@leonerd leonerd added the squash-before-merge Author must squash the commits down before merging to blead label Sep 5, 2024
@leonerd
Copy link
Contributor Author

leonerd commented Sep 5, 2024

Several updates made:

  • Pass context pointer as opaque structure rather than simply void *
  • Automatic linefeed handling in opdump_printf() to avoid caller needing to handle it themselves
  • Added documentation of newly-created items

@leonerd leonerd added squash-before-merge Author must squash the commits down before merging to blead and removed squash-before-merge Author must squash the commits down before merging to blead hasConflicts labels Sep 6, 2024
perl.h Outdated
#include "perly.h"

/* opaque struct type used to communicate between xop_dump and opdump_printf */
struct OpDumpContext;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a public (API) identifier without a Perl_ prefix. Do we want that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have hundreds of those - to pick a random example, typedef struct av AV in perl.h. But you're right; no need to make it worse. I'll stick a prefix on it.

@tonycoz
Copy link
Contributor

tonycoz commented Sep 8, 2024

"decalaration" in the first commit message

dump.c Outdated

while(msglen) {
if(ctx->indent_needed) {
PerlIO_printf(ctx->file, " ");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PerlIO_puts() to avoid the overhead of PerlIO_printf().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahyes; another one I copied from the other code. I'll fix it there too

@tonycoz
Copy link
Contributor

tonycoz commented Sep 9, 2024

Looks fine otherwise

When working with nontrivially-shaped custom ops, such as ones based on
UNOP_AUX with interesting op_aux arrays, it is often useful to be able
to peek into the contents of these op structures with `op_dump()`.
Perl's core dumper cannot know the contents of these aux arrays, but by
defining a helper function in the module that provides the custom op,
help can be achieved.

A helper function, `opdump_printf` is also provided that acts as a
printf()-alike function for outputting lines of content. The various
internal arguments to it (level, bar, file) are bundled up into an
opaque structure, so as to achieve a modicum of abstraction away from
the specific internals on how dump.c happens to work.
@leonerd
Copy link
Contributor Author

leonerd commented Sep 9, 2024

"decalaration" in the first commit message

Oops, fixed too.

@leonerd leonerd removed the squash-before-merge Author must squash the commits down before merging to blead label Sep 9, 2024
@leonerd leonerd merged commit 1eab73a into Perl:blead Sep 9, 2024
@leonerd leonerd deleted the xop-custom-dumper branch September 9, 2024 09:38
tonycoz added a commit to tonycoz/perl5 that referenced this pull request Nov 11, 2025
which I missed in the review of Perl#22572 but found when I tried to use
it.
tonycoz added a commit to tonycoz/perl5 that referenced this pull request Nov 11, 2025
not struct OpDumpContext *, OP *.

Which I missed in the review of Perl#22572 but found when I tried to use
it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants