Created attachment 149671 [details] [review]
Updates enum.pl script.

Oops, by mistake all enums were classified as enum, not as flags, when they evidently were flags. I fixed it and attach a new patch with more groovy git commit message.

Created attachment 149901 [details] [review]
gtkmm_withnewenumsscript.patch

The difference to gtk_enums.patch when using this new version of the script.

Using this on gtk+ shows some weird stuff compared to the current gtk_enums.defs in gtkmm. See the attached patch. I'd rather fix that before committing this.


Also, should we worry about these warnings that it prints?:

WARNING: G_SIGNAL_RUN_FIRST in GTK_RUN_FIRST in 'GtkSignalRunType' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.
WARNING: G_SIGNAL_RUN_LAST in GTK_RUN_LAST in 'GtkSignalRunType' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.
WARNING: G_SIGNAL_NO_RECURSE in GTK_RUN_NO_RECURSE in 'GtkSignalRunType' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,

Thanks for your response.

1. You probably meant GtkRcTokenType enum by "weird stuff". Problem is that with previous version of enum parser incorrect values were generated, because first value in this enum is G_TOKEN_LAST from Glib module. Neither previous version nor my does not know its value, but difference lies in handling unknown tokens.

Previous parser just puts #error, my - copies it, assuming that, what is written in warnings. When unknown token is found and next enum have no specified value it is assumed that is unknown token + 1.

I know, it could be done better - instead of printing:

'("invalid" "GTK_RC_TOKEN_INVALID" "G_TOKEN_LAST")
'("include" "GTK_RC_TOKEN_INCLUDE" "((G_TOKEN_LAST) + 1)")
'("normal" "GTK_RC_TOKEN_NORMAL" "(((G_TOKEN_LAST) + 1) + 1)")
'("active" "GTK_RC_TOKEN_ACTIVE" "((((G_TOKEN_LAST) + 1) + 1) + 1)")
'("prelight" "GTK_RC_TOKEN_PRELIGHT" "(((((G_TOKEN_LAST) + 1) + 1) + 1) + 1)")

better would be:

'("invalid" "GTK_RC_TOKEN_INVALID" "G_TOKEN_LAST")
'("include" "GTK_RC_TOKEN_INCLUDE" "((G_TOKEN_LAST) + 1)")
'("normal" "GTK_RC_TOKEN_NORMAL" "((G_TOKEN_LAST) + 2)")
'("active" "GTK_RC_TOKEN_ACTIVE" "((G_TOKEN_LAST) + 3)")
'("prelight" "GTK_RC_TOKEN_PRELIGHT" "((G_TOKEN_LAST) + 4)")

I'll try to fix it.

2. As of ToolPaletteDragTargets - I'll fix it. Definition of this enum differs from others in that instead of
typedef enum
{
...
} ToolPaletteDragTargets;

there is
typedef enum
{
...
}
ToolPaletteDragTargets;

Easier would be just putting ToolPaletteDragTargets in the same line as closing bracket.

3. About using hexadecimal values in enums (see: GTK_ANCHOR_N) - I'll look at it, but it is not critical, since number is a number.

===

And by the way - I recommend using --with-typedefs option - very helpful. It prints original typedef before every defs in ;;, like this:

;; From gtkentry.h

;; Original typedef:
;; typedef enum
;; {
;;   GTK_ENTRY_ICON_PRIMARY,
;;   GTK_ENTRY_ICON_SECONDARY
;; } GtkEntryIconPosition;

(define-enum-extended EntryIconPosition
  (in-module "Gtk")
  (c-name "GtkEntryIconPosition")
  (values
    '("primary" "GTK_ENTRY_ICON_PRIMARY" "0")
    '("secondary" "GTK_ENTRY_ICON_SECONDARY" "1")
  )
)

About warnings - no, mostly we should worry about them - enum parser prints them just to let you know that some enums uses values other that earlier defined. It is best seen in Gio module in GSocketMsgFlags.

Also, if you don't want deprecated enum or enum values to be parsed, please use --omit-deprecated option. Maybe omitting deprecated enum and enum values should be enabled by default and option renamed to --parse-deprecated.

I'll be very grateful for your opinions.

I'm happy as long as we don't create new build problems for gtkmm.

Please make the --with-typedefs option the default. Comments are always good. In fact, you can remove it as an option.

By default, we _do_ want deprecated enums to be shown, because we don't remove wrappers for deprecated API. Well, not until gtkmm3.

Created attachment 149951 [details]
enum.pl script

This is whole enum.pl script instead of patch. It fixes 1 and 2 from comment 4.

But now I got errors like this:

Broken lisp definition for flags-extended IOFlags.
unknown token (in-module "G") 
unknown token (c-name "GIOFlags") 
unknown token (values '("append" "G_IO_FLAG_APPEND" "1 << 0") '("nonblock" "G_IO_FLAG_NONBLOCK" "1 << 1") '("is-readable" "G_IO_FLAG_IS_READABLE" "1 << 2") '("is-writeable" "G_IO_FLAG_IS_WRITEABLE" "1 << 3") '("is-seekable" "G_IO_FLAG_IS_SEEKABLE" "1 << 4") '("mask" "G_IO_FLAG_MASK" "(1 << 5) - 1") '("get-mask" "G_IO_FLAG_GET_MASK" "0x1F") '("set-mask" "G_IO_FLAG_SET_MASK" "0x3") ) 
unknown token )

def looks like this:

(define-flags-extended IOFlags
  (in-module "G")
  (c-name "GIOFlags")
  (values
    '("append" "G_IO_FLAG_APPEND" "1 << 0")
    '("nonblock" "G_IO_FLAG_NONBLOCK" "1 << 1")
    '("is-readable" "G_IO_FLAG_IS_READABLE" "1 << 2")
    '("is-writeable" "G_IO_FLAG_IS_WRITEABLE" "1 << 3")
    '("is-seekable" "G_IO_FLAG_IS_SEEKABLE" "1 << 4")
    '("mask" "G_IO_FLAG_MASK" "(1 << 5) - 1")
    '("get-mask" "G_IO_FLAG_GET_MASK" "0x1F")
    '("set-mask" "G_IO_FLAG_SET_MASK" "0x3")
  )
)

For now I don't know why I'm getting this.

Problem is with gmmproc not accepting parentheses (by not parsing them) between quotes, like for example a G_IO_FLAG_MASK element's value in above enum. I'm trying to look at this, but I haven't grasped perl's split function yet.

It would be good to have support for parens inside quotes because of defines like below:

header.h:

#define MY_DEF = 1 << 5

other_header.h:

#include <header.h>

typedef enum
{
  MY_VAL1 = MY_DEF,
  MY_VAL2,
  MY_VAL3,
  ...
} MyEnum

In this case for enum.pl MY_VAL is unknown and produces defs like this:

(define-whatever-extended MyEnum
  ...
  (values
    '("val1" "MY_VAL1" "MY_DEF")
    '("val2" "MY_VAL2" "(MY_DEF) + 1")
    '("val3" "MY_VAL3" "(MY_DEF) + 2")
  )
)

Parens protect against having MY_VAL2 evaluated by preprocessor as '1 << 5 + 1' which equals to '1 << 6' instead of '(1 << 5) + 1'.

Maybe this example is bogus and if such protection is not needed I'll just change enum.pl to not use parens.

(In reply to comment #7) 
> #define MY_DEF = 1 << 5

should be:
#define MY_DEF 1 << 5

Created attachment 150807 [details] [review]
Changes splitters in GtkDefs.pm and in Enum.pm

This patch changes token splitters, so parentheses in strings (like in a description of a family property of GInetAddress or G_IO_FLAG_MASK value from defs in comment 6) are correctly parsed.

Created attachment 150808 [details] [review]
Another updated enum script patch.

This patch provides updated enum.pl. I don't remember if any changes were done in comparison of previous attachment (maybe some trailing whitespace cleanup), but I'm attaching it anyway.

Created attachment 150849 [details] [review]
Updated patch with new splitters.

Shortened one check with a regexp.

These patches works for me - building tested on glibmm, giomm, pangomm, atkmm, gdkmm and gtkmm. And I'm sure it will also work with libvtemm and gtksourceviewmm.

Hope you'll get the same results.

When I run this on giomm, I get the following errors.  is there any way to fix them (sorry, I know nothing about perl)?

$ ./tools/enum.pl /home/jonathon/.jhbuild-prefix/include/glib-2.0/gio/*.h ~/.jhbuild-prefix/include/gio-unix-2.0/gio/*.h > gio/src/gio_enums.defs 
WARNING: GLIB_SYSDEF_AF_UNIX in G_SOCKET_FAMILY_UNIX in 'GSocketFamily' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.
WARNING: GLIB_SYSDEF_AF_INET in G_SOCKET_FAMILY_IPV4 in 'GSocketFamily' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.
WARNING: GLIB_SYSDEF_AF_INET6 in G_SOCKET_FAMILY_IPV6 in 'GSocketFamily' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.
WARNING: GLIB_SYSDEF_MSG_OOB in G_SOCKET_MSG_OOB in 'GSocketMsgFlags' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.
WARNING: GLIB_SYSDEF_MSG_PEEK in G_SOCKET_MSG_PEEK in 'GSocketMsgFlags' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.
WARNING: GLIB_SYSDEF_MSG_DONTROUTE in G_SOCKET_MSG_DONTROUTE in 'GSocketMsgFlags' is an unknown token. It probably is:
         - a typo - send a patch to maintainer of this module,
         - preprocessor value - make sure you include a header defining this value,
         - enum value from other header or other module - see above.

I should note that this is not a regression since the unpatched enums.pl also failed on these enumerations, but it would still be nice to fix this.

Hmm.  I guess I should have looked more closely at the output before posting these last comments.  Because even though the warnings above were printed, the output seems to be generated correctly, whereas it was not generated correctly with the old enums.pl script.  Still, I'd like to know what those warnings are trying to tell me.

These warnings say, that enum.pl script does not know the value of these tokens - it is normal, when value is a preprocessor value [1] or enum value from other module [2] - enum.pl then assumes, that appropriate header defining these unknown values will be included. A typo is a third option, but it does not happen here. So these warnings are mostly harmless.

[1] GSocketMsgFlags is good example of it - GLIB_SYSDEF_MSG_ROUTE is a preprocessor value defined in glibconfig.h.

[2] gtk's GtkRcTokenType has first value defined as glib's GTokenType last value - G_TOKEN_LAST.

oh, I just noticed that murray had posted about the same warnings farther up.  Sorry for making so much noise about something that was already discussed.  

However, the warning message it prints out ("make sure you include a header defining this value") made me think that maybe I was supposed to add the header containing this symbol as an argument to enums.pl (e.g. './enums.pl header-with-symbol.h /path/to/gio/headers/*.h').  So I tried that but it still printed the same warnings.  But it's really trying to say that the header including this symbol should be included into the file where that enumeration is wrapped.  Perhaps that could be stated more obviously (?)

Good point - I'll fix it.

Created attachment 150868 [details] [review]
Reworked another updated enum script patch.

Fixes wording of warnings about unknown token.

Have you tried using this for gtkmm, for instance?

Probably yes, but that was some time ago, so I don't remember now. I'll test it after weekend.

Created attachment 153541 [details]
Comparison scripts.

I tested above patches on glibmm (glib and gio), pangomm, gtkmm (atk, gdk and gtk), libvtemm and gtksourceviewmm - all went well.

I tested it by pulling glibmm from git, applying both patches, then updating all enums.defs in all modules, touching all .hg and .ccg files, compiling and installing.



Also, for just curious, I'm attaching a tarball with four perl scripts doing a nice comparison between old and new enums.defs. I wrote it because diff output is not legible. There is a README inside tarball, so please read it first.

When I run this on the glib/gio/g*.h headers, to update glib/gio/src/gio_enums.defs, I get a lot of 
"Broken lisp definition for flags-extended"
errors from gmmproc.

I guess it has something to do with ( and ) in the values.

Sorry, I have already pushed your patch, so it would be nice if you take a look at this.

Seems that you didn't push a patch with new splitters. I was testing building modules with both patches applied to glibmm.

I applied your latest patch. Could you please provide a new patch against git master, or just commit a fix if you have access?

Sure, I just pushed the lacking patch (the "Updated patch with new splitters." in attachments section below). Now should work.

I'm closing this for now. If any problems with enum defs appear, just reopen this and I'll see to it.

Sorry to re-open this.  I was just rebuilding all modules in jhbuild and glibmm fails to build because of a stray '#' in an enum declaration in glibmm/unicode.h:

enum NormalizeMode
{
  NORMALIZE_DEFAULT = 0,
  NORMALIZE_NFD = #error,
  NORMALIZE_DEFAULT_COMPOSE = 1,
  NORMALIZE_NFC = 1,
  NORMALIZE_ALL = 2,
  NORMALIZE_NFKD = 2,
  NORMALIZE_ALL_COMPOSE = 3,
  NORMALIZE_NFKC = 3
};

Could you please look into that?  Thanks.

Seems that new enum splitter does not work with *_enums.defs generated by old enum.pl. But new enum.pl script was pushed to master recently, so could you regenerate glib_enums.defs (and probably other enums.defs too)? That should fix it.

Yes, that does fix it.  Thanks.

Could someone do me a favour and use this to fix the glibmm build by regenerating/fixing the .defs file? Sorry, don't have time right now.

Done.  Please let me know if something is not right.