Mailing List Archive

On Sat, Aug 07, 2021 at 01:50:05AM +0100, Paul "LeoNerd" Evans wrote:
> I've started a branch to tidy up some edges around the SvFLAGS
> constants.
>
> https://github.com/leonerd/perl5/tree/sv-flags-tidy
>
> So far I've:
>
> *) Created SVphv_HASAUX as an alias of SVf_OOK, so the flag/macro is
> named less confusingly there
>
> *) Renamed SVf_AMAGIC to SVphv_AMAGIC because it's only applied to
> stashes

These might break some XS modules, but probably not many.

> I've also noticed that the SVp_SCREAM bit is currently not used on
> regular scalars except SvROK ones, and the SVf_IVisUV bit is only used
> on numerical (non-referential) scalars. I'm now testing a change to
> move that latter bit out from its own current position, into the same
> value as SVp_SCREAM. If this works, that will give us two spare free
> bits on regular scalars, conveniently right next to the SVf_UTF8 bit.

I don't understand how this gives you two spare bits. At most it should
give you one more for non-ref scalars. But bear in mind that SVf_IVisUV
can be set on a POK scalar. This:

use Devel::Peek;
my $x = 0xffffffffffffffff;
my $y = "$x";
Dump $x;

gives:

SV = PVIV(0xdb0728) at 0xdafc68
REFCNT = 1
FLAGS = (IOK,POK,pIOK,pPOK,IsUV)
UV = 18446744073709551615
PV = 0xda2208 "18446744073709551615"\0
CUR = 20
LEN = 22


> We'll need two spare bits here if we want to track UNKNOWN|BYTES|TEXT
> state of strings.

If is a big word here.

> I imagine there probably isn't any compatibility problem with changing
> the value of a flag bit,

There's no problem changing the value of a flag, unless XS code is using
a "wrong" name for a flag. For example if a few years down the line XS
code is still testing for 'SVf_OOK' rather than 'SVphv_HASAUX' on hashes,
and we in the meantime stop them being synonyms and assign them different
values, then that XS will break.

> but perhaps to be on the safe side, if people
> think it best, I might give it new name while I'm moving it. Since it
> only applies (privately) to non-reference scalars, perhaps it should be
> called SVp_IVisUV instead?

Its not a private flag - its a public adjunct to the IOK flag to
distinguish signed from unsigned integers (normally only integers greater
in value than 0x7ff.....f get this flag set).

> It's possible that SVprv_WEAKREF will also get in the way here, and if
> so I may have to change its value too, but again the same compat
> questions apply.

Same answers apply.

--
"Foul and greedy Dwarf - you have eaten the last candle."
-- "Hordes of the Things", BBC Radio.

On Sat, 7 Aug 2021 12:39:42 +0100
Dave Mitchell <davem@iabyn.com> wrote:

> > *) Created SVphv_HASAUX as an alias of SVf_OOK, so the flag/macro
> > is named less confusingly there
> >
> > *) Renamed SVf_AMAGIC to SVphv_AMAGIC because it's only applied to
> > stashes
>
> These might break some XS modules, but probably not many.

I suspect some decent fraction of those would be mine ;)
(Future::AsyncAwait and Devel::MAT come to mind...)

Though so far all I've done is added extra names for the existing
macros - no code should break as the old names still exist, with their
current values. The changes just mean that whenever you see "SvOOK" in
the core code you now know it's about string PV offsets, and not HV AUX
structures.

> > I've also noticed that the SVp_SCREAM bit is currently not used on
> > regular scalars except SvROK ones, and the SVf_IVisUV bit is only
> > used on numerical (non-referential) scalars. I'm now testing a
> > change to move that latter bit out from its own current position,
> > into the same value as SVp_SCREAM. If this works, that will give us
> > two spare free bits on regular scalars, conveniently right next to
> > the SVf_UTF8 bit.
>
> I don't understand how this gives you two spare bits.

Oh oops I should have given more context. The SVp_IVisUV bit currently
occupies the very topmost flag bit (1<<31); the bit below that (1<<30)
is not currently used by regular scalar values. Those bits would both
be free to be used together.

> At most it
> should give you one more for non-ref scalars. But bear in mind that
> SVf_IVisUV can be set on a POK scalar. This:
>
> use Devel::Peek;
> my $x = 0xffffffffffffffff;
> my $y = "$x";
> Dump $x;
>
> gives:
>
> SV = PVIV(0xdb0728) at 0xdafc68
> REFCNT = 1
> FLAGS = (IOK,POK,pIOK,pPOK,IsUV)
> UV = 18446744073709551615
> PV = 0xda2208 "18446744073709551615"\0
> CUR = 20
> LEN = 22

Yeah that's OK - as far as I can make out, SvROK is mutually exclusive
with having any of the "normal" number or string fields. The effect of
my change is that SvFLAGS() & (1<<16) means SVprv_PCS_IMPORTED if
SvROK() is set, and SVf_IVisUV if it is clear.

A side-effect of this work is that I've been writing myself up a little
table of what flags have what meanings to what kinds of SV - I've found
it necessary to distinguish SvROK()=1 from SvROK()=0 cases, but within
the non-ROK it's all a big mix of state. I hope to have something
suitable to publish when I'm done, quite apart from any actual code
fixes.

> > We'll need two spare bits here if we want to track
> > UNKNOWN|BYTES|TEXT state of strings.
>
> If is a big word here.

"If" is a big word, for sure :) Even if that task doesn't happen, I'm
sure someone in the future will appreciate us tidying up the SV flags a
bit to make some space for whatever feature does turn up in that space.

> > I imagine there probably isn't any compatibility problem with
> > changing the value of a flag bit,
>
> There's no problem changing the value of a flag, unless XS code is
> using a "wrong" name for a flag. For example if a few years down the
> line XS code is still testing for 'SVf_OOK' rather than
> 'SVphv_HASAUX' on hashes, and we in the meantime stop them being
> synonyms and assign them different values, then that XS will break.

I guess this is less a question and more an invite to discussion. We
don't really have a formal spec for what things are "official API".
It's clear that CPAN XS authors make use of a great number of things
beyond that simply marked as such in embed.fnc, and many things there
that everyone would complain if we were to change.

I'm more after a sortof "gut feel" for where on the spectrum we feel we
could put the bitvalues of these flags. If we feel it's broadly OK to
change them from time to time, then I'm quite happy to go on record as
saying "I changed it, blame me; show me your code and I'll fix it" if
someone complains we broke their code.

> > but perhaps to be on the safe side, if people
> > think it best, I might give it new name while I'm moving it. Since
> > it only applies (privately) to non-reference scalars, perhaps it
> > should be called SVp_IVisUV instead?
>
> Its not a private flag - its a public adjunct to the IOK flag to
> distinguish signed from unsigned integers (normally only integers
> greater in value than 0x7ff.....f get this flag set).

Hmm - perhaps "private" isn't the right word. It's more that some of
these flags seem to apply to _any_ kind of SV, in that they relate to
the SV head itself (e.g. SVf_BREAK relates to refcount), whereas other
flags have a type-specific meaning (e.g. SVphv_HASAUX). SVf_IVisUV is
really in this latter category - it literally makes no sense on e.g. an
SVt_PVHV. I feel if I'm going change its value it might be safer to
give it a new name at the same time.

Also, given as we provide the macro SvIVisUV(), surely we can call this
flag "private" in the sense that users shouldn't be using it, they
should be using only the macro..?


However, at the moment the whole thing is somewhat of an academic
question. Having simply changed the bitwise value of the flag, the test
suite now fails... well actually I don't know how many tests. The full
list of failures scrolled off the top of my terminal. Suffice it to say
this one isn't ready yet and may require more work. ;)

--
Paul "LeoNerd" Evans

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