Closed Bug 324035 Opened 15 years ago Closed 4 years ago

Forward-declared non-IDL classes in IDL files

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: jhpedemonte, Unassigned)

References

Details

Attachments

(1 file)

Building XULRunner, and generating Java interface files for both $(SDK_XPIDLSRCS) and $(XPIDLSRCS).  When generating, ran into many errors related to forward declared classes in the IDL files, that are not themselves defined in IDL files.  Here's a list of the non-IDL interfaces:

nsIPresShell.h
nsIDocument.h
nsObjectFrame.h
nsIFrame.h
nsIContent.h
nsILayoutHistoryState.h
mdb.h (class nsIMdbEnv, nsIMdbTable, nsIMdbRow)
nsIChannelSecurityInfo (** this one is odd, not defined anywhere)
nsIUnicodeDecoder.h
nsIUnicodeEncoder.h
nsIServiceManagerObsolete.h
nsIWordBreaker.h
nsISecureEnv.h
nsIMenuItem.h
nsIScrollbarMediator.h
nsIObjectFrame.h
nsIScriptContext.h
nsIScriptGlobalObject.h
nsIScriptElement.h
nsIFrameSelection.h
nsIWidget.h
nsIMenuBar.h
nsMacWindow.h
Attached file Java errors log
Error log file from compiling Java interfaces.

There are two errors here not related to forward declaration of non-IDL interfaces:

nsILDAPURL.idl (nsILDAPSyncQuery.idl is from /extensions/pref/autoconfig, but depends on nsILDAPURL.idl from directory/xpcom, which wasn't built.  Build config error?)
nsICmdLineService.idl (nsIProfileInternal.idl from profile, depends on nsICmdLineService.idl from xpfe/components, which isn't built for xulrunner.)
Blocks: 333618
In bug 333618, bsmedberg suggested the following:
"Perhaps, to solve the issue of forward-declared non-IDL classes you could
invent a [notidl] interface marker for the forward-declared class. When the
java output encountered that interface, it could declare the Java version with
nsISupports (or not declare it at all). Seems to me that it would not be
necessary to expose the [notidl] marker in the XPT file, which is good because
we don't have many flag bits left."

Something like this: "[notidl] interface nsIPresShell;".

Seems to me like it would work.  When I have some time, I'll work up a patch.
Doesn't look like the approach in comment #2 will work.  I added "[notidl] interface nsIPresShell;" to one IDL file, and when running xpidl on it to output the Java interface, I got this warning:

idl/nsIAccessibleRetrieval.idl:44: Ignoring properties for forward declaration `nsIPresShell'

So it looks like properties are not supported on forward decls.
Is that being emitted by libIDL or by xpidl code?
Seems to be libIDL code, since I can't find any mention of it in xpidl code.
I just debugged this, and sure enough, the |properties| array of the |forward_decl| object is NULL.  So unless there is a way to override this behavior in libIDL, I don't think we can use "[notidl]".
Well, as a hack, I added a hardcoded list of the non-IDL interface names to xpidl, ran it over the the contents of the dist/idl dir, and got these warnings:

./imgIDecoderObserver.idl:96: Warning: imgIDecoderObserver is scriptable but inherits from the non-scriptable interface imgIContainerObserver

./nsIScriptSecurityManager.idl:277: Warning: nsIScriptSecurityManager is scriptable but inherits from the non-scriptable interface nsIXPCSecurityManager

./nsIWindowCreator2.idl:93: Warning: nsIWindowCreator2 is scriptable but inherits from the non-scriptable interface nsIWindowCreator

I'm not sure how to handle these.  It seems to me that scriptable interfaces should only inherit from other scriptable interfaces.  What do you guys think?
This is a continuation of bug 333618 comment #6 (this is the correct bug to discuss this):

What about making it a hard rule that all interfaces/classes used in IDL files must themselves be defined in IDLs?  Even if the interface is only used on the C++ side of things, this would still work.  In fact, there are already quite a few interfaces like this.

So, for example, we'd have nsIPresShell.idl, non-scriptable.  It would get compiled to a C++ header.  It wouldn't be usable by JS.  And for Java, xpidl could write out a "sparse" interface file as suggested by timeless in bug 333618 ("sparse" in that it would only contain the inheritance and an IID field; no methods).

This would also work on the 1.8 branch, since it wouldn't change the interfaces' structure;  they would just be defined in an IDL rather than a .h file.  Is there any reason this wouldn't work?
Darin, Benjamin, what do you think of my approach in comment #8?
It might work depending on whether it is possible to convert the existing .h interfaces into .idl interfaces. Seems to me that [noscript]ing the relevant methods might be simpler. What interfaces are at issue?
Some C++-only "interfaces" also have member variables and inline functions, you can't really do that with IDL (except in a %{C++} block I guess). But I don't know if those are ones that are referenced in IDLs...
(In reply to comment #10)
> What interfaces are at issue?

Those listed in the bug description above.

> Seems to me that [noscript]ing the relevant
> methods might be simpler. What interfaces are at issue?

Well, since all of these interfaces are only meant to be used by C++, I would just mark the whole interface as non-scriptable.

And as Biesinger suggests, any C++ constructs would just be wrapped with %{C++} blocks (many IDL files use those now).

So the full rule would be that any type referenced in an IDL file must itself be defined in an IDL file, must be an XPIDL base type ("octect", "short", "int",...), or must be declared as a "native" type.
or be a typedef for one of those types :)
Depends on: 350080
I'm guessing this is no longer an issue or at least isn't something of high enough priority to keep an open bug on file.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.