unexpected OP_CUSTOM (catch) at /usr/share/perl5/5.30/B/Deparse.pm line 1633.

[wchristian]

Ran into this, and suspect it's related to: https://metacpan.org/pod/Syntax::Keyword::Try

Can collect more info and poke at it when you let me know what would most help you.

Meanwhile, any thoughts on whether this might be easy to fix?

[pjcj]

Oh, interesting. And yes, I suspect you are correct. Are you able to do a plain Deparse on the code?

A simple solution might be to add a B::Deparse::pp_catch sub. If that were empty, things might "work" but probably with missing coverage for the catch block. If you were able to write the correct code in the pp_catch sub you could probably get things working correctly.

But I rather dislike the solution of writing code for a particular module. I wonder whether Paul Evans might have any ideas of how to solve it too.

[leonerd]

I've looked into this and in summary, B::Deparse's structure isn't flexible enough to actually deparse catch.

The syntax try { ... } catch { ... } ends up being parsed by Syntax::Keyword::Try into something whose outermost op looks like a do { ... } block to deparse. Currently, whatever code I try to write inside a hypothetical pp_catch would end up leaving that outer do in place, because I can't get at that code.

A better solution would involve more invasive changes into B::Deparse, sufficient to allow these custom op hooks to restructure bits of the optree, or somehow say they consume more of it so deparse doesn't have to do so much.

[haarg]

Why would it be a problem for the output of the deparse to include that do block?

[leonerd]

@haarg Oh, not a huge problem, just it felt a bit untidy. The reconstituted Perl syntax would not be the same as the original input. If you roundtrip it again you'll get another layer, and so on. It would be good enough for casual human inspection while debugging, say, but it wouldn't do for serialisation purposes.

[haarg]

That's pretty standard for deparse.

[wchristian]

This issue is becoming a little bigger with core becoming more and more flexible, and i just had a little chat with @leonerd on the topic:

13:24 [Mithaldu] unexpected OP_PUSHMARK at /usr/share/perl5/5.32/B/Deparse.pm line 1663. 13:24 [Mithaldu] unexpected OP_CUSTOM (await) at /usr/share/perl5/5.32/B/Deparse.pm line 1663. 13:24 [Mithaldu] unexpected OP_CUSTOM (leaveasync) at /usr/share/perl5/5.32/B/Deparse.pm line 1663. 13:24 [Mithaldu] :( 13:26 [LeoNerd] Oh, yeah don't try to Deparse any async/await-using code. ;) 13:36 [Mithaldu] LeoNerd: any chance at that ever working? because that would mean i can't get coverage whatsoever, since everything called downstream of an async sub doesn't get covered 13:40 [Mithaldu] like, there's a ton of functions here, and from what i can tell the only ones who even get acknowledged by devel-cover are the ones that aren't called inside an async function https://usercontent.irccloud-cdn.com/file/6j8Sem51/image.png 13:40 [Mithaldu] none of them even use async/await themselves 16:42 [LeoNerd] Mithaldu: B::Deparse..? Unlikely 16:42 [LeoNerd] This issue -would- be fixable with about 20 lines of code change in Deparse.pm 16:42 [LeoNerd] But Deparse.pm is core and not going to alter itself on behalf of some random thirdparty CPAN module 16:43 [LeoNerd] The basic structure of it isn't really ameniable to large-scale custom opcodes 16:43 [LeoNerd] B::Deparse can cope with tiny custom ops that "look and feel a bit like functions"; foo($x) or whatever 16:43 [Mithaldu] LeoNerd: so you're saying Devel::Cover would need to become pluggable 16:43 [LeoNerd] But the way that larger shapes like enterasync/leaveasync work as wrappers for the optree of an entire sub, isn't really possible to patch in without a lot of reshape to Deparse.pm itself 16:44 [LeoNerd] Correct 16:44 [Mithaldu] that gives hope :) 16:44 [LeoNerd] This is the price you pay for flexibility in the core. It means that other things that want to understand optrees get confused by the new stuff 16:45 [Mithaldu] yeah it makes sense 16:45 [Mithaldu] i'm just glad to hear that it's not impossible 16:45 [LeoNerd] The way that XS::Parse::Infix works is that it monkeypatches B::Deparse at load time, so at least all the infix operators you make -that- way work properly 16:45 [LeoNerd] I say "you make" - currently, the ones that /I/ make because it only runs on my PL_infix_plugin branch. ;) 16:46 [LeoNerd] I do have vague thoughts to try to get some automatic deparsing out of XPK, but even then it can only really work in some cases 16:46 [Mithaldu] i'm thinking devel::cover should try move away from B::Deparse anyhow, because i'm already in the situation that try/catch can't be analyzed by D::C, but newer B::Deparse can do it, but cygwin only has 5.32 available 16:46 [LeoNerd] The craziness of XPS means it's unlikely ever to be possible automatically in a general enough way 16:46 [Mithaldu] or rather, make it possible for D::C to use a different deparser 16:46 [LeoNerd] Yeah; not relying on B::Deparse is likely a good idea 16:47 [Mithaldu] damn 16:47 [LeoNerd] Do you need to deparse it at all? 16:47 [Mithaldu] i'll admit: i did not expect you to agree 16:47 [Mithaldu] i don't know 16:47 [Mithaldu] i haven't worked on it much 16:47 [LeoNerd] To a first approximation: I would have thought that coverage checking is basically a matter of asking "did all the OP_NEXTSTATEs get hit?" 16:47 [Mithaldu] i'll drop this log in a ticket i already have open and see what pjcj says 16:48 [Mithaldu] i think the part where it gets a bit more intricate is that it also wants to do branching analysis 16:48 [LeoNerd] You don't really need to know much else besides that.. If you find ones that didn't get hit they already know their file + linenumber anyway, so you can just report that 16:48 [Mithaldu] look at an if( .... ) construct and figure out ALL possible ways for it to play out 16:48 [LeoNerd] Yeah if you want to do $cond ? TRUE : FALSE branching you also need to know those 16:49 [Mithaldu] otoh 16:49 [LeoNerd] Oh, well if/else still creates block bodies that have OP_NEXTSTATEs at the start 16:49 [Mithaldu] if THAT causes the incompatibility i'm happy to have a dumb "check only which lines ran" mode 16:49 [Mithaldu] but i'm speculating, i don't know D::C code :) 16:50 [LeoNerd] The only thing I know of it is that it uses B::Deparse and I find that fact surprisingly odd

[leonerd]

This issue is becoming a little bigger with core becoming more and more flexible, and i just had a little chat with @leonerd on the topic:

I wouldn't say it's because core is becoming more flexible. It's always been that flexible. It's just more that I'm learning how flexible it is and writing more interesting CPAN modules to actually use it; doing things that honestly have always been possible but nobody else has really ever done before.

[wchristian]

in response to inquiry by @leonerd i just had this exchange with @pjcj on irc, so i'm documenting this here :)

<Mithaldu> morning, maybe a quick question, can you explain why Devel::Cover
           uses B::Deparse at all, and whether it might be possible to decouple
           them? :)
<pjcj> Hello!  Yes, there are two reasons:
<pjcj> 1. As a way to walk through the optree and collect data about the ops.
          This could certainly be replaced by code which just walked the optree
          and didn't try to deparse it, and I'd love to see that.
<pjcj> 2. To output the branch and condition details for those coverage
          criteria.  This is harder to replace because we do want to deparse
          that part of the optree.  The deparsing is not perfect at the moment
          because we don't have the context of how the section of the optree was
          entered, but it's mostly OK.