Thanks for the feedback! Also, if you are interested in r7rs in guile you might be interested in `r7rs-wip` branch of guile
Le dim. 20 janv. 2019 à 14:50, Jéssica Milaré <jessymil...@gmail.com> a écrit : > > > Em dom, 20 de jan de 2019 às 10:58, Amirouche Boubekki < > amirouche.boube...@gmail.com> escreveu: > >> Thanks! >> >> I have done mostly a cosmetic review of your code. Here is the summary: >> > > Your comments are very good, I'll do most of the suggested changes. > > >> - 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? >> > > These macros were copied from old SRFI-69 code. Regarding to > optimizations, I don't ask myself if something *should be* done by the > compiler, but if it *actually is* done by the compiler. Otherwise, we'll > end in a discussion involving a "sufficiently smart compiler", not a real > compiler. > > 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 >>>>>>>> >>>>>>>
