Skip Menu |
Report information
Id: 6997
Status: pending release
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


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