Skip to content

Commit 6c3c827

Browse files
committed
Check the right caller context for 'illegalproto' warnings in attributes.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.
1 parent c8bfe0f commit 6c3c827

File tree

2 files changed

+40
-4
lines changed

2 files changed

+40
-4
lines changed

ext/attributes/attributes.xs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,32 @@ modify_SV_attributes(pTHX_ SV *sv, SV **retlist, SV **attrlist, int numattrs)
9898
subname = newSVhek_mortal(hek);
9999
else
100100
subname=(SV *)CvGV((const CV *)sv);
101-
if (ckWARN(WARN_ILLEGALPROTO))
102-
Perl_validate_proto(aTHX_ subname, proto, TRUE, 0);
101+
{
102+
/* Need to check if the caller has WARN_ILLEGALPROTO
103+
* set. That might be PL_curcop or it might be higher
104+
* up, we'll have to check
105+
*/
106+
107+
COP *cop = PL_curcop;
108+
int caller_level = 0;
109+
const char *stashname;
110+
while (cop && (stashname = HvNAME(CopSTASH(cop))) &&
111+
strEQ(stashname, "attributes")) {
112+
// skip caller frames from attributes.pm itself
113+
caller_level++;
114+
cop = caller_cx(caller_level, NULL)->blk_oldcop;
115+
}
116+
117+
if (cop && cop_has_warning(cop, WARN_ILLEGALPROTO)) {
118+
ENTER;
119+
SAVESPTR(PL_curcop);
120+
PL_curcop = cop;
121+
122+
Perl_validate_proto(aTHX_ subname, proto, TRUE, 0);
123+
124+
LEAVE;
125+
}
126+
}
103127
Perl_cv_ckproto_len_flags(aTHX_ (const CV *)sv,
104128
(const GV *)subname,
105129
name+10,

t/op/attrproto.t

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ BEGIN {
1111
}
1212
use warnings;
1313

14-
plan tests => 48;
14+
plan tests => 52;
1515

1616
my @warnings;
1717
my ($attrs, $ret) = ("", "");
@@ -90,7 +90,7 @@ is @warnings, 0, "No more warnings"
9090

9191
# Use attributes
9292
@warnings = ();
93-
eval 'package Q; use attributes __PACKAGE__, \&B, "prototype(new)";';
93+
eval 'package Q; no warnings "illegalproto"; use attributes __PACKAGE__, \&B, "prototype(new)";';
9494
$ret = prototype \&Q::B;
9595
is $ret, "new", "use attributes also sets the prototype";
9696
like shift @warnings, qr/Prototype mismatch: sub Q::B \(baz\) vs \(new\)/,
@@ -105,6 +105,18 @@ is $ret, "new", "A malformed prototype doesn't reset it";
105105
like $@, qr/Unterminated attribute parameter in attribute list/, "Malformed prototype croaked";
106106
is @warnings, 0, "Malformed prototype isn't just a warning";
107107

108+
# Respects `use warnings 'illegalproto'` of the right caller
109+
@warnings = ();
110+
eval 'package Q; use warnings; sub C {} use attributes __PACKAGE__, \&C, "prototype(bad)";';
111+
$ret = prototype \&Q::C;
112+
is $ret, "bad", "illegal prototype is still set via use attributes";
113+
like shift @warnings, qr/^Illegal character in prototype for \*Q::C : bad at /,
114+
"Illegal prototype warning triggered";
115+
like shift @warnings, qr/^Prototype mismatch: sub Q::C: none vs \(bad\) at /,
116+
"Prototype mismatch warning triggered";
117+
is @warnings, 0, "No more warnings" or
118+
diag "More warning: $warnings[0]";
119+
108120
@warnings = ();
109121
eval 'use attributes __PACKAGE__, \&foo, "prototype($$\x{100}";';
110122
$ret = prototype \&Q::B;

0 commit comments

Comments
 (0)