-
Notifications
You must be signed in to change notification settings - Fork 602
Modernize attributes.pm to v5.40
#23769
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
Conversation
e55597e to
713a9fb
Compare
ext/attributes/attributes.pm
Outdated
| my $svref = shift; | ||
| my $svtype = uc reftype($svref); | ||
| my $stash = _guess_stash($svref); | ||
| $stash = caller unless defined $stash; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use // here:
my $stash = _guess_stash($svref) // caller;There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for 1; at the end of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahyes, both fixed
| %EXPORT_TAGS = (ALL => [@EXPORT, @EXPORT_OK]); | ||
|
|
||
| use strict; | ||
| our @EXPORT_OK = qw(get reftype); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could just do *reftype = \&builtin::reftype in this file and drop the reftype func from attributes.xs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that would be reasonable too. Annoyingly, we need to keep the symbol present, in case some code out there somewhere actually calls attributes::reftype() - i.e. we can't simply use a lexical import here.
* Reset `@warnings` array between tests so failures in one block don't leak into all the later ones * Print the next warning using `diag` if we fail because it isn't empty
…tes.xs Previously the XS code always checked its immediate caller for warnings. This was fine when core perl invokes it directly in a `:prototype` warning, but was checking the wrong layer when invoked indirectly via `attributes::import`. In that case, it meant that it was sensitive to, and printed warnings as it coming from attributes.pm itself, rather than the intended caller; i.e. the location of the `use attributes ...` statement. This is fixed by using `caller_cx()` to walk up the caller stack looking for a call frame not from the `attributes` package.
713a9fb to
004837f
Compare
* use v5.40 * `our` on package globals * Signatures on subs * Lexical subs when not required to be visible * `module_true` means no need for ending `1;` * Use `//` operator * Remove `reftype` from attributes.xs and use builtin::reftype instead
004837f to
dd15d23
Compare
|
LGTM 👍 |
ouron package globalsThis first required a bugfix in attributes.xs, whose details are outlined in its own commit.