Closed Bug 458936 Opened 16 years ago Closed 13 years ago

Replace binary xpidl xpt output with a python version hooked up to xpidl.py

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla8

People

(Reporter: benjamin, Assigned: khuey)

References

(
URL
)

Details

(Keywords: dev-doc-needed, Whiteboard: fixed-in-bs)

Attachments

(1 file, 4 obsolete files)

typelib.py
20.48 KB, patch
ted
: review+
Details | Diff | Splinter Review 
We currently are using the binary xpidl tool to produce XPT output. We should port this logic to python and get rid of the binary XPIDL parser for good.

Relevant links:
existing binary XPT output code:
http://mxr.mozilla.org/mozilla-central/source/xpcom/typelib/xpidl/xpidl_typelib.c

new python XPIDL parser core:
http://mxr.mozilla.org/mozilla-central/source/xpcom/idl-parser/xpidl.py

code to produce C++ headers from the python core:
http://mxr.mozilla.org/mozilla-central/source/xpcom/idl-parser/header.py

The magic here is, I'm afraid, that the XPT format is under-documented, so you get to have the fun of reverse-engineering parts of it. The available docs are here:
http://www.mozilla.org/scriptable/typelib_file.html
Needs to be able to both generate and read the xpt files, so it can replace xpidl and xpt_link. We should create some unit tests before even starting this. :)
I don't think it's necessary to replace xpt_link in this bug. xpt_link doesn't depend on glib/libIDL, as far as I know; even though it's still a damnable host program ;-)
I'll probably start off writing tests anyway.
What Python version are we writing for? Is 2.4 good enough, or do we need 2.3 compatibility? bsmedberg, getName/setName is really not very Pythonic (from xpidl.py)...

Ted mentioned we like 2.3 support in the build system.
We want 2.3 compat unless there's some really serious reason we need a later version.
I started looking into this, by writing a functional equivalent of xpt_dump in Python.  (It does not produce the same output format.)  I will attach it when I get back to the computer it's on, but here are some tidbits about the file format that might be useful:

 * http://www.mozilla.org/scriptable/typelib_file.html says that the
   interface_directory pointer in the file header is a zero-based offset
   from the beginning of the file.  THIS IS WRONG.  It is a *one*-based
   offset from the beginning of the file.  The data_pool pointer, however,
   is zero-based.

 * parent_interface_index zero is documented to mean nsISupports, but the
   existing tools actually use it to mean "this interface has no parent"
   (e.g. in the .xpt file for nsISupports itself).  nsISupports gets a regular
   entry in the interface directory of each file that needs to refer to it.

 * Type tags #13 (void) and #14 (uuid) can appear without the pointer flag.
   Also, #14 is not necessarily an IID.  (xpt_dump thinks #14 requires the
   pointer flag, but it's wrong.)

 * Reserved type tags #23, #24, #25 have been assigned meaning; if
   xpt_dump is to be believed, they are "UTF8String", "CString", and
   "AString" respectively, require the pointer flag, and use 
   the SimpleTypeDescriptor variant.

 * The first reserved ParamDescriptor flag bit (value 0x04, treating the
   entire set of flags as a byte quantity) has been assigned; it means
   "optional".

That's everything I can remember.

I'd be happy to write an update for typelib_file.html provided I can bug someone about which of the above were changes in format 1.2 and which were always that way just misdocumented.
Oh, also, if you're going for byte-for-byte compatibility with the existing generators, be aware that they do *not* unify duplicate strings.
Well, byte-for-byte compatibility seemed like the best way to verify that untested code was behaving sanely. If you want to use an alternative strategy (identical xpt_dump output, for example, or something similar), please feel free to propose something!

Unless the XPT version was changed after Firefox 1 (which I seriously doubt) then I don't think we need to worry about older versions at all.
I'm definitely down with byte-for-byte compatibility as a condition for switching over.  I just thought I'd mention the string thing as something that might surprise, and perhaps also as something that should be changed after the switchover.

I'm only interested in <1.2 versions for the documentation's sake, but perhaps it would be clearer to just not bother documenting what old versions were like.
Unless it's easy to find out, I think just documenting the current version is quite reasonable and probably clearer.
Attached file python xpt dumper (obsolete) — 
Here's my python xpt dumper.  It's probably not great as a basis for an xpt *writer*, but it may still be useful for reference.  I made a start on a more structured xpt library but didn't get far enough for it to be interesting, I think.
The URL is my Python XPT reading/writing library. It can properly round-trip all the xpt files produced in a Firefox build, and its xpt_dump output exactly matches the binary xpt_dump.
Blocks: 578790
URL: http://hg.mozilla.org/users/tmielczar...
Summary: Replace binary xpidl xpt output with a python version → Replace binary xpidl xpt output with a python version hooked up to xpidl.py
Assignee: dirkjan → nobody
Assignee: nobody → khuey
I have a patch that is correct enough to start the browser.  Tracking down the remaining issues now.
Attached patch typelib.py (obsolete) — — Splinter Review 
Throwing this at try now, but I think it's good enough for a review.
Attachment #346402 - Attachment is obsolete: true
Attachment #551743 - Flags: review?(ted.mielczarek)
Status: NEW → ASSIGNED
Attached patch typelib.py (obsolete) — — Splinter Review 
Fix a bug with jsvals being weird.
Attachment #551743 - Attachment is obsolete: true
Attachment #551754 - Flags: review?(ted.mielczarek)
Attachment #551743 - Flags: review?(ted.mielczarek)
Attached patch typelib.py (obsolete) — — Splinter Review 
Ugh, hg being dumb.
Attachment #551754 - Attachment is obsolete: true
Attachment #551755 - Flags: review?(ted.mielczarek)
Attachment #551754 - Flags: review?(ted.mielczarek)
Attached patch typelib.py — — Splinter Review 
And this handles size_is with strings.  This passes tests locally for me, throwing it at try to verify.
Attachment #551755 - Attachment is obsolete: true
Attachment #551856 - Flags: review?(ted.mielczarek)
Attachment #551755 - Flags: review?(ted.mielczarek)
Comment on attachment 551755 [details] [diff] [review]
typelib.py

Review of attachment 551755 [details] [diff] [review]:
-----------------------------------------------------------------

::: config/rules.mk
@@ +1553,5 @@
> +	$(PYTHON_PATH) \
> +	  -I$(topsrcdir)/other-licenses/ply \
> +	  -I$(topsrcdir)/xpcom/idl-parser \
> +	  -I$(topsrcdir)/xpcom/typelib/xpt/tools \
> +	  $(topsrcdir)/xpcom/idl-parser/typelib.py --cachedir=$(topsrcdir)/xpcom/idl-parser $(XPIDL_FLAGS) $(_VPATH_SRCS) -d $(MDDEPDIR)/$(@F).pp -o $@

You should file a followup on having a single pass invocation that generates both headers and typelibs in one shot. (Also we could probably make the script handle multiple IDL files in one shot, which would be even nicer. It could cache the parse tree for things like nsISupports which would probably save some time.)

::: xpcom/idl-parser/typelib.py
@@ +69,5 @@
> +    'cstring':            xpt.Type.Tags.CString,
> +    'jsval':              xpt.Type.Tags.jsval
> +}
> +
> +# XXXkhuey dipper types should go away

File a bug on this and list the bug number?

@@ +73,5 @@
> +# XXXkhuey dipper types should go away
> +def isDipperType(type):
> +    return type == xpt.Type.Tags.DOMString or type == xpt.Type.Tags.AString or type == xpt.Type.Tags.CString or type == xpt.Type.Tags.UTF8String
> +
> +def write_interface(iface, ifaces):

I'd call this something other than "write_interface", since it doesn't write anything, it just returns an xpt.Interface object. build_interface? populate_interface?

@@ +84,5 @@
> +        if isinstance(type, xpidl.Builtin):
> +            tag = TypeMap[type.name]
> +            return xpt.SimpleType(tag,
> +                                  pointer=(tag == xpt.Type.Tags.char_ptr or tag == xpt.Type.Tags.wchar_t_ptr),
> +                                  #XXXkhuey unique_pointer is completely unused

File a bug on removing it and mention the bug number here. Also, unique_pointer defaults to False, so you don't have to specify it here at all.

@@ +90,5 @@
> +                                  reference=False)
> +
> +        if isinstance(type, xpidl.Array):
> +            return xpt.ArrayType(get_type(type.type, calltype), size_is,
> +                                 #XXXkhuey length_is is not actually used,

Should probably say "length_is duplicates size_is", and file a bug on removing it and mention the bug number here...

@@ +97,5 @@
> +        if isinstance(type, xpidl.Interface) or isinstance(type, xpidl.Forward):
> +            xptiface = None
> +            for i in ifaces:
> +                if i.name == type.name:
> +                    xptiface = i

We should probably just build a dict of interface name -> interface and use that instead.

@@ +101,5 @@
> +                    xptiface = i
> +
> +            if not xptiface:
> +                xptiface = xpt.Interface(name=type.name)
> +                ifaces.append(xptiface)

Hrm. If you have a forward declaration of an interface, used in an interface declaration, and then an actual declaration of the interface later in the same file, this would probably produce a malformed typelib. (You'd put an unresolved and a resolved interface descriptor in the same file.) I doubt that ever happens in practice, but we should probably try to avoid it.

@@ +126,5 @@
> +                                      unique_pointer=False,
> +                                      reference=False)
> +
> +        print type(type)
> +        raise Exception("fooey")

Presumably you want this to say something more useful. :)

@@ +148,5 @@
> +        type = get_type(m.realtype, 'out')
> +        if isDipperType(type.tag):
> +            # NB: The retval bit needs to be set here, contrary to what the
> +            # xpt spec says.
> +            return xpt.Param(type, True, False, True, dipper=True)

You should probably pass all these args by keyword. I wrote that code and I have no idea what the args are here.

@@ +151,5 @@
> +            # xpt spec says.
> +            return xpt.Param(type, True, False, True, dipper=True)
> +        return xpt.Param(type, False, True, True)
> +
> +    def get_attr_param(a, get=False, set=False):

You might want to name these "getter" and "setter" for consistency with other code.

@@ +155,5 @@
> +    def get_attr_param(a, get=False, set=False):
> +        if not (get or set):
> +            raise Exception("Attribute param must be for a getter or a setter!")
> +
> +        type = get_type(a.realtype, get == True and 'out' or 'in')

"get == True" is redundant, just say "get". Also, Python has a true ternary expression, you could write this as:
'out' if get else 'in'
but some people don't like that syntax, so it's up to you.

@@ +171,5 @@
> +
> +    consts = []
> +    methods = []
> +
> +    def write_const(c):

Again, "write" sounds like a misnomer.

@@ +178,5 @@
> +    def write_method(m):
> +        params = []
> +
> +        def get_param(p):
> +            def findattr(p, attr):

This could be ...more usefully named, I think. "find_referenced_param" or something? A docstring comment wouldn't hurt either.

@@ +179,5 @@
> +        params = []
> +
> +        def get_param(p):
> +            def findattr(p, attr):
> +                if hasattr(p, attr) and getattr(p, attr):

Why the "and getattr"? To avoid None values? Seems a little weird.

@@ +182,5 @@
> +            def findattr(p, attr):
> +                if hasattr(p, attr) and getattr(p, attr):
> +                    for param in m.params:
> +                        if param.name == getattr(p, attr):
> +                            return m.params.index(param)

You could do:
for i, param in enumerate(m.params):
and just return i.

@@ +206,5 @@
> +            params.append(get_retval_param(m))
> +
> +        methods.append(xpt.Method(m.name, get_result_param(m), params, False,
> +                                  False, m.notxpcom, False, m.noscript,
> +                                  m.optional_argc, m.implicit_jscontext))

Again, can you use keyword args here, at least when you're just passing boolean literals?

@@ +238,5 @@
> +    parent = None
> +    if iface.base:
> +        for i in ifaces:
> +            if i.name == iface.base:
> +                parent = i

Really ought to be a dict lookup. Does idl.resolve not set these to the actual interface types for you? (If not we should probably make it do that.)

@@ +241,5 @@
> +            if i.name == iface.base:
> +                parent = i
> +        if not parent:
> +            parent = xpt.Interface(name=iface.base)
> +            ifaces.append(parent)

This has the same potential bug as I described earlier.

@@ +245,5 @@
> +            ifaces.append(parent)
> +
> +    return xpt.Interface(iface.name, iface.attributes.uuid, methods=methods,
> +                         constants=consts, resolved=True, parent=parent,
> +                         scriptable=not iface.attributes.noscript,

Shouldn't this technically be "iface.attributes.scriptable and not iface.attributes.noscript". I don't know why we support both of those on an interface, seems dumb.

@@ +262,5 @@
> +    typelib = xpt.Typelib(interfaces=ifaces)
> +    typelib.writefd(fd)
> +
> +if __name__ == '__main__':
> +    from optparse import OptionParser

How about you stick this all in a "def main()" and just call main() here, in case we get an opportunity to ditch pythonpath.py and still want to use this as a native command?

@@ +285,5 @@
> +        print >>sys.stderr, "-d requires -o"
> +        sys.exit(1)
> +
> +    if options.outfile is not None:
> +        outfd = open(options.outfile, 'wb')

Is there a reason you open the file here and not in write_typelib? You'd only have to pass down the filename if you did that.

@@ +286,5 @@
> +        sys.exit(1)
> +
> +    if options.outfile is not None:
> +        outfd = open(options.outfile, 'wb')
> +        closeoutfd = True

Feels like you just want to "with open(outfile, 'wb') as outfd". (You'll need from __future__ import with_statement to use that reliably.)

@@ +299,5 @@
> +    if closeoutfd:
> +        outfd.close()
> +
> +    if options.depfile is not None:
> +        depfd = open(options.depfile, 'w')

with open(options.depfile, 'w') as depfd: (Also it's a little weird to name these variables "fd", since they're really file objects. You can get real file descriptors in Python with os.open.)

::: xpcom/typelib/xpt/tools/xpt.py
@@ +145,5 @@
>          self.pointer = pointer
>          self.unique_pointer = unique_pointer
>          self.reference = reference
> +        if reference and not pointer:
> +            raise Exception("If reference is True pointer must be True too")

Good catch.

@@ +645,5 @@
>          self.optargc = optargc
>          self.implicit_jscontext = implicit_jscontext
>          self.params = list(params)
> +        if result and not isinstance(result, Param):
> +            raise Exception("result must be a Param!")

Good catch here too. Want to add unit tests for these two cases to the test suite?
Attachment #551755 - Attachment is obsolete: false
Attachment #551755 - Attachment is obsolete: true
Comment on attachment 551856 [details] [diff] [review]
typelib.py

Review of attachment 551856 [details] [diff] [review]:
-----------------------------------------------------------------

Overall this looks good. Any thoughts on a test suite? With slight refactoring of typelib.py you could make it take an IDL file and return the XPT object, and you could at least sanity check your mappings. The xpt.py tests already test round-tripping of data to and from a file, so that's reasonably well tested.

::: xpcom/idl-parser/typelib.py
@@ +84,5 @@
> +        if isinstance(type, xpidl.Builtin):
> +            if type.name == 'string' and size_is != None:
> +                  return xpt.StringWithSizeType(size_is, size_is)
> +            elif type.name == 'wstring' and size_is != None:
> +                  return xpt.WideStringWithSizeType(size_is, size_is)

I'd say this ought to be a lookup table, but there are only two entries anyway. :-/
Attachment #551856 - Flags: review?(ted.mielczarek) → review+
(In reply to Ted Mielczarek [:ted, :luser] from comment #19)
> Comment on attachment 551755 [details] [diff] [review] [diff] [details] [review]
> typelib.py
> 
> Review of attachment 551755 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: config/rules.mk
> @@ +1553,5 @@
> > +	$(PYTHON_PATH) \
> > +	  -I$(topsrcdir)/other-licenses/ply \
> > +	  -I$(topsrcdir)/xpcom/idl-parser \
> > +	  -I$(topsrcdir)/xpcom/typelib/xpt/tools \
> > +	  $(topsrcdir)/xpcom/idl-parser/typelib.py --cachedir=$(topsrcdir)/xpcom/idl-parser $(XPIDL_FLAGS) $(_VPATH_SRCS) -d $(MDDEPDIR)/$(@F).pp -o $@
> 
> You should file a followup on having a single pass invocation that generates
> both headers and typelibs in one shot. (Also we could probably make the
> script handle multiple IDL files in one shot, which would be even nicer. It
> could cache the parse tree for things like nsISupports which would probably
> save some time.)

I think I'll just repurpose bug 330124.

> ::: xpcom/idl-parser/typelib.py
> @@ +69,5 @@
> > +    'cstring':            xpt.Type.Tags.CString,
> > +    'jsval':              xpt.Type.Tags.jsval
> > +}
> > +
> > +# XXXkhuey dipper types should go away
> 
> File a bug on this and list the bug number?

Bug 677784.

> @@ +73,5 @@
> > +# XXXkhuey dipper types should go away
> > +def isDipperType(type):
> > +    return type == xpt.Type.Tags.DOMString or type == xpt.Type.Tags.AString or type == xpt.Type.Tags.CString or type == xpt.Type.Tags.UTF8String
> > +
> > +def write_interface(iface, ifaces):
> 
> I'd call this something other than "write_interface", since it doesn't write
> anything, it just returns an xpt.Interface object. build_interface?
> populate_interface?

build_interface it is

> @@ +84,5 @@
> > +        if isinstance(type, xpidl.Builtin):
> > +            tag = TypeMap[type.name]
> > +            return xpt.SimpleType(tag,
> > +                                  pointer=(tag == xpt.Type.Tags.char_ptr or tag == xpt.Type.Tags.wchar_t_ptr),
> > +                                  #XXXkhuey unique_pointer is completely unused
> 
> File a bug on removing it and mention the bug number here. Also,
> unique_pointer defaults to False, so you don't have to specify it here at
> all.

Bug 677787.

> @@ +90,5 @@
> > +                                  reference=False)
> > +
> > +        if isinstance(type, xpidl.Array):
> > +            return xpt.ArrayType(get_type(type.type, calltype), size_is,
> > +                                 #XXXkhuey length_is is not actually used,
> 
> Should probably say "length_is duplicates size_is", and file a bug on
> removing it and mention the bug number here...

Bug 677788

> @@ +97,5 @@
> > +        if isinstance(type, xpidl.Interface) or isinstance(type, xpidl.Forward):
> > +            xptiface = None
> > +            for i in ifaces:
> > +                if i.name == type.name:
> > +                    xptiface = i
> 
> We should probably just build a dict of interface name -> interface and use
> that instead.

I chose not to do that because it we'd have to maintain the dict and the list.

> @@ +101,5 @@
> > +                    xptiface = i
> > +
> > +            if not xptiface:
> > +                xptiface = xpt.Interface(name=type.name)
> > +                ifaces.append(xptiface)
> 
> Hrm. If you have a forward declaration of an interface, used in an interface
> declaration, and then an actual declaration of the interface later in the
> same file, this would probably produce a malformed typelib. (You'd put an
> unresolved and a resolved interface descriptor in the same file.) I doubt
> that ever happens in practice, but we should probably try to avoid it.

It does happen in practice, and it's handled when XPTs get merged.

> @@ +126,5 @@
> > +                                      unique_pointer=False,
> > +                                      reference=False)
> > +
> > +        print type(type)
> > +        raise Exception("fooey")
> 
> Presumably you want this to say something more useful. :)

Ah, this was debugging code.  This should just be unreached now.

> @@ +148,5 @@
> > +        type = get_type(m.realtype, 'out')
> > +        if isDipperType(type.tag):
> > +            # NB: The retval bit needs to be set here, contrary to what the
> > +            # xpt spec says.
> > +            return xpt.Param(type, True, False, True, dipper=True)
> 
> You should probably pass all these args by keyword. I wrote that code and I
> have no idea what the args are here.

Heh, I thought the same when I was writing it.

> @@ +151,5 @@
> > +            # xpt spec says.
> > +            return xpt.Param(type, True, False, True, dipper=True)
> > +        return xpt.Param(type, False, True, True)
> > +
> > +    def get_attr_param(a, get=False, set=False):
> 
> You might want to name these "getter" and "setter" for consistency with
> other code.

Done.

> @@ +155,5 @@
> > +    def get_attr_param(a, get=False, set=False):
> > +        if not (get or set):
> > +            raise Exception("Attribute param must be for a getter or a setter!")
> > +
> > +        type = get_type(a.realtype, get == True and 'out' or 'in')
> 
> "get == True" is redundant, just say "get". Also, Python has a true ternary
> expression, you could write this as:
> 'out' if get else 'in'
> but some people don't like that syntax, so it's up to you.

I copied the style of the existing pyxpidl code here, and I think I'll leave it.  I've dropped the unnecessary comparison.

> @@ +171,5 @@
> > +
> > +    consts = []
> > +    methods = []
> > +
> > +    def write_const(c):
> 
> Again, "write" sounds like a misnomer.

Changed to build.

> @@ +178,5 @@
> > +    def write_method(m):
> > +        params = []
> > +
> > +        def get_param(p):
> > +            def findattr(p, attr):
> 
> This could be ...more usefully named, I think. "find_referenced_param" or
> something? A docstring comment wouldn't hurt either.

I've changed it to build_param to match the others.

> @@ +179,5 @@
> > +        params = []
> > +
> > +        def get_param(p):
> > +            def findattr(p, attr):
> > +                if hasattr(p, attr) and getattr(p, attr):
> 
> Why the "and getattr"? To avoid None values? Seems a little weird.

Yes.  That happens, no idea why.

> @@ +182,5 @@
> > +            def findattr(p, attr):
> > +                if hasattr(p, attr) and getattr(p, attr):
> > +                    for param in m.params:
> > +                        if param.name == getattr(p, attr):
> > +                            return m.params.index(param)
> 
> You could do:
> for i, param in enumerate(m.params):
> and just return i.

Neat

> @@ +206,5 @@
> > +            params.append(get_retval_param(m))
> > +
> > +        methods.append(xpt.Method(m.name, get_result_param(m), params, False,
> > +                                  False, m.notxpcom, False, m.noscript,
> > +                                  m.optional_argc, m.implicit_jscontext))
> 
> Again, can you use keyword args here, at least when you're just passing
> boolean literals?

Done.

> @@ +238,5 @@
> > +    parent = None
> > +    if iface.base:
> > +        for i in ifaces:
> > +            if i.name == iface.base:
> > +                parent = i
> 
> Really ought to be a dict lookup. Does idl.resolve not set these to the
> actual interface types for you? (If not we should probably make it do that.)
>
> @@ +241,5 @@
> > +            if i.name == iface.base:
> > +                parent = i
> > +        if not parent:
> > +            parent = xpt.Interface(name=iface.base)
> > +            ifaces.append(parent)
> 
> This has the same potential bug as I described earlier.

And our code handles it fine.

> @@ +245,5 @@
> > +            ifaces.append(parent)
> > +
> > +    return xpt.Interface(iface.name, iface.attributes.uuid, methods=methods,
> > +                         constants=consts, resolved=True, parent=parent,
> > +                         scriptable=not iface.attributes.noscript,
> 
> Shouldn't this technically be "iface.attributes.scriptable and not
> iface.attributes.noscript". I don't know why we support both of those on an
> interface, seems dumb.

Yes.  I didn't know we had both!

> @@ +262,5 @@
> > +    typelib = xpt.Typelib(interfaces=ifaces)
> > +    typelib.writefd(fd)
> > +
> > +if __name__ == '__main__':
> > +    from optparse import OptionParser
> 
> How about you stick this all in a "def main()" and just call main() here, in
> case we get an opportunity to ditch pythonpath.py and still want to use this
> as a native command?

Ok.

> @@ +285,5 @@
> > +        print >>sys.stderr, "-d requires -o"
> > +        sys.exit(1)
> > +
> > +    if options.outfile is not None:
> > +        outfd = open(options.outfile, 'wb')
> 
> Is there a reason you open the file here and not in write_typelib? You'd
> only have to pass down the filename if you did that.

To match header.py.

> @@ +286,5 @@
> > +        sys.exit(1)
> > +
> > +    if options.outfile is not None:
> > +        outfd = open(options.outfile, 'wb')
> > +        closeoutfd = True
> 
> Feels like you just want to "with open(outfile, 'wb') as outfd". (You'll
> need from __future__ import with_statement to use that reliably.)

Again, just copied from header.py.  I think we should probably merge them eventually.

> @@ +299,5 @@
> > +    if closeoutfd:
> > +        outfd.close()
> > +
> > +    if options.depfile is not None:
> > +        depfd = open(options.depfile, 'w')
> 
> with open(options.depfile, 'w') as depfd: (Also it's a little weird to name
> these variables "fd", since they're really file objects. You can get real
> file descriptors in Python with os.open.)
> 
> ::: xpcom/typelib/xpt/tools/xpt.py
> @@ +145,5 @@
> >          self.pointer = pointer
> >          self.unique_pointer = unique_pointer
> >          self.reference = reference
> > +        if reference and not pointer:
> > +            raise Exception("If reference is True pointer must be True too")
> 
> Good catch.
> 
> @@ +645,5 @@
> >          self.optargc = optargc
> >          self.implicit_jscontext = implicit_jscontext
> >          self.params = list(params)
> > +        if result and not isinstance(result, Param):
> > +            raise Exception("result must be a Param!")
> 
> Good catch here too. Want to add unit tests for these two cases to the test
> suite?

I'll file a followup.
http://hg.mozilla.org/mozilla-central/rev/1a9b3ace8c68
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
I don't know if it's the good place but I had 2 small problems/regressions  when I switch from the old generator (source : https://hg.mozilla.org/projects/webcl/)

First,You can't chaining typedef anymore:
typedef long                    T_WebCLEnum;
typedef T_WebCLEnum             T_WebCLAddressingMode; // -> raise Unknow type


and conversions are different (python inside) :
  const long MY_TEST = 0xFFFFFFFF; doesn't work anymore
-> must write const long MY_TEST = -1;

I write that here in case someone has same issues.
Please file bugs with test case IDL files and assign them to me.
It would be helpful if someone could put together some basic notes (at least) on how this is used. Right now, the old xpidl is documented here: https://developer.mozilla.org/en/XPIDL/xpidl
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: