For example: PersistenceType.webidl: enum PersistenceType { "permanent", "temporary" }; IDBFactory.webidl: dictionary IDBOpenDBOptions { long long version; PersistenceType storage; }; IDBFactoryBinding.h: ... struct IDBOpenDBOptions : public MainThreadDictionaryBase { Optional<PersistenceType > mStorage; Optional<int64_t > mVersion; private: ... That doesn't compile since PersistenceType is not defined. I'm attaching a simple patch that works, but it adds a useless "self" include for the cases where enums/dictionaries live in the same file.

Boris, could you help me with this ?

The real basic problem here is that enums can't be forward-declared, right? That said, why do you need to put them in different webidl files to start with?

(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #2) > The real basic problem here is that enums can't be forward-declared, right? right > > That said, why do you need to put them in different webidl files to start > with? Ehsan suggested to do it in his review comments for temp storage, bug 785884. We will need the enum in different APIs, for example FileSystem API http://lists.w3.org/Archives/Public/public-webapps/2013AprJun/0382.html

Is it still true that the enum-class enums we use now cannot be forward-declared?

(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #4) > Is it still true that the enum-class enums we use now cannot be > forward-declared? interesting, I'll try to do it that way, thanks

(In reply to comment #4) > Is it still true that the enum-class enums we use now cannot be > forward-declared? We should be able to do that. You can do that by editing mfbt/TypedEnum.h, and add a macro called MOZ_FORWARD_DECLARE_ENUM_CLASS and in the MOZ_HAVE_CXX11_STRONG_ENUMS case, have it expand to |enum class Name : Type;| and in the other case have it expand to |class Name;|. I'd be happy to review a patch which does that.

(In reply to :Ehsan Akhgari (needinfo? me!) from comment #6) > (In reply to comment #4) > > Is it still true that the enum-class enums we use now cannot be > > forward-declared? > > We should be able to do that. You can do that by editing mfbt/TypedEnum.h, > and add a macro called MOZ_FORWARD_DECLARE_ENUM_CLASS and in the > MOZ_HAVE_CXX11_STRONG_ENUMS case, have it expand to |enum class Name : > Type;| and in the other case have it expand to |class Name;|. > > I'd be happy to review a patch which does that. I have a patch that does exactly the same, I'm testing it here: https://hg.mozilla.org/try/rev/df7c68aeb216

Comment on attachment 767753 [details] [diff] [review] patch v0.1 Why are we still adding the #include now that we're forward-declaring things?

(In reply to Boris Zbarsky (:bz) (reading mail, but on paternity leave) from comment #9) > Comment on attachment 767753 [details] [diff] [review] > patch v0.1 > > Why are we still adding the #include now that we're forward-declaring things? the #include is now in the generated cpp file when I remove it, I get this error: /Users/varga/Sources/Mozilla/obj-ff-dbg/dom/bindings/IDBFactoryBinding.cpp:64:61: error: use of undeclared identifier 'PersistenceTypeValues'; did you mean 'PersistenceType'? ...temp.ref(), PersistenceTypeValues::strings, "PersistenceType", ... ^~~~~~~~~~~~~~~~~~~~~ PersistenceType ./IDBFactoryBinding.h:14:24: note: 'PersistenceType' declared here MOZ_FORWARD_ENUM_CLASS(PersistenceType, uint32_t) ^ ../../dist/include/mozilla/TypedEnum.h:104:57: note: expanded from macro 'MOZ_FORWARD_ENUM_CLASS' # define MOZ_FORWARD_ENUM_CLASS(Name, type) enum class Name : type; ^ /Users/varga/Sources/Mozilla/obj-ff-dbg/dom/bindings/IDBFactoryBinding.cpp:64:61: error: incomplete type 'mozilla::dom::PersistenceType' named in nested name specifier ...temp.ref(), PersistenceTypeValues::strings, "PersistenceType", ...

Oh, in the .cpp file the #include makes perfect sense, yes!

Comment on attachment 767753 [details] [diff] [review] patch v0.1 Review of attachment 767753 [details] [diff] [review]: ----------------------------------------------------------------- r=me on the TypedEnum.h changes. Please ask somebody else (khuey/peterv/smaug/Ms2ger) to review the codegen changes, as I know nothing about that code. Thanks! ::: mfbt/TypedEnum.h @@ +100,5 @@ > * underlying type, so no extra check is needed. > */ > +# define MOZ_BEGIN_ENUM_CLASS(Name, type) enum class Name : type { > +# define MOZ_END_ENUM_CLASS(Name) }; > +# define MOZ_FORWARD_ENUM_CLASS(Name, type) enum class Name : type; Nit: I think MOZ_FORWARD_DECLARE_ENUM_CLASS is a better name. Also, please document the new macro above this block, perhaps at the end of the current documentation. Thanks!

It seems that the forward declaration can't be used on platforms which don't support strong enums. MOZ_FORWARD_DECLARE_ENUM_CLASS(PersistenceType, uint32_t) Optional<PersistenceType > mStorage; the macro translates to |class PersistenceType;| and the template fails to compile https://tbpl.mozilla.org/?tree=Try&rev=984b5e717fb0 Am I interpreting it right? I think we have to check MOZ_HAVE_CXX11_STRONG_ENUMS in the generated code and forward declare just the enum or include the entire file according to it.

Yeah, I tried to make forward declaration of enums work before, but I couldn't figure out any way to make it work on VS2010.

We might as well do the #include in all cases, then, and just skip it if it would be the same file. This probably involves telling CGHeaders which file it's for.

Yeah, let's give up on the idea of forward declaring enums here.

Comment on attachment 770358 [details] [diff] [review] patch v0.2 The checkin comment should probably be more like this: Bug 886755 - Include the correct binding header if a binding uses a WebIDL enumeration that's defined in a different .webidl file. CGHeader.__init__ needs to document the new argument in its docstring. Please document why the 'filename != prefix + ".h"' check is there. r=me with that

(In reply to :Ehsan Akhgari (not reading bugmail, needinfo? me!) from comment #16) > Yeah, let's give up on the idea of forward declaring enums here. Do we have a way to forward declare enums defined with MOZ_BEGIN_ENUM_CLASS today? I stumbled on this bug while looking for a way to do it.