Skip to content
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

signatures accept fancy assignment operators #16084

Closed
p5pRT opened this issue Jul 22, 2017 · 13 comments
Closed

signatures accept fancy assignment operators #16084

p5pRT opened this issue Jul 22, 2017 · 13 comments

Comments

@p5pRT
Copy link

p5pRT commented Jul 22, 2017

Migrated from rt.perl.org#131777 (status was 'resolved')

Searchable as RT131777$

@p5pRT
Copy link
Author

p5pRT commented Jul 22, 2017

From zefram@fysh.org

Created by zefram@fysh.org

$ perl5.26.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a } print z()'
3

In 5.26 the signature syntax now accepts any kind of assignment operator
(+=, ^=, &&=, etc.) to introduce the default value for a scalar parameter.
The semantics are always the same, simple defaulting, unaffected by the
binop type embedded in the assignment operator. This is clearly not an
intentional feature​: it's undocumented and misleading.

It was introduced by the reimplementation of signature parsing as part
of the yacc grammar. The original implementation signals an error for
such syntax.

$ perl5.24.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a } print z()'
Parse error at -e line 1.
syntax error at -e line 1, near "$a -"
Execution of -e aborted due to compilation errors.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.26.0:

Configured by zefram at Thu Jun  1 13:52:05 BST 2017.

Summary of my perl5 (revision 5 version 26 subversion 0) configuration:
   
  Platform:
    osname=linux
    osvers=3.16.0-4-amd64
    archname=x86_64-linux-thread-multi
    uname='linux barba.rous.org 3.16.0-4-amd64 #1 smp debian 3.16.7-ckt11-1+deb8u6 (2015-11-09) x86_64 gnulinux '
    config_args='-des -Dprefix=/home/zefram/usr/perl/perl_install/perl-5.26.0-i64-f52 -Duselargefiles -Dusethreads -Uafs -Ud_csh -Uusesfio -Uusenm -Duseshrplib -Dusedevel -Uversiononly -Ui_db'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
    optimize='-O2'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
    ccversion=''
    gccversion='4.9.2'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
    libs=-lpthread -lnsl -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.19.so
    so=so
    useshrplib=true
    libperl=libperl.so
    gnulibc_version='2.19'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E -Wl,-rpath,/home/zefram/usr/perl/perl_install/perl-5.26.0-i64-f52/lib/5.26.0/x86_64-linux-thread-multi/CORE'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'



@INC for perl 5.26.0:
    /home/zefram/usr/perl/perl_install/perl-5.26.0-i64-f52/lib/site_perl/5.26.0/x86_64-linux-thread-multi
    /home/zefram/usr/perl/perl_install/perl-5.26.0-i64-f52/lib/site_perl/5.26.0
    /home/zefram/usr/perl/perl_install/perl-5.26.0-i64-f52/lib/5.26.0/x86_64-linux-thread-multi
    /home/zefram/usr/perl/perl_install/perl-5.26.0-i64-f52/lib/5.26.0


Environment for perl 5.26.0:
    HOME=/home/zefram
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/zefram/usr/perl/perl_install/perl-5.26.0-i64-f52/bin:/home/zefram/usr/perl/util:/home/zefram/pub/x86_64-unknown-linux-gnu/bin:/home/zefram/pub/common/bin:/usr/bin:/bin:/usr/local/bin:/usr/games
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Jul 23, 2017

From @jkeenan

On Sat, 22 Jul 2017 00​:14​:03 GMT, zefram@​fysh.org wrote​:

This is a bug report for perl from zefram@​fysh.org,
generated with the help of perlbug 1.40 running under perl 5.26.0.

-----------------------------------------------------------------
[Please describe your issue here]

$ perl5.26.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a }
print z()'
3

In 5.26 the signature syntax now accepts any kind of assignment
operator
(+=, ^=, &&=, etc.) to introduce the default value for a scalar
parameter.
The semantics are always the same, simple defaulting, unaffected by
the
binop type embedded in the assignment operator. This is clearly not
an
intentional feature​: it's undocumented and misleading.

It was introduced by the reimplementation of signature parsing as part
of the yacc grammar. The original implementation signals an error for
such syntax.

$ perl5.24.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a }
print z()'
Parse error at -e line 1.
syntax error at -e line 1, near "$a -"
Execution of -e aborted due to compilation errors.

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.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jul 23, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Jul 23, 2017

From zefram@fysh.org

James E Keenan via RT wrote​:

I wonder if the 5.26 behavior, though unintended undocumented, might
actually be desirable in certain circumstances.

No, never. It would have been a reasonable design decision to use some
decorated form of "=" rather than bare "=" to represent the defaulting
semantic, in order to distinguish it from simple assignment. But it would
have had to be a novel form of decoration ("!!=", say), because the use
of any existing binop would imply that that binop's behaviour is involved,
and that's not the case for any existing binop. (I don't think the extra
length of a novel operator would be justified; I'm quite satisfied with
the bare "=", and that was an uncontroversial part of the syntax design.)

-zefram

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2017

From @tonycoz

On Fri, 21 Jul 2017 17​:14​:03 -0700, zefram@​fysh.org wrote​:

$ perl5.26.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a }
print z()'
3

In 5.26 the signature syntax now accepts any kind of assignment
operator
(+=, ^=, &&=, etc.) to introduce the default value for a scalar
parameter.
The semantics are always the same, simple defaulting, unaffected by
the
binop type embedded in the assignment operator. This is clearly not
an
intentional feature​: it's undocumented and misleading.

It was introduced by the reimplementation of signature parsing as part
of the yacc grammar. The original implementation signals an error for
such syntax.

$ perl5.24.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a }
print z()'
Parse error at -e line 1.
syntax error at -e line 1, near "$a -"
Execution of -e aborted due to compilation errors.

The attached prevents non-simple assignment from being accepted.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2017

From @tonycoz

0001-perl-131777-prevent-non-assign-ops-tokens-in-sub-sig.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2017

From dcmertens.perl@gmail.com

Missing closing single quote in docs?

use feature 'signatures;

On Mon, Jul 24, 2017 at 9​:52 PM, Tony Cook via RT <perlbug-followup@​perl.org

wrote​:

On Fri, 21 Jul 2017 17​:14​:03 -0700, zefram@​fysh.org wrote​:

$ perl5.26.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a }
print z()'
3

In 5.26 the signature syntax now accepts any kind of assignment
operator
(+=, ^=, &&=, etc.) to introduce the default value for a scalar
parameter.
The semantics are always the same, simple defaulting, unaffected by
the
binop type embedded in the assignment operator. This is clearly not
an
intentional feature​: it's undocumented and misleading.

It was introduced by the reimplementation of signature parsing as part
of the yacc grammar. The original implementation signals an error for
such syntax.

$ perl5.24.0 -Mexperimental=signatures -lwe 'sub z ($a -= 3) { $a }
print z()'
Parse error at -e line 1.
syntax error at -e line 1, near "$a -"
Execution of -e aborted due to compilation errors.

The attached prevents non-simple assignment from being accepted.

Tony

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=131777

From 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 d3d9da4 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 '\)'&#8203;:

--
2.1.4

--
"Debugging is twice as hard as writing the code in the first place.
  Therefore, if you write the code as cleverly as possible, you are,
  by definition, not smart enough to debug it." -- Brian Kernighan

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2017

From @tonycoz

On Mon, 24 Jul 2017 19​:50​:43 -0700, run4flat wrote​:

Missing closing single quote in docs?

use feature 'signatures;

Thanks, fixed in the attached.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jul 25, 2017

From @tonycoz

0001-perl-131777-prevent-non-assign-ops-tokens-in-sub-sig.patch
From 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

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2017

From @tonycoz

On Mon, 24 Jul 2017 21​:40​:20 -0700, tonyc wrote​:

On Mon, 24 Jul 2017 19​:50​:43 -0700, run4flat wrote​:

Missing closing single quote in docs?

use feature 'signatures;

Thanks, fixed in the attached.

Applied as 08ccc81.

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2017

@tonycoz - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

From @khwilliamson

Thank 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
resolved.

Perl 5.28.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.28.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT p5pRT closed this as completed Jun 23, 2018
@p5pRT
Copy link
Author

p5pRT commented Jun 23, 2018

@khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant