Opened 4 years ago

Closed 3 years ago

#1718 closed defect (fixed)

srfi-69 egg hash-table-delete!/-merge!/remove! return type doesn't match SRFI-69

Reported by: Kon Lovett Owned by:
Priority: not urgent at all Milestone: someday
Component: extensions Version: 5.2.0
Keywords: srfi-69 Cc:
Estimated difficulty: trivial

Description

all 3 have non-void return values & -delete! has wrong return type

(per SRFI): Procedure: hash-table-delete! hash-table key → undefined
(per .types): ... srfi-69#hash-table-delete! ((struct hash-table) *) boolean))

& the associated tests use the return value

Attachments (1)

srfi-69.diff (6.6 KB) - added by Kon Lovett 4 years ago.
SVN diff of suggested changes

Download all attachments as: .zip

Change History (4)

Changed 4 years ago by Kon Lovett

Attachment: srfi-69.diff added

SVN diff of suggested changes

comment:1 Changed 3 years ago by sjamaan

hmm, I think this change is intentional, and should be allowed even if undefined in the SRFI. To me it seems like a nice extension to have hash-table-merge! return the hash table into which was merged, analogously to hash-table-merge without the bang. However, if we decide to keep this we should fix the types entry because right now it's specified as returning undefined, which is inconsistent with the procedure's actual behaviour.

Delete returning whether something was deleted also seems to be intentional and a nice behaviour.

It looks like hash-table-remove! actually returns void already. At least I was unable to get it to return anything else. In the body the final form is #t, but note that that is inside a do loop which has a ##sys#setislot as its final action *which returns void) when the stop condition test returns true.

Finally, changing the behaviour (especially of hash-table-merge!) will probably break user code, so I'm not sure that's a great idea. Portable code should not be using the return value anyway (as it's undefined), so should not be affected, but CHICKEN-specific code that uses the actual behaviour rather than the documented behaviour will break. Note that hash-table-merge! in the CHICKEN documentation is explicitly documented to return its first argument:

hash-table-merge!
[procedure] (hash-table-merge! HASH-TABLE-1 HASH-TABLE-2)

Returns HASH-TABLE-1 as the union of HASH-TABLE-1 and HASH-TABLE-2. Keys that exist in both tables will be taken from HASH-TABLE-1.

What do the other core devs think?

comment:2 Changed 3 years ago by Kon Lovett

it was intentional. this report filed in a fit of standarditis. agree, document what is.

comment:3 Changed 3 years ago by sjamaan

Resolution: fixed
Status: newclosed

Fixed the inconsistent type declaration for hash-table-merge! with r40008; as Kon agrees, we should just keep the behaviour so I'm closing the ticket.

Note: See TracTickets for help on using tickets.