sv.h: SvPVx now equivalent to SvPV, et.al

[khwilliamson]

These macros suffixed with 'x' are guaranteed to evaluate their arguments just once. Prior to this commit, they used PL_Sv to accomplish that. But since 1ef9039 in 5.37, the macros they call only evaluate their arguments once, so the PL_Sv is superfluous.

• This set of changes does not require a perldelta entry.

[bulk88]

If this were a perldoc comment (seen by CPAN XS authors), I'd strongly vocalize a note mentioning that this remark can not be used outside of blead perl/core only code, b/c compat with old perls. Out of paranoia for future edits, I'd change a word or 2 in "/* The macros these now expand to no longer evaluate their arguments multiple times */" to somehow suggest this comment is NA for CPAN.

[richardleach]

this remark can not be used outside of blead perl/core only code, b/c compat with old perls.

@bulk88 - I'm unclear on why XS authors would have to be copying definitions out of sv.h? They surely should use the API and that hasn't changed. I could see that changing the definitions in ppport.h - if any of these are in there - would not be backwards compatible.

@khwilliamson - apart from the above concern, LTGM.

[bulk88]

this remark can not be used outside of blead perl/core only code, b/c compat with old perls.

@bulk88 - I'm unclear on why XS authors would have to be copying definitions out of sv.h? They surely should use the API and that hasn't changed. I could see that changing the definitions in ppport.h - if any of these are in there - would not be backwards compatible.

ppport.h doesnt have any public api documentation content AFAIK, I've never seen any content or ever used it as a reference for CPAN code. only ref info ppport.h provides is API/macro add/drop timeline history.

I did note above

If this were a perldoc comment (seen by CPAN XS authors), I'd strongly vocalize a note mentioning that this remark can not be used outside of blead perl/core only code, b/c compat with old perls.

this is a C comment that will not be seen by 90% of XS authors. There is a small risk of future doc edits, which are "improvements" elevating obfuscated/UB legalese to metacpan/perldocs visibility, Those doc edits will NEVER be done by the same person who named the fn/macro/token/var, Most likely a doc/comment edit, is the 1st C/XS/core commit ever by that person, who might be college aged or younger (GSOC/interns/etc, bkgnd high tech skills or beginner irrelevant), I'd rather duplicate crit info docs wise in multiple places in core vs a wheres waldo hunt with grep,

The original phrasing/wording DOES say "used to be multi eval" but it took me 3 passes of the sentence to realize "used to be multi eval" fact was included. 2-3 words can be moved/changed so that fact is understood on eyeball pass 1 not 3.

IDC, but adding a period at the end would be nice since rest of the comment is long and looks like a sentence. I wouldn't have written a PR reply just for the missing period.

[richardleach]

I'm not clear what the best wording would be in that case.
If technically accurate, something like the following?

/* The following macro expansions evaluate their arguments just once. In
 * earlier perl releases, PL_Sv had to be used as an intermediate in order
 * to prevent multiple evaluations. The expansion behaviour changed in 
 * commit 1ef9039bccbfe64f47f201b6cfb7d6d23e0b08a7. */

[bulk88]

/* The following macro expansions evaluate their arguments just once. In

• earlier perl releases, PL_Sv had to be used as an intermediate in order
• to prevent multiple evaluations. The expansion behaviour changed in
• commit 1ef9039. */

my proposed wording below. To keep it short by text length, I'm not adding the fact that these macros are 100% identical to certain other macros in ultra new perls. The end reader can put 1+1 together, phrase In earlier perl releases, these macros used to provides the hint that legacy/grandfather/history is involved in the particular API fn call's docs.

"anti-multi-eval" describes former/current/cpan/old-perl-safe purpose of these exact tokens. to use more efficient inline functions instead of macros and a global var provides enough engineering info for the curious cat to skip doing "trust but verify" archaeology with git blame/ML.

Changing public API is very alarming to some people with personality disorders. Certain Comm/Foss SDKs with public APIs, just do a big red "REMOVED" or a big red "EOL" stamp, on deprecated/EOL scheduled func calls. And not saying in src comments, HTML comments, git comments or public facing bug trackers, why the func got a future removal in ~1 to ~5 years.

• Was it priv esc security/exploit?
• performance?
• race conditions?
• unfixable bugs b/c the fix breaks existing code?-
• ABI/API/var names/capitalization/spelling/arg types and order/etc?
• This fn is now a slightly slower back compat shim b/c a Forklift replacement of internals happened?
• Day 1->Now, Official API docs, official sample usage, was buggy, only solution is PerlCritic blacklisting, since there is zero non-buggy production code in the world. The new fixed call will get a new token/identifier. Buggy old impl is untouched.
• Community survey/citizen outrage/pitchforks?
• BOFH reasserting his power [check reddit/HN/the register/slashdot for details]

Basically a project admin group should note to 3P End Users the following, DO ABSOLUTELY NOTHING with your prior private/CPAN XS code. DO NOT EDIT or grep+replace all your CPAN XS libs, either slowly or quickly. Changelogs like MetaClass::getRecords() was altered in Wave 45, are useless, what is the end user supposed to do?

My proposed wording.

The following macro expansions evaluate their arguments just once on all versions of Perl. In earlier perl releases, these macros used PL_Sv global var as an intermediate in order to prevent multiple evaluations. The implementation changed in commit 1ef9039bccbfe64f47f201b6cfb7d6d23e0b08a7 to use more efficient inline functions instead of macros and a global var for their implementation,