Fwd: [PATCH] hv_iterinit return value documented wrong? #13655
Comments
From @wolfsageCreated by wolfsage@gmail.comhv_iterinit's docs say: hv_iterinit But it actually returns HvTOTALKEYS(hv). This can differ, as shown by the example here: https://github.com/wolfsage/scratch/tree/master/Test-Keys Specifically, Test-Keys.xs: void printit(hv) And in Perl: use Test::Keys; my %hash = qw(cat dog mouse bird); Test::Keys::printit(\%hash); Which outputs: TOTAL: 2 Attached are two patches with two different attempts at fixing. The first updates the docs to say HvTOTALKEYS instead, with a little more info. The second instead updates the code to use HvUSEDKEYS instead, to be Is either of these correct? Thanks, -- Matthew Horsfall (alh) (Thanks to stevan and bulk88 - noticed this because of them) Perl Info |
From @wolfsage0001-Correct-hv_iterinit-s-return-value-documentation.patchFrom df0cffa2368c83c09f63fd7642499032232e9607 Mon Sep 17 00:00:00 2001
From: Matthew Horsfall <WolfSage@gmail.com>
Date: Tue, 11 Mar 2014 15:59:15 -0400
Subject: [PATCH] Correct hv_iterinit's return value documentation
---
hv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hv.c b/hv.c
index 9d80659..8aa3ddd 100644
--- a/hv.c
+++ b/hv.c
@@ -1989,8 +1989,8 @@ S_hv_auxinit(pTHX_ HV *hv) {
=for apidoc hv_iterinit
Prepares a starting point to traverse a hash table. Returns the number of
-keys in the hash (i.e. the same as C<HvUSEDKEYS(hv)>). The return value is
-currently only meaningful for hashes without tie magic.
+keys in the hash, including placeholders (i.e. the same as C<HvTOTALKEYS(hv)>).
+The return value is currently only meaningful for hashes without tie magic.
NOTE: Before version 5.004_65, C<hv_iterinit> used to return the number of
hash buckets that happen to be in use. If you still need that esoteric
--
1.7.9.5
|
From @wolfsage0001-Update-iterinit-s-return-to-match-documentation.patchFrom 9e23b36859e64ea3461b1600e4f0ac5cf1db5a4c Mon Sep 17 00:00:00 2001
From: Matthew Horsfall <WolfSage@gmail.com>
Date: Tue, 11 Mar 2014 16:13:48 -0400
Subject: [PATCH] Update iterinit's return to match documentation
---
hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hv.c b/hv.c
index 9d80659..a660e2c 100644
--- a/hv.c
+++ b/hv.c
@@ -2028,7 +2028,7 @@ Perl_hv_iterinit(pTHX_ HV *hv)
}
/* used to be xhv->xhv_fill before 5.004_65 */
- return HvTOTALKEYS(hv);
+ return HvUSEDKEYS(hv);
}
I32 *
--
1.7.9.5
|
From @bulk88- return HvTOTALKEYS(hv); the 2 macros aren't the same, idk enough to comment further -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Tue, Mar 11, 2014 at 01:17:04PM -0700, Matthew Horsfall wrote:
The only use of its return value in core is as a boolean: i.e. it's used So I conclude that the docs need fixing. -- |
From @jkeenanOn Wed, 12 Mar 2014 20:49:38 GMT, davem wrote:
Can we get an update on the status of the patches originally submitted by Matt in this RT? The first patch is documentation-only (though the location of that documentation within the file has changed). The second is a C-level source code change (which I think we would have to defer until after code freeze). Thank you very much. |
From @wolfsageOn Sun, Feb 26, 2017 at 5:46 PM, James E Keenan via RT
Dave believes the docs need fixing. With that being the case, we could Whether or not before 5.26.0 I leave to Sawyer though. -- Matthew Horsfall (alh) |
From @iabynOn Mon, Feb 27, 2017 at 09:30:30AM -0500, Matthew Horsfall (alh) wrote:
Having now had a quick at the history, that function has been returning I'm happy for the doc patch to be applied before 5.26.0. -- |
From @xsawyerxOn 02/28/2017 09:30 AM, Dave Mitchell wrote:
That's good. Contradicting oneself is a fickle situation. :)
Yes, please. Thanks guys. |
From @jkeenanOn Tue, 28 Feb 2017 09:49:48 GMT, xsawyerx@gmail.com wrote:
Consensus appears to be: Apply the doc patch now; throw out the code change. (If we have second thoughts on the latter, we can open a new RT.) Doc patch applied in commit fe7d7ed. Marking ticket Resolved. Thank you very much. -- |
|
@jkeenan - Status changed from 'open' to 'resolved' |
From @wolfsageThanks! -- Matthew Horsfall (alh) |
From @khwilliamsonOn 02/28/2017 02:48 AM, Sawyer X wrote:
unless you're a politician |
Migrated from rt.perl.org#121418 (status was 'resolved')
Searchable as RT121418$
The text was updated successfully, but these errors were encountered: