Grovelling CONSTANTENUM ignores BASE-TYPE

[Mark Cox]

The groveller ignores the BASE-TYPE argument for CONSTANTENUM forms.

This can be seen with the following example.

Create the following grovel file.

    ;; Grovelling for INADDR_BROADCAST
    (in-package "CL-USER")
    (include "netinet/in.h")

    (constantenum (example-signed :base-type :int)
      ((:inaddr-broadcast "INADDR_BROADCAST")))

    (constantenum (example-unsigned :base-type :unsigned-int)
      ((:inaddr-broadcast "INADDR_BROADCAST")))

Process the above file using CFFI-GROVEL:PROCESS-GROVEL-FILE to produce the following:

    (cl:in-package #:CL-USER)

    (cffi:defcenum (example-signed :int)
      (:inaddr-broadcast -1))
    (cffi:defcenum (example-unsigned :unsigned-int)
      (:inaddr-broadcast -1))

The expected output should be

    (cl:in-package #:CL-USER)

    (cffi:defcenum (example-signed :int)
      (:inaddr-broadcast -1))
    (cffi:defcenum (example-unsigned :unsigned-int)
      (:inaddr-broadcast 4294967295))

since the value of INADDR_BROADCAST is defined in netinet/in.h as (u_int32_t)0xffffffff.

This occurs with version 0.14.0 of CFFI.

[Luís Oliveira (luismbo)]

On Sat, Nov 22, 2014 at 3:00 AM, Mark Cox <email address hidden> wrote:
> The groveller ignores the BASE-TYPE argument for CONSTANTENUM forms.

Stelian, is this one of those cases where the groveller needs C++?

[Mark Cox (markcox80)]

The bug in (DEFINE-GROVEL-SYNTAX CONSTANTENUM) is
     ….
     (c-printf out "%i" c-name)
     ….

You could use the BASE-TYPE as an argument to FOREIGN-TYPE-TO-PRINTF-SPECIFICATION in order to obtain the correct printf(3) format string. The C-PRINTF form above would have to change as FOREIGN-TYPE-TO-PRINTF-SPECIFICATION includes double quotes i.e. "\"%i\"".

This problem exists in (DEFINE-GROVEL-SYNTAX CENUM) too.

[Luís Oliveira (luismbo)]

Mark, do you feel like taking a stab at fixing the issue?

[Mark Cox (markcox80)]

Sure. The patch is attached.

I have added a test to tests/grovel.lisp. The test uses the constant
UINT_LEAST32_MAX from stdint.h rather than INADDR_BROADCAST as used in
the bug report. INADDR_BROADCAST requires platform specific header
files, making the test case messy to implement. The header file
stdint.h is included in grovel/common.h and thus is not required to be
included.

The patch to (DEFINE-GROVEL-SYNTAX CONSTANTENUM) and
(DEFINE-GROVEL-SYNTAX CENUM) uses the same style found in
(DEFINE-GROVEL-SYNTAX BITFIELD).

[Luís Oliveira (luismbo)]

Awesome, pushed to the main repository with a minor stylistic change to the commit message. Thanks Mark!

[Mark Cox (markcox80)]

Unfortunately there is a problem with the patch I provided when the base type is also grovelled e.g.

    (ctype my-intmax "intmax_t")
    (cenum (my-enum :base-type my-intmax)
        …)

The attached patch fixes this bug and the above issue. It does this by performing the conversion of the grovelled constant to a value of type BASE-TYPE at read time.

Comments welcomed.

[Luís Oliveira (luismbo)]

Mark, I've only skimmed your patch so far, so this might be a silly question. Can you take advantage of canonicalize-foreign-type to simplify and future-proof convert-intmax-constant?

[Mark Cox (markcox80)]

Hi Luis. Yes it can. There has to be a keyword clause to terminate the recursion. e.g. if someone specifies a base type for which CANONICALIZE-FOREIGN-TYPE returns :double.

Thank you, I didn't know about the function CANONICALIZE-FOREIGN-TYPE.

[Mark Cox (markcox80)]

Modified previous patch to use CANONICALIZE-FOREIGN-TYPE.

[Luís Oliveira (luismbo)]

Thanks Mark. I gave it another go at refactoring to take advantage of CANONICALIZE-FOREIGN-TYPE (and noticed that the function just below could also use it!) and made some minor cosmetic tweaks. I hope I didn't break anything in the process. (Your tests were quite helpful, thanks!)