Mailing List Archive

On Mon, Mar 20, 2023 at 1:24?PM zefram via perl5-porters <
perl5-porters@perl.org> wrote:

FYI whatever mechanism you are using to send this email is rendering it
unusable for me. I receive only an attachment with no file extension that
Gmail cannot preview.

-Dan

>

I've copied that one to github as well:

https://github.com/Perl/perl5/issues/20948

Overall though, I can't help thinking that this process of my manually
copy-pasting emails into github issues is not the best way forward for
these bug reports.

Is there a way we can find to do this better?

--
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk | https://metacpan.org/author/PEVANS
http://www.leonerd.org.uk/ | https://www.tindie.com/stores/leonerd/

Op ma 20 mrt 2023 om 18:45 schreef Dan Book <grinnz@gmail.com>:
>
> FYI whatever mechanism you are using to send this email is rendering it unusable for me. I receive only an attachment with no file extension that Gmail cannot preview.


I'm seeing the same issue in Gmail, but if you look at the actual
plain e-mail, it starts with:

"This is a bug report for perl from zefram@fysh.org,
generated with the help of perlbug 1.43 running under perl 5.37.9."

So i guess it's either perlbug or Gmail's fault. There is not even a
MIME boundary defined in the headers, so there seems to be no good
reason for Gmail to treat the message as an attachment?

Maybe Gmail doesn't now how to render "Content-Type: text/markdown;
variant=GFM; charset=US-ASCII" ?

Zefram wrote in his annoying non-standard attachment[1]:

> Observe that the refaliasing operation is able to replace the scalar at
> index 1, whereas the splice operation attempting to do the same thing
> croaks because of the readonlyness. The equivalent also happens with a
> readonly hash:
>
> ```
> $ perl5.37.9 -MInternals=SetReadOnly -Mexperimental=refaliasing -lwe '%a
= (aa=>11,bb=>22,cc=>33); SetReadOnly(\%a); \$a{bb} = \555; print $a{bb};
delete $a{bb}'
> 555
> Attempt to delete readonly key 'bb' from a restricted hash at -e line 1.
> ```
>
> These readonly flags ought to be respected. Refaliasing should not
> provide an ability to bypass them.

At least in this case you are wrong. Marking a hash as readonly does not
make its values readonly. It means the hash itself is restricted and that
you cannot add keys to it that it did not contain when it was marked as
readonly. You can delete keys that it contained, or modify them. You can
also use functions in Hash::Util to mark specific keys as being legal. In
short, turning on the readonly flag on an HV does not make the hash
"readonly", it makes it "restricted" or "locked". If you want to mark a
value in a hash as readonly you must explictly mark *that* variable as
readonly.

We have functionality in Hash::Util which uses Internal::SvREADONLY(%hash,
$flag) to twiddle the readonly bits properly, and even then I have a patch
to replace all of that with a specialized function in Util.xs so that we do
not need to expose SvREADONLY() or SetReadOnly() [Which I didn't know about
until you mentioned it].

Internals::SetReadOnly is not a documented function, you as a user should
not be using it directly, and whatever results you get from using it are
debatable at best. You as a perl core hacker are of course free to use it,
but you are expected to know what the readonly flag means. For a very long
time the readonly flag by itself does not necessarily mean "readonly", it
is only when it is combined with the "protected" flag that it means what
you thinks, and even then you conflating "container" and contents in this
part of your bug report.

Yves
[1] No email client knows what to do with text/markdown variant github.
Just send your attachments as text please. It is annoying already that you
expect us to do work for you to satisfy your refusal to use github (which
makes no sense when your commits are going there anyway), but making it
even harder for us is just over the top. I don't mind being a forwarding
service for you, but i do mind that you refuse to work with me to make it
easy. I understand you have a philisophical objection to using github for
your own projects, but refusing to sign up and use it to interact with the
rest of us who use it as the main way to manage perl is just anti-social.







On Mon, 20 Mar 2023 at 18:24, zefram via perl5-porters <
perl5-porters@perl.org> wrote:

>

--
perl -Mre=debug -e "/just|another|perl|hacker/"

On 3/20/23 14:14, Sander Plas wrote:
> Op ma 20 mrt 2023 om 18:45 schreef Dan Book <grinnz@gmail.com>:
>> FYI whatever mechanism you are using to send this email is rendering it unusable for me. I receive only an attachment with no file extension that Gmail cannot preview.
>
> I'm seeing the same issue in Gmail, but if you look at the actual
> plain e-mail, it starts with:
>
> "This is a bug report for perl from zefram@fysh.org,
> generated with the help of perlbug 1.43 running under perl 5.37.9."
>
> So i guess it's either perlbug or Gmail's fault. There is not even a
> MIME boundary defined in the headers, so there seems to be no good
> reason for Gmail to treat the message as an attachment?
>
> Maybe Gmail doesn't now how to render "Content-Type: text/markdown;
> variant=GFM; charset=US-ASCII" ?

I use Thunderbird, and the original email appears in the body, with no
attachments.

demerphq wrote:
>At least in this case you are wrong. Marking a hash as readonly does not
>make its values readonly.

I didn't try to write to its value scalars. I was altering which
scalars it references, which is (part of) what the readonly flag is
meant to prevent.

> You can delete keys that it contained, or modify them.

If you look at my test case, you'll see that deleting an element, with
the "delete" operator, *is* prevented.

> If you want to mark a
>value in a hash as readonly you must explictly mark *that* variable as
>readonly.

I'm well aware of the distinction. Marking a value scalar as readonly
is not relevant to the test. If you want to try it, though, insert
"SetReadOnly(\$a{bb});" into the test case, and you'll see that that
also doesn't prevent the refaliasing operation.

I'm also aware that there's more to a restricted hash than readonlyness,
but the additional semantics don't seem relevant here.

>Internals::SetReadOnly is not a documented function,

Ah, this is confusing. What we have here is a collision between the
Internals module on CPAN and the builtin functions that the Perl core
puts into the Internals package. Internals::SetReadOnly() is from the
CPAN module, and it very much *is* documented in that module's POD, and
is in its @EXPORT_OK list. Whereas Internals::SvREADONLY() is supplied
by the core, and is neither documented in Internals.pm nor in @EXPORT_OK.

> and even then you conflating "container" and contents in this
>part of your bug report.

No, I'm really not.

> making it
>even harder for us

It's harder to handle when correctly labelled? That is a surprising
finding. If there's a consensus that the correct labelling is
counterproductive then sure, we can do it with incorrect labelling.

> is just over the top.

You write here as if I'm deliberately making things difficult. I'm not.
The recent bug reports in Markdown form are partly experimental, testing
the handling of Markdown in email. I did not know what the outcome
would be. I did not anticipate that the MIME labelling would actually
cause a problem, but that's the sort of thing that the experiment is
meant to discover. It also wasn't my idea in the first place to write
bug reports in Markdown.

> i do mind that you refuse to work with me to make it
>easy.

I'm not clear what kind of working with you to make it easy was missing
here. I don't recall you objecting in advance to the suggestion about
MIME headers. It seems to me that the appropriate next step in working
together on this was in fact to try out what had been suggested to see
how well it works, which is what has just happened.

> refusing to sign up and use it to interact with the
>rest of us who use it as the main way to manage perl is just anti-social.

If you want to say that non-GitHubbers should just go away, say it.

-zefram

On Mon, 20 Mar 2023 at 19:59, Zefram via perl5-porters <
perl5-porters@perl.org> wrote:

> demerphq wrote:
> >At least in this case you are wrong. Marking a hash as readonly does not
> >make its values readonly.
>
> I didn't try to write to its value scalars. I was altering which
> scalars it references, which is (part of) what the readonly flag is
> meant to prevent.
>

No, it is not. The readonly flag enables "restricted hash mode". It means
that you can only interact with specific keys in the hash, typically those
present in the hash when it was marked as readonly. Although in fact keys
can be registered as legal even if they arent in the hash at the time it is
marked readonly (actually "locked"). See Hash::Util::lock_keys_plus().

Anyway, in simple terms this is expected behavior:

perl -le'my %hash= (a=>1,b=>2); Internals::SvREADONLY(%hash,1); delete
$hash{a}; $hash{a}=[]; print $hash{a}'
ARRAY(0x55b2c63a35e8)


>
> > You can delete keys that it contained, or modify them.
>
> If you look at my test case, you'll see that deleting an element, with
> the "delete" operator, *is* prevented.
>

No, you are deleting an element that is marked as read-only itself. You
took a reference to the literal 555. Restricted hashes prevent deleting a
value that is marked as readonly. You just happened to use a test case that
stored a readonly value in the hash in the first place.


>
> > If you want to mark a
> >value in a hash as readonly you must explictly mark *that* variable as
> >readonly.
>
> I'm well aware of the distinction.


Obviously not as well as you think.


> Marking a value scalar as readonly
> is not relevant to the test.


Yes it is.


> If you want to try it, though, insert
> "SetReadOnly(\$a{bb});" into the test case, and you'll see that that
> also doesn't prevent the refaliasing operation.
>

Why would marking an anonymous scalar reference read-only prove anything?


> I'm also aware that there's more to a restricted hash than readonlyness,
> but the additional semantics don't seem relevant here.
>

Yes they are.


>
> >Internals::SetReadOnly is not a documented function,
>
> Ah, this is confusing. What we have here is a collision between the
> Internals module on CPAN and the builtin functions that the Perl core
> puts into the Internals package. Internals::SetReadOnly() is from the
> CPAN module,


Sorry? CPAN module? Internals is a namespace reserved to perl itself or
should be.


> and it very much *is* documented in that module's POD,


IMO It is an abuse of the internals that the Internals module even exists.
That is pretty infuriating.

and
> is in its @EXPORT_OK list. Whereas Internals::SvREADONLY() is supplied
> by the core, and is neither documented in Internals.pm nor in @EXPORT_OK.
>
> > and even then you conflating "container" and contents in this
> >part of your bug report.
>
> No, I'm really not.
>

Yes, really you are.

perl -le'my %hash= (a=>1,b=>2); Internals::SvREADONLY(%hash,1);
Internals::SvREADONLY($hash{a},1); delete $hash{a}; '
Attempt to delete readonly key 'a' from a restricted hash at -e line 1.


> > making it
> >even harder for us
>
> It's harder to handle when correctly labelled? That is a surprising
> finding. If there's a consensus that the correct labelling is
> counterproductive then sure, we can do it with incorrect labelling.
>

There is consensus that what you are doing is annoying everyone who uses
gmail to interact with perl5porters.


>
> > is just over the top.
>
> You write here as if I'm deliberately making things difficult. I'm not.
>

You are. You explained your justification for not signing up to github as
that you consider the legal terms onerous. That justification holds for
your own repositories, it does not hold for interacting with the rest of
the perl developers. Your refusal to use github means that those of us who
care about your opinion have to do extra work to deal with your
submissions, without actually changing anything that would occur if you
signed up, beyond making the process more error prone and removing your
ability to control what is happening. Your commits to perl will still be
created on github as PR's and your issues will still be filed as issues on
github. So in effect all it does is make your and our life difficult
without actually changing anything for your contributions.


> The recent bug reports in Markdown form are partly experimental, testing
> the handling of Markdown in email. I did not know what the outcome
> would be. I did not anticipate that the MIME labelling would actually
> cause a problem, but that's the sort of thing that the experiment is
> meant to discover. It also wasn't my idea in the first place to write
> bug reports in Markdown.
>

No, it was the perl projects decision to migrate to github so that we could
offload a bunch of costs of maintaining the project onto microsoft.


>
> > i do mind that you refuse to work with me to make it
> >easy.
>
> I'm not clear what kind of working with you to make it easy was missing
> here. I don't recall you objecting in advance to the suggestion about
> MIME headers. It seems to me that the appropriate next step in working
> together on this was in fact to try out what had been suggested to see
> how well it works, which is what has just happened.
>

If you want to perform experiments like that then it is common courtesy to
inform folks that they are part of your experiment. Did you post to p5p and
say "I am going to start sending perlbug output to p5p with customized
headers that look like this: X could you please let me know if that causes
issues"?



>
> > refusing to sign up and use it to interact with the
> >rest of us who use it as the main way to manage perl is just anti-social.
>
> If you want to say that non-GitHubbers should just go away, say it.
>

I do not want you to go away. I want you to think a little bit about how
you are externalizing costs on the rest of us without actually changing
anything with regard to your own contributions and how that is essentially
anti-social, and consider what you can do to absolutely minimize the cost
of us having to do extra work for your submissions.

cheers,
Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

demerphq wrote:
>took a reference to the literal 555. Restricted hashes prevent deleting a
>value that is marked as readonly. You just happened to use a test case that
>stored a readonly value in the hash in the first place.

Ah, I stand corrected, thank you. There *is* some relevance of the
restricted hash semantics, and, as you've shown, those semantics are such
that it is correct for them to not prevent the refaliasing operation in
my original test case. I certainly need to look again at the restricted
hash semantics and related subjects.

>Why would marking an anonymous scalar reference read-only prove anything?

There are multiple things going on with this question.

I think you've misunderstood the calls to Internals::SetReadOnly(),
presumably because you're thinking in terms of Internals::SvREADONLY().
They have different argument conventions. Internals::SvREADONLY() (from
the core) has a "\[$%@];$" prototype, meaning that (for bareword calls)
the first argument gets implicitly enreferenced. Internals::SetReadOnly()
(from CPAN) has no prototype, so no implicit enreferencement. Both
require the actual first argument to be a reference to the object
to operate on. So "Internals::SetReadOnly(\$x)" is equivalent to
"Internals::SvREADONLY($x, 1)", not to "Internals::SvREADONLY(\$x, 1)"
(which doesn't compile). So the "SetReadOnly(\$a{bb});" invocation that
I proposed isn't making a completely anonymous scalar (the RV referencing
$a{bb}) readonly, it's making $a{bb} readonly.

My intent was that making the hash element scalar readonly would clear
up a confusion that I thought that you had fallen into. I was mistaken;
you didn't actually have the misunderstanding that I thought you did.
So making that scalar readonly doesn't have the relevance that I thought
it did.

But actually it *does* prove something relevant. Making the hash
element scalar readonly when the hash itself is restricted invokes
exactly that bit of the restricted hash semantics that you explained my
test case accidentally invoked. With the hash element scalar readonly,
deletion via "delete" is prevented, so now ostensibly Perl doesn't allow
that scalar to be replaced. But the refaliasing operation still works,
so in *this* case the refaliasing breaks the hash's restriction:

$ perl -Mexperimental=refaliasing -lwe '%a = (aa=>11,bb=>22,cc=>33); Internals::SvREADONLY(%a, 1); Internals::SvREADONLY($a{bb}, 1); delete $a{bb}'
Attempt to delete readonly key 'bb' from a restricted hash at -e line 1.
$ perl -Mexperimental=refaliasing -lwe '%a = (aa=>11,bb=>22,cc=>33); Internals::SvREADONLY(%a, 1); Internals::SvREADONLY($a{bb}, 1); print \$a{bb}; \$a{bb} = \555; print \$a{bb}'
SCALAR(0x55f3eee968a0)
SCALAR(0x55f3eefd7aa8)

>Sorry? CPAN module? Internals is a namespace reserved to perl itself or
>should be.

As I said, confusing. It's a confusion of long standing.

> You explained your justification for not signing up to github as
>that you consider the legal terms onerous. That justification holds for
>your own repositories, it does not hold for interacting with the rest of
>the perl developers.

Joining GitHub for any purpose would mean accepting its legal terms.
My reason for not joining GitHub applies to having an account there at
all, not to only some of the activities that would be enabled by such
an account.

-zefram

On Mon, 20 Mar 2023 at 21:33, Zefram via perl5-porters <
perl5-porters@perl.org> wrote:

> $ perl -Mexperimental=refaliasing -lwe '%a = (aa=>11,bb=>22,cc=>33);
> Internals::SvREADONLY(%a, 1); Internals::SvREADONLY($a{bb}, 1); print
> \$a{bb}; \$a{bb} = \555; print \$a{bb}'
> SCALAR(0x55f3eee968a0)
> SCALAR(0x55f3eefd7aa8)
>

Yep, that is a bug.

Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"

On Mon, Mar 20, 2023 at 09:41:19PM +0100, demerphq wrote:
> On Mon, 20 Mar 2023 at 21:33, Zefram via perl5-porters <
> perl5-porters@perl.org> wrote:
>
> > $ perl -Mexperimental=refaliasing -lwe '%a = (aa=>11,bb=>22,cc=>33);
> > Internals::SvREADONLY(%a, 1); Internals::SvREADONLY($a{bb}, 1); print
> > \$a{bb}; \$a{bb} = \555; print \$a{bb}'
> > SCALAR(0x55f3eee968a0)
> > SCALAR(0x55f3eefd7aa8)
> >
>
> Yep, that is a bug.

Note that this is a general issue with aliasing, e.g. via 'local', not
just with the \ refaliasing mechanism:

%a = (aa=>11,bb=>22,cc=>33);
Internals::SvREADONLY(%a, 1);
Internals::SvREADONLY($a{bb}, 1);
local $a{bb} = 555 if @ARGV;
delete $a{bb};


$ pp
Attempt to delete readonly key 'bb' from a restricted hash at /home/davem/tmp/p line 9.
$ pp 1
$


--
It's not that I'm afraid to die, I just don't want to be there when it
happens.
-- Woody Allen

On Thu, 23 Mar 2023 12:20:52 +0000
Dave Mitchell <davem@iabyn.com> wrote:

> Note that this is a general issue with aliasing, e.g. via 'local', not
> just with the \ refaliasing mechanism:

Yes I was about to compare it with `local`. A refaliasing is really the
same as a local only without the scope-guarded undo part.

--
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk | https://metacpan.org/author/PEVANS
http://www.leonerd.org.uk/ | https://www.tindie.com/stores/leonerd/

On Thu, 23 Mar 2023 at 14:51, Paul "LeoNerd" Evans
<leonerd@leonerd.org.uk> wrote:
>
> On Thu, 23 Mar 2023 12:20:52 +0000
> Dave Mitchell <davem@iabyn.com> wrote:
>
> > Note that this is a general issue with aliasing, e.g. via 'local', not
> > just with the \ refaliasing mechanism:
>
> Yes I was about to compare it with `local`. A refaliasing is really the
> same as a local only without the scope-guarded undo part.

Let me know if you poke into this. I will take a look when i get a chance.

Yves


--
perl -Mre=debug -e "/just|another|perl|hacker/"

On Thu, 23 Mar 2023 15:06:16 +0100
demerphq <demerphq@gmail.com> wrote:

> On Thu, 23 Mar 2023 at 14:51, Paul "LeoNerd" Evans
> <leonerd@leonerd.org.uk> wrote:
> >
> > On Thu, 23 Mar 2023 12:20:52 +0000
> > Dave Mitchell <davem@iabyn.com> wrote:
> >
> > > Note that this is a general issue with aliasing, e.g. via
> > > 'local', not just with the \ refaliasing mechanism:
> >
> > Yes I was about to compare it with `local`. A refaliasing is really
> > the same as a local only without the scope-guarded undo part.
>
> Let me know if you poke into this. I will take a look when i get a
> chance.

I wasn't intending to "poke into" this. I was simply going to point out
that probably refaliasing and `local` ought to behave the same way. It
turns out that they already do, so maybe we don't have to do anything
at all.

Alternatively, if we decide we should change the way they work, we
should make sure to keep them consistent and change both of them at
once.

Changing the way that `local` works on SvREADONLY containers sounds
like a rather big and invasive change to make at this point.

Good luck.

--
Paul "LeoNerd" Evans

leonerd@leonerd.org.uk | https://metacpan.org/author/PEVANS
http://www.leonerd.org.uk/ | https://www.tindie.com/stores/leonerd/

On Thu, 23 Mar 2023 at 15:29, Paul "LeoNerd" Evans
<leonerd@leonerd.org.uk> wrote:
> Changing the way that `local` works on SvREADONLY containers sounds
> like a rather big and invasive change to make at this point.

For now im only interesting in the restricted hash case, which
actually turns out to be easy to fix under local. I havent worked out
how the refaliasing works yet.

Yves


--
perl -Mre=debug -e "/just|another|perl|hacker/"

Paul "LeoNerd" Evans wrote:
>Alternatively, if we decide we should change the way they work, we
>should make sure to keep them consistent and change both of them at
>once.

I concur. They should be consistent with each other, and I reckon that
both should change, because the current semantics make readonlyness of
a container somewhat meaningless.

>Changing the way that `local` works on SvREADONLY containers sounds
>like a rather big and invasive change to make at this point.

Yes, it's not for 5.38. If it had just been refaliasing then it could
have been, but local is too widely used to change it at such short notice.
(When I reported the bug I thought that it was just refaliasing, and
put it down to that being an underdeveloped feature.)

-zefram

On Thu, 23 Mar 2023 at 20:23, Zefram via perl5-porters
<perl5-porters@perl.org> wrote:
>
> Paul "LeoNerd" Evans wrote:
> >Alternatively, if we decide we should change the way they work, we
> >should make sure to keep them consistent and change both of them at
> >once.
>
> I concur. They should be consistent with each other, and I reckon that
> both should change, because the current semantics make readonlyness of
> a container somewhat meaningless.
>
> >Changing the way that `local` works on SvREADONLY containers sounds
> >like a rather big and invasive change to make at this point.

I think it is better to avoid making judgements like this until you
actually know what code is involved.

> Yes, it's not for 5.38. If it had just been refaliasing then it could
> have been, but local is too widely used to change it at such short notice.
> (When I reported the bug I thought that it was just refaliasing, and
> put it down to that being an underdeveloped feature.)

At least as far as hashes go it does not appear to be that much code,
as far as I can tell the logic is pretty well abstracted already.

cheers,
Yves


--
perl -Mre=debug -e "/just|another|perl|hacker/"

On Thu, 23 Mar 2023 at 21:46, demerphq <demerphq@gmail.com> wrote:
>
> On Thu, 23 Mar 2023 at 20:23, Zefram via perl5-porters
> <perl5-porters@perl.org> wrote:
> >
> > Paul "LeoNerd" Evans wrote:
> > >Alternatively, if we decide we should change the way they work, we
> > >should make sure to keep them consistent and change both of them at
> > >once.
> >
> > I concur. They should be consistent with each other, and I reckon that
> > both should change, because the current semantics make readonlyness of
> > a container somewhat meaningless.
> >
> > >Changing the way that `local` works on SvREADONLY containers sounds
> > >like a rather big and invasive change to make at this point.
>
> I think it is better to avoid making judgements like this until you
> actually know what code is involved.
>
> > Yes, it's not for 5.38. If it had just been refaliasing then it could
> > have been, but local is too widely used to change it at such short notice.
> > (When I reported the bug I thought that it was just refaliasing, and
> > put it down to that being an underdeveloped feature.)
>
> At least as far as hashes go it does not appear to be that much code,
> as far as I can tell the logic is pretty well abstracted already.

Needs tests, but it pretty much works afaict:

https://github.com/Perl/perl5/pull/20963

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

demerphq wrote:
>I think it is better to avoid making judgements like this until you
>actually know what code is involved.

The issue isn't that it's a lot of core code, but the extent of the
code written in Perl that could potentially be affected. local has been
around for a long time, including the ability to localise elements of a
read-only container. More of an issue for arrays than hashes, though,
I think, because restricted hashes aren't quite so old or widely used.

>Needs tests, but it pretty much works afaict:
>
>https://github.com/Perl/perl5/pull/20963

That prevents unpermitted changes being made by the inward phase of
localisation, but it's still possible for the restoration to make an
unpermitted change if the hash became more restricted during the scope
of the localisation:

$ perl -lwe '%a=qw(a z c d); { local $a{a} = "b"; Internals::SvREADONLY(%a, 1); Internals::SvREADONLY($a{a}, 1); print %a; } print %a'
cdab
cdaz

There's some question about the legitimacy of that particular test case:
local specifically supplies a mutable scalar, and is the user implicitly
promising not to make it readonly? In general I think it's legitimate
to make a thing readonly after a mutable construction phase, and in this
case that goes for both the hash and the scalar. But I don't have a
feel for what sort of things ought to be possible with a restricted hash.
I find them a very weird concept, especially in the way that mutability
of the hash container depends on mutability of the scalar object that
it contains.

There are many other permutations to consider. A particularly neat one,
because it doesn't require any explicit readonly flag setting within
the local scope, comes from combining local and refaliasing:

$ perl -Mexperimental=refaliasing,declared_refs -lwe '%a=qw(a z c d); Internals::SvREADONLY(%a, 1); { local \$a{a} = \"b"; print %a; } print %a'
abcd
azcd

It's arguable that that one is a distinct bug. If one makes sense of a
restricted hash by saying that the readonlyness of the element scalars is
semantically part of the container, then it is wrong for refaliasing to
be willing to put a readonly scalar into such a hash, whether localised
or not. But that's probably the wrong view of restricted hashes.

-zefram

On Fri, 24 Mar 2023 at 09:14, Zefram via perl5-porters
<perl5-porters@perl.org> wrote:
>
> demerphq wrote:
> >I think it is better to avoid making judgements like this until you
> >actually know what code is involved.
>
> The issue isn't that it's a lot of core code, but the extent of the
> code written in Perl that could potentially be affected. local has been
> around for a long time, including the ability to localise elements of a
> read-only container. More of an issue for arrays than hashes, though,
> I think, because restricted hashes aren't quite so old or widely used.
>
> >Needs tests, but it pretty much works afaict:
> >
> >https://github.com/Perl/perl5/pull/20963
>
> That prevents unpermitted changes being made by the inward phase of
> localisation, but it's still possible for the restoration to make an
> unpermitted change if the hash became more restricted during the scope
> of the localisation:
>
> $ perl -lwe '%a=qw(a z c d); { local $a{a} = "b"; Internals::SvREADONLY(%a, 1); Internals::SvREADONLY($a{a}, 1); print %a; } print %a'
> cdab
> cdaz

I dont see a problem here. The local is ending and we are putting it
back to the original state. I dont think it matters that the new value
is readonly.

> There's some question about the legitimacy of that particular test case:
> local specifically supplies a mutable scalar, and is the user implicitly
> promising not to make it readonly? In general I think it's legitimate
> to make a thing readonly after a mutable construction phase, and in this
> case that goes for both the hash and the scalar. But I don't have a
> feel for what sort of things ought to be possible with a restricted hash.
> I find them a very weird concept, especially in the way that mutability
> of the hash container depends on mutability of the scalar object that
> it contains.

AFAIK they were invented to get rid of pseudo-hashes, which themselves
were an attempt to make it possible to detect mistyped keys in a hash
based OO model. I think they are basically a failed experiment,
except that every time I say that someone pops up and says "but I like
them". :-(

> There are many other permutations to consider. A particularly neat one,
> because it doesn't require any explicit readonly flag setting within
> the local scope, comes from combining local and refaliasing:
>
> $ perl -Mexperimental=refaliasing,declared_refs -lwe '%a=qw(a z c d); Internals::SvREADONLY(%a, 1); { local \$a{a} = \"b"; print %a; } print %a'
> abcd
> azcd

The one is also fine, as the value is not readonly when the
localization happens.

>
> It's arguable that that one is a distinct bug. If one makes sense of a
> restricted hash by saying that the readonlyness of the element scalars is
> semantically part of the container, then it is wrong for refaliasing to
> be willing to put a readonly scalar into such a hash, whether localised
> or not. But that's probably the wrong view of restricted hashes.

I think there is a real danger of overthinking this.

Nothing says that unlocalizing a value that has been marked as
readonly is illegal, and IMO if we did consider it illegal it would be
pretty confusing, so lets simply NOT do that. On the other hand,
double localizing the value once it has been marked as readonly should
be illegal, and with my patch is illegal. On the other hand, it seems
that given a read-only value is not allowed to be updated or deleted,
it also shouldn't be allowed to be localized. Which my patch prevents

So I dont think there is any controversy here. All we need to do is
prevent aliases and localization from touching a read-only value in a
restricted hash.

Yves


--
perl -Mre=debug -e "/just|another|perl|hacker/"

demerphq wrote:
>I dont see a problem here. The local is ending and we are putting it
>back to the original state.

The state immediately before the end of the local scope is a restricted
hash, which appears to preclude replacing that key. One can't make that
change in the ordinary way, and so I reckon the restoration via local
should also be unable to make that change. local restoration doesn't
have a general exemption from the language semantics.

If local restoration is exempt from immutability (indeed, if anything is
semantically exempt from immutability) then that destroys the usefulness
of immutability. It would be impossible to rely on readonly values not
changing, and that's a really useful thing to rely on. In practice, of
course, people would just rely on them not changing anyway, and get burned
when they actually change. It's a formula for widespread subtle bugs.

If you really want local restoration to be exempt, though, then this is
a bug (still present in your branch):

$ perl -lwe '%a = qw(aa bb cc dd); { delete local $a{aa}; Internals::SvREADONLY(%a, 1); print %a; } print %a'
ccdd
Attempt to access disallowed key 'aa' in a restricted hash at -e line 1.

> I think they are basically a failed experiment,
>except that every time I say that someone pops up and says "but I like
>them". :-(

I'd like to introduce straightforward read-only hashes. It shouldn't be
much work. To coexist alongside restricted hashes we'd need one extra
flag on HVs saying which kind of read-only it is if it's SvREADONLY.

> and IMO if we did consider it illegal it would be
>pretty confusing,

Isn't it more confusing to have a read-only object change its value?

> given a read-only value is not allowed to be updated or deleted,
>it also shouldn't be allowed to be localized.

I disagree. If local restoration is allowed to alter the state of a
read-only object then I reckon it's equally legitimate for the forward
side of localisation to change it. It's not making a change to the
object in the dynamic scope where it had that read-only state; it's just
temporarily hiding that state. I don't think that allowing localisation
to affect read-only objects makes for good semantics, but at least it
would be consistent semantics if dynamic scope boundary crossings were
free to do this in both directions.

-zefram

On Sat, 25 Mar 2023 at 20:37, Zefram via perl5-porters
<perl5-porters@perl.org> wrote:
>
> demerphq wrote:
> >I dont see a problem here. The local is ending and we are putting it
> >back to the original state.
>
> The state immediately before the end of the local scope is a restricted
> hash, which appears to preclude replacing that key. One can't make that
> change in the ordinary way, and so I reckon the restoration via local
> should also be unable to make that change. local restoration doesn't
> have a general exemption from the language semantics.

I don't agree. We can do what we want with restricted hashes, and the
logical thing for this case is to allow local to undo whatever change
was made to the value since the localization, just like local undoes
any other change to a value.

>
> If local restoration is exempt from immutability (indeed, if anything is
> semantically exempt from immutability) then that destroys the usefulness
> of immutability.

That is the fallacy of the excluded middle. You are welcome to your
opinion, but I don't see any contradiction.

These are /restricted hashes/. You are imputing that restricted hashes
are immutable in a way that hasn't been agreed on. You already said
you don't understand restricted hashes, and now you are taking the
most extreme interpretation of what they mean. IMO that is not
reasonable.

> It would be impossible to rely on readonly values not
> changing, and that's a really useful thing to rely on. In practice, of
> course, people would just rely on them not changing anyway, and get burned
> when they actually change. It's a formula for widespread subtle bugs.

Sorry, I don't agree. I think it is an extreme interpretation that is
unnecessary to take. A key in a restricted hash is mutable until
marked as readonly. If someone localizes a mutable key, and marks the
localized version as readonly, that does not preclude the localization
ending, it merely precludes that anyone can modify that now readonly
key until they turn off the readonly flag, or the localization ends.
So for instance it precludes the localized item being localized again,
but that doesn't mean it should undo the localization. I can entirely
imagine someone localizing the key *because* they are going to mark it
readonly, and want to be sure that it gets restored to mutable if
there is an exception in their logic. That sounds like a much more
reasonable behavior than what you are suggesting.

Restricted hashes are designed to have their values readonlyness
change dynamically. It is part of the API. All we are saying is that
when they are delocalized they return to their previous state of
unlocked. (After all you can only localize an unlocked key.)

> If you really want local restoration to be exempt, though, then this is
> a bug (still present in your branch):
>
> $ perl -lwe '%a = qw(aa bb cc dd); { delete local $a{aa}; Internals::SvREADONLY(%a, 1); print %a; } print %a'
> ccdd
> Attempt to access disallowed key 'aa' in a restricted hash at -e line 1.

That looks like a "don't do that" bug to me. if you lock a hash after
localizing its keys you get to keep the exception that it triggers. We
can simply document "don't do that". And considering it does trigger
an exception you can be sure nobody is doing it. Ideally we could
check and see if a hash had any localized members and refuse to mark
it as restricted if it did, but that is probably infeasible so we
should just tell people "don't do that".

> > I think they are basically a failed experiment,
> >except that every time I say that someone pops up and says "but I like
> >them". :-(
>
> I'd like to introduce straightforward read-only hashes. It shouldn't be
> much work. To coexist alongside restricted hashes we'd need one extra
> flag on HVs saying which kind of read-only it is if it's SvREADONLY.

I am curious what you mean by that. I have implemented protected
hashes in #20928, but they are pretty much the same as restricted
hashes with the caveat that they dont throw an exception when someone
reads a key that does not exist.

> > and IMO if we did consider it illegal it would be
> >pretty confusing,
>
> Isn't it more confusing to have a read-only object change its value?

No, not really. With restricted hashes the notion that a value or the
entire hash might change from "readonly" to "mutable" during process
lifetime is part of the API. That it happens when the localization is
undone is pretty reasonable. Its no different from the value changing
from "a" to "b".

> > given a read-only value is not allowed to be updated or deleted,
> >it also shouldn't be allowed to be localized.
>
> I disagree. If local restoration is allowed to alter the state of a
> read-only object then I reckon it's equally legitimate for the forward
> side of localisation to change it.

Well, I think it is consistent and i think its what most people
familiar with restricted hashes would expect.

I start off with a hash with unrestricted values but restricted keys,
i localize one of the unrestricted keys, I expect when that
localization is undone for the localization to be undone /whatever
happened to value/. If I lock one of the keys, i then expect that it
cant be localized or otherwise changed until it is unlocked. If i
lock a key that was localized then when it gets unlocalized it gets
unlocked. Seems pretty straight forward, consistent with established
norms and reasonable to me.

> It's not making a change to the
> object in the dynamic scope where it had that read-only state; it's just
> temporarily hiding that state. I don't think that allowing localisation
> to affect read-only objects makes for good semantics, but at least it
> would be consistent semantics if dynamic scope boundary crossings were
> free to do this in both directions.

I don't agree. I think if you showed sample code implementing the
simple interpretation I propose to most Perl hackers they would
consider it pretty reasonable (at least from the point of view of how
restricted hashes already behave).

There is no reason that consistency is required between localization
and delocalization. You seem fixated on that without any good reason,
and your personal definition of "consistency" isn't a good reason IMO.
I think "consistency with how local works generally" is more important
than "consistency with how locked hashes work when the key is locked".
After all the local came first historically, and (assuming my patches
are applied) it came first imperatively in any program as you can't
localize a locked key.

cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"

demerphq wrote:
>These are /restricted hashes/. You are imputing that restricted hashes
>are immutable in a way that hasn't been agreed on.

With respect to restricted hashes, I'm only expecting them to maintain
the level of immutability that they display in normal hash operations.
Some aspects of them are immutable, others are not, and it seems to be
pretty well established which parts are which, excluding localisation
and refaliasing. But I think restricted hashes are a distraction here.
The same principles also apply to read-only arrays, which have much more
straightforward semantics. Most of the same reasoning about language
semantics is applicable there.

Ultimately I'm more interested in the semantics of read-only arrays,
and of the not-restricted-hash read-only hashes that I'd like to
add. I've been debating on the basis that the immutable aspects of
read-only arrays should behave the same as the immutable aspects of
restricted hashes, assuming that consistency between them is desirable.
But restricted hashes could end up getting even weirder semantics that
would break that analogy.

>I am curious what you mean by that.

My idea of straightforward read-only hashes is that the read-only-ness
of the hash governs the mapping of keys to element scalars. That is,
in a read-only hash one would be unable to insert any new key, delete
any existing key, or change to which scalar any existing key refers.
This differs from a restricted hash in that existing elements can't be
deleted even if the element scalar is writable, and there's no concept
of a deleted key that it is permitted to re-add. (Also lookups of
non-existent keys wouldn't die, but that's a superficial issue.)

I find this kind of read-only-ness to be natural: within a data structure
it means that we have a read-only flag controlling everything mutable in
the range from the hash identity up to the point where other read-only
flags (on the element scalars) take over. It's also the direct parallel
of the read-only-ness of arrays, which is similarly natural.

-zefram