Skip to content

vutil.c: Use new SvVSTRING() macro - #23154

Closed
leonerd wants to merge 1 commit into
Perl:bleadfrom
leonerd:use-SvVSTRING-in-vutil-c
Closed

vutil.c: Use new SvVSTRING() macro#23154
leonerd wants to merge 1 commit into
Perl:bleadfrom
leonerd:use-SvVSTRING-in-vutil-c

Conversation

@leonerd

@leonerd leonerd commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

The new SvVSTRING macro means callers don't have to be aware of the current (magic-based) implementation of vstrings. When I added it in 80f24f4 I overlooked this file for changes. Here's two more places it should be used from.

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

@Leont

Leont commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

This file's upstream is version.pm, and should not be modified in core.

@Leont

Leont commented Mar 24, 2025

Copy link
Copy Markdown
Contributor

This file's upstream is version.pm, and should not be modified in core.

Effectively that means SvVSTRING needs to be added to ppport.h first.

@leonerd

leonerd commented Mar 24, 2025

Copy link
Copy Markdown
Contributor Author

Effectively that means SvVSTRING needs to be added to ppport.h first.

Sounds like something for @khwilliamson then

@Leont Leont added the Closable? We might be able to close this ticket, but we need to check with the reporter label Mar 25, 2025
@leonerd
leonerd force-pushed the use-SvVSTRING-in-vutil-c branch from ebed084 to e86607e Compare March 26, 2025 16:07
@Leont

Leont commented Mar 26, 2025

Copy link
Copy Markdown
Contributor

I think this can be closed, and instead a PR should be opened against version.pm

jkeenan pushed a commit to jkeenan/version.pm that referenced this pull request Jun 9, 2025
Committer: Originally submitted by Paul Evans to Perl core distribution as
Perl/perl5#23154.  It was recommended that it be
submitted upstream first.
@jkeenan

jkeenan commented Jun 9, 2025

Copy link
Copy Markdown
Contributor

Discussion moved upstream to Perl/version.pm#45. Closing this ticket.

@jkeenan jkeenan closed this Jun 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Closable? We might be able to close this ticket, but we need to check with the reporter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants