Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

Enhancement: which *expression* (not just scalar) was uninitialized in eq #9828

Open p5pRT opened 15 years ago

p5pRT commented 15 years ago

Migrated from rt.perl.org#68534 (status was 'open')

Searchable as RT68534$

p5pRT commented 15 years ago

From DanVDascalescu@yahoo.com

perl 5.10.0​:

$ perl -we 'my $x; print $x == 1' Use of uninitialized value $x in numeric eq (==) at -e line 1.

$ perl -we 'my $x; print $x->{foo} == 1' Use of uninitialized value in numeric eq (==) at -e line 1.

Same for string eq.

-- Dan Dascalescu (dandv) http​://wiki.dandascalescu.com/.list/Perl

 

p5pRT commented 14 years ago

From myfwhite@gmail.com

Thanks for the bug report!

I've assigned this to the wishlist severity\, because it's not urgent for 5.12.0.

p5pRT commented 14 years ago

@xdg - Status changed from 'new' to 'open'

p5pRT commented 11 years ago

From @jkeenan

On Fri Aug 14 20​:20​:03 2009\, dandv wrote​:

perl 5.10.0​:

$ perl -we 'my $x; print $x == 1' Use of uninitialized value $x in numeric eq (==) at -e line 1.

$ perl -we 'my $x; print $x->{foo} == 1' Use of uninitialized value in numeric eq (==) at -e line 1.

Same for string eq.

-- Dan Dascalescu (dandv) http​://wiki.dandascalescu.com/.list/Perl

This request was placed on the wishlist before Perl 5.12.0. Is this something we would be willing to consider?

If so\, would someone be willing to take it on? (Hint​: Perl_report_uninit in sv.c looks relevant.)

Thank you very much. Jim Keenan

p5pRT commented 11 years ago

From @cpansprout

On Thu Aug 08 16​:25​:29 2013\, jkeenan wrote​:

On Fri Aug 14 20​:20​:03 2009\, dandv wrote​:

perl 5.10.0​:

$ perl -we 'my $x; print $x == 1' Use of uninitialized value $x in numeric eq (==) at -e line 1.

$ perl -we 'my $x; print $x->{foo} == 1' Use of uninitialized value in numeric eq (==) at -e line 1.

Same for string eq.

-- Dan Dascalescu (dandv) http​://wiki.dandascalescu.com/.list/Perl

This request was placed on the wishlist before Perl 5.12.0. Is this something we would be willing to consider?

Yes.

If so\, would someone be willing to take it on?

Yes\, but not yet. :-)

(Hint​: Perl_report_uninit in sv.c looks relevant.)

Yes.

Note that this already includes the hash element​:

$ perl -we 'my %x; print $x{foo} == 1' Use of uninitialized value $x{"foo"} in numeric eq (==) at -e line 1.

Extending that to hashes referenced by scalars is no doubt possible.

--

Father Chrysostomos

p5pRT commented 11 years ago

From @davidnicol

On Thu\, Aug 8\, 2013 at 7​:57 PM\, Father Chrysostomos

Note that this already includes the hash element​:

$ perl -we 'my %x; print $x{foo} == 1' Use of uninitialized value $x{"foo"} in numeric eq (==) at -e line 1.

Extending that to hashes referenced by scalars is no doubt possible.

Extending it to arbitrary anything seems like it happen several different ways​: 1​: add special case-handling code for a slowly growing but incomplete subset of expressions (the current path\, and suboptimal) 2​: present the whole text of the expression (would have to remember all the expression texts everywhere\, taking gobs of memory and adding a lot of complexity\, for instance what about multi-line expressions? do we reformat them?) 3​: option 2\, plus some formatting rules pertaining to length of the expression (still awfully complicated) 4​: drop the current repetition of a limited class of expressions in favor of two different warnings\, the first stating "uninitialized left expression in numeric eq (==) at -e line 1" (could conceivably break something somewhere\, if someone somehow is parsing these warnings as part of their very fragile process\, but slightly easier to do than option 5\, but means discarding perfectly good function) 5​: keep what's there now and switch to option-4 style for anything more complicated than what gets displayed now

I'm for option 5.

-- You've got to wiggle before you can crawl

p5pRT commented 11 years ago

From @davidnicol

On Fri\, Aug 9\, 2013 at 7​:17 PM\, David Nicol \davidnicol@​gmail\.com wrote​:

On Thu\, Aug 8\, 2013 at 7​:57 PM\, Father Chrysostomos

Note that this already includes the hash element​:

$ perl -we 'my %x; print $x{foo} == 1' Use of uninitialized value $x{"foo"} in numeric eq (==) at -e line 1.

4​: drop the current repetition of a limited class of expressions in favor of two different warnings\, the first stating "uninitialized left expression in numeric eq (==) at -e line 1" (could conceivably break something somewhere\, if someone somehow is parsing these warnings as part of their very fragile process\, but slightly easier to do than option 5\, but means discarding perfectly good function) 5​: keep what's there now and switch to option-4 style for anything more complicated than what gets displayed now

option 4 would be easier done by simply inserting "on left" or "on right"\, instead of null-string\, when the expression is beyond what the warning can handle.

p5pRT commented 7 years ago

From jaderd+bitcard@gmail.com

On Fri\, 09 Aug 2013 17​:22​:14 -0700\, davidnicol@​gmail.com wrote​:

On Fri\, Aug 9\, 2013 at 7​:17 PM\, David Nicol \davidnicol@​gmail\.com wrote​:

On Thu\, Aug 8\, 2013 at 7​:57 PM\, Father Chrysostomos

Note that this already includes the hash element​:

$ perl -we 'my %x; print $x{foo} == 1' Use of uninitialized value $x{"foo"} in numeric eq (==) at -e line 1.

4​: drop the current repetition of a limited class of expressions in favor of two different warnings\, the first stating "uninitialized left expression in numeric eq (==) at -e line 1" (could conceivably break something somewhere\, if someone somehow is parsing these warnings as part of their very fragile process\, but slightly easier to do than option 5\, but means discarding perfectly good function) 5​: keep what's there now and switch to option-4 style for anything more complicated than what gets displayed now

option 4 would be easier done by simply inserting "on left" or "on right"\, instead of null-string\, when the expression is beyond what the warning can handle.

I would like to upvote this request and the proposed solution #5.

Test script

  $ perl -we'sub a {} sub b {} print a == b'

Current output​:

  Use of uninitialized value in numeric eq (==) at -e line 1.

Desired output​:

  Use of uninitialized value at the left of numeric eq (==) at -e line 1.

p5pRT commented 7 years ago

From @xsawyerx

On 08/03/2017 01​:41 PM\, Jader Dias via RT wrote​:

On Fri\, 09 Aug 2013 17​:22​:14 -0700\, davidnicol@​gmail.com wrote​:

On Fri\, Aug 9\, 2013 at 7​:17 PM\, David Nicol \davidnicol@​gmail\.com wrote​:

On Thu\, Aug 8\, 2013 at 7​:57 PM\, Father Chrysostomos

Note that this already includes the hash element​:

$ perl -we 'my %x; print $x{foo} == 1' Use of uninitialized value $x{"foo"} in numeric eq (==) at -e line 1.

4​: drop the current repetition of a limited class of expressions in favor of two different warnings\, the first stating "uninitialized left expression in numeric eq (==) at -e line 1" (could conceivably break something somewhere\, if someone somehow is parsing these warnings as part of their very fragile process\, but slightly easier to do than option 5\, but means discarding perfectly good function) 5​: keep what's there now and switch to option-4 style for anything more complicated than what gets displayed now

option 4 would be easier done by simply inserting "on left" or "on right"\, instead of null-string\, when the expression is beyond what the warning can handle. I would like to upvote this request

+1

and the proposed solution #5.

Test script

$ perl \-we'sub a \{\} sub b \{\} print a == b'

Current output​:

Use of uninitialized value in numeric eq \(==\) at \-e line 1\.

Desired output​:

Use of uninitialized value at the left of numeric eq \(==\) at \-e line 1\.

Bikeshed​: Is it "at the left of" or "on the left of"?

p5pRT commented 7 years ago

From @iabyn

On Fri\, Aug 04\, 2017 at 10​:29​:15AM +0200\, Sawyer X wrote​:

On 08/03/2017 01​:41 PM\, Jader Dias via RT wrote​:

On Fri\, 09 Aug 2013 17​:22​:14 -0700\, davidnicol@​gmail.com wrote​:

On Fri\, Aug 9\, 2013 at 7​:17 PM\, David Nicol \davidnicol@​gmail\.com wrote​:

On Thu\, Aug 8\, 2013 at 7​:57 PM\, Father Chrysostomos

Note that this already includes the hash element​:

$ perl -we 'my %x; print $x{foo} == 1' Use of uninitialized value $x{"foo"} in numeric eq (==) at -e line 1.

4​: drop the current repetition of a limited class of expressions in favor of two different warnings\, the first stating "uninitialized left expression in numeric eq (==) at -e line 1" (could conceivably break something somewhere\, if someone somehow is parsing these warnings as part of their very fragile process\, but slightly easier to do than option 5\, but means discarding perfectly good function) 5​: keep what's there now and switch to option-4 style for anything more complicated than what gets displayed now

option 4 would be easier done by simply inserting "on left" or "on right"\, instead of null-string\, when the expression is beyond what the warning can handle. I would like to upvote this request

+1

and the proposed solution #5.

Test script

$ perl \-we'sub a \{\} sub b \{\} print a == b'

Current output​:

Use of uninitialized value in numeric eq \(==\) at \-e line 1\.

Desired output​:

Use of uninitialized value at the left of numeric eq \(==\) at \-e line 1\.

Bikeshed​: Is it "at the left of" or "on the left of"?

Before the bikeshedding\, we need to decide whether its possible. The short answer is that its not.

Or more precisely\, at the point where the 'uninit' warning is triggered on a binary op\, both args have been pushed onto the stack\, then popped off and assigned to local vars\, then one or both of them will have had SvPV() or SVIV() or whatever called on them\, which triggers the call to Perl_report_uninit() and the warning.

At this point Perl_report_uninit() has no idea whether the SV passed to it was the LH or RH arg of a binary op (or something altogether different).

The way that Perl_report_uninit() works at the moment\, is that it examines the optree associated with PL_op and tries to work out where this particular value came from. This involves a whole series of ad-hoc rules\, which is what was complained about earlier in this ticket.

One of those rules is that if one of those args is a direct hash lookup with a constant key\, look up that hash element and if the SVs match\, print '$hash{foo}'.

We could add further rules and code to handle more common cases\, e.g. '$ref->{foo}' as was suggested earlier.

Any ad-hoc rule we added to enable us to display 'LH' or 'RH' could just as easily be used to display something more meaningful\, such as '$ref->{foo}'.

-- I before E. Except when it isn't.

p5pRT commented 7 years ago

From zefram@fysh.org

Dave Mitchell wrote​:

Any ad-hoc rule we added to enable us to display 'LH' or 'RH' could just as easily be used to display something more meaningful\, such as '$ref->{foo}'.

Presumably the requester imagines that we could pass down an explicit argument into Perl_report_uninit() that would say which operand the value came from. If that could be done\, it could be done regardless of how complicated the optree that generated the value was\, whereas the current type of rules\, however much extended\, can only find the scalar in a limited number of places. If we had mechanisms of both these types available\, it would be worth preferring the current specific kind of description and falling back to identifying which operand it was.

The problem is that providing that explicit argument would be a lot of hassle\, because it has to be passed through a lot of function calls and macro layers that don't have a place for it. At minimum\, we could assign a couple of flag bits for it\, and change an enormous number of SvPV()\, etc.\, calls into SvPV_flags(). The question isn't so much whether it can be done as whether it's worth that much additional source code. It's probably worth a pilot project (implement it for a couple of ops) to see how much it takes.

-zefram

p5pRT commented 7 years ago

From @cpansprout

On Fri\, 04 Aug 2017 04​:38​:28 -0700\, zefram@​fysh.org wrote​:

Dave Mitchell wrote​:

Any ad-hoc rule we added to enable us to display 'LH' or 'RH' could just as easily be used to display something more meaningful\, such as '$ref->{foo}'.

Presumably the requester imagines that we could pass down an explicit argument into Perl_report_uninit() that would say which operand the value came from. If that could be done\, it could be done regardless of how complicated the optree that generated the value was\, whereas the current type of rules\, however much extended\, can only find the scalar in a limited number of places. If we had mechanisms of both these types available\, it would be worth preferring the current specific kind of description and falling back to identifying which operand it was.

The problem is that providing that explicit argument would be a lot of hassle\, because it has to be passed through a lot of function calls and macro layers that don't have a place for it.

Thankfully we have global variables....

If it is only pp functions that assign to (say) PL_larg and PL_rarg\, and that via common macros\, then what are the chances of them getting stomped on?

At minimum\, we could assign a couple of flag bits for it\, and change an enormous number of SvPV()\, etc.\, calls into SvPV_flags(). The question isn't so much whether it can be done as whether it's worth that much additional source code. It's probably worth a pilot project (implement it for a couple of ops) to see how much it takes.

-zefram

--

Father Chrysostomos

p5pRT commented 7 years ago

From zefram@fysh.org

Father Chrysostomos via RT wrote​:

Thankfully we have global variables....

Oh yuck\, please don't do that.

If it is only pp functions that assign to (say) PL_larg and PL_rarg\, and that via common macros\, then what are the chances of them getting stomped on?

The thing that worries me is them *not* getting stomped on\, and being applied to SV operations that are nothing to do with the original operands to the op that set them. The existing use of PL_op is bad enough.

-zefram

p5pRT commented 7 years ago

From @iabyn

On Fri\, Aug 04\, 2017 at 10​:16​:52PM +0100\, Zefram wrote​:

Father Chrysostomos via RT wrote​:

Thankfully we have global variables....

Oh yuck\, please don't do that.

If it is only pp functions that assign to (say) PL_larg and PL_rarg\, and that via common macros\, then what are the chances of them getting stomped on?

The thing that worries me is them *not* getting stomped on\, and being applied to SV operations that are nothing to do with the original operands to the op that set them. The existing use of PL_op is bad enough.

I think everyone needs to bear in mind that identifying the var etc in 'uninitialised' warnings is purely a helpful debugging hint.

Perl currently makes a reasonable effort to do so for some common cases; its no huge loss when the var can't be identified.

I think anything that slows down all binary ops on every execution just in case we might emit a warning\, would be unacceptable.

-- The optimist believes that he lives in the best of all possible worlds. As does the pessimist.

p5pRT commented 7 years ago

From @xsawyerx

On 08/05/2017 01​:21 PM\, Dave Mitchell wrote​:

On Fri\, Aug 04\, 2017 at 10​:16​:52PM +0100\, Zefram wrote​:

Father Chrysostomos via RT wrote​:

Thankfully we have global variables.... Oh yuck\, please don't do that.

If it is only pp functions that assign to (say) PL_larg and PL_rarg\, and that via common macros\, then what are the chances of them getting stomped on? The thing that worries me is them *not* getting stomped on\, and being applied to SV operations that are nothing to do with the original operands to the op that set them. The existing use of PL_op is bad enough.

I think everyone needs to bear in mind that identifying the var etc in 'uninitialised' warnings is purely a helpful debugging hint.

Perl currently makes a reasonable effort to do so for some common cases; its no huge loss when the var can't be identified.

What is difficult is when it's not a variable and needs to have these hints. I was just assuming that in a two-op comparison you have two sides and you can say which one is undefined. :/

I think anything that slows down all binary ops on every execution just in case we might emit a warning\, would be unacceptable.

I agree. Maybe this means we should simply close the ticket with the explanation that the only way to accomplish this would require adding additional rules that make the binary ops slower which is undesired. The only question is\, is it even feasible to just have a left vs. right comparison uninit warning. What would it take to be able to say "this left side in this comparison is uninitialized"?

p5pRT commented 7 years ago

From @cpansprout

On Sun\, 06 Aug 2017 01​:35​:19 -0700\, xsawyerx@​gmail.com wrote​:

On 08/05/2017 01​:21 PM\, Dave Mitchell wrote​:

On Fri\, Aug 04\, 2017 at 10​:16​:52PM +0100\, Zefram wrote​:

Father Chrysostomos via RT wrote​:

Thankfully we have global variables.... Oh yuck\, please don't do that.

If it is only pp functions that assign to (say) PL_larg and PL_rarg\, and that via common macros\, then what are the chances of them getting stomped on? The thing that worries me is them *not* getting stomped on\, and being applied to SV operations that are nothing to do with the original operands to the op that set them. The existing use of PL_op is bad enough.

I think everyone needs to bear in mind that identifying the var etc in 'uninitialised' warnings is purely a helpful debugging hint.

Perl currently makes a reasonable effort to do so for some common cases; its no huge loss when the var can't be identified.

What is difficult is when it's not a variable and needs to have these hints. I was just assuming that in a two-op comparison you have two sides and you can say which one is undefined. :/

Yes\, but that information does not get propagated through the functions calls that eventually trigger an uninit warning.

Solving that either slows down all binary ops (my suggestion) or entails a lot of work and a high risk of breakage (Zefram’s suggestion).

Another way to do it would be to modify all binary ops to do an explicit undef check up front and warn about it\, but that may well slow everything down\, too.

I think anything that slows down all binary ops on every execution just in case we might emit a warning\, would be unacceptable.

I agree. Maybe this means we should simply close the ticket with the explanation that the only way to accomplish this would require adding additional rules that make the binary ops slower which is undesired.

We can still extend it to $hashref->{...} without slowing down any code that does not warn\, as per the original request. I still think that is a good idea.

The only question is\, is it even feasible to just have a left vs. right comparison uninit warning. What would it take to be able to say "this left side in this comparison is uninitialized"?

See the previous messages in the thread. Either we add extra parameters to numerous functions\, which is a lot of work and (I think) entails a high risk of introducing regressions\, or we use global variables and slow down every binary op\, or we do my new suggestion of having each binary op check immediately before passing the values to other functions. They all have downsides\, which are probably unacceptable.

--

Father Chrysostomos