From @karenetheridge

From a conversation between ribasushi and Father C. I just learned about the existence of overload​::mycan. However, this is not documented at all. If it is a public interface, it should be documented in the overload pod. If it's not a public interface, it should start with a leading _ (underscore).

From @cpansprout

On Wed Sep 10 10​:24​:18 2014, ether wrote​:

From a conversation between ribasushi and Father C. I just learned
about the existence of overload​::mycan. However, this is not
documented at all. If it is a public interface, it should be
documented in the overload pod. If it's not a public interface, it
should start with a leading _ (underscore).

It’s definitely not public. There are actually quite a few subroutines in overload.pm that are not documented. Some of them go all the way back to perl 5.002, which introduced overload (see 4633a7c).

Let’s see​:

- OVERLOAD​:
  overload​::import does $package->overload​::OVERLOAD, so this could
  be folded into import. But Class​::Trait calls it directly.
- OverloadedStringify and AddrRef
  Originally used as subroutines by StrVal, these are no longer used
  by overload.pm itself. But Smart​::Match uses OverloadedStringify
  and Devel​::StrackTrace uses AddrRef (which is now an alias for
  StrVal).

Later, these were added​:

- nil
  An empty sub used as a placeholder recognized by gv.c. We could
  rename it _nil and change gv.c to match. But Role​::Tiny uses that.
- ov_method
  Internal, but XML​::Declare uses it.
- can
  Ditto.

A leading underscore is the usual convention, but not everything has to follow that. My usual rule is​: If it’s not documented, don’t use it if you don’t want to risk having your code break.

If we really wanted to make these things private, then why not use lexical subroutines since we have them (except for nil)? overload.pm is core-only, after all.

But seeing that overload.pm has been very stable for years, I’m not sure it’s worth breaking code for cosmetic reasons. (I.e., these modules are taking the risk and we’ll break them if we have to, but only if we have to.)

--

Father Chrysostomos

The RT System itself - Status changed from 'new' to 'open'

From @karenetheridge

On Wed Sep 10 10​:53​:04 2014, sprout wrote​:

It’s definitely not public. There are actually quite a few
subroutines in overload.pm that are not documented...

- nil
An empty sub used as a placeholder recognized by gv.c. We could
rename it _nil and change gv.c to match. But Role​::Tiny uses that.

Role​::Tiny now looks for _nil as well​: ;)

moose/Role-Tiny@6c9c9b5

From @cpansprout

On Wed Sep 10 11​:03​:16 2014, ether wrote​:

On Wed Sep 10 10​:53​:04 2014, sprout wrote​:

It’s definitely not public. There are actually quite a few
subroutines in overload.pm that are not documented...

- nil
An empty sub used as a placeholder recognized by gv.c. We could
rename it _nil and change gv.c to match. But Role​::Tiny uses
that.

Role​::Tiny now looks for _nil as well​: ;)

https://github.com/moose/Role-
Tiny/commit/6c9c9b5995c4e613bd6819d8ef52824f6167d7e6

OK, so what is the underscore for, then? :-)

Anyway, I just stopped at the first module I found for each function. These also use nil​:

MooseX​::Role​::WithOverloading
Devel​::OverloadInfo
Test​::Modern
Package​::Anon (in its tests)
Test​::CleanNamespaces

--

Father Chrysostomos

From @karenetheridge

On Wed, Sep 10, 2014 at 11​:18​:12AM -0700, Father Chrysostomos via RT wrote​:

Role​::Tiny now looks for _nil as well​: ;)

https://github.com/moose/Role-
Tiny/commit/6c9c9b5995c4e613bd6819d8ef52824f6167d7e6

OK, so what is the underscore for, then? :-)

A more public indication of​: "we're using a private sub and we understand
that this might break at any time. we'll accept those consequences,
whatever they are."

Anyway, I just stopped at the first module I found for each function. These also use nil​:

MooseX​::Role​::WithOverloading
Test​::CleanNamespaces

I pushed updates for those as well.

Devel​::OverloadInfo
Test​::Modern
Package​::Anon (in its tests)

The authors of these are active and responsible, and should be able to
quickly apply any necessary patches.

From @khwilliamson

On 09/10/2014 12​:48 PM, Karen Etheridge wrote​:

OK, so what is the underscore for, then?​:-)
A more public indication of​: "we're using a private sub and we understand
that this might break at any time. we'll accept those consequences,
whatever they are."

Preceding the name of something with an underscore comes AFAIK from C++.
  It has come to be a pretty universal convention that means what Karen
said above.

From @ribasushi

On 09/10/2014 07​:53 PM, Father Chrysostomos via RT wrote​:

Let’s see​:

- OverloadedStringify

On a related note - shouldn't this method do exactly what I am trying to
do with is_plain_value from SQL​::Abstract?

I tried to plug it into the SQLA test suite - it doesn't seem to behave
identically. Is the method broken, or does it not follow the same logic
I am using?

From @ribasushi

On 09/10/2014 07​:53 PM, Father Chrysostomos via RT wrote​:

On Wed Sep 10 10​:24​:18 2014, ether wrote​:

A leading underscore is the usual convention, but not everything has to follow that. My usual rule is​: If it’s not documented, don’t use it if you don’t want to risk having your code break.

If we really wanted to make these things private, then why not use lexical subroutines since we have them (except for nil)? overload.pm is core-only, after all.

But seeing that overload.pm has been very stable for years, I’m not sure it’s worth breaking code for cosmetic reasons. (I.e., these modules are taking the risk and we’ll break them if we have to, but only if we have to.)

+1. Cleanup for the sake of cleanup is a nasty time waster, and is only
liable to introduce subtle breakages.

From @cpansprout

On Wed Sep 10 22​:44​:06 2014, ribasushi@​cpan.org wrote​:

On 09/10/2014 07​:53 PM, Father Chrysostomos via RT wrote​:

Let’s see​:

- OverloadedStringify

On a related note - shouldn't this method do exactly what I am trying to
do with is_plain_value from SQL​::Abstract?

I tried to plug it into the SQLA test suite - it doesn't seem to behave
identically. Is the method broken, or does it not follow the same logic
I am using?

Looking at it more closely, I see it ignores the fallback setting. In fact, not only does it not serve its original purpose, but the value it now returns is not useful or informative in any way.

Its original purpose was to be a quick check to see whether StrVal needed to bless the object temporarily into a non-overloaded class to stringify it, so false positives were fine. Now it’s heavyweight, not quick.

Since it gives false positives, the method it returns is not useful.

--

Father Chrysostomos

From @ribasushi

On 09/11/2014 07​:57 AM, Father Chrysostomos via RT wrote​:

On Wed Sep 10 22​:44​:06 2014, ribasushi@​cpan.org wrote​:

On 09/10/2014 07​:53 PM, Father Chrysostomos via RT wrote​:

Let’s see​:

- OverloadedStringify

On a related note - shouldn't this method do exactly what I am trying to
do with is_plain_value from SQL​::Abstract?

I tried to plug it into the SQLA test suite - it doesn't seem to behave
identically. Is the method broken, or does it not follow the same logic
I am using?

Looking at it more closely, I see it ignores the fallback setting. In fact, not only does it not serve its original purpose, but the value it now returns is not useful or informative in any way.

Its original purpose was to be a quick check to see whether StrVal needed to bless the object temporarily into a non-overloaded class to stringify it, so false positives were fine. Now it’s heavyweight, not quick.

Since it gives false positives, the method it returns is not useful.

This was my reading of the source as well, I just wanted to make sure I
am not missing something.

A cursory search via http​://grep.cpan.me/?q=OverloadedStringify reveals
two problematic uses on CPAN​:
https://metacpan.org/source/LEONT/SmartMatch-Sugar-0.05/lib/SmartMatch/Sugar.pm#L90
https://metacpan.org/source/LEONT/Smart-Match-0.007/lib/Smart/Match.pm#L158