352485 - Would like enum type in xpidl

Status: RESOLVED DUPLICATE of bug 8781

(Core :: XPCOM, enhancement)

Description

[Håkan Waara]

I'm surprised that I can't find any bug about this. The closest are about xpidl's handling of const ints (turning them into C++ anonymous enums). See bug 262000, and bug 239460.

I'd like an enum type, so we can avoid having to do stuff like this (http://landfill.mozilla.org/mxr-test/mozilla/source/accessible/public/nsIAccessible.idl#271) in interfaces, when you want to expose constants in your IDL.

If this is something IDL is missing, would it be frowned upon to implement it in xpidl only?

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

bug 8781?

Comment 2

[Håkan Waara]

Ah, damn...

*** This bug has been marked as a duplicate of 8781 ***

Comment 3

[John Bandhauer]

FWIW, your link above points to a set of bitflags. The automatic numbering of a C-style enum declaration would not have helped here. What you *could* have done is used bit shift notation: 1 << 0, 1 << 1, etc. to make this easier to confirm by eye. That is mentioned in the xpconnect/xpidl faq:
http://www.mozilla.org/scriptable/faq.html#i2

You'd still want to confirm at least once that the xpidl output is as expected :)

I agree that it is nice when the compiler helps with numbering. But, in the lines below that in the interface you linked to, the code's author could have helped him/herself by tabbing out the numbers so that they would be easy to scan by eye for errors.

My real arguments against adding enums to xpidl were against enums as types for interface params. The issue of compile-time auto-numbering helpers is a completely different thing. The problem there is more about munging the IDL compiler we are dependent upon to support a desired syntax. But, I'd still support the notion that the generated output has to map to one xpidl's fixed size types and not a C/C++ typed enum.

Comment 4

[Håkan Waara]

(In reply to comment #3)
> FWIW, your link above points to a set of bitflags. The automatic numbering of a
> C-style enum declaration would not have helped here.

Sorry, my link was mispointed. Look again here: http://landfill.mozilla.org/mxr-test/mozilla/source/accessible/public/nsIAccessible.idl#377

My main issue is that it looks really bad to have so many constants in an interface. At least an enum is easier on the eye, in that you do not have to manually number them all.

The constant name is exposed to JS as well, so I don't see why the users would have to care about the internal value of these ints.

However, I suppose the issue you raised about unknown storage sizes still applies. Thanks for the educational post. :)

Comment 5

[John Bandhauer]

I should just let this drop, but I want to point out some more things...

Your file is interesting because if has a few examples of why auto-numbered emuns are dangerous or not useful:

1) the bitflag set that I already pointed out.

2) the set of 'RELATION_*' constants that need to match external 'ATK_RELATION_*' values (I gather these are somewhat standardized?). When I googled I saw at least one other header in the world that used an enum for typing the set, but gave each entry an explicit value. Nevertheless, you really want explicit values in your declaration to ensure that they values all match what is expected. It is just too easy to screw up the matching without explicit numbering.

3) I would argue that even the place where you would like to use auto-numbering is not necessarily a good place. The nice thing about manual numbering is that it forces the code maintainer to *think* when adding a new item or making any other change to the ordered set of items. In a closed situation where you can recompile all code and not worry about compatability with previous versions, it would not matter if you added a new item in the middle of the list, or reordered them, or removed an unused item and let some of the members get automatically re-numbered. But, in an *interface* used by external code, that might need to stay stable from version to version, it is *good* that the manual aspect of this discourages a maintainer from making unnecessary changes (which might even be unintentional). For instance, I know there have been recent interface changes in this code having to do with method reordering that have caused some people 'pain'.

Also, most users of the interface don't care about the actual values. But, they are not hurt by the presence of values either. However, when debugging, it can be very handy to not have to *manually* figure out what each auto-numbered value is likely to be. Explicit is not always bad.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

When debugging, it would be even more convenient to be shown the name of the constant rather than the value, which can be done by giving it an enum type... (that is in fact the main advantage of enums, in my opinion)

Comment 7

[Mats Palmgren (inactive)]

We have a lot of code like this:
    PRUint32 additionEvent = isAsynch ? nsIAccessibleEvent::EVENT_ASYNCH_SHOW :
                                        nsIAccessibleEvent::EVENT_DOM_CREATE;

Modern versions of gcc complains about it:
nsDocAccessible.cpp:2028: warning: enumeral mismatch in conditional expression: 'nsIAccessibleEvent::<anonymous enum>' vs 'nsIAccessibleEvent::<anonymous enum>'

which teaches people to ignore such warnings, which is bad.

Comment 8

[David Bradley]

I created Bug 497880 to consider changing the anonymous enum's to static const declarations. That should address the warning issue.