S_new_SV: mark args unused, make inline, correct typo #22208
Conversation
richardleach
force-pushed
the
hydahy/22125_leaking_scalars_inline
branch
from
b3717a0
to
250be70
Compare
|
Something that I should have picked up from the original commit: S_new_SV() is visible outside of perl itself, and like the other visible inline functions it should use a Looks fine otherwise. |
richardleach
force-pushed
the
hydahy/22125_leaking_scalars_inline
branch
from
250be70
to
1c16f24
Compare
|
I think S_new_SV() should be made always defined, not just existing on DEBUG_LEAKING_SCALARS builds; but with the extra sv->sv_debug* and logging lines wrapped in ifdef DEBUG_LEAKING_SCALARS. Then the macro new_SV() should stop being a multi-line macro and just become a call to the inline function S_new_SV(). Then the code comment "provide a real function for a debugger to play with" becomes redundant. The weird "sometimes a macro, sometimes a function" arrangement was put in place by me back before we could use inline functions. It should probably also have an entry in embed.fnc? |
|
Yes it should have an embed.fnc entry |
|
Ok, will work on that. |
richardleach
force-pushed
the
hydahy/22125_leaking_scalars_inline
branch
from
1c16f24
to
fdbd16f
Compare
| { | ||
| SV* sv; | ||
| PERL_UNUSED_ARG(file); | ||
| PERL_UNUSED_ARG(line); | ||
| PERL_UNUSED_ARG(func); |
There was a problem hiding this comment.
These three variables are used when DEBUG_LEAKING_SCALARS is defined. Maybe these should be wrapped with a #ifndef DEBUG_LEAKING_SCALARS ... #endif.
There was a problem hiding this comment.
Sure, I can do that.
richardleach
force-pushed
the
hydahy/22125_leaking_scalars_inline
branch
from
fdbd16f
to
ddecdf3
Compare
When sv_inline.h was created in Perl@75acd14 and a number of things moved into it, the S_new_SV debugging function should have been made PERL_STATIC_INLINE and given the Perl_ prefix. This commit now does those things. It also marks the arguments to Perl_new_SV as PERL_UNUSED_ARG, reducing warnings on some builds. Additionally, now that we can use inline functions, the new_SV() macro is now just a call to this inline function, rather than being that only on DEBUG_LEAKING_SCALARS builds and a multi-line macro the rest of the time.
richardleach
force-pushed
the
hydahy/22125_leaking_scalars_inline
branch
from
ddecdf3
to
ececbcf
Compare
When sv_inline.h was created in
75acd14
and a number of things moved into it, the S_new_SV debugging function should
have been made PERL_STATIC_INLINE. This commit now does that.It also marks
the arguments to S_new_SV as PERL_UNUSED_ARG, reducing warnings on some
builds, and corrects a transcription error in the FUNCTION argument.
The first two changes were suggested in #22125 and the third noticed during
preparation of this PR.