Perl / perl5

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

Use `xV_FROM_REF()` macros in place of casting result of SvRV() #22417

Closed leonerd closed 2 months ago

leonerd commented 3 months ago

This results in shorter neater code, and additional debugging assertions that the dereferenced SVs really are the requested type when built under -DDEBUGGING.

When I added the xV_FROM_REF() macros, I searched for (TYPE)SvRV style cast expressions, but forgot to additionally look for MUTABLE_xV() calls.

There are additionally two spots in pp_hot.c that cannot be modified, because despite casting the result to a CV pointer, the SV isn't actually a CV. I've added a comment on these lines as to why they're not altered.

khwilliamson commented 2 months ago

But, I noticed that the macro these call use a variable named _ref. Names with a leading underscore are reserved for the C implementation itself. ref_ would be legal for us to use. The consequences of the name in use are unlikely to bite us, so this is for future reference for anyone reading this

mauke commented 2 months ago

Names with a leading underscore are reserved for the C implementation itself

... but only at file scope. _ref is fine as a local variable.

khwilliamson commented 2 months ago
Names with a leading underscore are reserved for the C implementation itself

... but only at file scope. _ref is fine as a local variable.

If I'm wrong about this, please review "Choosing legal symbol names" in perlhacktips

mauke commented 2 months ago

If I'm wrong about this, please review "Choosing legal symbol names" in perlhacktips

https://perldoc.perl.org/5.41.2/perlhacktips#Choosing-legal-symbol-names agrees with me:

C reserves for its implementation any symbol whose name begins with an underscore followed immediately by either an uppercase letter [A-Z] or another underscore. C++ further reserves any symbol containing two consecutive underscores, and further reserves in the global name space any symbol beginning with an underscore, not just ones followed by a capital.

leonerd commented 2 months ago

I notice other macros use ref_ in this situation anyway, so I'll fix this one accordingly.