SV is "PVNV" despite being assigned/used in numeric context only

[sisyphus]

This came up in #19585.
AFAICS, it's happening in current blead, all the way back to perl-5.8.8.
Maybe it's nothing to worry about.

Steps to Reproduce

$ perl -MDevel::Peek -le '$x = 1.0; $y = $x + 0; Dump $x;'
SV = PVNV(0x23426160) at 0x23448a38
REFCNT = 1
FLAGS = (IOK,NOK,pIOK,pNOK)
IV = 1
NV = 1
PV = 0

Expected behavior

Given that $x was created as a number, and then used only in numeric context, the thing that I did not expect was its description as "PVNV" in the first line of output.
Should I just ignore that ?
I'm unaware of any serious consequences associated with this behaviour.

Perl configuration

AFAIK, it happens on any and all perl configurations going back to 5.8.8 and perhaps earlier.

[haarg]

It needs to be PVNV to be able to store both the IV and NV values. You are adding 0, which is an IV, so it tries to do integer math if possible. I don't see any problems with the behavior here.

[demerphq]

On Wed, 6 Apr 2022 at 15:09, Graham Knop ***@***.***> wrote: It needs to be PVNV to be able to store both the IV and NV values. You are adding 0, which is an IV, so it tries to do integer math if possible. I don't see any problems with the behavior here.

At one level I agree with you and at another I don't. So there are two cases which I think we should consider differently. The first is adding the constant 0. I dont think that should trigger an upgrade. Adding a 0 in a variable is a harder case. For what its worth I ran this through the debugger and he is the code that is executed: $ gdb --args ./perl -le'my $x = 1.0; print \$x; my $discard = $x + 0; <>' [trimmed] Reading symbols from ./perl... (gdb) b pp_hot.c:1405 Breakpoint 1 at 0x23486d: file pp_hot.c, line 1405. (gdb) r Starting program: /home/yorton/git_tree/perl/perl -lemy\ \$x\ =\ 1.0\;\ print\ \\\$x\;\ my\ \$discard\ =\ \$x\ +\ 0\;\ \<\> [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". SCALAR(0x555555c0c068) Breakpoint 1, Perl_pp_add () at pp_hot.c:1405 1405 tryAMAGICbin_MG(add_amg, AMGf_assign|AMGf_numeric); (gdb) n 1406 svr = TOPs; (gdb) 1407 svl = TOPm1s; (gdb) 1412 if (!((svl->sv_flags|svr->sv_flags) & (SVf_IVisUV|SVs_GMG))) { (gdb) x svl 0x555555c0c068: 0x55c0c050 (gdb) x svr 0x555555c0c0b0: 0x55c0c0a0 (gdb) s 1414 U32 flags = (svl->sv_flags & svr->sv_flags); (gdb) 1415 if (flags & SVf_IOK) { (gdb) 1435 else if (flags & SVf_NOK) { (gdb) 1453 useleft = USE_LEFT(svl); (gdb) 1500 if (SvIV_please_nomg(svr)) { (gdb) 1504 UV auv = 0; (gdb) 1505 bool auvok = FALSE; (gdb) 1506 bool a_valid = 0; (gdb) 1508 if (!useleft) { (gdb) 1516 if (SvIV_please_nomg(svl)) { (gdb) Perl_sv_2iv_flags (sv=0x55be7df0, flags=32767) at sv.c:2249 2249 { (gdb) 2250 PERL_ARGS_ASSERT_SV_2IV_FLAGS; (gdb) 2252 assert (SvTYPE(sv) != SVt_PVAV && SvTYPE(sv) != SVt_PVHV (gdb) 2255 if (SvGMAGICAL(sv) && (flags & SV_GMAGIC)) (gdb) 2258 if (SvROK(sv)) { (gdb) 2271 if (SvVALID(sv) || isREGEXP(sv)) { (gdb) 2315 if (SvTHINKFIRST(sv)) { (gdb) 2323 if (!SvIOKp(sv)) { (gdb) 2324 if (S_sv_2iuv_common(aTHX_ sv)) (gdb) S_sv_2iuv_common (sv=0x200000000) at sv.c:1974 1974 { (gdb) 1975 PERL_ARGS_ASSERT_SV_2IUV_COMMON; (gdb) 1977 if (SvNOKp(sv)) { (gdb) 1984 if (SvTYPE(sv) == SVt_NV) (gdb) 1985 sv_upgrade(sv, SVt_PVNV); (gdb) q A debugging session is active. Inferior 1 [process 123900] will be killed. Quit anyway? (y or n) y That is an *awful* lot of code to evaluate $x + 0. I feel like we should be able to do better and not upgrade to a PVNV for this operation. Eg, why is 0 an integer? Why isn't it a NOK/IOK value where we can just add the NV's together and be done with it? You could even argue that 0+ or +0 should have their own opcode that simply numifies its "other argument". For the case of 0 in a variable, again we could check much earlier in this codepath that one of the values is 0 and simply short circuit a bunch of work. I agree with you that this is "not a bug", but it does feel at least like an undesirable thing that we should consider trying to improve on. Yves

…

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

[haarg]

Some thoughts without any real conclusions.

It may be worth changing how the upgrading is done to avoid setting the SVf flags, similar to the change made to prevent integers from setting SVf_POK when used as a string. On its own, this wouldn't prevent the upgrading to PVNV, since it would only be a change to how the flags are set. I suspect that it would be more problematic than the string case though, because when used as a string, the PV will still be used even if only SVp_POK is set, but for numbers the cached values may be worthless if only the SVp flag is set.

Currently, there is no pure-perl operation to numify a value. 0+$val kind of works, but not in all cases. It has an impact on flags, as seen in this ticket. Additionally, for objects with overloads, it will prefer the + overload, not the 0+ overload. This means the only way to reliably do numification and not any other operation is with sprintf or pack. It would be nice to have something to numify reliably.

[richardleach]

Related: #17777, #18914, #17814

[richardleach]

Actionable suggestions (if viable and approved) from the issues above:

• Preemptively upgrade constant IV/NV to PVNV at compile time (should fix Yves' specific example above)
• Make SvIV_please_nomg(sv) convert an undef SV to an IV with value 0

Also coming out of those discussions was Tony's use float branch (#17813, #17831), but that hasn't been merged (1) amid uncertainty as to if users would find it beneficial, and (2) because of concerns about the cost of COPs

[iabyn]

On Wed, Apr 06, 2022 at 06:18:32AM -0700, Yves Orton wrote: That is an *awful* lot of code to evaluate $x + 0. I feel like we should be able to do better and not upgrade to a PVNV for this operation.

As a general comment, pp_add() is heavily optimised for the cases of int+int and num+num, and generally falls back to slow and messy code paths once things start to get mixed up (e.g. int + num, int + string etc) since there's usually a whole big mess of edge cases that quickly come into play.

…

-- That he said that that that that is is is debatable, is debatable.

[sisyphus]

Fixed by #19580