Bug #72505 for IO-Compress: Misuse of length(@array)

Thu Nov 17 16:15:39 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Ticket created

CC:	perl5-porters [...] perl.org, wolfsage [...] gmail.com
Subject:	Misuse of length(@array)

length(@array) puts @array in scalar context, so @array evaluates to the length of the array, and then length() takes the length of the resulting number. This is almost never correct, so Matthew Horsfall submitted a patch to perl to add a warning (<nntp://nntp.perl.org/CAJ0K8bgyMLK30p6cJTx8ESye-CrguigjwQf2WKES-DuwJYGbwA@mail.gmail.com>). This causes test failures, due to the length(@$data) in Extra.pm: length() used on @array (did you mean "scalar(@array)"?) at ../lib/IO/Compress/Zlib/Extra.pm line 198. I’m pretty sure that line of code is wrong, but I haven’t looked at it too closely. Is there any chance you could make a new IO-Compress release with that fixed, so we can integrate it into blead and apply Mr. Horsfall’s patch? You can test it against a patched perl by checking out the sprout/length-warning branch or by downloading a snapshot from <http://perl5.git.perl.org/perl.git/snapshot/7187e4df1c22bf5f70c9e024abbc3b67c1f2ad7e.tar.gz>.

Thu Nov 17 18:43:03 2011 pmqs [...] cpan.org - Correspondence added

Thanks for the bug report. As it happens the same issue was reported last week in #72329. Amazing - this bug has probably been lurking for a few years, then it gets reported twice in the space of a week. Good idea having the new warning, by the way. I can't think of a valid use of length(@array) off hand. I've checked my fix works with the new warnings patch. Just senting a refresh (version 2.042) of all the *::Compress::* modules to CPAN now. cheers Paul

Thu Nov 17 18:43:03 2011 The RT System itself - Status changed from 'new' to 'open'

Thu Nov 17 19:07:25 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Thu Nov 17 18:43:03 2011, PMQS wrote: Show quoted text

> Thanks for the bug report. As it happens the same issue was reported > last week in #72329. Amazing - this bug has probably been lurking for a > few years, then it gets reported twice in the space of a week.

Actually, it’s not so amazing, as it was the same person who reported that bug and submitted the warning patch. :-) Show quoted text

> > Good idea having the new warning, by the way. I can't think of a valid > use of length(@array) off hand. > > I've checked my fix works with the new warnings patch. > > Just senting a refresh (version 2.042) of all the *::Compress::* modules > to CPAN now. > > cheers > Paul

Fri Nov 18 05:59:22 2011 pmqs [...] cpan.org - Correspondence added

On Thu Nov 17 19:07:25 2011, SPROUT wrote: Show quoted text

> On Thu Nov 17 18:43:03 2011, PMQS wrote:

> > Thanks for the bug report. As it happens the same issue was reported > > last week in #72329. Amazing - this bug has probably been lurking

> for a

> > few years, then it gets reported twice in the space of a week.

> > Actually, it’s not so amazing, as it was the same person who reported > that bug and submitted > the warning patch. :-)

Duh! :-)

Fri Nov 18 06:55:03 2011 pmqs [...] cpan.org - Status changed from 'open' to 'resolved'

Fri Nov 18 06:55:04 2011 pmqs [...] cpan.org - Fixed in 2.042 added

Fri Nov 18 11:39:24 2011 WolfSage [...] cpan.org - Correspondence added

Oh, sorry about that. I knew about the test failures and meant to ask for a new release so it wouldn't cause problems for make test in blead once my patch was incorporated but I sort of dropped the ball on that one. Thanks FC for taking care of that, sorry to waste your time :) And thanks Paul for handling that so quick! Cheers, -- Matthew Horsfall (alh)

Fri Nov 18 11:39:26 2011 The RT System itself - Status changed from 'resolved' to 'open'

Fri Nov 18 12:14:25 2011 pmqs [...] cpan.org - Correspondence added

On Fri Nov 18 11:39:24 2011, WOLFSAGE wrote: Show quoted text

> Oh, sorry about that.

No worries Matthew. The "Duh" was to myself for not noticing that the two bug repors ogiginated from the same source. Paul

Sat Nov 19 06:33:10 2011 pmqs [...] cpan.org - Status changed from 'open' to 'resolved'

Sun Mar 25 10:40:34 2012 gisle [...] ActiveState.com - Correspondence added

CC:	perl5-porters [...] perl.org, wolfsage [...] gmail.com
Subject:	Re: [rt.cpan.org #72505] Misuse of length(@array)
Date:	Sun, 25 Mar 2012 16:40:12 +0200
To:	bug-IO-Compress [...] rt.cpan.org
From:	Gisle Aas <gisle [...] activestate.com>

On 18. nov. 2011, at 00:43, Paul Marquess via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=72505 > > > Thanks for the bug report. As it happens the same issue was reported > last week in #72329. Amazing - this bug has probably been lurking for a > few years, then it gets reported twice in the space of a week. > > Good idea having the new warning, by the way. I can't think of a valid > use of length(@array) off hand.

I upgraded to 5.16-tobe today and it started complaining about my code, saying: length() used on @res (did you mean "scalar(@res)"?) This was was perfectly fine code that I now had to change because of this warning. The code looked like this: my $count_width = length(@res); for (@res) { $count++; printf "%*d: %s %s\n", $count_width, $count, $_->[0], $_->[1]; } As you can see there are certainly valid uses of length(@array). I would prefer for that warning to not be there. --Gisle

Sun Mar 25 10:40:35 2012 The RT System itself - Status changed from 'resolved' to 'open'

Sun Mar 25 12:35:07 2012 ikegami [...] adaelis.com - Correspondence added

CC:	bug-IO-Compress [...] rt.cpan.org, perl5-porters [...] perl.org, wolfsage [...] gmail.com
Subject:	Re: [rt.cpan.org #72505] Misuse of length(@array)
Date:	Sun, 25 Mar 2012 12:34:55 -0400
To:	Gisle Aas <gisle [...] activestate.com>
From:	Eric Brine <ikegami [...] adaelis.com>

On Sun, Mar 25, 2012 at 10:40 AM, Gisle Aas <gisle@activestate.com> wrote: Show quoted text

> This was was perfectly fine code that I now had to change because of this > warning. > > As you can see there are certainly valid uses of length(@array). I would > prefer for that warning to not be there. >

There are certainly valid uses of numification of non-numeric strings, stringification of undef, deep recursion, ambiguous use of operators, etc, but those warn too. The criteria isn't whether it has valid uses or not, it's a question of how many time it's used, and for what percentage of those times it's used incorrectly. - Eric

Sun Mar 25 15:31:29 2012 gisle [...] ActiveState.com - Correspondence added

CC:	bug-IO-Compress [...] rt.cpan.org, perl5-porters [...] perl.org, wolfsage [...] gmail.com
Subject:	Re: [rt.cpan.org #72505] Misuse of length(@array)
Date:	Sun, 25 Mar 2012 21:31:11 +0200
To:	Eric Brine <ikegami [...] adaelis.com>
From:	Gisle Aas <gisle [...] activestate.com>

On 25. mars 2012, at 18:34, Eric Brine wrote: Show quoted text

> On Sun, Mar 25, 2012 at 10:40 AM, Gisle Aas <gisle@activestate.com> wrote: > This was was perfectly fine code that I now had to change because of this warning. > > As you can see there are certainly valid uses of length(@array). I would prefer for that warning to not be there. > > There are certainly valid uses of numification of non-numeric strings, stringification of undef, deep recursion, ambiguous use of operators, etc, but those warn too. The criteria isn't whether it has valid uses or not, it's a question of how many time it's used, and for what percentage of those times it's used incorrectly.

It's not so problematic for me that there are warnings for run-time conditions that might indicate errors. These are conditions that the programmer might not have expected and it's good to make her be explicit about it. But complaining about length(@array) is a pure static warning. If we don't want people to be able to say that, why not just make it a syntax error then? We can even introduce this using a feature to stay backwards compatible. I don't think I ever seen a program that use length(@array) incorrectly. Are we for instance able to point to code on CPAN that use this construct incorrectly? I'm sure there have been novice Perl programmers that have attempted to write length(@array) when they shouldn't, but the error would be pretty obvious from the result you get. --Gisle

Sun Mar 25 19:38:59 2012 doy [...] tozt.net - Correspondence added

CC:	'spro^^%^6ut# [...] &$%*c.cpan.rt.develooper.com, perl5-porters [...] perl.org, wolfsage [...] gmail.com
Subject:	Re: [rt.cpan.org #72505] Misuse of length(@array)
Date:	Sun, 25 Mar 2012 18:38:50 -0500
To:	Gisle Aas via RT <bug-IO-Compress [...] rt.cpan.org>
From:	Jesse Luehrs <doy [...] tozt.net>

On Sun, Mar 25, 2012 at 03:31:30PM -0400, Gisle Aas via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=72505 > > > > On 25. mars 2012, at 18:34, Eric Brine wrote: >

> > On Sun, Mar 25, 2012 at 10:40 AM, Gisle Aas <gisle@activestate.com> wrote: > > This was was perfectly fine code that I now had to change because of this warning. > > > > As you can see there are certainly valid uses of length(@array). I would prefer for that warning to not be there. > > > > There are certainly valid uses of numification of non-numeric > > strings, stringification of undef, deep recursion, ambiguous use of > > operators, etc, but those warn too. The criteria isn't whether it > > has valid uses or not, it's a question of how many time it's used, > > and for what percentage of those times it's used incorrectly.

> > It's not so problematic for me that there are warnings for run-time > conditions that might indicate errors. These are conditions that the > programmer might not have expected and it's good to make her be > explicit about it. But complaining about length(@array) is a pure > static warning. If we don't want people to be able to say that, why > not just make it a syntax error then? We can even introduce this > using a feature to stay backwards compatible.

Making it a syntax error would not be in line with our deprecation policy. Deprecating existing syntax requires a warning for at least one full stable release cycle before we can actually make it an error. Show quoted text

> I don't think I ever seen a program that use length(@array) > incorrectly. Are we for instance able to point to code on CPAN that > use this construct incorrectly?

I can see quite a few misuses of length(@array) in http://grep.cpan.me/?q=%5Cblength%5C%28%5C%40 Show quoted text

> I'm sure there have been novice Perl programmers that have attempted > to write length(@array) when they shouldn't, but the error would be > pretty obvious from the result you get.

Only if the code is on a common execution path which is actually tested (and if the test actually would be different - testing length(@a) == 1 would succeed in both interpretations, for instance). -doy

Mon Mar 26 17:32:45 2012 gisle [...] ActiveState.com - Correspondence added

CC:	Gisle Aas via RT <bug-IO-Compress [...] rt.cpan.org>, perl5-porters [...] perl.org, wolfsage [...] gmail.com
Subject:	Re: [rt.cpan.org #72505] Misuse of length(@array)
Date:	Mon, 26 Mar 2012 23:32:16 +0200
To:	Jesse Luehrs <doy [...] tozt.net>
From:	Gisle Aas <gisle [...] activestate.com>

On 26. mars 2012, at 01:38, Jesse Luehrs wrote: Show quoted text

>> I don't think I ever seen a program that use length(@array) >> incorrectly. Are we for instance able to point to code on CPAN that >> use this construct incorrectly?

> > I can see quite a few misuses of length(@array) in > http://grep.cpan.me/?q=%5Cblength%5C%28%5C%40

Indeed. I withdraw my complaint and I have rewritten my code to use length(scalar(@array)). --Gisle

Sat Apr 14 14:53:47 2012 pmqs [...] cpan.org - Status changed from 'open' to 'resolved'