Skip to content

Commit 2f48e46

Browse files
committed
warn about logical negation as operand of isa
Code like `!$obj isa Some::Class` doesn't make sense, so emit a precedence warning. (If you really want to test whether a boolean is an instance of some class, write `(!$obj) isa Some::Class`.)
1 parent 716d8ca commit 2f48e46

File tree

3 files changed

+35
-2
lines changed

3 files changed

+35
-2
lines changed

op.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15242,16 +15242,24 @@ Perl_ck_length(pTHX_ OP *o)
1524215242
OP *
1524315243
Perl_ck_isa(pTHX_ OP *o)
1524415244
{
15245-
OP *classop = cBINOPo->op_last;
15246-
1524715245
PERL_ARGS_ASSERT_CK_ISA;
1524815246

15247+
OP *const classop = cBINOPo->op_last;
15248+
1524915249
/* Convert barename into PV */
1525015250
if(classop->op_type == OP_CONST && classop->op_private & OPpCONST_BARE) {
1525115251
/* TODO: Optionally convert package to raw HV here */
1525215252
classop->op_private &= ~(OPpCONST_BARE|OPpCONST_STRICT);
1525315253
}
1525415254

15255+
OP *const objop = cBINOPo->op_first;
15256+
/* !$x isa Some::Class # probably meant !($x isa Some::Class) */
15257+
if (objop->op_type == OP_NOT && !(objop->op_flags & OPf_PARENS)) {
15258+
Perl_ck_warner(aTHX_ packWARN(WARN_PRECEDENCE),
15259+
"Possible precedence problem on isa operator"
15260+
);
15261+
}
15262+
1525515263
return o;
1525615264
}
1525715265

pod/perldiag.pod

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5508,6 +5508,19 @@ If instead you intended to match the word 'foo' at the end of the line
55085508
followed by whitespace and the word 'bar' on the next line then you can use
55095509
C<m/$(?)\/> (for example: C<m/foo$(?)\s+bar/>).
55105510

5511+
=item Possible precedence problem on isa operator
5512+
5513+
(W precedence) You wrote something like
5514+
5515+
!$obj isa Some::Class
5516+
5517+
but because C<!> has higher precedence than C<isa>, this is interpreted as
5518+
C<(!$obj) isa Some::Class>, which checks whether a boolean is an instance of
5519+
C<Some::Class>. If you want to negate the result of C<isa> instead, use one of:
5520+
5521+
!($obj isa Some::Class) # explicit parentheses
5522+
not $obj isa Some::Class # low-precedence 'not' operator
5523+
55115524
=item Possible unintended interpolation of %s in string
55125525

55135526
(W ambiguous) You said something like '@foo' in a double-quoted string

t/lib/warnings/op

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,6 +1577,18 @@ Possible precedence problem on bitwise |. operator at - line 26.
15771577
Possible precedence problem on bitwise &. operator at - line 27.
15781578
########
15791579
# op.c
1580+
use warnings 'precedence';
1581+
use feature 'isa';
1582+
$a = !$b isa Some::Class; # should warn
1583+
$a = (!$b) isa Some::Class;
1584+
$a = !($b) isa Some::Class; # should warn
1585+
$a = !($b isa Some::Class);
1586+
$a = not $b isa Some::Class;
1587+
EXPECT
1588+
Possible precedence problem on isa operator at - line 4.
1589+
Possible precedence problem on isa operator at - line 6.
1590+
########
1591+
# op.c
15801592
use integer;
15811593
use warnings 'precedence';
15821594
$a = $b & $c == $d;

0 commit comments

Comments
 (0)