Navigation Menu

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

Change 649d02de (unary prototypes) changes precedence #10546

Closed
p5pRT opened this issue Aug 15, 2010 · 13 comments
Closed

Change 649d02de (unary prototypes) changes precedence #10546

p5pRT opened this issue Aug 15, 2010 · 13 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 15, 2010

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

Searchable as RT77234$

@p5pRT
Copy link
Author

p5pRT commented Aug 15, 2010

From @cpansprout

See http​://www.nntp.perl.org/group/perl.perl5.porters/2010/08/msg162945.html

The problem is that sub foo(;$) is now parsed as a unary function. So code like this will break​:

sub foo(;$) { ... }
foo $a < $b;

because it now parses as foo($a)<$b rather than foo($a<$b).

I thought I had considered all the possibilities of breakage, but this one eluded me.

Should we make this a �feature� feature?

use feature 'sane_prototype_parsing'; # too long
use feature 'unary_proto'; # a bit arcane
use 5.014; # makes the precise name irrelevant

I would be sad to see 649d02d reverted, as the bug it fixes defeats the purpose of change 1db4d19 (making tie overridable) and the original purpose of prototypes (to make subs parse like built-ins).


Flags​:
  category=core
  severity=high


Site configuration information for perl 5.13.3​:

Configured by sprout at Thu Aug 12 17​:53​:37 PDT 2010.

Summary of my perl5 (revision 5 version 13 subversion 3 patch v5.13.3-193-g798ae1b) configuration​:
  Snapshot of​: 798ae1b
  Platform​:
  osname=darwin, osvers=10.4.0, archname=darwin-2level
  uname='darwin pint.local 10.4.0 darwin kernel version 10.4.0​: fri apr 23 18​:28​:53 pdt 2010; root​:xnu-1504.7.4~1release_i386 i386 '
  config_args='-de -Dusedevel'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=undef, use64bitall=undef, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include',
  optimize='-O3',
  cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.2.1 (Apple Inc. build 5664)', gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib
  libs=-ldbm -ldl -lm -lutil -lc
  perllibs=-ldl -lm -lutil -lc
  libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false, libperl=libperl.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 


@​INC for perl 5.13.3​:
  /usr/local/lib/perl5/site_perl/5.13.3/darwin-2level
  /usr/local/lib/perl5/site_perl/5.13.3
  /usr/local/lib/perl5/5.13.3/darwin-2level
  /usr/local/lib/perl5/5.13.3
  /usr/local/lib/perl5/site_perl
  .


Environment for perl 5.13.3​:
  DYLD_LIBRARY_PATH (unset)
  HOME=/Users/sprout
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Aug 15, 2010

From @cpansprout

On Aug 15, 2010, at 1​:17 PM, Father Chrysostomos wrote​:

See http​://www.nntp.perl.org/group/perl.perl5.porters/2010/08/msg162945.html

The problem is that sub foo(;$) is now parsed as a unary function. So code like this will break​:

sub foo(;$) { ... }
foo $a < $b;

because it now parses as foo($a)<$b rather than foo($a<$b).

I thought I had considered all the possibilities of breakage, but this one eluded me.

Should we make this a �feature� feature?

use feature 'sane_prototype_parsing'; # too long
use feature 'unary_proto'; # a bit arcane
use 5.014; # makes the precise name irrelevant

In case this makes any difference to the decision, the existing behaviour is not just undocumented; it actually contradicts the documented behaviour​:

...
  sub mygrep (&@​) mygrep { /foo/ } $a, $b, $c
  sub myrand (;$) myrand 42
  sub mytime () mytime
...
  "myrand()" is parsed as a true unary operator with unary precedence the
  same as "rand()", and "mytime()" is truly without arguments, just like
...

I would be sad to see 649d02d reverted, as the bug it fixes defeats the purpose of change 1db4d19 (making tie overridable) and the original purpose of prototypes (to make subs parse like built-ins).

---
Flags​:
category=core
severity=high
---
Site configuration information for perl 5.13.3​:

Configured by sprout at Thu Aug 12 17​:53​:37 PDT 2010.

Summary of my perl5 (revision 5 version 13 subversion 3 patch v5.13.3-193-g798ae1b) configuration​:
Snapshot of​: 798ae1b
Platform​:
osname=darwin, osvers=10.4.0, archname=darwin-2level
uname='darwin pint.local 10.4.0 darwin kernel version 10.4.0​: fri apr 23 18​:28​:53 pdt 2010; root​:xnu-1504.7.4~1release_i386 i386 '
config_args='-de -Dusedevel'
hint=recommended, useposix=true, d_sigaction=define
useithreads=undef, usemultiplicity=undef
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=undef, use64bitall=undef, uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler​:
cc='cc', ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include',
optimize='-O3',
cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
ccversion='', gccversion='4.2.1 (Apple Inc. build 5664)', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
alignbytes=8, prototype=define
Linker and Libraries​:
ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib'
libpth=/usr/local/lib /usr/lib
libs=-ldbm -ldl -lm -lutil -lc
perllibs=-ldl -lm -lutil -lc
libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false, libperl=libperl.a
gnulibc_version=''
Dynamic Linking​:
dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches​:

---
@​INC for perl 5.13.3​:
/usr/local/lib/perl5/site_perl/5.13.3/darwin-2level
/usr/local/lib/perl5/site_perl/5.13.3
/usr/local/lib/perl5/5.13.3/darwin-2level
/usr/local/lib/perl5/5.13.3
/usr/local/lib/perl5/site_perl
.

---
Environment for perl 5.13.3​:
DYLD_LIBRARY_PATH (unset)
HOME=/Users/sprout
LANG=en_US.UTF-8
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin
PERL_BADLANG (unset)
SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Aug 15, 2010

From zefram@fysh.org

Father Chrysostomos wrote​:

Should we make this a "feature" feature?

use feature 'sane_prototype_parsing'; # too long

Please don't. A much better approach would be to add a facility to the
prototype language to enable the parsing that you favour, such as an
explicit flag controlling whether it parses as a unary operator.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Aug 15, 2010

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

@p5pRT
Copy link
Author

p5pRT commented Aug 15, 2010

From @nwc10

On Sun, Aug 15, 2010 at 01​:17​:37PM -0700, Father Chrysostomos wrote​:

See http​://www.nntp.perl.org/group/perl.perl5.porters/2010/08/msg162945.html

The problem is that sub foo(;$) is now parsed as a unary function. So code like this will break​:

sub foo(;$) { ... }
foo $a < $b;

because it now parses as foo($a)<$b rather than foo($a<$b).

1​: But the changed behaviour is the correct behaviour, by the letter of the
  documentation?

I thought I had considered all the possibilities of breakage, but this one eluded me.

2​: Is it only the specific prototype ';$' where this change will cause
  breakage/fixage?
3​: Or are there other prototypes which will now cause the tokeniser to parse
  differently? [You can infer from that question that I'm not considering
  this stuff one my my strong points].
4​: If the answer to that is "yes", do all the prototypes that now change
  behaviour start with ';'? (ie optional arguments)

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Aug 16, 2010

From @cpansprout

On Sun, 15 Aug 2010 21​:35​:08 +0100, Nicholas Clark wrote​:

On Sun, Aug 15, 2010 at 01​:17​:37PM -0700, Father Chrysostomos wrote​:

See http​://www.nntp.perl.org/group/perl.perl5.porters/2010/08/msg162945.html

The problem is that sub foo(;$) is now parsed as a unary function. So code like this will break​:

sub foo(;$) { ... }
foo $a < $b;

because it now parses as foo($a)<$b rather than foo($a<$b).

1​: But the changed behaviour is the correct behaviour, by the letter of the
documentation?

Yes. Ironically (;$), the prototype that Switch is using, is precisely the one that perlsub uses to explain the effect that prototypes have on precedence.

I thought I had considered all the possibilities of breakage, but this one eluded me.

2​: Is it only the specific prototype ';$' where this change will cause
breakage/fixage?
3​: Or are there other prototypes which will now cause the tokeniser to parse
differently? [You can infer from that question that I'm not considering
this stuff one my my strong points].
4​: If the answer to that is "yes", do all the prototypes that now change
behaviour start with ';'? (ie optional arguments)

(*) (;$) (;*) are the only prototypes affected. (The (\...) and (;\...) prototypes were syntax errors in all the cases that break for those three.)

It only happens when one of these is used in an unparenthesised argument​:

  nonassoc < > <= >= lt gt le ge
  nonassoc == != <=> eq ne cmp ~~
  left &
  left | ^
  left &&
  left || //
  nonassoc .. ...
  right ?​:
  right = += -= *= etc.

I think the best solution in this case is to patch Switch.pm and document this under perldelta as a possible cause for breakage. After all, the documentation is quite explicit.

Iâ��ve submitted a patch for Switch​: http​://rt.cpan.org/Ticket/Display.html?id=60380

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2010

From @iabyn

On Sun, Aug 15, 2010 at 05​:45​:46PM -0700, Father Chrysostomos wrote​:

I think the best solution in this case is to patch Switch.pm and document this under perldelta as a possible cause for breakage. After all, the documentation is quite explicit.

Was a perldelta entry added? If not, could write one?

--
Justice is when you get what you deserve.
Law is when you get what you pay for.

@p5pRT
Copy link
Author

p5pRT commented Aug 29, 2010

From @cpansprout

On Aug 25, 2010, at 6​:42 AM, Dave Mitchell wrote​:

On Sun, Aug 15, 2010 at 05​:45​:46PM -0700, Father Chrysostomos wrote​:

I think the best solution in this case is to patch Switch.pm and document this under perldelta as a possible cause for breakage. After all, the documentation is quite explicit.

Was a perldelta entry added?

Only the fact that it broke Switch.

If not, could write one?

Here is that describes it in greater detail, so users will know what to look out for.

@p5pRT
Copy link
Author

p5pRT commented Aug 29, 2010

From @cpansprout

From​: Father Chrysostomos <sprout@​cpan.org>

[perl #77234] Change 649d02d (unary prototypes) changes precedence

This patch retroactively adds a description of the breakage to
perl5134delta so it will be copied eventually into perl5140delta.

Inline Patch
--- blead/pod/perl5134delta.pod.orig	2010-08-26 20:46:07.000000000 -0700
+++ blead/pod/perl5134delta.pod	2010-08-26 21:22:15.000000000 -0700
@@ -102,6 +102,29 @@ exception if they don't match.
 Some bit fields have been reordered; therefore, this release will not be binary
 compatible with any previous Perl release.
 
+=head2 Change in the parsing of certain prototypes
+
+Due to a bug fix, functions using the C<(*)>, C<(;$)> and C<(;*)>
+prototypes are parsed with higher precedence than before. So in the
+following example:
+
+  sub foo($);
+  foo $a < $b;
+
+the second line is now parsed correctly as C<< foo($a) < $b >>, rather than
+C<< foo($a < $b) >>. This happens when one of these operators is used in
+an unparenthesised argument:
+
+  < > <= >= lt gt le ge
+  == != <=> eq ne cmp ~~
+  &
+  | ^
+  &&
+  || //
+  .. ...
+  ?:
+  = += -= *= etc.
+
 =head1 Deprecations
 
 =head2 List assignment to C<$[>

@p5pRT
Copy link
Author

p5pRT commented Aug 29, 2010

From @rafl

Father Chrysostomos <sprout@​cpan.org> writes​:

On Aug 25, 2010, at 6​:42 AM, Dave Mitchell wrote​:

On Sun, Aug 15, 2010 at 05​:45​:46PM -0700, Father Chrysostomos wrote​:

I think the best solution in this case is to patch Switch.pm and
document this under perldelta as a possible cause for
breakage. After all, the documentation is quite explicit.

Was a perldelta entry added?

Only the fact that it broke Switch.

There also was an entry about the changed parsing in perl5134delta, but
it wasn't nearly as good as what you provided in your patch.

If not, could write one?

Here is that describes it in greater detail, so users will know what
to look out for.

What should we do with this? 5.13.4, which contains the change, is
already out. We don't usually provide explanations of previous changes
in later perldeltas. Should this be applied to 5134delta, and the
original entry be removed, so compiling the 5140delta becomes easier?

@p5pRT
Copy link
Author

p5pRT commented Aug 30, 2010

From @cpansprout

On Aug 29, 2010, at 3​:19 PM, Florian Ragwitz wrote​:

Father Chrysostomos <sprout@​cpan.org> writes​:

On Aug 25, 2010, at 6​:42 AM, Dave Mitchell wrote​:

On Sun, Aug 15, 2010 at 05​:45​:46PM -0700, Father Chrysostomos wrote​:

I think the best solution in this case is to patch Switch.pm and
document this under perldelta as a possible cause for
breakage. After all, the documentation is quite explicit.

Was a perldelta entry added?

Only the fact that it broke Switch.

There also was an entry about the changed parsing in perl5134delta, but
it wasn't nearly as good as what you provided in your patch.

If not, could write one?

Here is that describes it in greater detail, so users will know what
to look out for.

What should we do with this? 5.13.4, which contains the change, is
already out. We don't usually provide explanations of previous changes
in later perldeltas. Should this be applied to 5134delta, and the
original entry be removed, so compiling the 5140delta becomes easier?

That was the idea, except the original entry should stay, as it is the only place that mentions all the prototypes that were fixed. But maybe it could be moved to �Selected Bug Fixes�, since it�s not a new feature.

@p5pRT
Copy link
Author

p5pRT commented Aug 31, 2010

From @rafl

Father Chrysostomos <sprout@​cpan.org> writes​:

On Aug 29, 2010, at 3​:19 PM, Florian Ragwitz wrote​:

What should we do with this? 5.13.4, which contains the change, is
already out. We don't usually provide explanations of previous changes
in later perldeltas. Should this be applied to 5134delta, and the
original entry be removed, so compiling the 5140delta becomes easier?

That was the idea, except the original entry should stay, as it is the
only place that mentions all the prototypes that were fixed. But maybe
it could be moved to �Selected Bug Fixes�, since it�s not a new
feature.

Good point. I've applied your patch and moved those two entries
together. Thanks!

@p5pRT
Copy link
Author

p5pRT commented Aug 31, 2010

@rafl - Status changed from 'open' 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