Ticket #2008 (closed patch: wontfix)

Opened 11 years ago

Last modified 10 years ago

[PCT] Patch/feature request: No pop_eh in try PAST::Op

Reported by: arnsholt Owned by: pmichaud
Priority: normal Milestone:
Component: PCT Version: 3.0.0
Severity: medium Keywords:
Cc: cotto, benabik Language:
Patch status: Platform:


For my HLL I needed a different layout of the pop_eh instructions in my exception handler code. The simplest way to achieve this was to suppress PAST::Compiler from outputting those and supplying my own.

The attached patch implements this by adding a nopopeh attribute to PAST::Op and code in PAST::Compiler that skips the pop_eh instructions when the attribute has a true value.

Rakudo HEAD spectest with the patch applied has the same failures as without (fails tests 184-186 in S05-mass/properties-derived.rakudo, 20 in S12-enums/basic.rakudo, 2-3 in S19-command-line/dash-e.t) on my machine.


nopopeh.patch Download (2.4 KB) - added by arnsholt 11 years ago.

Change History

Changed 11 years ago by arnsholt

Changed 10 years ago by jkeenan

  • cc cotto, benabik added

Would someone be able to evaluate the patch attached to this ticket?

Thank you very much.


Changed 10 years ago by pmichaud

I'm against the patch, as I feel that any structure (such as 'try') that creates an exception handler should also be responsible for removing it properly. I'm also not wanting to make a lot of special-purpose attributes in nodes like this one.

I'd like to see more details about the use case where the pop_eh needs to be suppressed, I suspect there's a better solution than suppression.


Changed 10 years ago by benabik

I pretty much agree with pmichaud here. This intuitively feels wrong to me, and adding such an option to 'core' PCT seems like a good way to have future HLLs get the exception handler stack very very wrong. I'd suggest that if his HLL really needs it, he may want to use a new pasttype and extent PAST::Compiler to move the pop_eh nodes later. Or, if he never needs traditional try semantics, he could use something like tree-optimization to rearrange the trees.

Changed 10 years ago by jkeenan

  • status changed from new to closed
  • resolution set to wontfix

Based on these comments, I am going to reject the patch. I hope the original submitter will contribute to Parrot in the future.

Thank you very much.


Note: See TracTickets for help on using tickets.