Skip to content

Perlapi: document Sv[GSA]MAGICAL#24361

Open
richardleach wants to merge 1 commit intoPerl:bleadfrom
richardleach:perlapi_magical
Open

Perlapi: document Sv[GSA]MAGICAL#24361
richardleach wants to merge 1 commit intoPerl:bleadfrom
richardleach:perlapi_magical

Conversation

@richardleach
Copy link
Copy Markdown
Contributor

This PR adds some simple descriptions for the following macros for the benefit of perlapi:

  • SvMAGICAL
  • SvGMAGICAL
  • SvSMAGICAL
  • SvAMAGICAL

Closes #18956


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

@richardleach
Copy link
Copy Markdown
Contributor Author

@khwilliamson - please let me know if you feel the corresponding _on and _off macros should also be documented.

@Leont
Copy link
Copy Markdown
Contributor

Leont commented Apr 12, 2026

please let me know if you feel the corresponding _on and _off macros should also be documented.

IMO those are internals, and nothing other than a few core functions involved in adding/removing magic should ever set/unset these flags.

Comment on lines +1149 to +1150
This is typically used prior to assigning a value to the SV, as magic it
could influence exactly how and where the value is actually stored.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You normally check it after modifying the SV, and typically via SvSETMAGIC()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO those are internals

Agreed. In a better of all possible worlds, we would document these as internal. But my thinking is it's not worth it. I think their usage is obvious to someone who's messing with the internals, so there are more important things to do instead.

This is typically used prior to assigning a value to the SV, as magic it
could influence exactly how and where the value is actually stored.

=for apidoc Am|U32|SvAMAGICAL|SV* sv
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such macro - did you mean SvAMAGIC()?

@tonycoz
Copy link
Copy Markdown
Contributor

tonycoz commented Apr 13, 2026

This misses SvRMAGICAL(), which is similar to Sv[GS]?MAGICAL() in that it just checks the flags of the given SV, while SvAMAGIC() should have get magic checked first.


Remove any string offset.

=for apidoc Am|U32|SvMAGICAL|SV* sv
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought a blank line had to separate the apidoc line from the explanatory text, but if it works as-is I don't care

But it is suspicious to me that the return is a U32, when I would be expecting a bool from the description. I think that needs to be addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[doc] perlapi doesn’t define SvMAGICAL

5 participants