Thanks! I have done mostly a cosmetic review of your code. Here is the summary:
- There is two big procedures that I think should be split ` get-hash-functions` and `hash-table` to help readability - I am not very fond of generic-hash-table macros, I would rather use apply / call-with-values, if there is an optimisation to be done it should be done in the compiler... no? Otherwise: - You follow the GNU guidelines for commit message that is a good thing (minor niptick, you should use present tense "Fix" instead of "Fixed", "Implement" instead of "Implemented" - The code is very readable I don't know about whether that code requires additions to the manual since it's R7RS and SRFI. I cc'ed Mark Weaver. Le jeu. 17 janv. 2019 à 22:14, Jéssica Milaré <jessymil...@gmail.com> a écrit : > *Note > > Em qui, 17 de jan de 2019 às 19:13, Jéssica Milaré <jessymil...@gmail.com> > escreveu: > >> Done. >> >> Not that there are many commits. The first commit only fixes bug 33827 of >> SRFI 69, and SRFI 69 is reimplemented in the third commit, which pretty >> much erases the changes of the first commit. >> >> https://github.com/jessymilare/guile/pull/1 >> >> Em qua, 16 de jan de 2019 às 22:19, Amirouche Boubekki < >> amirouche.boube...@gmail.com> escreveu: >> >>> I will try to look at the commits. One last niptick, can you push guile >>> master branch to you repository, and make a pull request against it so that >>> I can directly comment in the pull requests. please. It's easier to way to >>> see the overall changes in particular. >>> >>> Le dim. 13 janv. 2019 à 23:52, Jéssica Milaré <jessymil...@gmail.com> a >>> écrit : >>> >>>> Finally finished the libraries. >>>> >>>> SRFI-125 is implemented. I took the liberty to create a procedure >>>> scm_hash_n_items in 'libguile/hashtab.c' that works with both normal and >>>> weak hash tables, so the GENERIC-HASH-TABLES module don't need to keep >>>> track of the hash table size anymore. >>>> >>>> `make check` runs everything ok. I believe it's ready for testing. Any >>>> feedback is welcome. >>>> >>>> Regards, >>>> Jéssica >>>> >>>> Em sáb, 12 de jan de 2019 às 18:34, Jéssica Milaré < >>>> jessymil...@gmail.com> escreveu: >>>> >>>>> (It seems I mistakenly responded only to a personal e-mail, sorry, >>>>> responded now to everyone in the list with updates). >>>>> >>>>> So here are the news: >>>>> >>>>> I've created a module called (ice-9 generic-hash-tables), which is >>>>> similar to SRFI-125, and used it to implement SRFI-69, (RNRS HASHTABLES), >>>>> SRFI-126 and SRFI-125. Since SRFI-125 depends on SRFI-128, I've >>>>> implemented >>>>> it as well. My public repository already has SRFI-128, I'm just finishing >>>>> some tests with SRFI-125 and, once they are done, I'll push it as well. >>>>> >>>>> The module (ice-9 generic-hash-tables), is quite usable and all >>>>> exported procedures are documented. I've created tests for it and also >>>>> ported standard tests from SRFI-126. >>>>> >>>>> Now, I see that 'libguile/hashtab.h' code keeps track of the number of >>>>> elements in hash tables, but the field is not visible from Scheme. Can >>>>> that >>>>> be changed? Then generic hash tables wouldn't need to also keep track of >>>>> the number of elements (like SRFI-69 currently does) and that would >>>>> simplify its code. >>>>> >>>>> Besides, what about creating new versions HASHX-* procedures that >>>>> accept an equivalence procedure instead of an assoc procedure? Perhaps >>>>> prefixed by 'NEW-' or post-fixed by a '*'. >>>>> >>>>> I've also found inconsistencies between SRFI-125 and it's reference >>>>> implementation and standard tests. I've implemented according to the >>>>> specification - so, for instance, HASH-TABLE=? checks if the equivalence >>>>> function of both hash tables are the same and HASH-TABLE returns an >>>>> immutable hash table. >>>>> >>>>> Code is public and suggestions are always welcome :) >>>>> https://github.com/jessymilare/guile >>>>> >>>>> Regargs, >>>>> Jéssica >>>>> >>>>> Em dom, 30 de dez de 2018 às 15:50, Amirouche Boubekki < >>>>> amirou...@hypermove.net> escreveu: >>>>> >>>>>> Le 2018-12-28 17:11, Jéssica Milaré a écrit : >>>>>> > Hello, >>>>>> > >>>>>> > As I said in a previous e-mail, currently SRFI-69 is broken for weak >>>>>> > hash tables - and I've sent a patch to fix it. However, I think >>>>>> there >>>>>> > are many other problems with current implementation of hash tables. >>>>>> > There are guile standard hash tables, SRFI-69 hash tables (which is >>>>>> > implemented on top of standard hash tables) and also R6RS hash >>>>>> tables >>>>>> > (which is implemented on top or SRFI-69 and completely lacks support >>>>>> > for weak keys and/or values). >>>>>> > >>>>>> > I think that should be fixed and guile should have only two kinds of >>>>>> > hash tables: the standard guile hash table and another extended hash >>>>>> > table type that will be used directly by R6RS, SRFI-125 and SRFI-69. >>>>>> > In my opinion, it should be based on SRFI-125, which is part of R7RS >>>>>> > Red Edition, but also supports some other procedures to make it >>>>>> > compatible with R6RS and SRFI-69, supporting weakness and immutable >>>>>> > hash tables. >>>>>> > >>>>>> > I'm already implementing the SRFI-125 based hash tables library for >>>>>> > myself, so, if that is accepted, I can also make a patch for guile. >>>>>> > >>>>>> >>>>>> Yes please make a patch and attach it to the bug report. >>>>>> >>>>>> Don't forget to send an announcement when you SRFI-125 implementation >>>>>> is ready for testing. Is it already available anywhere? >>>>>> >>>>>> TIA >>>>>> >>>>>
