Re: CRUNCH feedback

On Mon Aug 11, 2025 at 8:55 PM CEST, Diogo via Chicken-users wrote:
>
> I've recently tried CRUNCH a bit and would like to give some feedback:
>
> Installation:
> - worked fine on macOS
> - failed on NetBSD 10.1, for two reasons:
>   - -Wno-parenthesis-equality is not available (gcc 10.5.0)
>   - popcount32 (tests/embedded) has conflicting type with popcount32 defined in
>   /usr/include/strings.h: former is long->long and latter is uint32->uint32.
>   - After fixing that, the egg compiled, and installed fine.

Hi!

Thanks for reporting this. I renamed the procedure in the example file
and dropped the compiler option, which is unfortunate, but its effect
can at least on clang be accomplished with a pragma.

>
> CRUNCH_NO_UTF:
> - I found a bit weird that only an object file is offered, why not a static
>   library? Concerns with LTO?

It just saves one step of creating the .a and bother with the options.
It really makes no difference.

> - If only an object is installedi (as currently is), can we have a command
>   option to get the full path to the object?

"chicken-crunch -libs" will give you the full path to the file and required
libraries (-lm in this case).

> - CRUNCH_NO_UTF does not work properly because the function crunch_read_string
>   (in crunch.h) has a call to crunch_utf_decode outside the #ifdef guard.
>

Indeed, that was forgotten, I corrected this.

> crunch.h:
> - crunch.h file defines several functions as static, but if they are not
>   used, the compiler complains. (I typically use -Wall when compiling). Why
>   can't all these functions be marked as inline? Then the compiler won't
>   complain and it can still decide not inline them if it prefers.

Inline functions can not refer to static data, I already tried this before.
Moreover, -Wall mostly alerts about stylistic options in my experience and
makes no sense for machine generated code. I recommend you move hand-written
code into a separate compilation unit and you can use then any option you
like.

> - crunch.h needs some documentation at the top to explain the compile options
>   (CRUNCH_NO_UTF, CRUNCH_FREESTANDING, CRUNCH_EMBEDDED, ...). Also a license
>   notice in the file would be useful in case the developer wants to create a
>   customized crunch.h and redistribute it.
>

The macros are already described in the CRUNCH manual on the wiki 
(/eggref/6/crunch), so probably this doesn't need to be duplicated.

I added a license header to crunch.h.

> So for each section in crunch.h refering to a library, I've created an #ifdef
> guard and disabled most of them.  Something like this:
>
> #ifndef CRUNCH_IMPORT_SCHEME_FILE
> /* (scheme file) */
> ...
> #endif
>
> I have disabled (scheme file), (scheme time), (scheme complex), (scheme
> inexact), and (crunch process-context). My suggestion here would be to add such
> guards to all libraries supported by CRUNCH and then only enable them (by
> defining CRUNCH_IMPORT_<LIB>) if the respective <LIB> has been imported in the
> .scm file.

I nice idea. I have implemented this and added guards to the relevant sections
in crunch.h. I may have missed some interdependencies, but it seemd to work
fine for the testcases I have.

>
> Besides the libraries, I also had to make optional (and disabled) all interfaces
> with complex numbers, 128bit integers, and which use fmemopen. From three, the
> most intrusive change was regarding the complex numbers because they are spread
> all over the file (similarly as `CRUNCH_NO_UTF`).
>

I added guards depending on CRUNCH_IMPORT_SCHEME_COMPLEX for all uses of
complex numbers (unless I missed some).

> For the moment this is all feedback I have. Thanks for releasing this nice
> feature to the CHICKEN ecosystem. 

And thanks again for the feedback! I have tagged a new version (0.96)
which should be available presently.


cheers,
felix