Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upuntil( assignment ) doesn't warn when while (assignment ) does #15138
Comments
This comment has been minimized.
This comment has been minimized.
From @tonycozThis is similar to #127122, but doesn't have quite as simple a fix. $ ./perl -Ilib -Mwarnings=syntax -le 'while ($x = 0) { }' $ ./perl -Ilib -V Characteristics of this binary (from libperl): |
This comment has been minimized.
This comment has been minimized.
From @dcollinsnOn Wed Jan 20 17:14:21 2016, tonyc wrote:
You're right. I started by looking at what you did for #127122, but the "hack" of treating an unless as an if won't work here. Ultimately, I added a few lines of code to op.c to accept this special case - the condition which currently generates the warning, but with an OP_NOT around it. I also added a test for this bug to t/lib/warnings/op, as well as a test for the related while(assignment) case, which didn't have a test. The tests reduce to while(0) and until(1) so as to not hang the entire test suite. All tests pass for me. Patch attached, hopefully formatted correctly. |
This comment has been minimized.
This comment has been minimized.
From @dcollinsn0001-perl-127333-add-warning-for-until-assignment.patchFrom 90d93e7911a4471ec779afda1c669a3fe7664d49 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Mon, 6 Jun 2016 21:04:46 -0400
Subject: [PATCH] [perl #127333] add warning for until(assignment)
while ($a = 1) emits a warning "Found = in conditional, should be ==".
However, until ($a = 1) does not. The parser breaks down until
(condition) into while (!(condition)), which defeats the check in
S_scalarboolean, since it is looking for a conditional that /is/ an
assignment, but we have a conditional that is a thinly veiled assignment.
A similar bug was [perl #127122]. That bug was fixed by treating
unless (a) {b} else {c} as if (a) {c} else {b}, instead of treating it
as if (!a) {b} else {c}. That approach will not work here.
Instead, the test in S_scalarboolean has been widened. It previously
looked for an OP_SASSIGN with op_first OP_CONST, it now also detects an
OP_NOT surrounding that construct.
Tests for both while() and until() were also added to t/lib/warnings/op.
---
op.c | 7 +++++--
t/lib/warnings/op | 22 ++++++++++++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/op.c b/op.c
index 619c6e3..9a1f8a4 100644
--- a/op.c
+++ b/op.c
@@ -1532,8 +1532,11 @@ S_scalarboolean(pTHX_ OP *o)
{
PERL_ARGS_ASSERT_SCALARBOOLEAN;
- if (o->op_type == OP_SASSIGN && cBINOPo->op_first->op_type == OP_CONST
- && !(cBINOPo->op_first->op_flags & OPf_SPECIAL)) {
+ if ((o->op_type == OP_SASSIGN && cBINOPo->op_first->op_type == OP_CONST &&
+ !(cBINOPo->op_first->op_flags & OPf_SPECIAL)) ||
+ (o->op_type == OP_NOT && cUNOPo->op_first->op_type == OP_SASSIGN &&
+ cBINOPx(cUNOPo->op_first)->op_first->op_type == OP_CONST &&
+ !(cBINOPx(cUNOPo->op_first)->op_first->op_flags & OPf_SPECIAL))) {
if (ckWARN(WARN_SYNTAX)) {
const line_t oldline = CopLINE(PL_curcop);
diff --git a/t/lib/warnings/op b/t/lib/warnings/op
index cf7a798..cc0cf46 100644
--- a/t/lib/warnings/op
+++ b/t/lib/warnings/op
@@ -134,6 +134,28 @@ Found = in conditional, should be == at - line 3.
Found = in conditional, should be == at - line 4.
########
# op.c
+# NAME while with assignment as condition
+use warnings 'syntax';
+1 while $a = 0;
+while ($a = 0) {
+ 1;
+}
+EXPECT
+Found = in conditional, should be == at - line 3.
+Found = in conditional, should be == at - line 4.
+########
+# op.c
+# NAME until with assignment as condition
+use warnings 'syntax';
+1 until $a = 1;
+until ($a = 1) {
+ 1;
+}
+EXPECT
+Found = in conditional, should be == at - line 3.
+Found = in conditional, should be == at - line 4.
+########
+# op.c
use warnings 'syntax' ;
@a[3];
@a{3};
--
2.8.1
|
This comment has been minimized.
This comment has been minimized.
|
The RT System itself - Status changed from 'new' to 'open' |
This comment has been minimized.
This comment has been minimized.
From @dcollinsnTony reports that that didn't apply. Didn't apply for me either. Probably a line endings issue. This one should work. |
This comment has been minimized.
This comment has been minimized.
From @dcollinsn0001-perl-127333-add-warning-for-until-assignment.patchFrom 9e26b2c7ce3faf108ded08a9d11d3e37a4515a24 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Mon, 6 Jun 2016 21:04:46 -0400
Subject: [PATCH] [perl #127333] add warning for until(assignment)
while ($a = 1) emits a warning "Found = in conditional, should be ==".
However, until ($a = 1) does not. The parser breaks down until
(condition) into while (!(condition)), which defeats the check in
S_scalarboolean, since it is looking for a conditional that /is/ an
assignment, but we have a conditional that is a thinly veiled assignment.
A similar bug is [perl #127122]. That bug was fixed by treating
unless (a) {b} else {c} as if (a) {c} else {b}, instead of treating it
as if (!a) {b} else {c}. That approach will not work here.
Instead, the test in S_scalarboolean has been widened. It previously
looked for an OP_SASSIGN with op_first OP_CONST, it now also detects an
OP_NOT surrounding that construct.
Tests for both while() and until() were also added to t/lib/warnings/op.
---
op.c | 7 +++++--
t/lib/warnings/op | 22 ++++++++++++++++++++++
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/op.c b/op.c
index 128cdc4..6986610 100644
--- a/op.c
+++ b/op.c
@@ -1532,8 +1532,11 @@ S_scalarboolean(pTHX_ OP *o)
{
PERL_ARGS_ASSERT_SCALARBOOLEAN;
- if (o->op_type == OP_SASSIGN && cBINOPo->op_first->op_type == OP_CONST
- && !(cBINOPo->op_first->op_flags & OPf_SPECIAL)) {
+ if ((o->op_type == OP_SASSIGN && cBINOPo->op_first->op_type == OP_CONST &&
+ !(cBINOPo->op_first->op_flags & OPf_SPECIAL)) ||
+ (o->op_type == OP_NOT && cUNOPo->op_first->op_type == OP_SASSIGN &&
+ cBINOPx(cUNOPo->op_first)->op_first->op_type == OP_CONST &&
+ !(cBINOPx(cUNOPo->op_first)->op_first->op_flags & OPf_SPECIAL))) {
if (ckWARN(WARN_SYNTAX)) {
const line_t oldline = CopLINE(PL_curcop);
diff --git a/t/lib/warnings/op b/t/lib/warnings/op
index cf7a798..cc0cf46 100644
--- a/t/lib/warnings/op
+++ b/t/lib/warnings/op
@@ -134,6 +134,28 @@ Found = in conditional, should be == at - line 3.
Found = in conditional, should be == at - line 4.
########
# op.c
+# NAME while with assignment as condition
+use warnings 'syntax';
+1 while $a = 0;
+while ($a = 0) {
+ 1;
+}
+EXPECT
+Found = in conditional, should be == at - line 3.
+Found = in conditional, should be == at - line 4.
+########
+# op.c
+# NAME until with assignment as condition
+use warnings 'syntax';
+1 until $a = 1;
+until ($a = 1) {
+ 1;
+}
+EXPECT
+Found = in conditional, should be == at - line 3.
+Found = in conditional, should be == at - line 4.
+########
+# op.c
use warnings 'syntax' ;
@a[3];
@a{3};
--
2.8.1
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@tonycoz - Status changed from 'open' to 'pending release' |
This comment has been minimized.
This comment has been minimized.
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been Perl 5.26.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
This comment has been minimized.
This comment has been minimized.
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#127333 (status was 'resolved')
Searchable as RT127333$