"comma at end of enumerator list" error due to macros in libqof

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

"comma at end of enumerator list" error due to macros in libqof

Tim M-6
Hello,

Per my previous email I am looking at compiler warnings/errors and I
found an interesting one for which I could use some advice.  During
make, libqof came up with a fairly large number of errors, including
quite a few "comma at end of enumerator list" errors.  Some of the
errors were quite simple to fix because they were simple enum lists
with a comma after the last enum item.  Of course, remove the extra
comma and voila - no more error.

It took me a little time to decipher the same compiler error for this
line, however:

   DEFINE_ENUM (QofLogLevel, LOG_LEVEL_LIST)

After a little research, I found that basically, the enum list is
defined in a macro:

   #define LOG_LEVEL_LIST(_) \
     _(QOF_LOG_FATAL,   = G_LOG_LEVEL_ERROR)   \
     _(QOF_LOG_ERROR,   = G_LOG_LEVEL_CRITICAL)   \
     _(QOF_LOG_WARNING, = G_LOG_LEVEL_WARNING) \
     _(QOF_LOG_MESSAGE, = G_LOG_LEVEL_MESSAGE) \
     _(QOF_LOG_INFO,    = G_LOG_LEVEL_INFO)    \
     _(QOF_LOG_DEBUG,   = G_LOG_LEVEL_DEBUG)

The DEFINE_ENUM macro then looks like this:

   #define DEFINE_ENUM(name, list)          \
       typedef enum {                       \
           list(ENUM_BODY)                  \
       }name;

So DEFINE_ENUM in turn calls LOG_LEVEL_LIST(ENUM_BODY).  ENUM_BODY
looks like this:

   #define ENUM_BODY(name, value)           \
       name value,

ENUM_BODY then expands to "name value," for every entry in
LOG_LEVEL_LIST. This then causes the "comma at end of enumerator list"
error mentioned at top because the enum generated by this macro
combination always has a comma after the last entry:

   typedef enum {
     QOF_LOG_FATAL    = G_LOG_LEVEL_ERROR,
     QOF_LOG_ERROR    = G_LOG_LEVEL_CRITICAL,
     QOF_LOG_WARNING  = G_LOG_LEVEL_WARNING,
     QOF_LOG_MESSAGE  = G_LOG_LEVEL_MESSAGE,
     QOF_LOG_INFO     = G_LOG_LEVEL_INFO,
     QOF_LOG_DEBUG    = G_LOG_LEVEL_DEBUG,
   }QofLogLevel;


I can remove the comma from the ENUM_BODY and instead put it in the
LOG_LEVEL_LIST define like this:

   #define ENUM_BODY(name, value)           \
       name value

   #define LOG_LEVEL_LIST(_) \
     _(QOF_LOG_FATAL,   = G_LOG_LEVEL_ERROR),   \
     _(QOF_LOG_ERROR,   = G_LOG_LEVEL_CRITICAL),   \
     _(QOF_LOG_WARNING, = G_LOG_LEVEL_WARNING), \
     _(QOF_LOG_MESSAGE, = G_LOG_LEVEL_MESSAGE), \
     _(QOF_LOG_INFO,    = G_LOG_LEVEL_INFO),    \
     _(QOF_LOG_DEBUG,   = G_LOG_LEVEL_DEBUG)

This change prevents the make warning/error, but after making this
change Eclipse then reports a syntax error at the DEFINE_ENUM
(QofLogLevel, LOG_LEVEL_LIST) line (qoflog.h, line 102), and will no
longer explore the macro expansion.  I'm not sure then if this is
actually a syntax error (since the compiler does not throw any errors
or warnings) or a bug in Eclipse, but having a macro for such a simple
enum is a bit of a headache.

These macros are used throughout gnucash in a few different places so
I am wary of simply tossing out the DEFINE_ENUM and related macros
completely and replacing them with explicit typedefs since the macros
seem to have at least some usefulness to them, so it seems like a less
invasive fix is needed.  I am not sure what the best solution is other
than the one above which causes the Eclipse error.  At the moment I
don't have any better ideas and it is quite late so I must return to
this later.  There are other similar macros in libqof which seem to
share this structure so I expect to see errors from the other macros
as well at some point.  Any suggestions?

Thanks,
-Tim
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Christian Stimming-4
Am Montag, 6. Juni 2011 schrieb Tim M:

> I can remove the comma from the ENUM_BODY and instead put it in the
> LOG_LEVEL_LIST define like this:
>
>    #define ENUM_BODY(name, value)           \
>        name value
>
>    #define LOG_LEVEL_LIST(_) \
>      _(QOF_LOG_FATAL,   = G_LOG_LEVEL_ERROR),   \
>      _(QOF_LOG_ERROR,   = G_LOG_LEVEL_CRITICAL),   \
>      _(QOF_LOG_WARNING, = G_LOG_LEVEL_WARNING), \
>      _(QOF_LOG_MESSAGE, = G_LOG_LEVEL_MESSAGE), \
>      _(QOF_LOG_INFO,    = G_LOG_LEVEL_INFO),    \
>      _(QOF_LOG_DEBUG,   = G_LOG_LEVEL_DEBUG)
>
> This change prevents the make warning/error, ...

I agree with your change here. From what this twisted interleaving of macros
should look like, the comma should be set as you say and not as the current
macros would do. If they make eclipse break, well, we probably can fix this
for now.

But in general: I don't see the value of those macros here anyway. I mean, the
list of enum values need to appear in the definition of LOG_LEVEL_LIST anyway.
Passing this list through two other macros doesn't really save any typing
except for the two words "typedef enum { ... }". The macros might have made a
little bit of sense if it were for a unified generation of some wrapper
function (e.g. the conversion functions from and to string)... oh, I see: Some
of the enum declaration actually use those: AS_STRING_DEC, AS_STRING_FUNC,
FROM_STRING_DEC, FROM_STRING_FUNC.

It's a classical tradeoff between saving of typing and making the code less
readable: IMHO it is a big disadvantage that those functions are hidden behind
this non-understandable macros. Because of that, I would rather prefer writing
all the declaration and also those conversion functions in plain text instead
of hiding them behind macros.

But in any case, those enums which don't use the conversion functions also
don't need to use the DEFINE_ENUM macro. Hence, feel free to submit a patch
that replaces DEFINE_ENUM by a normal declaration for all enums which don't
use the conversion function macros. Thanks!

Christian
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Derek Atkins
Christian Stimming <[hidden email]> writes:

> But in general: I don't see the value of those macros here anyway. I
> mean, the list of enum values need to appear in the definition of
> LOG_LEVEL_LIST anyway.  Passing this list through two other macros
> doesn't really save any typing except for the two words "typedef enum
> { ... }". The macros might have made a little bit of sense if it were
> for a unified generation of some wrapper function (e.g. the conversion
> functions from and to string)... oh, I see: Some of the enum
> declaration actually use those: AS_STRING_DEC, AS_STRING_FUNC,
> FROM_STRING_DEC, FROM_STRING_FUNC.

Yes, there is value to make sure that you only have to make changes in
one place.

> It's a classical tradeoff between saving of typing and making the code
> less readable: IMHO it is a big disadvantage that those functions are
> hidden behind this non-understandable macros. Because of that, I would
> rather prefer writing all the declaration and also those conversion
> functions in plain text instead of hiding them behind macros.

This "macro maze" was done to make it easier to maintain the list of
macros.  Before, if you added a macro you had to make changes in
about three or four places!  Now you only need to make changes in one
place and it all just works.  The idea here is that you can define the
enumerator and it automatically creates the enum->string and
string->enum functions for you.

> But in any case, those enums which don't use the conversion functions also
> don't need to use the DEFINE_ENUM macro. Hence, feel free to submit a patch
> that replaces DEFINE_ENUM by a normal declaration for all enums which don't
> use the conversion function macros. Thanks!

Sure, if you *never* use the conversion functions then yes, I feel you
can use a regular enum instead of the DEFINE_ENUM macro.

> Christian

-derek
--
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Tim M-6
On Mon, Jun 6, 2011 at 9:02 AM, Derek Atkins <[hidden email]> wrote:

> Christian Stimming <[hidden email]> writes:
>
>> But in general: I don't see the value of those macros here anyway. I
>> mean, the list of enum values need to appear in the definition of
>> LOG_LEVEL_LIST anyway.  Passing this list through two other macros
>> doesn't really save any typing except for the two words "typedef enum
>> { ... }". The macros might have made a little bit of sense if it were
>> for a unified generation of some wrapper function (e.g. the conversion
>> functions from and to string)... oh, I see: Some of the enum
>> declaration actually use those: AS_STRING_DEC, AS_STRING_FUNC,
>> FROM_STRING_DEC, FROM_STRING_FUNC.
>
> Yes, there is value to make sure that you only have to make changes in
> one place.
>
>> It's a classical tradeoff between saving of typing and making the code
>> less readable: IMHO it is a big disadvantage that those functions are
>> hidden behind this non-understandable macros. Because of that, I would
>> rather prefer writing all the declaration and also those conversion
>> functions in plain text instead of hiding them behind macros.
>
> This "macro maze" was done to make it easier to maintain the list of
> macros.  Before, if you added a macro you had to make changes in
> about three or four places!  Now you only need to make changes in one
> place and it all just works.  The idea here is that you can define the
> enumerator and it automatically creates the enum->string and
> string->enum functions for you.
>

The problem here is that it creates improperly formed code, so the
question is whether this issue can/should be fixed somehow within the
qofutil macros, or by simply swapping the comma separators out of
ENUM_BODY and into the definitions of the typedefs such as the
LOG_LEVEL_LIST(_) suggestion above, or another solution.  Looking at
the code some more, I think swapping the commas outright would break
the other conversion functions.

Based on your feedback I have the impression that simply eliminating
DEFINE_ENUM might not be the best approach and I will keep looking for
a better solution; however, at minimum any enums which are defined
using DEFINE_ENUM could instead be explicitly defined if they do _not_
use the other conversion functions.

I will need to think about the macros some more to see if there is a
better way to get properly formed code from DEFINE_ENUM, while
maintaining compatibility with the other conversion functions.

Thanks,
-Tim

>> But in any case, those enums which don't use the conversion functions also
>> don't need to use the DEFINE_ENUM macro. Hence, feel free to submit a patch
>> that replaces DEFINE_ENUM by a normal declaration for all enums which don't
>> use the conversion function macros. Thanks!
>
> Sure, if you *never* use the conversion functions then yes, I feel you
> can use a regular enum instead of the DEFINE_ENUM macro.
>
>> Christian
>
> -derek
> --
>       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
>       Member, MIT Student Information Processing Board  (SIPB)
>       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
>       [hidden email]                        PGP key available
> _______________________________________________
> gnucash-devel mailing list
> [hidden email]
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Jim Paris
Tim M wrote:

> The problem here is that it creates improperly formed code, so the
> question is whether this issue can/should be fixed somehow within the
> qofutil macros, or by simply swapping the comma separators out of
> ENUM_BODY and into the definitions of the typedefs such as the
> LOG_LEVEL_LIST(_) suggestion above, or another solution.  Looking at
> the code some more, I think swapping the commas outright would break
> the other conversion functions.
>
> Based on your feedback I have the impression that simply eliminating
> DEFINE_ENUM might not be the best approach and I will keep looking for
> a better solution; however, at minimum any enums which are defined
> using DEFINE_ENUM could instead be explicitly defined if they do _not_
> use the other conversion functions.
>
> I will need to think about the macros some more to see if there is a
> better way to get properly formed code from DEFINE_ENUM, while
> maintaining compatibility with the other conversion functions.

DEFINE_ENUM can just add a dummy element to the end which will follow
the trailing comma:

#define DEFINE_ENUM(name, list)             \
       typedef enum {                       \
           list(ENUM_BODY)                  \
           __dummy__##name                  \
       } name;

-jim
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Derek Atkins
In reply to this post by Tim M-6
Tim M <[hidden email]> writes:

> The problem here is that it creates improperly formed code

How is the generated code "improperly formed"?  Are you talking about
the trailing comma?  That's only invalid in C89, which you've enabled
yourself.  It's perfectly legal in "modern" C, and indeed if it were
truly a problem we'd have been having many many more people claiming
they can't build GnuCash!

-derek
--
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Derek Atkins
In reply to this post by Jim Paris
Jim Paris <[hidden email]> writes:

> DEFINE_ENUM can just add a dummy element to the end which will follow
> the trailing comma:
>
> #define DEFINE_ENUM(name, list)             \
>        typedef enum {                       \
>            list(ENUM_BODY)                  \
>   __dummy__##name                  \
>        } name;

This would cause problems elsewhere, because any switch(name) { }
construct would need to be modified to either have a default or
__dummy__name case statement!   So you would break existing code.

> -jim

-derek
--
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Tim M-6
Since all roads lead to nowhere, I will just leave it alone.

Next time I have time, I'll disable iso-c and see if I can find more
important issues to address :)

Thanks,
-Tim

On Tue, Jun 7, 2011 at 12:16 PM, Derek Atkins <[hidden email]> wrote:

> Jim Paris <[hidden email]> writes:
>
>> DEFINE_ENUM can just add a dummy element to the end which will follow
>> the trailing comma:
>>
>> #define DEFINE_ENUM(name, list)             \
>>        typedef enum {                       \
>>            list(ENUM_BODY)                  \
>>          __dummy__##name                  \
>>        } name;
>
> This would cause problems elsewhere, because any switch(name) { }
> construct would need to be modified to either have a default or
> __dummy__name case statement!   So you would break existing code.
>
>> -jim
>
> -derek
> --
>       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
>       Member, MIT Student Information Processing Board  (SIPB)
>       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
>       [hidden email]                        PGP key available
>

_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Derek Atkins
Tim M <[hidden email]> writes:

> Since all roads lead to nowhere, I will just leave it alone.

Okay.

> Next time I have time, I'll disable iso-c and see if I can find more
> important issues to address :)

I highly recommend removing the iso-c configure option and working from
there!  Great idea!  :)

> Thanks,
> -Tim

-derek

--
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       [hidden email]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Geert Janssens
In reply to this post by Tim M-6
On woensdag 8 juni 2011, Tim M wrote:
> Since all roads lead to nowhere, I will just leave it alone.
>
> Next time I have time, I'll disable iso-c and see if I can find more
> important issues to address :)
>
> Thanks,
> -Tim
>
Out of curiosity, why were you attempting to generate C89 compatible code ?
Are there still actively used compilers out there that can't handle more
modern C standards ?

Geert
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel
Reply | Threaded
Open this post in threaded view
|

Re: "comma at end of enumerator list" error due to macros in libqof

Tim M-6
I am just getting into C development so I wasn't clear on what all the
compiler switches were for.  Also, I come from a background where
standards-based compliance is very important, so it seemed sensible to
code to existing standards - I just didn't realize when I flipped the
ISO-C switch that it was going to be validating against a 30-year old
standard!  It was my mistake.

Thanks,
-Tim

On Fri, Jun 10, 2011 at 8:28 AM, Geert Janssens
<[hidden email]> wrote:

> On woensdag 8 juni 2011, Tim M wrote:
>
>> Since all roads lead to nowhere, I will just leave it alone.
>
>>
>
>> Next time I have time, I'll disable iso-c and see if I can find more
>
>> important issues to address :)
>
>>
>
>> Thanks,
>
>> -Tim
>
>>
>
> Out of curiosity, why were you attempting to generate C89 compatible code ?
> Are there still actively used compilers out there that can't handle more
> modern C standards ?
>
> Geert
_______________________________________________
gnucash-devel mailing list
[hidden email]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel