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

Owner: Nobody
Requestors: pnewton [at] gmx.de
Cc:
AdminCc:

Operating System: All
PatchStatus: HasPatch
Severity: low
Type:
Perl Version:
  • 5.15.9
  • 5.25.3
Fixed In: 5.27.7

Attachments
0001-perl-6997-ck_concat-is-setting-OPf_STACKED-when-it-s.patch
0002-RT-6997-Test-for-warnings-for-a.-b.-c.patch



From: "Philip Newton" <pnewton [...] gmx.de>
To: perlbug [...] perl.org
Date: Tue, 15 May 2001 13:22:00 +0200
Subject: "Useless use of contatenation" warning only appears with two children?
Download (untitled) / with headers
text/plain 498b
$ ./perl -w -e '($a,$b)=(42)x2; $a . $b' Useless use of concatenation (.) or string in void context at -e line 1. $ ./perl -w -e '($a,$b,$c)=(42)x3; $a . $b . $c' (nothing) To me, the second concatenation is also in void context. Even if you parenthesize it, (($a . $b) . $c), the first concatenation is in scalar context because its value is used for the second concatenation -- but the second concatenation, at least, is in void context, isn't it? category=core severity=low Cheers, Philip
RT-Send-CC: perl5-porters [...] perl.org
On Mon May 14 21:22:06 2001, pnewton@gmx.de wrote: Show quoted text
> $ ./perl -w -e '($a,$b)=(42)x2; $a . $b' > Useless use of concatenation (.) or string in void context at -e line 1. > $ ./perl -w -e '($a,$b,$c)=(42)x3; $a . $b . $c' > (nothing) > > To me, the second concatenation is also in void context.
Agreed. This seems to stem from some old code that I'm not sure is necessary any more. Perl_ck_concat does: if (kid->op_type == OP_CONCAT && !(kid->op_private & OPpTARGET_MY) && !(kUNOP->op_first->op_flags & OPf_MOD)) o->op_flags |= OPf_STACKED; But why does it do this? Removing Perl_ck_concat makes $a . $b . $c warn properly, and doesn't break the test suite (legit, right?). Compare (pre-patch) how multiple add ops are constructed versus multiple concat ops: mhorsfall@tworivers:~/p5/perl$ perl -MO=Concise,-exec -e '$x + $a' 1 <0> enter 2 <;> nextstate(main 1 -e:1) v:{ 3 <#> gvsv[*x] s 4 <#> gvsv[*a] s 5 <2> add[t3] vK/2 6 <@> leave[1 ref] vKP/REFC -e syntax OK mhorsfall@tworivers:~/p5/perl$ perl -MO=Concise,-exec -e '$x + $a + $a' 1 <0> enter 2 <;> nextstate(main 1 -e:1) v:{ 3 <#> gvsv[*x] s 4 <#> gvsv[*a] s 5 <2> add[t3] sK/2 6 <#> gvsv[*a] s 7 <2> add[t5] vK/2 8 <@> leave[1 ref] vKP/REFC -e syntax OK mhorsfall@tworivers:~/p5/perl$ perl -MO=Concise,-exec -e '$x . $a' 1 <0> enter 2 <;> nextstate(main 1 -e:1) v:{ 3 <#> gvsv[*x] s 4 <#> gvsv[*a] s 5 <2> concat[t3] vK/2 6 <@> leave[1 ref] vKP/REFC -e syntax OK mhorsfall@tworivers:~/p5/perl$ perl -MO=Concise,-exec -e '$x . $a . $a' 1 <0> enter 2 <;> nextstate(main 1 -e:1) v:{ 3 <#> gvsv[*x] s 4 <#> gvsv[*a] s 5 <2> concat[t3] sK/2 6 <#> gvsv[*a] s 7 <2> concat[t5] vKS/2 <-- OPf_STACKED 8 <@> leave[1 ref] vKP/REFC With Perl_ck_concat, the final concat is modified to be OPf_STACKED, which causes the void warning to not trigger. Is . fundamentally different than + in some way I don't know, or is this just old code that's no longer needed? Possible patch attached. -- Matthew Horsfall (alh)
Subject: 0001-perl-6997-ck_concat-is-setting-OPf_STACKED-when-it-s.patch
From 17b48bfcbeedf308e8ff26037de7dd8de0eaae4f Mon Sep 17 00:00:00 2001 From: Matthew Horsfall <wolfsage@gmail.com> Date: Sat, 16 Jan 2016 16:29:47 -0500 Subject: [PATCH] [perl #6997] - ck_concat is setting OPf_STACKED when it shouldn't. In fact, why do we even have a ck_concat? It was introduced with perl 5.0, but seems to serve no purpose now? --- embed.h | 1 - op.c | 14 -------------- opcode.h | 2 +- proto.h | 5 ----- regen/opcodes | 2 +- t/lib/warnings/2use | 2 ++ 6 files changed, 4 insertions(+), 22 deletions(-) diff --git a/embed.h b/embed.h index 73c02d2..63dff89 100644 --- a/embed.h +++ b/embed.h @@ -1110,7 +1110,6 @@ #define ck_backtick(a) Perl_ck_backtick(aTHX_ a) #define ck_bitop(a) Perl_ck_bitop(aTHX_ a) #define ck_cmp(a) Perl_ck_cmp(aTHX_ a) -#define ck_concat(a) Perl_ck_concat(aTHX_ a) #define ck_defined(a) Perl_ck_defined(aTHX_ a) #define ck_delete(a) Perl_ck_delete(aTHX_ a) #define ck_each(a) Perl_ck_each(aTHX_ a) diff --git a/op.c b/op.c index ee31adc..a481001 100644 --- a/op.c +++ b/op.c @@ -9386,20 +9386,6 @@ Perl_ck_cmp(pTHX_ OP *o) } OP * -Perl_ck_concat(pTHX_ OP *o) -{ - const OP * const kid = cUNOPo->op_first; - - PERL_ARGS_ASSERT_CK_CONCAT; - PERL_UNUSED_CONTEXT; - - if (kid->op_type == OP_CONCAT && !(kid->op_private & OPpTARGET_MY) && - !(kUNOP->op_first->op_flags & OPf_MOD)) - o->op_flags |= OPf_STACKED; - return o; -} - -OP * Perl_ck_spair(pTHX_ OP *o) { dVAR; diff --git a/opcode.h b/opcode.h index e711e65..0747f91 100644 --- a/opcode.h +++ b/opcode.h @@ -1440,7 +1440,7 @@ EXT Perl_check_t PL_check[] /* or perlvars.h */ Perl_ck_null, /* i_add */ Perl_ck_null, /* subtract */ Perl_ck_null, /* i_subtract */ - Perl_ck_concat, /* concat */ + Perl_ck_null, /* concat */ Perl_ck_stringify, /* stringify */ Perl_ck_bitop, /* left_shift */ Perl_ck_bitop, /* right_shift */ diff --git a/proto.h b/proto.h index 1bbdace..188a915 100644 --- a/proto.h +++ b/proto.h @@ -288,11 +288,6 @@ PERL_CALLCONV OP * Perl_ck_cmp(pTHX_ OP *o) #define PERL_ARGS_ASSERT_CK_CMP \ assert(o) -PERL_CALLCONV OP * Perl_ck_concat(pTHX_ OP *o) - __attribute__warn_unused_result__; -#define PERL_ARGS_ASSERT_CK_CONCAT \ - assert(o) - PERL_CALLCONV OP * Perl_ck_defined(pTHX_ OP *o) __attribute__warn_unused_result__; #define PERL_ARGS_ASSERT_CK_DEFINED \ diff --git a/regen/opcodes b/regen/opcodes index 9ea0753..f179cef 100644 --- a/regen/opcodes +++ b/regen/opcodes @@ -133,7 +133,7 @@ add addition (+) ck_null IfsT2 S S i_add integer addition (+) ck_null ifsT2 S S subtract subtraction (-) ck_null IfsT2 S S i_subtract integer subtraction (-) ck_null ifsT2 S S -concat concatenation (.) or string ck_concat fsT2 S S +concat concatenation (.) or string ck_null fsT2 S S stringify string ck_stringify fsT@ S left_shift left bitshift (<<) ck_bitop fsT2 S S diff --git a/t/lib/warnings/2use b/t/lib/warnings/2use index c0d203a..c4b724e 100644 --- a/t/lib/warnings/2use +++ b/t/lib/warnings/2use @@ -75,8 +75,10 @@ Reversed += operator at - line 3. -w no warnings 'reserved' ; foo.bar; +foo.bar.baz; EXPECT Useless use of concatenation (.) or string in void context at - line 3. +Useless use of concatenation (.) or string in void context at - line 4. ######## --FILE-- abc -- 1.9.1
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 206b
Matt's patch works, except for the test, which no longer applies. The test he was patching has since been changed. the attached patch adds a test, which fails in blead and passes when applying Matt's patch.
Subject: 0002-RT-6997-Test-for-warnings-for-a.-b.-c.patch
From 315ed2c3ebb4fbe8a649d6ed8df084fd7a97c644 Mon Sep 17 00:00:00 2001 From: Dan Collins <dcollinsn@gmail.com> Date: Mon, 4 Jul 2016 19:11:05 -0400 Subject: [PATCH] [RT #6997] Test for warnings for $a.$b.$c --- t/lib/warnings/2use | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/t/lib/warnings/2use b/t/lib/warnings/2use index 4e10d4b..dc7c8c7 100644 --- a/t/lib/warnings/2use +++ b/t/lib/warnings/2use @@ -377,3 +377,13 @@ $#; EXPECT $* is no longer supported at - line 3. $# is no longer supported at - line 5. +######## + +# Useless use of concatenation should appear for any number of args +use warnings; +($a, $b, $c) = (42)x3; +$a.$b; +$a.$b.$c; +EXPECT +Useless use of concatenation (.) or string in void context at - line 5. +Useless use of concatenation (.) or string in void context at - line 6. -- 2.8.1
Date: Thu, 29 Sep 2016 10:18:16 +0100
To: Matthew Horsfall via RT <perlbug-comment [...] perl.org>
CC: perl5-porters [...] perl.org
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #6997] "Useless use of concatenation" warning not triggered by multiple concatenations.
Download (untitled) / with headers
text/plain 1.4k
On Sat, Jan 16, 2016 at 01:40:08PM -0800, Matthew Horsfall via RT wrote: Show quoted text
> On Mon May 14 21:22:06 2001, pnewton@gmx.de wrote:
> > $ ./perl -w -e '($a,$b)=(42)x2; $a . $b' > > Useless use of concatenation (.) or string in void context at -e line 1. > > $ ./perl -w -e '($a,$b,$c)=(42)x3; $a . $b . $c' > > (nothing) > > > > To me, the second concatenation is also in void context.
> > Agreed. > > This seems to stem from some old code that I'm not sure is necessary any more. > > Perl_ck_concat does: > > if (kid->op_type == OP_CONCAT && !(kid->op_private & OPpTARGET_MY) && > !(kUNOP->op_first->op_flags & OPf_MOD)) > o->op_flags |= OPf_STACKED; > > But why does it do this? > > Removing Perl_ck_concat makes $a . $b . $c warn properly, and doesn't break the test suite (legit, right?). >
It's an optimisation, so I suggest not removing it. The (($a . $b) . $c) is being compiled as (essentially) (($a . $b) .= $c) i.e. the temporary (padtmp) result of the first concat is just reused and appended to, rather than using a second temporary and copying both strings to it. As an aside, I've occasionally thought that we need an OP_CONCATLIST op, where rpeep() converts a nested tree of concat ops into a (PUSHMARK, arg, arg, ..., CONCATLIST) sequence, where the CONCATLIST op is a bit like join('', ....) (but more efficient). -- Overhead, without any fuss, the stars were going out. -- Arthur C Clarke
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 429b
On Thu Sep 29 02:18:43 2016, davem wrote: Show quoted text
> As an aside, I've occasionally thought that we need an OP_CONCATLIST > op, > where rpeep() converts a nested tree of concat ops into a (PUSHMARK, > arg, > arg, ..., CONCATLIST) sequence, where the CONCATLIST op is a bit like > join('', ....) (but more efficient).
If it turns out to be more efficient, then join '', ... should be optimised into CONCATLIST. -- Father Chrysostomos
Date: Thu, 14 Dec 2017 04:27:28 +0000
To: perl5-porters [...] perl.org
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #6997] "Useless use of concatenation" warning not triggered by multiple concatenations.
Fixed in commit 3d033384b2016a58c3eed89524dc658660c759ce. -zefram
Download (untitled) / with headers
text/plain 317b
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.


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