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

Concat with uninitialized vars #15269

Closed
p5pRT opened this issue Apr 11, 2016 · 10 comments
Closed

Concat with uninitialized vars #15269

p5pRT opened this issue Apr 11, 2016 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Apr 11, 2016

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

Searchable as RT127877$

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2016

From @VadimPushtaev

We discovered that `my` and `our` variables behave differently in terms of using uninitialized vars in concatenation.

Try this​:

perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx"'
perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'
perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx" . undef'
perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx" . undef'
perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx"'
perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx"'
perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx" . undef'
perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx" . undef'

Get this​:

1) perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx"'

2) perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'
# None here. WHY!?

3) perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx" . undef'
Use of uninitialized value in concatenation (.) or string at -e line 1.

4) perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx" . undef'
Use of uninitialized value $x in concatenation (.) or string at -e line 1.
Use of uninitialized value in concatenation (.) or string at -e line 1.

5) perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx"'

6) perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx"'
Use of uninitialized value $x in concatenation (.) or string at -e line 1.

7) perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx" . undef'
Use of uninitialized value in concatenation (.) or string at -e line 1.

8) perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx" . undef'
Use of uninitialized value $x in concatenation (.) or string at -e line 1.
Use of uninitialized value in concatenation (.) or string at -e line 1.

There is no consistency in this behavior​:
* (2) and (6) behave differently though the only difference is `my` and `our`.
* If (2) doesn't throw the warning, why does (4) throw TWO of them?

Btw, there is no such problem with the addition (`+`).

P.S. I also don't quite understand why `.=` doesn't throw any warnings, but this behavior is understandable and consistent, so no problem here.

P.P.S. The problem was originally discovered by ivan.yazykov@​gmail.com

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2016

From @demerphq

On 11 April 2016 at 12​:56, Vadim Pushtaev <perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Vadim Pushtaev
# Please include the string​: [perl #127877]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127877 >

We discovered that `my` and `our` variables behave differently in terms of using uninitialized vars in concatenation.

Try this​:

perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx"'
perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'
perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx" . undef'
perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx" . undef'
perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx"'
perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx"'
perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx" . undef'
perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx" . undef'

Get this​:

1) perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx"'

This is expected.

2) perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'
# None here. WHY!?

Yes, agreed. I find this not warning to be odd, and a bug. I wonder if
it is a regression.

3) perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx" . undef'
Use of uninitialized value in concatenation (.) or string at -e line 1.

This is expected.

4) perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx" . undef'
Use of uninitialized value $x in concatenation (.) or string at -e line 1.
Use of uninitialized value in concatenation (.) or string at -e line 1.

This is expected.

5) perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx"'

This is expected.

6) perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx"'
Use of uninitialized value $x in concatenation (.) or string at -e line 1.

This is expected. And is what I expect #2 to do.

7) perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx" . undef'
Use of uninitialized value in concatenation (.) or string at -e line 1.

This is expected.

8) perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx" . undef'
Use of uninitialized value $x in concatenation (.) or string at -e line 1.
Use of uninitialized value in concatenation (.) or string at -e line 1.

Also expected.

There is no consistency in this behavior​:
* (2) and (6) behave differently though the only difference is `my` and `our`.

Yes, I agree this is a bug.

* If (2) doesn't throw the warning, why does (4) throw TWO of them?

2 is buggy. What it does should not be used to determine what 4 does.

We get two warnings because there are two undefined values involved.

my $x; $x = $x . "xxx" . undef

The first is $x starting off undefined, and the second is the undef call.

Btw, there is no such problem with the addition (`+`).

Interesting.

P.S. I also don't quite understand why `.=` doesn't throw any warnings, but this behavior is understandable and consistent, so no problem here.

.= is a mutator. Most mutators DONT warn on uninitialized value on the
lhs but a few do (IIRC it DWIM pretty well). This is documented
somewhere in the perldocs.

That is why ++ does not warn, and why +=1 doesnt warn, etc.

So the only bug here is that

perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'

does not warn when it should.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2016

From @VadimPushtaev

On Mon Apr 11 05​:01​:17 2016, demerphq wrote​:

On 11 April 2016 at 12​:56, Vadim Pushtaev <perlbug-followup@​perl.org>
wrote​:

# New Ticket Created by Vadim Pushtaev
# Please include the string​: [perl #127877]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=127877 >

We discovered that `my` and `our` variables behave differently in
terms of using uninitialized vars in concatenation.

Try this​:

perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx"'
perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'
perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx" . undef'
perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx" .
undef'
perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx"'
perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx"'
perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx" . undef'
perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx" .
undef'

Get this​:

1) perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx"'

This is expected.

2) perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'
# None here. WHY!?

Yes, agreed. I find this not warning to be odd, and a bug. I wonder if
it is a regression.

3) perl -e 'use warnings q{uninitialized}; my $x; $x .= "xxx" .
undef'
Use of uninitialized value in concatenation (.) or string at -e line
1.

This is expected.

4) perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx" .
undef'
Use of uninitialized value $x in concatenation (.) or string at -e
line 1.
Use of uninitialized value in concatenation (.) or string at -e line
1.

This is expected.

5) perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx"'

This is expected.

6) perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx"'
Use of uninitialized value $x in concatenation (.) or string at -e
line 1.

This is expected. And is what I expect #2 to do.

7) perl -e 'use warnings q{uninitialized}; our $x; $x .= "xxx" .
undef'
Use of uninitialized value in concatenation (.) or string at -e line
1.

This is expected.

8) perl -e 'use warnings q{uninitialized}; our $x; $x = $x . "xxx" .
undef'
Use of uninitialized value $x in concatenation (.) or string at -e
line 1.
Use of uninitialized value in concatenation (.) or string at -e line
1.

Also expected.

There is no consistency in this behavior​:
* (2) and (6) behave differently though the only difference is `my`
and `our`.

Yes, I agree this is a bug.

* If (2) doesn't throw the warning, why does (4) throw TWO of them?

2 is buggy. What it does should not be used to determine what 4 does.

We get two warnings because there are two undefined values involved.

Yes, that's exactly what I meant.

my $x; $x = $x . "xxx" . undef

The first is $x starting off undefined, and the second is the undef
call.

Btw, there is no such problem with the addition (`+`).

Interesting.

P.S. I also don't quite understand why `.=` doesn't throw any
warnings, but this behavior is understandable and consistent, so no
problem here.

.= is a mutator. Most mutators DONT warn on uninitialized value on the
lhs but a few do (IIRC it DWIM pretty well). This is documented
somewhere in the perldocs.

I know it's unrelated, but I still can't find it in perdocs.

That is why ++ does not warn, and why +=1 doesnt warn, etc.

So the only bug here is that

perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'

does not warn when it should.

Yves

@p5pRT
Copy link
Author

p5pRT commented Apr 11, 2016

From @demerphq

On 11 April 2016 at 14​:17, Vadim Pushtaev via RT
<perlbug-followup@​perl.org> wrote​:

On Mon Apr 11 05​:01​:17 2016, demerphq wrote​:

.= is a mutator. Most mutators DONT warn on uninitialized value on the
lhs but a few do (IIRC it DWIM pretty well). This is documented
somewhere in the perldocs.

I know it's unrelated, but I still can't find it in perdocs.

It is in perlsyn, section "Declarations"​:

Operators such as C<++>, C<-->, C<+=>,
C<-=>, and C<.=>, that operate on undefined variables such as​:

  undef $a;
  $a++;

are also always exempt from such warnings.

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented May 8, 2016

From @arc

demerphq <demerphq@​gmail.com> wrote​:

So the only bug here is that

perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'

does not warn when it should.

Fix attached; I plan to apply it early in the 5.25 cycle.

--
Aaron Crane ** http​://aaroncrane.co.uk/

@p5pRT
Copy link
Author

p5pRT commented May 8, 2016

From @arc

0001-perl-127877-Emit-undef-warning-on-sassign-concat.patch
From 22fd7a732f3df8b11bbf167562a79756e3a34df0 Mon Sep 17 00:00:00 2001
From: Aaron Crane <arc@cpan.org>
Date: Sun, 8 May 2016 18:13:30 +0100
Subject: [PATCH] [perl #127877] Emit undef warning on sassign+concat

Code like this:

    my $x;
    $x .= 'a';

is specifically exempted from "use of uninitialized value" warnings,
according to the "Declarations" section of perlsyn, to allow the idiom of
building up a value piecemeal. The same is true of the += and -= operators.
However, breaking the combined assignment up into the underlying operator
and a simple assignment, as in this code:

    my $x;
    $x = $x . 'a';

*should* produce a warning.

That warning was correctly being emitted for addition and subtraction, but
concatenation was behaving as the ".=" case, because "$x = $x . EXPR" is
optimized to the equivalent of "$x .= EXPR".

So we now explicitly detect this case, and emit the desired warning.
---
 pp_hot.c               |  7 +++++--
 t/lib/warnings/9uninit | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/pp_hot.c b/pp_hot.c
index d6cb1aa..223169b 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -284,8 +284,11 @@ PP(pp_concat)
     }
     else { /* $l .= $r   and   left == TARG */
 	if (!SvOK(left)) {
-	    if (left == right && ckWARN(WARN_UNINITIALIZED)) /* $l .= $l */
-		report_uninit(right);
+            if ((left == right                          /* $l .= $l */
+                 || (PL_op->op_private & OPpTARGET_MY)) /* $l = $l . $r */
+                && ckWARN(WARN_UNINITIALIZED)
+                )
+                report_uninit(left);
 	    sv_setpvs(left, "");
 	}
         else {
diff --git a/t/lib/warnings/9uninit b/t/lib/warnings/9uninit
index ef9b4f6..42915d9 100644
--- a/t/lib/warnings/9uninit
+++ b/t/lib/warnings/9uninit
@@ -2138,3 +2138,24 @@ Use of uninitialized value $i in array element at - line 12.
 Use of uninitialized value $k in hash element at - line 12.
 Use of uninitialized value $i in array element at - line 13.
 Use of uninitialized value $k in hash element at - line 13.
+########
+# perl #127877
+use warnings 'uninitialized';
+my ($p, $q, $r, $s, $t, $u, $v, $w, $x, $y);
+$p = $p . "a";
+$q .= "a";
+$r = $r + 17;
+$s += 17;
+$t = $t - 17;
+$u -= 17;
+use integer;
+$v = $v + 17;
+$w += 17;
+$x = $x - 17;
+$y -= 17;
+EXPECT
+Use of uninitialized value $p in concatenation (.) or string at - line 4.
+Use of uninitialized value $r in addition (+) at - line 6.
+Use of uninitialized value $t in subtraction (-) at - line 8.
+Use of uninitialized value $v in integer addition (+) at - line 11.
+Use of uninitialized value $x in integer subtraction (-) at - line 13.
-- 
2.7.4

@p5pRT
Copy link
Author

p5pRT commented May 9, 2016

From @arc

Applied as 51f69a2; resolving ticket.

--
Aaron Crane ** http​://aaroncrane.co.uk/

@p5pRT p5pRT closed this as completed May 9, 2016
@p5pRT
Copy link
Author

p5pRT commented May 9, 2016

@arc - Status changed from 'open' to 'resolved'

@p5pRT
Copy link
Author

p5pRT commented May 9, 2016

From @VadimPushtaev

On Sun May 08 10​:26​:07 2016, arc wrote​:

demerphq <demerphq@​gmail.com> wrote​:

So the only bug here is that

perl -e 'use warnings q{uninitialized}; my $x; $x = $x . "xxx"'

does not warn when it should.

Fix attached; I plan to apply it early in the 5.25 cycle.

Thanks! We actually suspected that some optimization trick involed :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant