Backed out changeset 5beaad8a185b (bug 662341) for linting failure on a CLOSED TREE
authorAndreea Pavel <apavel@mozilla.com>
Thu, 15 Nov 2018 06:49:53 +0200
changeset 502907 5a4abcac5a0beffbc3f7705332afd1696d154db0
parent 502906 479f5e79077c06df25227e8628c29191c1e91ae6
child 502908 3a0b29de3cfb7c1abff8406f53aefb052c41b72c
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)
bugs662341
milestone65.0a1
backs out5beaad8a185bf4d01dd729d53e31429c8cdebf27
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
Backed out changeset 5beaad8a185b (bug 662341) for linting failure on a CLOSED TREE
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,96 +28,70 @@ else:
 def firstCap(str):
     return str[0].upper() + str[1:]
 
 
 def attributeParamName(a):
     return "a" + firstCap(a.name)
 
 
-def attributeParamNames(a, getter):
-    if getter and a.notxpcom:
-        l = []
-    else:
-        l = [attributeParamName(a)]
+def attributeParamNames(a):
+    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, getter, macro):
+def attributeReturnType(a, 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:
-        if macro == "NS_IMETHOD":
-            # This is the declaration.
-            ret = "virtual %s" % ret
+        ret = macro == "NS_IMETHOD" and "virtual nsresult" or "nsresult"
     else:
-        if ret == "nsresult":
-            ret = macro
-        else:
-            ret = "%s_(%s)" % (macro, ret)
-
+        ret = macro
     if a.must_use:
         ret = "MOZ_MUST_USE " + ret
     return ret
 
 
 def attributeParamlist(a, getter):
-    if getter and a.notxpcom:
-        l = []
-    else:
-        l = ["%s%s" % (a.realtype.nativeType(getter and 'out' or 'in'),
-                       attributeParamName(a))]
+    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, getter, declType),
+    params = {'returntype': attributeReturnType(a, 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.notxpcom:
-        ret = m.realtype.nativeType('in').strip()
+    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())
     else:
-        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)
-            
+        ret = macro
     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),
@@ -536,24 +510,23 @@ 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)))
-                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)})
+                fd.write(tmpl % {'asNative': attributeAsNative(member, True),
+                                 'nativeName': attributeNativeName(member, True),
+                                 'paramList': attributeParamNames(member)})
                 if not member.readonly:
-                    fd.write(attr_tmpl % {'asNative': attributeAsNative(member, False),
-                                          'nativeName': attributeNativeName(member, False),
-                                          'paramList': attributeParamNames(member, False)})
+                    fd.write(tmpl % {'asNative': attributeAsNative(member, False),
+                                     'nativeName': attributeNativeName(member, False),
+                                     'paramList': attributeParamNames(member)})
             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,31 +197,26 @@ 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
-        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,
+        param = mk_param(get_type(a.realtype, 'out'), out=1)
+        methods.append(mk_method(a.name, [param], getter=1, 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,
-                                     notxpcom=a.notxpcom, hidden=a.noscript,
+            methods.append(mk_method(a.name, [param], setter=1, 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 and attributes return their results directly by value. The x86
-# windows stdcall ABI returns aggregates by value differently for methods than
+# notxpcom methods 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 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.
+# 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.
 
 # 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,52 +115,41 @@ 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):
-    if getter and a.notxpcom:
-        l = []
-    else:
-        l = [(attributeParamName(a),
-              a.realtype.rustType('out' if getter else 'in'))]
+    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) -> %s" % \
+        return "pub %s: unsafe extern \"system\" fn (%s) -> ::nserror::nsresult" % \
             (attributeNativeName(m, getter),
-             attributeParamList(iface, m, getter),
-             attributeReturnType(m, getter))
+             attributeParamList(iface, 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):
@@ -259,23 +248,22 @@ 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'),
             }
 
-        param_list = attributeRawParamList(iface, m, getter)
-        params = ["%s: %s" % x for x in param_list]
+        rust_type = m.realtype.rustType('out' if getter else 'in')
         return method_impl_tmpl % {
             'name': attributeNativeName(m, getter),
-            'params': ', '.join(params),
-            'ret_ty': attributeReturnType(m, getter),
-            'args': '' if getter and m.notxpcom else name,
+            'params': name + ': ' + rust_type,
+            'ret_ty': '::nserror::nsresult',
+            'args': 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,20 +677,19 @@ 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' 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".
+            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".
                 #
                 # 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
 
@@ -988,17 +987,16 @@ 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
 
@@ -1019,18 +1017,16 @@ 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
@@ -1058,17 +1054,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 or self.notxpcom)
+        return not self.noscript
 
     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