signatures accept fancy assignment operators #16084
Comments
From zefram@fysh.orgCreated by zefram@fysh.org$ perl5.26.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a } print z()' In 5.26 the signature syntax now accepts any kind of assignment operator It was introduced by the reimplementation of signature parsing as part $ perl5.24.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a } print z()' Perl Info |
From @jkeenanOn Sat, 22 Jul 2017 00:14:03 GMT, zefram@fysh.org wrote:
I have confirmed the change of behavior between 5.24 and 5.26 via your examples. Given that we're using a pragma labeled "experimental" here, I wonder if the 5.26 behavior, though unintended undocumented, might actually be desirable in certain circumstances. Note that, since I've never used subroutine signatures, I don't have much of a stake in this. But since we're still "experimenting", I figured it wouldn't hurt to ask. Thank you very much. |
|
The RT System itself - Status changed from 'new' to 'open' |
From zefram@fysh.orgJames E Keenan via RT wrote:
No, never. It would have been a reasonable design decision to use some -zefram |
From @tonycozOn Fri, 21 Jul 2017 17:14:03 -0700, zefram@fysh.org wrote:
The attached prevents non-simple assignment from being accepted. Tony |
From @tonycoz0001-perl-131777-prevent-non-assign-ops-tokens-in-sub-sig.patchFrom 4f05cf783cf449bb227d8c4813fe6e4052ce2878 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 25 Jul 2017 11:48:44 +1000
Subject: (perl #131777) prevent non-'=' assign ops tokens in sub signatures
The yacc grammar introduced in d3d9da4a7 uses ASSIGNOP to
represent the '=' used to introduce default values in subroutine
signatures, unfortunately the parser returns ASSIGNOP for non-simple
assignments, which allowed:
sub foo ($x += 1) { ... }
to default $x to 1.
Modify yylex to accept only the simple assignment operator after a
subroutine parameter.
I'm not especially happy with the error recovery here.
---
pod/perldiag.pod | 11 +++++++++++
t/lib/croak/toke | 9 +++++++++
t/op/signatures.t | 16 ++++++++++------
toke.c | 37 ++++++++++++++++++++++++++++++++++---
4 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 7e8d827..872a722 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -2617,6 +2617,17 @@ this error when Perl was built using standard options. For some
reason, your version of Perl appears to have been built without
this support. Talk to your Perl administrator.
+=item Illegal operator following parameter in a subroutine signature
+
+(F) A parameter in a subroutine signature, was followed by something
+other than C<=> introducing a default, C<,> or C<)>.
+
+ use feature 'signatures;
+ sub foo ($=1) {} # legal
+ sub foo ($a = 1) {} # legal
+ sub foo ($a += 1) {} # illegal
+ sub foo ($a == 1) {} # illegal
+
=item Illegal character following sigil in a subroutine signature
(F) A parameter in a subroutine signature contained an unexpected character
diff --git a/t/lib/croak/toke b/t/lib/croak/toke
index 2603224..847bf13 100644
--- a/t/lib/croak/toke
+++ b/t/lib/croak/toke
@@ -394,3 +394,12 @@ $a = <<~ ;
EXPECT
Use of bare << to mean <<"" is forbidden at - line 1.
+########
+# NAME signature with non-"=" assignop #131777
+use feature 'signatures';
+no warnings 'experimental::signatures';
+sub foo ($a += 1)
+EXPECT
+Illegal operator following parameter in a subroutine signature at - line 3, near "($a += 1"
+syntax error at - line 3, near "($a += 1"
+Execution of - aborted due to compilation errors.
diff --git a/t/op/signatures.t b/t/op/signatures.t
index f0e1b93..8ab8db3 100644
--- a/t/op/signatures.t
+++ b/t/op/signatures.t
@@ -1095,17 +1095,21 @@ syntax error at foo line 8, near ", 123"
EOF
eval "#line 8 foo\nno warnings; sub t096 (\$a 123) { }";
-is $@, qq{syntax error at foo line 8, near "\$a 123"\n};
+is $@, <<'EOF';
+Illegal operator following parameter in a subroutine signature at foo line 8, near "($a 123"
+syntax error at foo line 8, near "($a 123"
+EOF
eval "#line 8 foo\nsub t097 (\$a { }) { }";
-is $@, <<EOF;
-syntax error at foo line 8, near "\$a { "
+is $@, <<'EOF';
+Illegal operator following parameter in a subroutine signature at foo line 8, near "($a { }"
+syntax error at foo line 8, near "($a { }"
EOF
eval "#line 8 foo\nsub t098 (\$a; \$b) { }";
-is $@, <<EOF;
-syntax error at foo line 8, at EOF
-syntax error at foo line 8, near "\$b) "
+is $@, <<'EOF';
+Illegal operator following parameter in a subroutine signature at foo line 8, near "($a; "
+syntax error at foo line 8, near "($a; "
EOF
eval "#line 8 foo\nsub t099 (\$\$) { }";
diff --git a/toke.c b/toke.c
index 6aa5f26..60a0aac 100644
--- a/toke.c
+++ b/toke.c
@@ -5107,12 +5107,43 @@ Perl_yylex(pTHX)
0, cBOOL(UTF), FALSE);
*dest = '\0';
assert(PL_tokenbuf[1]); /* we have a variable name */
+ }
+ else {
+ *PL_tokenbuf = 0;
+ PL_in_my = 0;
+ }
+
+ s = skipspace(s);
+ /* parse the = for the default ourselves to avoid '+=' etc being accepted here
+ * as the ASSIGNOP, and exclude other tokens that start with =
+ */
+ if (*s == '=' && (!s[1] || strchr("=~>", s[1]) == 0)) {
+ /* save now to report with the same context as we did when
+ * all ASSIGNOPS were accepted */
+ PL_oldbufptr = s;
+
+ ++s;
+ NEXTVAL_NEXTTOKE.ival = 0;
+ force_next(ASSIGNOP);
+ PL_expect = XTERM;
+ }
+ else if (*s == ',' || *s == ')') {
+ PL_expect = XOPERATOR;
+ }
+ else {
+ /* make sure the context shows the unexpected character and
+ * hopefully a bit more */
+ if (*s) ++s;
+ while (*s && *s != '$' && *s != '@' && *s != '%' && *s != ')')
+ s++;
+ PL_bufptr = s; /* for error reporting */
+ yyerror("Illegal operator following parameter in a subroutine signature");
+ PL_in_my = 0;
+ }
+ if (*PL_tokenbuf) {
NEXTVAL_NEXTTOKE.ival = sigil;
force_next('p'); /* force a signature pending identifier */
}
- else
- PL_in_my = 0;
- PL_expect = XOPERATOR;
break;
case ')':
--
2.1.4
|
From dcmertens.perl@gmail.comMissing closing single quote in docs?
On Mon, Jul 24, 2017 at 9:52 PM, Tony Cook via RT <perlbug-followup@perl.org
-- |
From @tonycozOn Mon, 24 Jul 2017 19:50:43 -0700, run4flat wrote:
Thanks, fixed in the attached. Tony |
From @tonycoz0001-perl-131777-prevent-non-assign-ops-tokens-in-sub-sig.patchFrom 653e061cc3f9608a0f900a0205fb1fac2e8bcefb Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 25 Jul 2017 14:36:28 +1000
Subject: (perl #131777) prevent non-'=' assign ops tokens in sub signatures
The yacc grammar introduced in d3d9da4a7 uses ASSIGNOP to
represent the '=' used to introduce default values in subroutine
signatures, unfortunately the parser returns ASSIGNOP for non-simple
assignments, which allowed:
sub foo ($x += 1) { ... }
to default $x to 1.
Modify yylex to accept only the simple assignment operator after a
subroutine parameter.
I'm not especially happy with the error recovery here.
---
pod/perldiag.pod | 11 +++++++++++
t/lib/croak/toke | 9 +++++++++
t/op/signatures.t | 16 ++++++++++------
toke.c | 37 ++++++++++++++++++++++++++++++++++---
4 files changed, 64 insertions(+), 9 deletions(-)
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 7e8d827..d8d1316 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -2617,6 +2617,17 @@ this error when Perl was built using standard options. For some
reason, your version of Perl appears to have been built without
this support. Talk to your Perl administrator.
+=item Illegal operator following parameter in a subroutine signature
+
+(F) A parameter in a subroutine signature, was followed by something
+other than C<=> introducing a default, C<,> or C<)>.
+
+ use feature 'signatures';
+ sub foo ($=1) {} # legal
+ sub foo ($a = 1) {} # legal
+ sub foo ($a += 1) {} # illegal
+ sub foo ($a == 1) {} # illegal
+
=item Illegal character following sigil in a subroutine signature
(F) A parameter in a subroutine signature contained an unexpected character
diff --git a/t/lib/croak/toke b/t/lib/croak/toke
index 2603224..847bf13 100644
--- a/t/lib/croak/toke
+++ b/t/lib/croak/toke
@@ -394,3 +394,12 @@ $a = <<~ ;
EXPECT
Use of bare << to mean <<"" is forbidden at - line 1.
+########
+# NAME signature with non-"=" assignop #131777
+use feature 'signatures';
+no warnings 'experimental::signatures';
+sub foo ($a += 1)
+EXPECT
+Illegal operator following parameter in a subroutine signature at - line 3, near "($a += 1"
+syntax error at - line 3, near "($a += 1"
+Execution of - aborted due to compilation errors.
diff --git a/t/op/signatures.t b/t/op/signatures.t
index f0e1b93..8ab8db3 100644
--- a/t/op/signatures.t
+++ b/t/op/signatures.t
@@ -1095,17 +1095,21 @@ syntax error at foo line 8, near ", 123"
EOF
eval "#line 8 foo\nno warnings; sub t096 (\$a 123) { }";
-is $@, qq{syntax error at foo line 8, near "\$a 123"\n};
+is $@, <<'EOF';
+Illegal operator following parameter in a subroutine signature at foo line 8, near "($a 123"
+syntax error at foo line 8, near "($a 123"
+EOF
eval "#line 8 foo\nsub t097 (\$a { }) { }";
-is $@, <<EOF;
-syntax error at foo line 8, near "\$a { "
+is $@, <<'EOF';
+Illegal operator following parameter in a subroutine signature at foo line 8, near "($a { }"
+syntax error at foo line 8, near "($a { }"
EOF
eval "#line 8 foo\nsub t098 (\$a; \$b) { }";
-is $@, <<EOF;
-syntax error at foo line 8, at EOF
-syntax error at foo line 8, near "\$b) "
+is $@, <<'EOF';
+Illegal operator following parameter in a subroutine signature at foo line 8, near "($a; "
+syntax error at foo line 8, near "($a; "
EOF
eval "#line 8 foo\nsub t099 (\$\$) { }";
diff --git a/toke.c b/toke.c
index 6aa5f26..60a0aac 100644
--- a/toke.c
+++ b/toke.c
@@ -5107,12 +5107,43 @@ Perl_yylex(pTHX)
0, cBOOL(UTF), FALSE);
*dest = '\0';
assert(PL_tokenbuf[1]); /* we have a variable name */
+ }
+ else {
+ *PL_tokenbuf = 0;
+ PL_in_my = 0;
+ }
+
+ s = skipspace(s);
+ /* parse the = for the default ourselves to avoid '+=' etc being accepted here
+ * as the ASSIGNOP, and exclude other tokens that start with =
+ */
+ if (*s == '=' && (!s[1] || strchr("=~>", s[1]) == 0)) {
+ /* save now to report with the same context as we did when
+ * all ASSIGNOPS were accepted */
+ PL_oldbufptr = s;
+
+ ++s;
+ NEXTVAL_NEXTTOKE.ival = 0;
+ force_next(ASSIGNOP);
+ PL_expect = XTERM;
+ }
+ else if (*s == ',' || *s == ')') {
+ PL_expect = XOPERATOR;
+ }
+ else {
+ /* make sure the context shows the unexpected character and
+ * hopefully a bit more */
+ if (*s) ++s;
+ while (*s && *s != '$' && *s != '@' && *s != '%' && *s != ')')
+ s++;
+ PL_bufptr = s; /* for error reporting */
+ yyerror("Illegal operator following parameter in a subroutine signature");
+ PL_in_my = 0;
+ }
+ if (*PL_tokenbuf) {
NEXTVAL_NEXTTOKE.ival = sigil;
force_next('p'); /* force a signature pending identifier */
}
- else
- PL_in_my = 0;
- PL_expect = XOPERATOR;
break;
case ')':
--
2.1.4
|
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release yesterday of Perl 5.28.0, this and 185 other issues have been Perl 5.28.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#131777 (status was 'resolved')
Searchable as RT131777$
The text was updated successfully, but these errors were encountered: