Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

always run finally blocks no matter how we leave the try block (RT #112099) #1

Closed
wants to merge 4 commits into from

Conversation

mauke
Copy link

@mauke mauke commented Feb 19, 2016

No description provided.

@karenetheridge
Copy link
Member

@karenetheridge
Copy link
Member

I'll pass this around for people to eyeball today, and if there are no screaming objections, send it out as -TRIAL tonight.

@mauke
Copy link
Author

mauke commented Feb 19, 2016

Cool, thanks. Note that this does change behavior a bit: try { exit } finally { ... } now runs ... whereas it didn't before.

@shadowcat-mst
Copy link

I ... think ideally, if a user does that, a small gnome exits the monitor and hits the developer with a mallet.

Also, that would quite possibly fix a bug in CGI scripts compiled to be persistent with an overriden exit.

So, on the whole, LGTM

@@ -29,6 +29,8 @@ BEGIN {
*_HAS_SUBNAME = ($su || $sn) ? sub(){1} : sub(){0};
}

our @_finally_guards;

Choose a reason for hiding this comment

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

our @some_variable is generally perceived as an invite of please feel free to fondle me if you think it is justified. Even (or often especially) if rhe variable is _ prefixed.

In the case of T::T there is no scenario where such "fondling" is justified, therefore a my %stackstate with local() on a key is a better solution.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think that's going to be a problem in practice. Try::Tiny already contains the following fondleables: $VERSION, @EXPORT, _subname, _HAS_SUBNAME, and an entire hidden package: Try::Tiny::ScopeGuard.

@karenetheridge
Copy link
Member

Thanks! merged and released as 0.25-TRIAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants