Skip to content

Use C99 named initialisers in core structs that define MGVTBLs #22086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: blead
Choose a base branch
from
Open

Use C99 named initialisers in core structs that define MGVTBLs #22086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
625cd98
Use C99 named initialisers in core structs that define MGVTBLs
leonerd Mar 18, 2024
6911fd7
Also use C99 named initialisers in mg_vtable.h
leonerd Mar 19, 2024
9375687
Use C99 designated array initalisers as well in mg_vtable.h
ilmari Mar 19, 2024
fe2e0d3
Keep trailing comma in PL_magic_vtable(_name)s definitions
ilmari Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions doio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1348,17 +1348,10 @@ If we have unlinkat(), renameat(), fchmodat(), dirfd() we also keep:
6: the DIR * for the current directory when we open the file, stored as an IV
*/

static const MGVTBL argvout_vtbl =
{
NULL, /* svt_get */
NULL, /* svt_set */
NULL, /* svt_len */
NULL, /* svt_clear */
S_argvout_free, /* svt_free */
NULL, /* svt_copy */
S_argvout_dup, /* svt_dup */
NULL /* svt_local */
};
static const MGVTBL argvout_vtbl = {
.svt_free = S_argvout_free,
.svt_dup = S_argvout_dup,
};

static bool
S_is_fork_open(const char *name) {
Expand Down
2 changes: 1 addition & 1 deletion ext/PerlIO-encoding/encoding.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package PerlIO::encoding;

use strict;
our $VERSION = '0.31';
our $VERSION = '0.32';
our $DEBUG = 0;
$DEBUG and warn __PACKAGE__, " called by ", join(", ", caller), "\n";

Expand Down
2 changes: 1 addition & 1 deletion ext/PerlIO-encoding/encoding.xs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ typedef struct {

#define NEEDS_LINES 1

static const MGVTBL PerlIOEncode_tag = { 0, 0, 0, 0, 0, 0, 0, 0 };
static const MGVTBL PerlIOEncode_tag = {0};

static SV *
PerlIOEncode_getarg(pTHX_ PerlIO * f, CLONE_PARAMS * param, int flags)
Expand Down
2 changes: 1 addition & 1 deletion ext/PerlIO-via/via.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
package PerlIO::via;
our $VERSION = '0.19';
our $VERSION = '0.20';
require XSLoader;
XSLoader::load();
1;
Expand Down
2 changes: 1 addition & 1 deletion ext/PerlIO-via/via.xs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ typedef struct
CV *UTF8;
} PerlIOVia;

static const MGVTBL PerlIOVia_tag = { 0, 0, 0, 0, 0, 0, 0, 0 };
static const MGVTBL PerlIOVia_tag = {0};

#define MYMethod(x) #x,&s->x

Expand Down
2 changes: 1 addition & 1 deletion ext/XS-APItest/APItest.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use strict;
use warnings;
use Carp;

our $VERSION = '1.35';
our $VERSION = '1.36';

require XSLoader;

Expand Down
6 changes: 3 additions & 3 deletions ext/XS-APItest/APItest.xs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ S_myset_set_dies(pTHX_ SV* sv, MAGIC* mg)


static MGVTBL vtbl_foo, vtbl_bar;
static MGVTBL vtbl_myset = { 0, S_myset_set, 0, 0, 0, 0, 0, 0 };
static MGVTBL vtbl_myset_dies = { 0, S_myset_set_dies, 0, 0, 0, 0, 0, 0 };
static MGVTBL vtbl_myset = { .svt_set = S_myset_set };
static MGVTBL vtbl_myset_dies = { .svt_set = S_myset_set_dies };

static int
S_mycopy_copy(pTHX_ SV *sv, MAGIC* mg, SV *nsv, const char *name, I32 namlen) {
Expand All @@ -192,7 +192,7 @@ S_mycopy_copy(pTHX_ SV *sv, MAGIC* mg, SV *nsv, const char *name, I32 namlen) {
return 0;
}

STATIC MGVTBL vtbl_mycopy = { 0, 0, 0, 0, 0, S_mycopy_copy, 0, 0 };
STATIC MGVTBL vtbl_mycopy = { .svt_copy = S_mycopy_copy };

/* indirect functions to test the [pa]MY_CXT macros */

Expand Down
166 changes: 76 additions & 90 deletions mg_vtable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,106 +105,92 @@ enum { /* pass one of these to get_vtbl */

#ifdef DOINIT
EXTCONST char * const PL_magic_vtable_names[magic_vtable_max] = {
"arylen",
"arylen_p",
"backref",
"checkcall",
"collxfrm",
"dbline",
"debugvar",
"defelem",
"destruct",
"env",
"envelem",
"hints",
"hintselem",
"hook",
"hookelem",
"isa",
"isaelem",
"lvref",
"mglob",
"nkeys",
"nonelem",
"ovrld",
"pack",
"packelem",
"pos",
"regdata",
"regdatum",
"regexp",
"sig",
"sigelem",
"substr",
"sv",
"taint",
"utf8",
"uvar",
"vec"
[want_vtbl_arylen] = "arylen",
[want_vtbl_arylen_p] = "arylen_p",
[want_vtbl_backref] = "backref",
[want_vtbl_checkcall] = "checkcall",
[want_vtbl_collxfrm] = "collxfrm",
[want_vtbl_dbline] = "dbline",
[want_vtbl_debugvar] = "debugvar",
[want_vtbl_defelem] = "defelem",
[want_vtbl_destruct] = "destruct",
[want_vtbl_env] = "env",
[want_vtbl_envelem] = "envelem",
Comment on lines +108 to +118
Copy link
Contributor

Choose a reason for hiding this comment

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

Array designated initializers are not C++ compatible, and since we have people who build perl with a C++ compiler, it's excluded by the guide in perlhacktips.pod.

g++ supports it as an extension. but MSVC doesn't.

Copy link
Contributor

Choose a reason for hiding this comment

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

@leonerd, @tonycoz identified some problems with this p.r. back in March. Do you expect to move forward with the p.r.? (It has begun to acquire merge conflicts.) Thanks.

[want_vtbl_hints] = "hints",
[want_vtbl_hintselem] = "hintselem",
[want_vtbl_hook] = "hook",
[want_vtbl_hookelem] = "hookelem",
[want_vtbl_isa] = "isa",
[want_vtbl_isaelem] = "isaelem",
[want_vtbl_lvref] = "lvref",
[want_vtbl_mglob] = "mglob",
[want_vtbl_nkeys] = "nkeys",
[want_vtbl_nonelem] = "nonelem",
[want_vtbl_ovrld] = "ovrld",
[want_vtbl_pack] = "pack",
[want_vtbl_packelem] = "packelem",
[want_vtbl_pos] = "pos",
[want_vtbl_regdata] = "regdata",
[want_vtbl_regdatum] = "regdatum",
[want_vtbl_regexp] = "regexp",
[want_vtbl_sig] = "sig",
[want_vtbl_sigelem] = "sigelem",
[want_vtbl_substr] = "substr",
[want_vtbl_sv] = "sv",
[want_vtbl_taint] = "taint",
[want_vtbl_utf8] = "utf8",
[want_vtbl_uvar] = "uvar",
[want_vtbl_vec] = "vec",
};
#else
EXTCONST char * const PL_magic_vtable_names[magic_vtable_max];
#endif

/* These all need to be 0, not NULL, as NULL can be (void*)0, which is a
* pointer to data, whereas we're assigning pointers to functions, which are
* not the same beast. ANSI doesn't allow the assignment from one to the other.
* (although most, but not all, compilers are prepared to do it)
*/

/* order is:
get
set
len
clear
free
copy
dup
local
*/

#ifdef DOINIT
/* These named initialisers will upset C++ compilers before C++20, but the
* DOINIT macro is only defined within globals.c so this should be fine.
*/
EXT_MGVTBL PL_magic_vtables[magic_vtable_max] = {
{ (int (*)(pTHX_ SV *, MAGIC *))Perl_magic_getarylen, Perl_magic_setarylen, 0, 0, 0, 0, 0, 0 },
{ 0, 0, 0, Perl_magic_cleararylen_p, Perl_magic_freearylen_p, 0, 0, 0 },
{ 0, 0, 0, 0, Perl_magic_killbackrefs, 0, 0, 0 },
{ 0, 0, 0, 0, 0, Perl_magic_copycallchecker, 0, 0 },
[want_vtbl_arylen] = { .svt_get = (int (*)(pTHX_ SV *, MAGIC *))Perl_magic_getarylen, .svt_set = Perl_magic_setarylen },
[want_vtbl_arylen_p] = { .svt_clear = Perl_magic_cleararylen_p, .svt_free = Perl_magic_freearylen_p },
[want_vtbl_backref] = { .svt_free = Perl_magic_killbackrefs },
[want_vtbl_checkcall] = { .svt_copy = Perl_magic_copycallchecker },
#ifdef USE_LOCALE_COLLATE
{ 0, Perl_magic_setcollxfrm, 0, 0, Perl_magic_freecollxfrm, 0, 0, 0 },
[want_vtbl_collxfrm] = { .svt_set = Perl_magic_setcollxfrm, .svt_free = Perl_magic_freecollxfrm },
#else
{ 0, 0, 0, 0, 0, 0, 0, 0 },
{0},
#endif
{ 0, Perl_magic_setdbline, 0, 0, 0, 0, 0, 0 },
{ Perl_magic_getdebugvar, Perl_magic_setdebugvar, 0, 0, 0, 0, 0, 0 },
{ Perl_magic_getdefelem, Perl_magic_setdefelem, 0, 0, 0, 0, 0, 0 },
{ 0, 0, 0, 0, Perl_magic_freedestruct, 0, 0, 0 },
{ 0, Perl_magic_set_all_env, 0, Perl_magic_clear_all_env, 0, 0, 0, 0 },
{ 0, Perl_magic_setenv, 0, Perl_magic_clearenv, 0, 0, 0, 0 },
{ 0, 0, 0, Perl_magic_clearhints, 0, 0, 0, 0 },
{ 0, Perl_magic_sethint, 0, Perl_magic_clearhint, 0, 0, 0, 0 },
{ 0, Perl_magic_sethookall, 0, Perl_magic_clearhookall, 0, 0, 0, 0 },
{ 0, Perl_magic_sethook, 0, Perl_magic_clearhook, 0, 0, 0, 0 },
{ 0, Perl_magic_setisa, 0, Perl_magic_clearisa, 0, 0, 0, 0 },
{ 0, Perl_magic_setisa, 0, 0, 0, 0, 0, 0 },
{ 0, Perl_magic_setlvref, 0, 0, 0, 0, 0, 0 },
{ 0, Perl_magic_setmglob, 0, 0, Perl_magic_freemglob, 0, 0, 0 },
{ Perl_magic_getnkeys, Perl_magic_setnkeys, 0, 0, 0, 0, 0, 0 },
{ 0, Perl_magic_setnonelem, 0, 0, 0, 0, 0, 0 },
{ 0, 0, 0, 0, Perl_magic_freeovrld, 0, 0, 0 },
{ 0, 0, Perl_magic_sizepack, Perl_magic_wipepack, 0, 0, 0, 0 },
{ Perl_magic_getpack, Perl_magic_setpack, 0, Perl_magic_clearpack, 0, 0, 0, 0 },
{ Perl_magic_getpos, Perl_magic_setpos, 0, 0, 0, 0, 0, 0 },
{ 0, 0, Perl_magic_regdata_cnt, 0, 0, 0, 0, 0 },
{ Perl_magic_regdatum_get, Perl_magic_regdatum_set, 0, 0, 0, 0, 0, 0 },
{ 0, Perl_magic_setregexp, 0, 0, 0, 0, 0, 0 },
{ 0, Perl_magic_setsigall, 0, 0, 0, 0, 0, 0 },
{ Perl_magic_getsig, Perl_magic_setsig, 0, Perl_magic_clearsig, 0, 0, 0, 0 },
{ Perl_magic_getsubstr, Perl_magic_setsubstr, 0, 0, 0, 0, 0, 0 },
{ Perl_magic_get, Perl_magic_set, 0, 0, 0, 0, 0, 0 },
{ Perl_magic_gettaint, Perl_magic_settaint, 0, 0, 0, 0, 0, 0 },
{ 0, Perl_magic_setutf8, 0, 0, Perl_magic_freeutf8, 0, 0, 0 },
{ Perl_magic_getuvar, Perl_magic_setuvar, 0, 0, 0, 0, 0, 0 },
{ Perl_magic_getvec, Perl_magic_setvec, 0, 0, 0, 0, 0, 0 }
[want_vtbl_dbline] = { .svt_set = Perl_magic_setdbline },
[want_vtbl_debugvar] = { .svt_get = Perl_magic_getdebugvar, .svt_set = Perl_magic_setdebugvar },
[want_vtbl_defelem] = { .svt_get = Perl_magic_getdefelem, .svt_set = Perl_magic_setdefelem },
[want_vtbl_destruct] = { .svt_free = Perl_magic_freedestruct },
[want_vtbl_env] = { .svt_set = Perl_magic_set_all_env, .svt_clear = Perl_magic_clear_all_env },
[want_vtbl_envelem] = { .svt_set = Perl_magic_setenv, .svt_clear = Perl_magic_clearenv },
[want_vtbl_hints] = { .svt_clear = Perl_magic_clearhints },
[want_vtbl_hintselem] = { .svt_set = Perl_magic_sethint, .svt_clear = Perl_magic_clearhint },
[want_vtbl_hook] = { .svt_set = Perl_magic_sethookall, .svt_clear = Perl_magic_clearhookall },
[want_vtbl_hookelem] = { .svt_set = Perl_magic_sethook, .svt_clear = Perl_magic_clearhook },
[want_vtbl_isa] = { .svt_set = Perl_magic_setisa, .svt_clear = Perl_magic_clearisa },
[want_vtbl_isaelem] = { .svt_set = Perl_magic_setisa },
[want_vtbl_lvref] = { .svt_set = Perl_magic_setlvref },
[want_vtbl_mglob] = { .svt_set = Perl_magic_setmglob, .svt_free = Perl_magic_freemglob },
[want_vtbl_nkeys] = { .svt_get = Perl_magic_getnkeys, .svt_set = Perl_magic_setnkeys },
[want_vtbl_nonelem] = { .svt_set = Perl_magic_setnonelem },
[want_vtbl_ovrld] = { .svt_free = Perl_magic_freeovrld },
[want_vtbl_pack] = { .svt_len = Perl_magic_sizepack, .svt_clear = Perl_magic_wipepack },
[want_vtbl_packelem] = { .svt_get = Perl_magic_getpack, .svt_set = Perl_magic_setpack, .svt_clear = Perl_magic_clearpack },
[want_vtbl_pos] = { .svt_get = Perl_magic_getpos, .svt_set = Perl_magic_setpos },
[want_vtbl_regdata] = { .svt_len = Perl_magic_regdata_cnt },
[want_vtbl_regdatum] = { .svt_get = Perl_magic_regdatum_get, .svt_set = Perl_magic_regdatum_set },
[want_vtbl_regexp] = { .svt_set = Perl_magic_setregexp },
[want_vtbl_sig] = { .svt_set = Perl_magic_setsigall },
[want_vtbl_sigelem] = { .svt_get = Perl_magic_getsig, .svt_set = Perl_magic_setsig, .svt_clear = Perl_magic_clearsig },
[want_vtbl_substr] = { .svt_get = Perl_magic_getsubstr, .svt_set = Perl_magic_setsubstr },
[want_vtbl_sv] = { .svt_get = Perl_magic_get, .svt_set = Perl_magic_set },
[want_vtbl_taint] = { .svt_get = Perl_magic_gettaint, .svt_set = Perl_magic_settaint },
[want_vtbl_utf8] = { .svt_set = Perl_magic_setutf8, .svt_free = Perl_magic_freeutf8 },
[want_vtbl_uvar] = { .svt_get = Perl_magic_getuvar, .svt_set = Perl_magic_setuvar },
[want_vtbl_vec] = { .svt_get = Perl_magic_getvec, .svt_set = Perl_magic_setvec },
};
#else
EXT_MGVTBL PL_magic_vtables[magic_vtable_max];
Expand Down
11 changes: 1 addition & 10 deletions op.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15328,16 +15328,7 @@ custom_op_register_free(pTHX_ SV *sv, MAGIC *mg)


static const MGVTBL custom_op_register_vtbl = {
0, /* get */
0, /* set */
0, /* len */
0, /* clear */
custom_op_register_free, /* free */
0, /* copy */
0, /* dup */
#ifdef MGf_LOCAL
0, /* local */
#endif
.svt_free = custom_op_register_free,
};


Expand Down
38 changes: 12 additions & 26 deletions regen/mg_vtable.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ BEGIN
my @names = sort keys %vtable_conf;
{
my $want = join ",\n ", (map {"want_vtbl_$_"} @names), 'magic_vtable_max';
my $names = join qq{",\n "}, @names;
my $names = join qq{\n }, map {qq{[want_vtbl_$_] = "$_",}} @names;

print $vt <<"EOH";

Expand All @@ -487,7 +487,7 @@ BEGIN

#ifdef DOINIT
EXTCONST char * const PL_magic_vtable_names[magic_vtable_max] = {
"$names"
$names
};
#else
EXTCONST char * const PL_magic_vtable_names[magic_vtable_max];
Expand All @@ -497,24 +497,10 @@ BEGIN
}

print $vt <<'EOH';
/* These all need to be 0, not NULL, as NULL can be (void*)0, which is a
* pointer to data, whereas we're assigning pointers to functions, which are
* not the same beast. ANSI doesn't allow the assignment from one to the other.
* (although most, but not all, compilers are prepared to do it)
*/

/* order is:
get
set
len
clear
free
copy
dup
local
*/

#ifdef DOINIT
/* These named initialisers will upset C++ compilers before C++20, but the
* DOINIT macro is only defined within globals.c so this should be fine.
*/
EXT_MGVTBL PL_magic_vtables[magic_vtable_max] = {
EOH

Expand All @@ -525,20 +511,20 @@ BEGIN
my $data = $vtable_conf{$name};
push @vtable_names, $name;
my @funcs = map {
$data->{$_} ? "Perl_magic_$data->{$_}" : 0;
my $cast = ( $_ eq "get" and $data->{const} ) ?
"(int (*)(pTHX_ SV *, MAGIC *))" :
"";

$data->{$_} ? ( ".svt_$_ = ${cast}Perl_magic_$data->{$_}" ) : ();
} qw(get set len clear free copy dup local);

$funcs[0] = "(int (*)(pTHX_ SV *, MAGIC *))" . $funcs[0] if $data->{const};
my $funcs = join ", ", @funcs;

# Because we can't have a , after the last {...}
my $comma = @names ? ',' : '';

print $vt "$data->{cond}\n" if $data->{cond};
print $vt " { $funcs }$comma\n";
print $vt " [want_vtbl_$name] = { $funcs },\n";
print $vt <<"EOH" if $data->{cond};
#else
{ 0, 0, 0, 0, 0, 0, 0, 0 }$comma
{0},
#endif
EOH
foreach(@{$data->{alias}}) {
Expand Down