Skip Menu |
Report information
Id: 128478
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors:
Cc:
AdminCc:

Operating System: All
PatchStatus: (no value)
Severity: low
Type:
Perl Version:
  • 5.18.0
  • 5.25.2
Fixed In: (no value)



Subject: Change in behaviour of "$foo::$bar" in 5.18
Download (untitled) / with headers
text/plain 1.9k
In perl 5.16 and earlier, "$foo::$bar" parsed as: $foo:: . $bar In perl 5.18, it accidentally started being parsed as: $foo . "::" . $bar (I say accidentally, because commit 07f7264624e0, which made the change, does not appear to have been made for this purpose, and its commit message does not even hint at this particular change in parsing.) Now one module, Sub::Attribute (admittedly brand new and still at 0.01), has inadvertently started relying on the new behaviour. (See <https://rt.cpan.org/Ticket/Display.html?id=115570>.) The *real* problem is that we now have a discrepancy between "$foo::$bar" and "$foo::@bar": $ ./perl -Ilib -MO=Deparse,-q -e '"$foo::$bar"' $foo . '::' . $bar; -e syntax OK $ ./perl -Ilib -MO=Deparse,-q -e '"$foo::@bar"' $foo:: . join($", @bar); -e syntax OK So, what should we do about this? Should we (a) revert to the old behaviour? (Fixing that one module is easy enough.) Or should we (b) make "$foo::@bar" consistent with "$foo::$bar". I prefer option a. Option b is harder, in that we have to decide where exactly to draw the line. Do we stop $foo:: from being interpolated at all? Do we only cut it off at $foo before $ and @? Etc. The change, BTW, did not only affect double-quoted strings, but also print $foo::$bar: $ perl5.14.4 -e 'print $foo::$bar' Scalar found where operator expected at -e line 1, near "$foo::$bar" (Missing operator before $bar?) Can't use an undefined value as a symbol reference at -e line 1. (The ‘Scalar found where operator expected’ is bogus. [I reported that in another ticket some years ago.] The fact that it does with ‘Can't use an undefined value’ shows that the program compiles and runs.) $ perl5.18.1 -e 'print $foo::$bar' Bad name after :: at -e line 1. Now we have a very unhelpful error message. (The same unhelpful message you get for ‘foo::bar::$baz’, at least as far back as 5.8.7.) Also, B::Deparse is wrong now: $ ./perl -Ilib -MO=Deparse -e '"${foo::}$a"' "$foo::$a"; -e syntax OK -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 358b
So far no response.... On Fri Jun 24 21:51:13 2016, sprout wrote: Show quoted text
> So, what should we do about this? Should we (a) revert to the old > behaviour? (Fixing that one module is easy enough.) Or should we (b) > make "$foo::@bar" consistent with "$foo::$bar". > > I prefer option a.
And here is a patch for it. Should I apply it? -- Father Chrysostomos
Subject: open_YHrrB0p9.txt
From b00321ceb71e521c8cef2ddc1cc63f1f890f901d Mon Sep 17 00:00:00 2001 From: Father Chrysostomos <sprout@cpan.org> Date: Sun, 26 Jun 2016 21:45:22 -0700 Subject: [PATCH] [perl #128478] Restore former "$foo::$bar" parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The function scan_word, in toke.c, is used to parse barewords. The scan_ident function is used to scan an identifier after a sigil. Prior to v5.17.9-108-g07f7264, both functions had their own parsing loops, and scan_ident actually had two, one for $foo and another for ${foo}. The state purpose of 07f7264 was to fix discrepancies in the parsing of $foo vs ${foo}, by making the two forms use the same parsing code. In accomplishing this, the commit in question merged not only the two loops in scan_ident, but all three loops, including the one in scan_word, by introducing a new function, parse_ident, that the others call. One result was that some logic appropriate only to scan_word started to be applied also to scan_ident; namely, that ::$ would be explicitly checked for and disallowed (the parsing would stop before the ::), for the sake of the “Bad name after Foo::” error. The consequence was that "$foo::$bar" started to be parsed as $foo."::".$bar, instead of $foo:: . $bar, as previously. Now, "$foo::@bar" was unaffected, so by fixing one form of inconsis- tency we ended up form, including B::Deparse bugs (because B::Deparse was not consistent with the core). This commit restores the previous behaviour by giving parse_ident an extra parameter, making the ::$ check optional. diff --git a/embed.fnc b/embed.fnc index cd4b0ff..fe5eb68 100644 --- a/embed.fnc +++ b/embed.fnc @@ -2548,7 +2548,7 @@ s |int |deprecate_commaless_var_list s |int |ao |int toketype s |void|parse_ident|NN char **s|NN char **d \ |NN char * const e|int allow_package \ - |bool is_utf8 + |bool is_utf8|bool check_dollar # if defined(PERL_CR_FILTER) s |I32 |cr_textfilter |int idx|NULLOK SV *sv|int maxlen s |void |strip_return |NN SV *sv diff --git a/embed.h b/embed.h index 84f647e..de5a590 100644 --- a/embed.h +++ b/embed.h @@ -1784,7 +1784,7 @@ #define lop(a,b,c) S_lop(aTHX_ a,b,c) #define missingterm(a) S_missingterm(aTHX_ a) #define no_op(a,b) S_no_op(aTHX_ a,b) -#define parse_ident(a,b,c,d,e) S_parse_ident(aTHX_ a,b,c,d,e) +#define parse_ident(a,b,c,d,e,f) S_parse_ident(aTHX_ a,b,c,d,e,f) #define pending_ident() S_pending_ident(aTHX) #define scan_const(a) S_scan_const(aTHX_ a) #define scan_formline(a) S_scan_formline(aTHX_ a) diff --git a/proto.h b/proto.h index 42d78cb..9bdbac7 100644 --- a/proto.h +++ b/proto.h @@ -5496,7 +5496,7 @@ STATIC SV* S_new_constant(pTHX_ const char *s, STRLEN len, const char *key, STRL STATIC void S_no_op(pTHX_ const char *const what, char *s); #define PERL_ARGS_ASSERT_NO_OP \ assert(what) -STATIC void S_parse_ident(pTHX_ char **s, char **d, char * const e, int allow_package, bool is_utf8); +STATIC void S_parse_ident(pTHX_ char **s, char **d, char * const e, int allow_package, bool is_utf8, bool check_dollar); #define PERL_ARGS_ASSERT_PARSE_IDENT \ assert(s); assert(d); assert(e) STATIC int S_pending_ident(pTHX); diff --git a/t/base/lex.t b/t/base/lex.t index fe46f14..4ac2b5b 100644 --- a/t/base/lex.t +++ b/t/base/lex.t @@ -1,6 +1,6 @@ #!./perl -print "1..105\n"; +print "1..107\n"; $x = 'x'; @@ -528,3 +528,10 @@ eval q|s##[}#e|; eval ('/@0{0*->@*/*]'); print "ok $test - 128171\n"; $test++; } + +$foo = "WRONG"; $foo:: = "bar"; $bar = "baz"; +print "not " unless "$foo::$bar" eq "barbaz"; +print qq|ok $test - [perl #128478] "\$foo::\$bar"\n|; $test++; +@bar = ("baz","bonk"); +print "not " unless "$foo::@bar" eq "barbaz bonk"; +print qq|ok $test - [perl #128478] "\$foo::\@bar"\n|; $test ++; diff --git a/toke.c b/toke.c index 327d984..aebeebb 100644 --- a/toke.c +++ b/toke.c @@ -8819,7 +8819,8 @@ S_new_constant(pTHX_ const char *s, STRLEN len, const char *key, STRLEN keylen, } PERL_STATIC_INLINE void -S_parse_ident(pTHX_ char **s, char **d, char * const e, int allow_package, bool is_utf8) { +S_parse_ident(pTHX_ char **s, char **d, char * const e, int allow_package, + bool is_utf8, bool check_dollar) { PERL_ARGS_ASSERT_PARSE_IDENT; for (;;) { @@ -8855,7 +8856,7 @@ S_parse_ident(pTHX_ char **s, char **d, char * const e, int allow_package, bool * the code path that triggers the "Bad name after" warning * when looking for barewords. */ - && (*s)[2] != '$') { + && !(check_dollar && (*s)[2] == '$')) { *(*d)++ = *(*s)++; *(*d)++ = *(*s)++; } @@ -8877,7 +8878,7 @@ S_scan_word(pTHX_ char *s, char *dest, STRLEN destlen, int allow_package, STRLEN PERL_ARGS_ASSERT_SCAN_WORD; - parse_ident(&s, &d, e, allow_package, is_utf8); + parse_ident(&s, &d, e, allow_package, is_utf8, TRUE); *d = '\0'; *slp = d - dest; return s; @@ -8925,7 +8926,7 @@ S_scan_ident(pTHX_ char *s, char *dest, STRLEN destlen, I32 ck_uni) } } else { /* See if it is a "normal" identifier */ - parse_ident(&s, &d, e, 1, is_utf8); + parse_ident(&s, &d, e, 1, is_utf8, FALSE); } *d = '\0'; d = dest; @@ -8994,7 +8995,7 @@ S_scan_ident(pTHX_ char *s, char *dest, STRLEN destlen, I32 ck_uni) (the later check for } being at the expected point will trap cases where this doesn't pan out.) */ d += is_utf8 ? UTF8SKIP(d) : 1; - parse_ident(&s, &d, e, 1, is_utf8); + parse_ident(&s, &d, e, 1, is_utf8, TRUE); *d = '\0'; tmp_copline = CopLINE(PL_curcop); if (s < PL_bufend && isSPACE(*s)) { @@ -11875,7 +11876,8 @@ S_parse_opt_lexvar(pTHX) s = PL_bufptr; d = PL_tokenbuf + 1; PL_tokenbuf[0] = (char)sigil; - parse_ident(&s, &d, PL_tokenbuf + sizeof(PL_tokenbuf) - 1, 0, cBOOL(UTF)); + parse_ident(&s, &d, PL_tokenbuf + sizeof(PL_tokenbuf) - 1, 0, + cBOOL(UTF), FALSE); PL_bufptr = s; if (d == PL_tokenbuf+1) return NULL;
Subject: Re: [perl #128478] Change in behaviour of "$foo::$bar" in 5.18
From: Brian Fraser <fraserbn [...] gmail.com>
To: Brian Fraser via RT <perlbug-followup [...] perl.org>
CC: Perl5 Porters Mailing List <perl5-porters [...] perl.org>
Date: Mon, 27 Jun 2016 09:08:57 +0200
Download (untitled) / with headers
text/plain 516b
On 27 June 2016 at 06:53, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> So far no response.... > > On Fri Jun 24 21:51:13 2016, sprout wrote:
>> So, what should we do about this? Should we (a) revert to the old >> behaviour? (Fixing that one module is easy enough.) Or should we (b) >> make "$foo::@bar" consistent with "$foo::$bar". >> >> I prefer option a.
> > And here is a patch for it. Should I apply it?
++ I entirely missed this when writing that commit; the fix looks spot-on to me.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 666b
On Mon Jun 27 00:09:29 2016, Hugmeir wrote: Show quoted text
> On 27 June 2016 at 06:53, Father Chrysostomos via RT > <perlbug-followup@perl.org> wrote:
> > So far no response.... > > > > On Fri Jun 24 21:51:13 2016, sprout wrote:
> >> So, what should we do about this? Should we (a) revert to the old > >> behaviour? (Fixing that one module is easy enough.) Or should we > >> (b) > >> make "$foo::@bar" consistent with "$foo::$bar". > >> > >> I prefer option a.
> > > > And here is a patch for it. Should I apply it?
> > ++ > > I entirely missed this when writing that commit; the fix looks spot-on > to me.
Thank you. I have pushed it as d9d2b74. -- Father Chrysostomos
To: perl5-porters [...] perl.org
Subject: Re: [perl #128478] Change in behaviour of "$foo::$bar" in 5.18
From: Sawyer X <xsawyerx [...] gmail.com>
Date: Wed, 29 Jun 2016 22:24:59 +0200
Download (untitled) / with headers
text/plain 921b
On 06/27/2016 03:33 PM, Father Chrysostomos via RT wrote: Show quoted text
> On Mon Jun 27 00:09:29 2016, Hugmeir wrote:
>> On 27 June 2016 at 06:53, Father Chrysostomos via RT >> <perlbug-followup@perl.org> wrote:
>>> So far no response.... >>> >>> On Fri Jun 24 21:51:13 2016, sprout wrote:
>>>> So, what should we do about this? Should we (a) revert to the old >>>> behaviour? (Fixing that one module is easy enough.) Or should we >>>> (b) >>>> make "$foo::@bar" consistent with "$foo::$bar". >>>> >>>> I prefer option a.
>>> And here is a patch for it. Should I apply it?
>> ++ >> >> I entirely missed this when writing that commit; the fix looks spot-on >> to me.
> Thank you. I have pushed it as d9d2b74.
I didn't get to respond, but mine was ++ as well. If we start with making "$foo::@bar" do something else, we'll have to also add "$foo::%bar", and then bikeshed any DWIM angle for it. Can of $something. :) Thank you.
Date: Thu, 30 Jun 2016 07:53:35 +0200
Subject: Re: [perl #128478] Change in behaviour of "$foo::$bar" in 5.18
From: Andreas Koenig <andreas.koenig.7os6VVqR [...] franz.ak.mind.de>
CC: perl5-porters [...] perl.org
To: "Father Chrysostomos via RT" <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 440b
Show quoted text
>>>>> On Mon, 27 Jun 2016 06:33:51 -0700, "Father Chrysostomos via RT" <perlbug-followup@perl.org> said:
Show quoted text
> Thank you. I have pushed it as d9d2b74.
Since this ticket is not closed yet, we probably need no BBC ticket. Please look into http://matrix.cpantesters.org/?dist=Tie-SecureHash-1.10 Does the report of my smoker http://www.cpantesters.org/cpan/report/7d363fa4-3dc0-11e6-b130-d1b85bc2a771 match your expectations? -- andreas
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 657b
On Wed Jun 29 22:54:13 2016, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote: Show quoted text
> >>>>> On Mon, 27 Jun 2016 06:33:51 -0700, "Father Chrysostomos via RT" > >>>>> <perlbug-followup@perl.org> said:
>
> > Thank you. I have pushed it as d9d2b74.
> > Since this ticket is not closed yet, we probably need no BBC ticket. > > Please look into http://matrix.cpantesters.org/?dist=Tie-SecureHash- > 1.10 > > Does the report of my smoker > http://www.cpantesters.org/cpan/report/7d363fa4-3dc0-11e6-b130- > d1b85bc2a771 > match your expectations?
Yes, it does. I have filed a ticket at <https://rt.cpan.org/Ticket/Display.html?id=115772>. -- Father Chrysostomos
Download (untitled) / with headers
text/plain 313b
Thank 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 resolved. Perl 5.26.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.26.0 If you find that the problem persists, feel free to reopen this ticket.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org