Clarify pod for some cophh functions

[khwilliamson]

This includes adding information about several variants of cophh_exists

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

[tonycoz]

The hash "computed automatically at compile time" is not true for any of the functions described here as far as I can tell.

[jkeenan]

The hash "computed automatically at compile time" is not true for any of the functions described here as far as I can tell.

@khwilliamson, is there any part of this pull request that, given @tonycoz's observations above, can go forward?

[khwilliamson]

There are a bunch of functions listed in perlapi that claim that the hash is computed automatically at compile time when the parameter is a string literal, hence known at compile time. That claim is false for all the ones I have just investigated. There's another version of the claim that says that it is "determinable" at compile time, which is more true, but gives false hope to the reader. Does anyone know if these claims were once true?

Could the claim be made valid if instead of passing 0 to the underlying function, we pass the result of PERL_HASH?

[tonycoz]

It may be you were confused by (removed by 704ee14 and replaced by the compile-time wording):

  The
C<hash> parameter is the precomputed hash value; if it is zero then
Perl will compute it.

and assumed that happened at compile-time.

There's the SV_CONST() macro, but again, those are demand calculated and irrelevant to the s suffix APIs.

Could the claim be made valid if instead of passing 0 to the underlying function, we pass the result of PERL_HASH?

PERL_HASH() depends on PL_hash_seed_w (often referenced via PL_hash_seed) which is initialised during perl startup, it isn't a compile-time constant and even if it was I don't see a C compiler[1] evaluating the hash function at compile-time for it.

Passing 0 simply triggers:

    else if (!hash)
        PERL_HASH(hash, key, klen);

in hv_common(), so I don't think it helps, either way it's computed at runtime and on each call.

I didn't see anything relevant with "determinable" but I didn't look hard either.

[1] C++ can if you ask nicely, but of course it would require all compile-time known input, which PL_hash_seed isn't.I didn't see anything relevant with "determinable" but I didn't look hard either.

[khwilliamson]

This doesn't sound to me like a mistake I would be inclined to make. But I can't find where the statement originally came from. So, I will submit a PR to remove the similar false claims from other functions not covered by this one. In the meantime, I've removed them from this PR

[tonycoz]

It may be worth mentioning the wrapper cop_hints_* APIs which take a COP* instead of a COPHH *. (there's no cop_hints_store_*() which may be a good thing)

I don't think these are ever used outside of a COP.

[khwilliamson]

I didn't see a wrapper for delete either. Changed to refer to the wrappers for fetch and exists

[tonycoz]

I don't think these are ever used outside of a COP.

Given our discussion in IRC, I'll expand on this.

A COP is the type used for nextstate and dbstate ops, it stores the filename and line number, warning bits, hints bits, feature flags, etc and a sort of incremental representation of the hints hash for the following code.

Where you might test the hash from hints_hash method for a B::COP in Perl code, in XS you'll use these functions (or the cop_hints_*() variants).

AFAIK there's no use of COPHH (aka struct refcounted_he) outside of COPs.

So you typically want to use the cop_hints_*() entry points.

[Leont]

AFAIK there's no use of COPHH (aka struct refcounted_he) outside of COPs.

So you typically want to use the cop_hints_*() entry points.

Agreed. Any normal module would use cop_hints_*, only direct use I can imagine for cophh_* would be something that creates optrees.

[khwilliamson]

I'm unsure of what you're asking

To be clear, I'm trying to make perlapi more user friendly by combining closely related entries to remove redundancies in the pod, and to enable the reader to see all the similar possibilities at a glance, so as to more easily compare and contrast them, and hence to choose the best form for their needs.

I'm doing that with functions that I know next to nothing about, just taking the existing text and massaging it, without making things less clear, hopefully.

What I think I'm hearing is that the existing text should be changed anyway, that maybe even the cophh forms shouldn't have been publicly exposed.? I'm happy to make such changes, but I don't know what I'm talking about. I don't know if the hints @tonycoz gave are enough.

[tonycoz]

What I think I'm hearing is that the existing text should be changed anyway, that maybe even the cophh forms shouldn't have been publicly exposed.? I'm happy to make such changes, but I don't know what I'm talking about. I don't know if the hints @tonycoz gave are enough.

It's more a matter of emphasis, you gave:

See L</cop_hints_fetch_pv> for equivalent forms that take a COP* argument
instead of a COPHH* one.

I'd be more inclined to give them a harder push to the cop_hints_*() functions:

You should probably be using the wrappers for these functions that take a C<COP*>, see L</cop_hints_fetch_pv>.

The _store and _delete variants should only be used if you're building an OP tree from scratch, if you have an XS import() method that uses these (on &PL_compiling->cop_hints_hash) any calling code that uses %^H is going to get confused.

I'll add some comments on the text too, some is potentially confusing.

[tonycoz]

It might be worth always referring to the cop hints hash as "cophh", since the references to "hash" here might be confused with the parameter called "hash".

Similarly for delete.

[khwilliamson]

Done, I think