Recognise a :reader attribute on class fields

[leonerd]

Generates simple reader accessors.

Not yet addressed - any kind of writer or mutator.

perldelta to be written in a separate commit

[Leont]

Look good to me (but I'm no expert on OPcodes)

[leonerd]

Look good to me (but I'm no expert on OPcodes)

The opcode stuff was mostly copy-pasted from Object::Pad's implementation anyway ;)

[leonerd]

If @tonycoz has a moment I'd appreciate a look at the various UTF-8 handling around the padnames in particular.

[leonerd]

On a casual commandline test using Espernato (and therefore non-Latin1 Unicode), it appears to work for me:

$ ./perl -Ilib -CD -E 'use utf8; use feature "class"; class X { field $sandviĉon :param :reader } say X->new( sandviĉon => "fromaĝo" )->sandviĉon'
class is experimental at -e line 1.
field is experimental at -e line 1.
Wide character in say at -e line 1.
fromaĝo

[rwp0]

but a different name can be specified in the attribute

How? If by :reader(NAME) let's document that too please (or give example thereof).

[leonerd]

Example added to docs.

[tonycoz]

If @tonycoz has a moment I'd appreciate a look at the various UTF-8 handling around the padnames in particular.

It looks good to me, but I think it should have some UTF-8 name tests.

[perigrin]

Should Test2->new->f also work here? The POD suggests not but doesn't state it explicitly and there's no test confirming it doesn't.

[leonerd]

Test added

[Grinnz]

not that I expect it to be an issue here, but perhaps Test2 isn't the most unique class name? :)

[leonerd]

Fair. I'll rename these to Testcase1 etc, and also fix up the names in the other .t files in a separate commit.

[leonerd]

I did this directly in 4650b6a

[leonerd]

It looks good to me, but I think it should have some UTF-8 name tests.

@tonycoz Yes certainly, though I have a whole bunch I plan to steal from https://metacpan.org/release/PEVANS/Object-Pad-0.808/source/t/95utf8.t which test all of the class stuff. I'll do that in a separate test-only PR. (At least, I hope it'll be test-only. Maybe it will uncover a bug :) )

[wolfsage]

This causes a segfault, is this known:

use v5.36;
use feature 'class';
no warnings 'experimental::class';

class Testcase1 {
  field $s :reader();
}

alh@emondsfield:~/p5/perl$ ./perl -Ilib nc.pl
Use of uninitialized value at nc.pl line 6.
Use of uninitialized value at nc.pl line 6.
Segmentation fault (core dumped)

Looks like we get a null or freed sv in apply_field_attribute_reader(pTHX_ PADNAME *pn, SV *value) for value maybe?

(Likewise, this crashes in a different way - attempting to eat all memory or ... some other bug?

use v5.36;
use feature 'class';
use warnings;
no warnings 'experimental::class';

class Testcase1 {
  field $s :param();
}

alh@emondsfield:~/p5/perl$ ./perl -Ilib nc.pl
Use of uninitialized value at nc.pl line 6.
Out of memory in perl:util:safesysrealloc

It can also crash like this:

alh@emondsfield:~/p5/perl$ ./perl -Ilib nc.pl
Use of uninitialized value at nc.pl line 7.
perl: sv_inline.h:928: Perl_SvPV_helper: Assertion `PL_valid_types_PVX[SvTYPE(_svcur) & SVt_MASK]' failed.
Aborted (core dumped)

Also, what happens in this case?

use v5.36;
use feature 'class';
no warnings 'experimental::class';

class Testcase1 {
  field $s :reader("foo");
}

I guess we create a method call "foo". It compiles fine, gives no warnings, but $obj->foo isn't usable which might surprise some folks.

Also if you do :reader(DESTROY) it looks like we will call the reader in the destructor - no harm done but definitely weird. So these magic attributes that can generate CVs can do some weird things....

[leonerd]

This causes a segfault, is this known:
...
Looks like we get a null or freed sv in apply_field_attribute_reader(pTHX_ PADNAME *pn, SV *value) for value maybe?

Hrm; I don't seem to be able to reproduce that:

$ ./perl -Ilib -Mexperimental=class -E 'class Point { field $x :reader() = 123; } say Point->new->x'
123

Also, what happens in this case?
...
field $s :reader("foo");
...
I guess we create a method call "foo". It compiles fine, gives no warnings, but $obj->foo isn't usable which might surprise some folks.

Surprising perhaps, yes. Similar would already happen with :param("foo"). I suppose we could add a warning similar in style to e.g. this one:

$ perl -Mwarnings -E 'qw( foo, bar )'
Possible attempt to separate words with commas at -e line 1.

Also if you do :reader(DESTROY) it looks like we will call the reader in the destructor - no harm done but definitely weird. So these magic attributes that can generate CVs can do some weird things....

That feels like a comment to make in documentation - "you can do this but you very likely should not."

[leonerd]

Hrm; I don't seem to be able to reproduce that:

Ah but wait a moment, maybe something more subtle going on:

$ valgrind ./perl -Ilib -Mexperimental=class -E 'class Point { field $x :param() :reader() = 123; } say Point->new( x => 456 )->x'
==164071== Memcheck, a memory error detector
==164071== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==164071== Using Valgrind-3.20.0 and LibVEX; rerun with -h for copyright info
==164071== Command: ./perl -Ilib -Mexperimental=class -E class\ Point\ {\ field\ $x\ :param()\ :reader()\ =\ 123;\ }\ say\ Point-\>new(\ x\ =\>\ 456\ )-\>x
==164071== 
==164071== Conditional jump or move depends on uninitialised value(s)
==164071==    at 0x1D09F9: apply_field_attribute_param (class.c:925)
==164071==    by 0x1D1327: S_class_apply_field_attribute (class.c:1066)
==164071==    by 0x1D149C: Perl_class_apply_field_attributes (class.c:1091)
==164071==    by 0x298355: Perl_yyparse (perly.y:1574)
==164071==    by 0x1A0360: S_parse_body (perl.c:2609)
==164071==    by 0x19DED0: perl_parse (perl.c:1912)
==164071==    by 0x14F285: main (perlmain.c:106)
==164071== 
==164071== Conditional jump or move depends on uninitialised value(s)
==164071==    at 0x1D0C49: apply_field_attribute_reader (class.c:955)
==164071==    by 0x1D1327: S_class_apply_field_attribute (class.c:1066)
==164071==    by 0x1D149C: Perl_class_apply_field_attributes (class.c:1091)
==164071==    by 0x298355: Perl_yyparse (perly.y:1574)
==164071==    by 0x1A0360: S_parse_body (perl.c:2609)
==164071==    by 0x19DED0: perl_parse (perl.c:1912)
==164071==    by 0x14F285: main (perlmain.c:106)
==164071== 
...

[leonerd]

Turns out to have been a good ol' uninitialised pointer. Fixed by 4b75b72

[leonerd]

Accepting that there should be a (warning? error?) when attempting to create a :reader with a name that's not a valid perl symbol, and also that there should be another test file to check some UTF-8 names, I'll do both of those in a subsequent PR.
That aside, I think this looks good to go?