[PATCH] overload::OverloadedStringify docs and tests #17038
Comments
From @SmylersThis is a bug report for perl from smylers@stripey.com, The overload module has an internal function OverloadedStringify which The function is used by Moose, among other things: It used to be used internally (by Larry Wall, in the initial So documenting it and adding tests for it seems more sensible. I wasn't entirely sure where the best place in overload.t was to add the • Straight after the overload::Overloaded tests made sense, but some of But, glancing at the verbose test output, it seems that the test • At the end of the file would seem another obvious place for new tests, However, there are several tests after the one with that comment, so Flags: Site configuration information for perl 5.31.1: Configured by smylers at Thu Jun 6 11:46:24 BST 2019. Summary of my perl5 (revision 5 version 31 subversion 1) configuration: Locally applied patches: @INC for perl 5.31.1: Environment for perl 5.31.1: |
From @Smylers0001-Doc-overload-implementation-not-changing-RSN.patchFrom 44f58ffee6a6e924a381d9c1a64644811ec5323e Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Thu, 6 Jun 2019 11:07:18 +0100
Subject: [PATCH 1/3] Doc: overload implementation not changing RSN
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Given this hasn't changed in the 24��years, since Larry wrote it in
4633a7c4, it seems inaccurate to suggest that any changes would be ���real
soon now���.
(Continue to label the implementation as subject to change, just without
implying there are imminent changes planned.)
---
lib/overload.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git lib/overload.pm lib/overload.pm
index 30f810b0..4345ca9b 100644
--- lib/overload.pm
+++ lib/overload.pm
@@ -1046,7 +1046,7 @@ From these methods they may be called as
=head1 IMPLEMENTATION
-What follows is subject to change RSN.
+What follows is subject to change.
The table of methods for all operations is cached in magic for the
symbol table hash for the package. The cache is invalidated during
--
1.9.1
|
From @Smylers0002-Using-done_testing-in-overload.t.patchFrom f3ff56ad41170c41079f26ae34085c91ce33bb6a Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Thu, 6 Jun 2019 10:18:51 +0100
Subject: [PATCH 2/3] Using done_testing in overload.t
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Get rid of magic number of tests, per perlhack's ���Make updating the
'1..42' string unnecessary.���
---
lib/overload.t | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git lib/overload.t lib/overload.t
index 5f2e0c29..3f31be77 100644
--- lib/overload.t
+++ lib/overload.t
@@ -48,7 +48,6 @@ package main;
$| = 1;
BEGIN { require './test.pl'; require './charset_tools.pl' }
-plan tests => 5363;
use Scalar::Util qw(tainted);
@@ -3193,3 +3192,5 @@ package RT33789 {
}
::is($destroy, 1, "RT #133789: delayed destroy");
}
+
+done_testing;
--
1.9.1
|
From @Smylers0003-overload-OverloadedStringify-docs-and-tests.patchFrom 6d4abcdaf9a79aa74fdfd074c75d8015725cfb72 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Thu, 6 Jun 2019 11:33:08 +0100
Subject: [PATCH 3/3] overload::OverloadedStringify docs and tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Document the overload module's OverloadedStringify function as being
available for users: it exists, it's useful, and some Cpan modules already
rely on it.
The OverloadedStringify function was party of Larry's initial
implementation of the overload module in 4633a7c4, Perl��5.002 beta��1.
It hasn't been used internally since 1b1d102f, 15��years ago.
However, it is a useful external function: it answers the question ���Is
this an object that provide overloaded stringification by any means?���.
That differs from using overload::Method $obj, '""' in a boolean context,
which would only indicate if '""' has specifically been overloaded, but
not if stringification is provided in some other way, such as by
overloading numification (which is then coerced to a string).
And the function has been used by Cpan modules, including Moose. As such,
even though it's no longer used internally, it would break things to get
rid of it.
So stop hiding it away. Document what it does, and test it.
---
lib/overload.pm | 8 +++++++-
lib/overload.t | 12 ++++++++++++
pod/perldelta.pod | 6 ++++++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git lib/overload.pm lib/overload.pm
index 4345ca9b..c37cae12 100644
--- lib/overload.pm
+++ lib/overload.pm
@@ -1,6 +1,6 @@
package overload;
-our $VERSION = '1.31';
+our $VERSION = '1.32';
%ops = (
with_assign => "+ - * / % ** << >> x .",
@@ -975,6 +975,12 @@ C<Scalar::Util::refaddr()>, which is faster.
Returns true if C<arg> is subject to overloading of some operations.
+=item overload::OverloadedStringify(arg)
+
+Returns whether C<arg> provides overloaded stringification, either
+directly (by overloading C<'""'>) or by overloading another operator (such as
+C<'0+'>) that provides fallback stringification.
+
=item overload::Method(obj,op)
Returns C<undef> or a reference to the method that implements C<op>.
diff --git lib/overload.t lib/overload.t
index 3f31be77..b038b28c 100644
--- lib/overload.t
+++ lib/overload.t
@@ -2825,6 +2825,18 @@ print length $o, "\n";
}
+{
+ use overload;
+ my $stringy = symbolic->new;
+ my $booly = bless \(my $ignore), 'B';
+ my $neither = bit->new;
+ ok overload::OverloadedStringify($stringy), 'string overloading found';
+ ok overload::OverloadedStringify($booly), 'boolify does stringification';
+ ok !overload::OverloadedStringify($neither), 'no string overloading';
+ ok !overload::OverloadedStringify(0), 'no overloading if not object';
+}
+
+
{ # undefining the overload stash -- KEEP THIS TEST LAST
package ant;
use overload '+' => 'onion';
diff --git pod/perldelta.pod pod/perldelta.pod
index 1541525c..04aa5d3e 100644
--- pod/perldelta.pod
+++ pod/perldelta.pod
@@ -125,6 +125,12 @@ XXX Remove this section if not applicable.
=item *
+L<overload> has been upgraded from version 1.31 to 1.32. It now provides
+C<OverloadedStringify> for determining whether an object has overloaded
+stringification by any means.
+
+=item *
+
L<XXX> has been upgraded from version A.xx to B.yy.
If there was something important to note about this change, include that here.
--
1.9.1
|
From @hvdsA minor comment: not sure if we have a policy on it, but for me there should be a preference to have commit messages ASCII where possible. All three of your commit messages have some utf8 in there, and in no case does it look particularly necessary. (If you do change them, you may also want to change the "party" of the third part, to "part".) Hugo |
|
The RT System itself - Status changed from 'new' to 'open' |
From @ilmari"Hugo van der Sanden via RT" <perlbug-followup@perl.org> writes:
Why? This is 2019, everything should be Unicode-capable by now, and ~/src/perl$ git log -P --grep='[^[:ascii:]]' --oneline |wc -l The earliest non-ASCII commit message is: commit f31700d don't use © in Test.pm (suggested by M.J.T. Guy) p4raw-id: //depot/maint-5.005/perl@1749
A lot of the non-ASCII in our commit history not "particularly - ilmari |
From @hvdsOn Thu, 06 Jun 2019 08:41:23 -0700, ilmari wrote:
Maybe it's just me, but for the range of systems perl aspires to port to it seems unlikely they'll all be equally Unicode-capable. I noticed it because they showed up as garbage in my email client, but maybe that was only because of the way RT wrapped the commits as attachments.
Yeah, that's the sort of stuff. If I'm the only one that considers such things to reduce the quality of the commit history, I'll have to live with it. Hugo |
From @iabynOn Thu, Jun 06, 2019 at 09:20:35AM -0700, Hugo van der Sanden via RT wrote:
The attachments, as seen on p5p, appear to have garbage in the commit ef bf bd ef bf bd ef bf bd which when decoded as utf8, form these characters "\x{fffd}\x{fffd}\x{fffd}" So I think the patches probably need resubmitting. -- |
From @SmylersDave Mitchell writes:
Ascii-only patches attached. (That was an opening double-quote mark when I typed it, U+201C. It Dagfinn Ilmari Mannsåker writes:
I agree. Especially since some committers have non-Ascii characters in And as you show, there are plenty of commit messages already The attachments in my ‘sent’ folder are unmangled if I open them with Or maybe only folk with commit bits are free to use any characters in Hugo van der Sanden via RT writes:
Thanks for spotting the issue, Hugo (and the other typo). Apologies for the distraction, all. Smylers |
From @Smylers0001-Doc-overload-implementation-not-changing-RSN.patchFrom 3199402284e4e446be5d12d05d796e9069ecc125 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Thu, 6 Jun 2019 11:07:18 +0100
Subject: [PATCH 1/3] Doc: overload implementation not changing RSN
Given this hasn't changed in the 24 years, since Larry wrote it in
4633a7c4, it seems inaccurate to suggest that any changes would be "real
soon now".
(Continue to label the implementation as subject to change, just without
implying there are imminent changes planned.)
---
lib/overload.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git lib/overload.pm lib/overload.pm
index 30f810b0..4345ca9b 100644
--- lib/overload.pm
+++ lib/overload.pm
@@ -1046,7 +1046,7 @@ From these methods they may be called as
=head1 IMPLEMENTATION
-What follows is subject to change RSN.
+What follows is subject to change.
The table of methods for all operations is cached in magic for the
symbol table hash for the package. The cache is invalidated during
--
1.9.1
|
From @Smylers0002-Using-done_testing-in-overload.t.patchFrom 89383a0ac2a0eaee7b2ef24e1014087e8a1b36ee Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Thu, 6 Jun 2019 10:18:51 +0100
Subject: [PATCH 2/3] Using done_testing in overload.t
Get rid of magic number of tests, per perlhack's "Make updating the
'1..42' string unnecessary."
---
lib/overload.t | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git lib/overload.t lib/overload.t
index 5f2e0c29..3f31be77 100644
--- lib/overload.t
+++ lib/overload.t
@@ -48,7 +48,6 @@ package main;
$| = 1;
BEGIN { require './test.pl'; require './charset_tools.pl' }
-plan tests => 5363;
use Scalar::Util qw(tainted);
@@ -3193,3 +3192,5 @@ package RT33789 {
}
::is($destroy, 1, "RT #133789: delayed destroy");
}
+
+done_testing;
--
1.9.1
|
From @Smylers0003-overload-OverloadedStringify-docs-and-tests.patchFrom 2a1ca66777f264e0aa5badaa844c5d842b1acba8 Mon Sep 17 00:00:00 2001
From: Smylers <Smylers@stripey.com>
Date: Thu, 6 Jun 2019 11:33:08 +0100
Subject: [PATCH 3/3] overload::OverloadedStringify docs and tests
Document the overload module's OverloadedStringify function as being
available for users: it exists, it's useful, and some Cpan modules already
rely on it.
The OverloadedStringify function was part of Larry's initial
implementation of the overload module in 4633a7c4, Perl 5.002 beta 1.
It hasn't been used internally since 1b1d102f, 15 years ago.
However, it is a useful external function: it answers the question 'Is
this an object that provide overloaded stringification by any means?'.
That differs from using overload::Method $obj, '""' in a boolean context,
which would only indicate if '""' has specifically been overloaded, but
not if stringification is provided in some other way, such as by
overloading numification (which is then coerced to a string).
And the function has been used by Cpan modules, including Moose. As such,
even though it's no longer used internally, it would break things to get
rid of it.
So stop hiding it away. Document what it does, and test it.
---
lib/overload.pm | 8 +++++++-
lib/overload.t | 12 ++++++++++++
pod/perldelta.pod | 6 ++++++
3 files changed, 25 insertions(+), 1 deletion(-)
diff --git lib/overload.pm lib/overload.pm
index 4345ca9b..c37cae12 100644
--- lib/overload.pm
+++ lib/overload.pm
@@ -1,6 +1,6 @@
package overload;
-our $VERSION = '1.31';
+our $VERSION = '1.32';
%ops = (
with_assign => "+ - * / % ** << >> x .",
@@ -975,6 +975,12 @@ C<Scalar::Util::refaddr()>, which is faster.
Returns true if C<arg> is subject to overloading of some operations.
+=item overload::OverloadedStringify(arg)
+
+Returns whether C<arg> provides overloaded stringification, either
+directly (by overloading C<'""'>) or by overloading another operator (such as
+C<'0+'>) that provides fallback stringification.
+
=item overload::Method(obj,op)
Returns C<undef> or a reference to the method that implements C<op>.
diff --git lib/overload.t lib/overload.t
index 3f31be77..b038b28c 100644
--- lib/overload.t
+++ lib/overload.t
@@ -2825,6 +2825,18 @@ print length $o, "\n";
}
+{
+ use overload;
+ my $stringy = symbolic->new;
+ my $booly = bless \(my $ignore), 'B';
+ my $neither = bit->new;
+ ok overload::OverloadedStringify($stringy), 'string overloading found';
+ ok overload::OverloadedStringify($booly), 'boolify does stringification';
+ ok !overload::OverloadedStringify($neither), 'no string overloading';
+ ok !overload::OverloadedStringify(0), 'no overloading if not object';
+}
+
+
{ # undefining the overload stash -- KEEP THIS TEST LAST
package ant;
use overload '+' => 'onion';
diff --git pod/perldelta.pod pod/perldelta.pod
index 1541525c..04aa5d3e 100644
--- pod/perldelta.pod
+++ pod/perldelta.pod
@@ -125,6 +125,12 @@ XXX Remove this section if not applicable.
=item *
+L<overload> has been upgraded from version 1.31 to 1.32. It now provides
+C<OverloadedStringify> for determining whether an object has overloaded
+stringification by any means.
+
+=item *
+
L<XXX> has been upgraded from version A.xx to B.yy.
If there was something important to note about this change, include that here.
--
1.9.1
|
From @tonycozOn Fri, 07 Jun 2019 02:26:42 -0700, smylers@stripey.com wrote:
The (old version of) RT we're using seems to corrupt UTF-8 in attached patches received by email, including in the copy of the email sent to the list. This: +L<overload> has been upgraded from version 1.31 to 1.32. It now provides seems inaccurate - isn't the idea that it's always been provided (for useful versions of always) and we're now documenting and testing it. Tony |
Thanks for the explanation, Tony. Hopefully that's no longer a problem. On your more substantive matter:
That's rather a philosophical point! In practice, yes, the function has always been there. Does that count as being “provided”? I presume that in general P5P wants users to use documented functions (with documented behaviour) and not to start using an undocumented internal function they've somehow come across and happens to work for them? And specifically that if a user did use such an internal function and then P5P removed it or changed it, that wouldn't count as breaking Perl's backwards- compatibility, because the user shouldn't've been using an internal function in the first place. In this case arguably what we have is an public function that we forgot to document, rather than an internal function. But in general that's quite a hard distinction to make: possibly the defining feature of an internal function is it's one that isn't documented. So it seemed safer to go for the wording above: if you only use publicly documented functions, then it effectively wasn't provided until now; adding this documentation is what provides the function. However, I now see the disadvantage of that is that it incorrectly makes users think they need to upgrade to get this function. It would be nice to have wording which retrospectively blesses use of it in older versions of perl. Specifically that would enable its use in Cpan modules with minimum perl version requirements. Does anybody know of a suitable form of words from a previous time when something like this has happened? If not, I'll have a go at coming up with some. (And I'll try not to wait 3 years this time. Apologies for gap; I guess I went on holiday or something and then it slipped my mind — then this morning I encountered a comment in our code-base which links to this bug report.) Smylers |
|
On Mon, 8 Aug 2022 at 09:46, Smylers ***@***.***> wrote:
From @tonycoz <https://github.com/tonycoz>
The (old version of) RT we're using seems to corrupt UTF-8 in attached
patches received by email, including in the copy of the email sent to the
list.
Thanks for the explanation, Tony. Hopefully that's no longer a problem. On
your more substantive matter:
This:
+L has been upgraded from version 1.31 to 1.32. It now provides
+C for determining whether an object has overloaded
+stringification by any means.
seems inaccurate - isn't the idea that it's always been provided (for
useful > versions of always) and we're now documenting and testing it.
That's rather a philosophical point! In practice, yes, the function has
always been there. Does that count as being “provided”?
I presume that in general P5P wants users to use documented functions
(with documented behaviour) and not to start using an undocumented internal
function they've somehow come across and happens to work for them?
I personally consider some widely distributed modules to have stable apis
even if we dont document them. For instance I would object if
Data::Dumper::qquote() was removed.
And specifically that if a user did use such an internal function and then
P5P removed it or changed it, that wouldn't count as breaking Perl's
backwards- compatibility, because the user shouldn't've been using an
internal function in the first place.
In theory, sure, in practice it's harder to say that. Why should we treat
using an undocumented (but obviously stable) api any differently than an
undocumented and unintentional behavior in perl any differently? And we
often commit to preserving such behavior. In practice if people have been
using it for a while and its going to cause trouble if we remove it we
treat it as baked in
In this case arguably what we have is an public function that we forgot to
document, rather than an internal function. But in general that's quite a
hard distinction to make: possibly the defining feature of an internal
function is it's one that isn't documented.
I think in practice it is one that is documented as internal. :-)
So it seemed safer to go for the wording above: if you only use publicly
documented functions, then it effectively *wasn't* provided until now;
adding this documentation is what provides the function
I have no opinion on the wording, I am just speaking to my view of the
bigger picture as your comment reminded me of all the times I have used
Data::Dumper::qquote() in production code even though it wasn't/isn't
documented.
cheers,
Yves
…--
perl -Mre=debug -e "/just|another|perl|hacker/"
|
Migrated from rt.perl.org#134180 (status was 'open')
Searchable as RT134180$
The text was updated successfully, but these errors were encountered: