Bug 662341. Add support for [notxpcom] annotations on xpidl attributes. r=nika
☠☠ backed out by 5a4abcac5a0b ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 15 Nov 2018 04:13:36 +0000
changeset 502905 5beaad8a185bf4d01dd729d53e31429c8cdebf27
parent 502904 386ec4aee534e7e5b0c48b83f09c5f0bae8dcadb
child 502906 479f5e79077c06df25227e8628c29191c1e91ae6
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs662341
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 662341. Add support for [notxpcom] annotations on xpidl attributes. r=nika Differential Revision: https://phabricator.services.mozilla.com/D11796
xpcom/idl-parser/xpidl/header.py
xpcom/idl-parser/xpidl/jsonxpt.py
xpcom/idl-parser/xpidl/rust.py
xpcom/idl-parser/xpidl/xpidl.py
--- a/xpcom/idl-parser/xpidl/header.py
+++ b/xpcom/idl-parser/xpidl/header.py
@@ -28,70 +28,96 @@ else:
 def firstCap(str):
     return str[0].upper() + str[1:]
 
 
 def attributeParamName(a):
     return "a" + firstCap(a.name)
 
 
-def attributeParamNames(a):
-    l = [attributeParamName(a)]
+def attributeParamNames(a, getter):
+    if getter and a.notxpcom:
+        l = []
+    else:
+        l = [attributeParamName(a)]
     if a.implicit_jscontext:
         l.insert(0, "cx")
     return ", ".join(l)
 
 
 def attributeNativeName(a, getter):
     binaryname = a.binaryname is not None and a.binaryname or firstCap(a.name)
     return "%s%s" % (getter and 'Get' or 'Set', binaryname)
 
 
-def attributeReturnType(a, macro):
+def attributeReturnType(a, getter, macro):
     """macro should be NS_IMETHOD or NS_IMETHODIMP"""
+    # Pick the type to be returned from the getter/setter.
+    if a.notxpcom:
+        ret = a.realtype.nativeType('in').strip() if getter else "void"
+    else:
+        ret = "nsresult"
+
+    # Set calling convention and virtual-ness
     if a.nostdcall:
-        ret = macro == "NS_IMETHOD" and "virtual nsresult" or "nsresult"
+        if macro == "NS_IMETHOD":
+            # This is the declaration.
+            ret = "virtual %s" % ret
     else:
-        ret = macro
+        if ret == "nsresult":
+            ret = macro
+        else:
+            ret = "%s_(%s)" % (macro, ret)
+
     if a.must_use:
         ret = "MOZ_MUST_USE " + ret
     return ret
 
 
 def attributeParamlist(a, getter):
-    l = ["%s%s" % (a.realtype.nativeType(getter and 'out' or 'in'),
-                   attributeParamName(a))]
+    if getter and a.notxpcom:
+        l = []
+    else:
+        l = ["%s%s" % (a.realtype.nativeType(getter and 'out' or 'in'),
+                       attributeParamName(a))]
     if a.implicit_jscontext:
         l.insert(0, "JSContext* cx")
 
     return ", ".join(l)
 
 
 def attributeAsNative(a, getter, declType='NS_IMETHOD'):
-    params = {'returntype': attributeReturnType(a, declType),
+    params = {'returntype': attributeReturnType(a, getter, declType),
               'binaryname': attributeNativeName(a, getter),
               'paramlist': attributeParamlist(a, getter)}
     return "%(returntype)s %(binaryname)s(%(paramlist)s)" % params
 
 
 def methodNativeName(m):
     return m.binaryname is not None and m.binaryname or firstCap(m.name)
 
 
 def methodReturnType(m, macro):
     """macro should be NS_IMETHOD or NS_IMETHODIMP"""
-    if m.nostdcall and m.notxpcom:
-        ret = "%s%s" % (macro == "NS_IMETHOD" and "virtual " or "",
-                        m.realtype.nativeType('in').strip())
-    elif m.nostdcall:
-        ret = "%snsresult" % (macro == "NS_IMETHOD" and "virtual " or "")
-    elif m.notxpcom:
-        ret = "%s_(%s)" % (macro, m.realtype.nativeType('in').strip())
+    if m.notxpcom:
+        ret = m.realtype.nativeType('in').strip()
     else:
-        ret = macro
+        ret = "nsresult"
+
+    # Set calling convention and virtual-ness
+    if m.nostdcall:
+        if macro == "NS_IMETHOD":
+            # This is the declaration
+            ret = "virtual %s" % ret
+    else:
+        if ret == "nsresult":
+            ret = macro
+        else:
+            ret = "%s_(%s)" % (macro, ret)
+            
     if m.must_use:
         ret = "MOZ_MUST_USE " + ret
     return ret
 
 
 def methodAsNative(m, declType='NS_IMETHOD'):
     return "%s %s(%s)" % (methodReturnType(m, declType),
                           methodNativeName(m),
@@ -510,23 +536,24 @@ def write_interface(iface, fd):
     def emitTemplate(forward_infallible, tmpl, tmpl_notxpcom=None):
         if tmpl_notxpcom is None:
             tmpl_notxpcom = tmpl
         for member in iface.members:
             if isinstance(member, xpidl.Attribute):
                 if forward_infallible and member.infallible:
                     fd.write("\\\n  using %s::%s; " %
                              (iface.name, attributeNativeName(member, True)))
-                fd.write(tmpl % {'asNative': attributeAsNative(member, True),
-                                 'nativeName': attributeNativeName(member, True),
-                                 'paramList': attributeParamNames(member)})
+                attr_tmpl = tmpl_notxpcom if member.notxpcom else tmpl
+                fd.write(attr_tmpl % {'asNative': attributeAsNative(member, True),
+                                      'nativeName': attributeNativeName(member, True),
+                                      'paramList': attributeParamNames(member, True)})
                 if not member.readonly:
-                    fd.write(tmpl % {'asNative': attributeAsNative(member, False),
-                                     'nativeName': attributeNativeName(member, False),
-                                     'paramList': attributeParamNames(member)})
+                    fd.write(attr_tmpl % {'asNative': attributeAsNative(member, False),
+                                          'nativeName': attributeNativeName(member, False),
+                                          'paramList': attributeParamNames(member, False)})
             elif isinstance(member, xpidl.Method):
                 if member.notxpcom:
                     fd.write(tmpl_notxpcom % {'asNative': methodAsNative(member),
                                               'nativeName': methodNativeName(member),
                                               'paramList': paramlistNames(member)})
                 else:
                     fd.write(tmpl % {'asNative': methodAsNative(member),
                                      'nativeName': methodNativeName(member),
--- a/xpcom/idl-parser/xpidl/jsonxpt.py
+++ b/xpcom/idl-parser/xpidl/jsonxpt.py
@@ -197,26 +197,31 @@ def build_interface(iface):
             params.append(mk_param(get_type(m.realtype, 'out'), out=1))
 
         methods.append(mk_method(
             m.name, params, notxpcom=m.notxpcom, hidden=m.noscript,
             optargc=m.optional_argc, context=m.implicit_jscontext,
             hasretval=hasretval, symbol=m.symbol))
 
     def build_attr(a):
+        assert a.realtype.name != 'void'
         # Write the getter
-        param = mk_param(get_type(a.realtype, 'out'), out=1)
-        methods.append(mk_method(a.name, [param], getter=1, hidden=a.noscript,
+        getter_params = []
+        if not a.notxpcom:
+            getter_params.append(mk_param(get_type(a.realtype, 'out'), out=1))
+        methods.append(mk_method(a.name, getter_params, getter=1,
+                                 notxpcom=a.notxpcom, hidden=a.noscript,
                                  context=a.implicit_jscontext, hasretval=1,
                                  symbol=a.symbol))
 
         # And maybe the setter
         if not a.readonly:
             param = mk_param(get_type(a.realtype, 'in'), in_=1)
-            methods.append(mk_method(a.name, [param], setter=1, hidden=a.noscript,
+            methods.append(mk_method(a.name, [param], setter=1,
+                                     notxpcom=a.notxpcom, hidden=a.noscript,
                                      context=a.implicit_jscontext,
                                      symbol=a.symbol))
 
     for member in iface.members:
         if isinstance(member, xpidl.ConstMember):
             build_const(member)
         elif isinstance(member, xpidl.Attribute):
             build_attr(member)
--- a/xpcom/idl-parser/xpidl/rust.py
+++ b/xpcom/idl-parser/xpidl/rust.py
@@ -8,29 +8,29 @@
 
 # --- Safety Hazards ---
 
 # We currently don't generate some bindings for some IDL methods in rust code,
 # due to there being ABI safety hazards if we were to do so. This is the
 # documentation for the reasons why we don't generate certain types of bindings,
 # so that we don't accidentally start generating them in the future.
 
-# notxpcom methods return their results directly by value. The x86 windows
-# stdcall ABI returns aggregates by value differently for methods than
+# notxpcom methods and attributes return their results directly by value. The x86
+# windows stdcall ABI returns aggregates by value differently for methods than
 # functions, and rust only exposes the function ABI, so that's the one we're
-# using. The correct ABI can be emulated for notxpcom methods returning
-# aggregates by passing an &mut ReturnType parameter as the second parameter.
-# This strategy is used by the winapi-rs crate.
+# using. The correct ABI can be emulated for notxpcom methods returning aggregates
+# by passing an &mut ReturnType parameter as the second parameter.  This strategy
+# is used by the winapi-rs crate.
 # https://github.com/retep998/winapi-rs/blob/7338a5216a6a7abeefcc6bb1bc34381c81d3e247/src/macros.rs#L220-L231
 #
-# Right now we can generate code for notxpcom methods, as we don't support
-# passing aggregates by value over these APIs ever (the types which are allowed
-# in xpidl.py shouldn't include any aggregates), so the code is correct. In the
-# future if we want to start supporting returning aggregates by value, we will
-# need to use a workaround such as the one used by winapi.rs.
+# Right now we can generate code for notxpcom methods and attributes, as we don't
+# support passing aggregates by value over these APIs ever (the types which are
+# allowed in xpidl.py shouldn't include any aggregates), so the code is
+# correct. In the future if we want to start supporting returning aggregates by
+# value, we will need to use a workaround such as the one used by winapi.rs.
 
 # nostdcall methods on x86 windows will use the thiscall ABI, which is not
 # stable in rust right now, so we cannot generate bindings to them.
 
 # In general, passing C++ objects by value over the C ABI is not a good idea,
 # and when possible we should avoid doing so. We don't generate bindings for
 # these methods here currently.
 
@@ -115,41 +115,52 @@ def firstCap(str):
 
 
 # Attribute VTable Methods
 def attributeNativeName(a, getter):
     binaryname = rustSanitize(a.binaryname if a.binaryname else firstCap(a.name))
     return "%s%s" % ('Get' if getter else 'Set', binaryname)
 
 
+def attributeReturnType(a, getter):
+    if a.notxpcom:
+        if getter:
+            return a.realtype.rustType('in').strip()
+        return "::libc::c_void"
+    return "::nserror::nsresult"
+
 def attributeParamName(a):
     return "a" + firstCap(a.name)
 
 
 def attributeRawParamList(iface, a, getter):
-    l = [(attributeParamName(a),
-          a.realtype.rustType('out' if getter else 'in'))]
+    if getter and a.notxpcom:
+        l = []
+    else:
+        l = [(attributeParamName(a),
+              a.realtype.rustType('out' if getter else 'in'))]
     if a.implicit_jscontext:
         raise xpidl.RustNoncompat("jscontext is unsupported")
     if a.nostdcall:
         raise xpidl.RustNoncompat("nostdcall is unsupported")
     return l
 
 
 def attributeParamList(iface, a, getter):
     l = ["this: *const " + iface.name]
     l += ["%s: %s" % x for x in attributeRawParamList(iface, a, getter)]
     return ", ".join(l)
 
 
 def attrAsVTableEntry(iface, m, getter):
     try:
-        return "pub %s: unsafe extern \"system\" fn (%s) -> ::nserror::nsresult" % \
+        return "pub %s: unsafe extern \"system\" fn (%s) -> %s" % \
             (attributeNativeName(m, getter),
-             attributeParamList(iface, m, getter))
+             attributeParamList(iface, m, getter),
+             attributeReturnType(m, getter))
     except xpidl.RustNoncompat as reason:
         return """\
 /// Unable to generate binding because `%s`
 pub %s: *const ::libc::c_void""" % (reason, attributeNativeName(m, getter))
 
 
 # Method VTable generation functions
 def methodNativeName(m):
@@ -248,22 +259,23 @@ def attrAsWrapper(iface, m, getter):
 
         if getter and m.infallible and m.realtype.kind == 'builtin':
             # NOTE: We don't support non-builtin infallible getters in Rust code.
             return infallible_impl_tmpl % {
                 'name': attributeNativeName(m, getter),
                 'realtype': m.realtype.rustType('in'),
             }
 
-        rust_type = m.realtype.rustType('out' if getter else 'in')
+        param_list = attributeRawParamList(iface, m, getter)
+        params = ["%s: %s" % x for x in param_list]
         return method_impl_tmpl % {
             'name': attributeNativeName(m, getter),
-            'params': name + ': ' + rust_type,
-            'ret_ty': '::nserror::nsresult',
-            'args': name,
+            'params': ', '.join(params),
+            'ret_ty': attributeReturnType(m, getter),
+            'args': '' if getter and m.notxpcom else name,
         }
 
     except xpidl.RustNoncompat:
         # Dummy field for the doc comments to attach to.
         # Private so that it's not shown in rustdoc.
         return "const _%s: () = ();" % attributeNativeName(m, getter)
 
 
--- a/xpcom/idl-parser/xpidl/xpidl.py
+++ b/xpcom/idl-parser/xpidl/xpidl.py
@@ -677,19 +677,20 @@ class Interface(object):
         self.doccomments = doccomments
         self.nativename = name
         self.implicit_builtinclass = False
 
         for m in members:
             if not isinstance(m, CDATA):
                 self.namemap.set(m)
 
-            if m.kind == 'method' and m.notxpcom and name != 'nsISupports':
-                # An interface cannot be implemented by JS if it has a
-                # notxpcom method. Such a type is an "implicit builtinclass".
+            if ((m.kind == 'method' or m.kind == 'attribute') and
+                m.notxpcom and name != 'nsISupports'):
+                # An interface cannot be implemented by JS if it has a notxpcom
+                # method or attribute. Such a type is an "implicit builtinclass".
                 #
                 # XXX(nika): Why does nostdcall not imply builtinclass?
                 # It could screw up the shims as well...
                 self.implicit_builtinclass = True
 
     def __eq__(self, other):
         return self.name == other.name and self.location == other.location
 
@@ -987,16 +988,17 @@ class CEnum(object):
     def __str__(self):
         body = ', '.join('%s = %s' % v for v in self.variants)
         return "\tcenum %s : %d { %s };\n" % (self.name, self.width, body)
 
 
 class Attribute(object):
     kind = 'attribute'
     noscript = False
+    notxpcom = False
     readonly = False
     symbol = False
     implicit_jscontext = False
     nostdcall = False
     must_use = False
     binaryname = None
     infallible = False
 
@@ -1017,16 +1019,18 @@ class Attribute(object):
                 self.binaryname = value
                 continue
 
             if value is not None:
                 raise IDLError("Unexpected attribute value", aloc)
 
             if name == 'noscript':
                 self.noscript = True
+            elif name == 'notxpcom':
+                self.notxpcom = True
             elif name == 'symbol':
                 self.symbol = True
             elif name == 'implicit_jscontext':
                 self.implicit_jscontext = True
             elif name == 'nostdcall':
                 self.nostdcall = True
             elif name == 'must_use':
                 self.must_use = True
@@ -1054,17 +1058,17 @@ class Attribute(object):
     def toIDL(self):
         attribs = attlistToIDL(self.attlist)
         readonly = self.readonly and 'readonly ' or ''
         return "%s%sattribute %s %s;" % (attribs, readonly, self.type, self.name)
 
     def isScriptable(self):
         if not self.iface.attributes.scriptable:
             return False
-        return not self.noscript
+        return not (self.noscript or self.notxpcom)
 
     def __str__(self):
         return "\t%sattribute %s %s\n" % (self.readonly and 'readonly ' or '',
                                           self.type, self.name)
 
     def count(self):
         return self.readonly and 1 or 2