Need support for enums in XPIDL




18 years ago
2 years ago


(Reporter: Alec Flett, Assigned: jcranmer)



Firefox Tracking Flags

(Not tracked)



(1 attachment)



18 years ago
Just talking to mccabe on IRC - I need enum support for some messenger stuff...

I need:
- C++ generated enum code
- typelib support for enums (mccabe says they'll come out as constants)

looks like some of the code is already there, but it's not complete.

Comment 1

18 years ago
What is it exactly that want/need?

What we have is constants. These generate one off enums in the C++ header (in
class scope). And you get the name/value pair reflected into JS.

What you don't get is enums as types for parameters. We decided to not do this
for a good reason... C++ allows the C++ compiler to optimize the storage of
enums to fit their possible values. That means that for an enum with a small set
of values the compiler *might* use a small size holder, while for larger values
it might use some other size. This makes for a gray area in xptcall-like
mapping. We can't have that.

Also, the enums that the (Corba) compiler front end allow don't support setting
explicit values in the source. This sucks big time.

See my discussion of this in:
search for the phase:
"Added support in header and typelib output for constants"

Note that idl constants allow for inline math. Also, we can easily fo bitflags
like in

I strongly argue that idl enums should not be added.

Comment 2

18 years ago
added warren to cc list since he just called me and asked be about enums in

Comment 3

18 years ago
hmm... so the concern is basically that the enum is of an unknown size so
XPConnecting it would be unreliable?

grr. I thought C garanteed that enums were ints, does C++ remove this

Comment 4

18 years ago
ansi C spec ( says: "Each enumerated type shall be compatible with an
integer type: the choice of type is implementation defined".

I don't have the C++ standard handy, but Stroustrup's the C++ Prog Lang 3rd.
says on p. 78: (and I paraphrase :) sizeof(some enum) <= sizeof(int), could be 1
or maybe 4 but not 8 on a machine where sizeof(int) == 4.

The real point is that the size is dependent on the enum's possible values and
the whims of the *particular* compiler used to compile the code. This is too
problematic for an xpcom interface (I argue). C++ compile-time enum legal value
checking is also a stretch to xpcom and ought not be counted upon. I think that
reliable component software ought to incur the cost of runtime checking of these
sorts of params. xpcom != C++

Comment 5

18 years ago
I thought the only compiler that had a issue with this was Metrowerks, but there
are project settings where you can declare that enums are always 32-bit ints.

I'm sure win16 had problems with this too, but we don't care about that anymore.

All I can say is that it sure would be nice if enums were types in xpidl.

Comment 6

18 years ago
Yes, and I'm not asking for XPCOM to range-check enums as they cross interfaces,
just that:
1) symbolic enum values are accessable from JS, and passable to JS-reflected
XPCOM interfaces
2) Generated C++ headers declare enums, rather than ints, to pass across

I'm going to bring this up on mozilla.xpcom with one more argument for why we
should not care about the C/C++ specification of enums.


18 years ago

Comment 7

18 years ago
Marking as assigned, to keep track until I can boil discussion of this into a

Comment 8

18 years ago
well, I eventually decided to just go with interface consts.
But I agree that this should go into a FAQ


18 years ago
Last Resolved: 18 years ago
Resolution: --- → WONTFIX

Comment 9

18 years ago
Done - jband answered this on

Marking won't fix.

(and thanks for getting back to this and reminding me to resolve it.)

Comment 10

11 years ago
*** Bug 352485 has been marked as a duplicate of this bug. ***


9 years ago
Component: xpidl → XPCOM
QA Contact: mike+mozilla → xpcom
I would like to revisit this issue.  The use of ints here imposes an unnecessary lack of type-safety in code that uses IDLs.  C++11 allows you to specify the underlying type of an enum, like

  enum EFoo : PRInt32 { foo, bar, baz };

(See N2347, part 3.2: <>.)  This is available in gcc since 4.4, Clang since 2.9, and VC++ for a long time, according to pages I found online.  Is there any compiler we care about that doesn't support this language feature, *and* doesn't default all enums to four bytes?  (In particular, I believe gcc historically defaults all enums to four bytes.)  If not, I don't think there's any good reason to give up type safety here for C++ code.  We can ensure that no one tries to use a different compiler by using a static assert on the sizeof the enum.

(We do want to make sure that the binding code for all languages ensures that the function is only called with correct enum values.  Otherwise people will probably write C++ functions that assume the values must be in the allowed set, and get very confused when they aren't.)

This might be nontrivial to do, mind you, and I'm not volunteering, but I don't think this should be WONTFIX at this point.
Resolution: WONTFIX → ---

Comment 12

5 years ago
I would argue that you probably want enum classes (w/ underlying types). So the IDL:

enum nsFooValue {

would become in C++ bindings:

MOZ_BEGIN_ENUM_CLASS(nsFooValue, uint32_t)

The XPT stuff could be generated as if all uses of nsFooValue were really unsigned long values (for everyone who supports enum classes, the ABI should be the same; the only question then is whether or not MSVC uses the same ABI in this case), and assuming that the nsFooValue were really defined like so:
interface nsFooValue {
  const unsigned long OptA = 0;
  const unsigned long OptB = 1;

This would allow JS people to use it via things like Components.interface.nsFooValue.OptA.

Comment 13

5 years ago
Created attachment 723156 [details] [diff] [review]
Add enum support to xpidl via TypedEnum.h

I honestly have no idea who needs to review this, but this is a simple implementation based on my last comment. xpidl generates enums using MFBT's MOZ_ENUM_CLASS support, and generates it according to an interface with only const members. I've chosen int32_t as the underlying type, since that is the general baseline as far as ABI is concerned without unsigned constants.
Assignee: mike+mozilla → Pidgeot18
Attachment #723156 - Flags: review?(benjamin)


5 years ago
Keywords: dev-doc-needed

Comment 14

5 years ago
Note that I'm not implementing the following features:
1. Support for limiting the enums to specific values. That requires invasive xpconnect and typelib modifications, which are well beyond my scope of knowledge.
2. Support for types other than int32_t. Underlying types aren't 100% supported on all platforms (the odd one out here is gcc 4.4). Also, our const support is in effect limited to 16 and 32 bit signed/unsigned types.
3. Support for specifying the value on an enumeration. This isn't hard to do in a follow-on, though.

The main purpose of this as it stands is to be able to replace interfaces that act as enum holders with actual enums, which would also help both the utility of warnings (we would pick up warnings on "not using all the enum types in a switch", e.g.) and reduce false positive (anonymous enum mismatches).

Comment 15

5 years ago
I'm very pleased to see that xpidl seems to have been rewritten in Python.

I'm amazed that this issue is (nearly) resolved after ALMOST 14 YEARS.

Comment 16

5 years ago
(In reply to Mike McCabe from comment #15)
> I'm very pleased to see that xpidl seems to have been rewritten in Python.
> I'm amazed that this issue is (nearly) resolved after ALMOST 14 YEARS.

Because this issue was marked as WONTFIX for 13 of those years and was impractical to fix before C++11 came around.

Comment 17

5 years ago
I probably won't be able to get to this review until after the perf workweek, so the week of 25-March.

Comment 18

2 years ago
I think we should WONTFIX this (again). There's probably no reason to add new features to the XPCOM binding system at this point.
Last Resolved: 18 years ago2 years ago
Resolution: --- → WONTFIX


2 years ago
Attachment #723156 - Flags: review?(benjamin)
I removed the useless ddn as this has been WONTFIXed
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.