Bug 1545299 - Make UniquePtr work more like the other types with a base type. r=nika
authorAndrew McCreight <continuation@gmail.com>
Wed, 24 Apr 2019 20:19:49 +0000
changeset 530025 3fa69313f15951fef2c5f285b9301345432f511d
parent 530024 bae1f74ffaa3ae350e31c91ceb234a201d681cfe
child 530026 8441524b064b9703b12aec9b3242fa84a4d8293a
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1545299
milestone68.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 1545299 - Make UniquePtr work more like the other types with a base type. r=nika This consolidates array, maybe and unique ptrs in IPDL into a single "has base type" qualifier, for types that wrap another type. I'm not sure this patch fixes everything, but I think it is at least more correct. It also adds checking for the stuff inside the UniquePtr<>, because the intent seems to be to allow things like protocol types in there. Differential Revision: https://phabricator.services.mozilla.com/D28571
ipc/ipdl/ipdl/lower.py
ipc/ipdl/ipdl/type.py
ipc/ipdl/test/ipdl/error/PBadArrayBase.ipdl
ipc/ipdl/test/ipdl/error/PBadUniquePtrBase.ipdl
ipc/ipdl/test/ipdl/error/PUniquePtrRecursive.ipdl
ipc/ipdl/test/ipdl/error/PUniquePtrSelfRecStruct.ipdl
ipc/ipdl/test/ipdl/error/PUniquePtrSelfRecUnion.ipdl
ipc/ipdl/test/ipdl/ok/PUniquePtrBasic.ipdl
ipc/ipdl/test/ipdl/ok/PUniquePtrOfActors.ipdl
ipc/ipdl/test/ipdl/ok/PUniquePtrOfActorsSub.ipdl
ipc/ipdl/test/ipdl/ok/PUniquePtrRecUnion.ipdl
ipc/ipdl/test/ipdl/ok/PUniquePtrUnion.ipdl
--- a/ipc/ipdl/ipdl/lower.py
+++ b/ipc/ipdl/ipdl/lower.py
@@ -244,17 +244,17 @@ E.g., |Foo[]| --> |ArrayOfFoo|."""
 
 
 def _hasVisibleActor(ipdltype):
     """Return true iff a C++ decl of |ipdltype| would have an Actor* type.
 For example: |Actor[]| would turn into |Array<ActorParent*>|, so this
 function would return true for |Actor[]|."""
     return (ipdltype.isIPDL()
             and (ipdltype.isActor()
-                 or ((ipdltype.isArray() or ipdltype.isMaybe())
+                 or (ipdltype.hasBaseType()
                      and _hasVisibleActor(ipdltype.basetype))))
 
 
 def _abortIfFalse(cond, msg):
     return StmtExpr(ExprCall(
         ExprVar('MOZ_RELEASE_ASSERT'),
         [cond, ExprLiteral.String(msg)]))
 
@@ -552,17 +552,17 @@ def _cxxConstRefType(ipdltype, side):
     if ipdltype.isIPDL() and ipdltype.isActor():
         return t
     if ipdltype.isIPDL() and ipdltype.isShmem():
         t.ref = True
         return t
     if ipdltype.isIPDL() and ipdltype.isByteBuf():
         t.ref = True
         return t
-    if ipdltype.isIPDL() and (ipdltype.isArray() or ipdltype.isMaybe()):
+    if ipdltype.isIPDL() and ipdltype.hasBaseType():
         # Keep same constness as inner type.
         inner = _cxxConstRefType(ipdltype.basetype, side)
         t.const = inner.const or not inner.ref
         t.ref = True
         return t
     if ipdltype.isCxx() and ipdltype.isMoveonly():
         t.ref = True
         return t
@@ -598,17 +598,17 @@ def _cxxTypeNeedsMove(ipdltype):
 def _cxxTypeNeedsMoveForSend(ipdltype):
     if ipdltype.isUniquePtr():
         return True
 
     if ipdltype.isCxx():
         return ipdltype.isMoveonly()
 
     if ipdltype.isIPDL():
-        if ipdltype.isMaybe() or ipdltype.isArray():
+        if ipdltype.hasBaseType():
             return _cxxTypeNeedsMove(ipdltype.basetype)
         return (ipdltype.isShmem() or
                 ipdltype.isByteBuf() or
                 ipdltype.isEndpoint() or
                 ipdltype.isManagedEndpoint())
 
     return False
 
--- a/ipc/ipdl/ipdl/type.py
+++ b/ipc/ipdl/ipdl/type.py
@@ -65,16 +65,19 @@ class TypeVisitor:
             component.accept(self, *args)
 
     def visitArrayType(self, a, *args):
         a.basetype.accept(self, *args)
 
     def visitMaybeType(self, m, *args):
         m.basetype.accept(self, *args)
 
+    def visitUniquePtrType(self, m, *args):
+        m.basetype.accept(self, *args)
+
     def visitShmemType(self, s, *args):
         pass
 
     def visitByteBufType(self, s, *args):
         pass
 
     def visitShmemChmodType(self, c, *args):
         c.shmem.accept(self)
@@ -83,19 +86,16 @@ class TypeVisitor:
         pass
 
     def visitEndpointType(self, s, *args):
         pass
 
     def visitManagedEndpointType(self, s, *args):
         pass
 
-    def visitUniquePtrType(self, s, *args):
-        pass
-
 
 class Type:
     def __cmp__(self, o):
         return cmp(self.fullname(), o.fullname())
 
     def __eq__(self, o):
         return (self.__class__ == o.__class__
                 and self.fullname() == o.fullname())
@@ -220,16 +220,18 @@ class IPDLType(Type):
     def isAsync(self): return self.sendSemantics == ASYNC
 
     def isSync(self): return self.sendSemantics == SYNC
 
     def isInterrupt(self): return self.sendSemantics is INTR
 
     def hasReply(self): return (self.isSync() or self.isInterrupt())
 
+    def hasBaseType(self): return False
+
     @classmethod
     def convertsTo(cls, lesser, greater):
         if (lesser.nestedRange[0] < greater.nestedRange[0] or
                 lesser.nestedRange[1] > greater.nestedRange[1]):
             return False
 
         # Protocols that use intr semantics are not allowed to use
         # message nesting.
@@ -389,17 +391,17 @@ are in a cycle in the type graph rooted 
 looks for such a cycle and returns True if found.'''
         if exploring is None:
             exploring = set()
 
         if t.isAtom():
             return False
         elif t is self or t in self.mutualRec:
             return True
-        elif t.isArray() or t.isMaybe():
+        elif t.hasBaseType():
             isrec = self.mutuallyRecursiveWith(t.basetype, exploring)
             if isrec:
                 self.mutualRec.add(t)
             return isrec
         elif t in exploring:
             return False
 
         exploring.add(t)
@@ -449,29 +451,33 @@ class UnionType(_CompoundType):
 class ArrayType(IPDLType):
     def __init__(self, basetype):
         self.basetype = basetype
 
     def isAtom(self): return False
 
     def isArray(self): return True
 
+    def hasBaseType(self): return True
+
     def name(self): return self.basetype.name() + '[]'
 
     def fullname(self): return self.basetype.fullname() + '[]'
 
 
 class MaybeType(IPDLType):
     def __init__(self, basetype):
         self.basetype = basetype
 
     def isAtom(self): return False
 
     def isMaybe(self): return True
 
+    def hasBaseType(self): return True
+
     def name(self): return self.basetype.name() + '?'
 
     def fullname(self): return self.basetype.fullname() + '?'
 
 
 class ShmemType(IPDLType):
     def __init__(self, qname):
         self.qname = qname
@@ -532,40 +538,44 @@ class ManagedEndpointType(IPDLType):
 
     def name(self):
         return self.qname.baseid
 
     def fullname(self):
         return str(self.qname)
 
 
-class UniquePtrType(Type):
-    def __init__(self, innertype):
-        self.innertype = innertype
+class UniquePtrType(IPDLType):
+    def __init__(self, basetype):
+        self.basetype = basetype
+
+    def isAtom(self): return False
 
     def isUniquePtr(self): return True
 
+    def hasBaseType(self): return True
+
     def name(self):
-        return 'UniquePtr<' + self.innertype.fullname() + '>'
+        return 'UniquePtr<' + self.basetype.name() + '>'
 
     def fullname(self):
-        return 'mozilla::UniquePtr<' + self.innertype.fullname() + '>'
+        return 'mozilla::UniquePtr<' + self.basetype.fullname() + '>'
 
 
 def iteractortypes(t, visited=None):
     """Iterate over any actor(s) buried in |type|."""
     if visited is None:
         visited = set()
 
     # XXX |yield| semantics makes it hard to use TypeVisitor
     if not t.isIPDL():
         return
     elif t.isActor():
         yield t
-    elif t.isArray() or t.isMaybe():
+    elif t.hasBaseType():
         for actor in iteractortypes(t.basetype, visited):
             yield actor
     elif t.isCompound() and t not in visited:
         visited.add(t)
         for c in t.itercomponents():
             for actor in iteractortypes(c, visited):
                 yield actor
 
@@ -1162,17 +1172,17 @@ def fullyDefined(t, exploring=None):
   defined(struct f1 f2...)  := defined(f1) and defined(f2) and ...
   defined(union c1 c2 ...)  := defined(c1) or defined(c2) or ...
 '''
     if exploring is None:
         exploring = set()
 
     if t.isAtom():
         return True
-    elif t.isArray() or t.isMaybe():
+    elif t.hasBaseType():
         return fullyDefined(t.basetype, exploring)
     elif t.defined:
         return True
     assert t.isCompound()
 
     if t in exploring:
         return False
 
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/error/PBadArrayBase.ipdl
@@ -0,0 +1,7 @@
+//error: argument typename `X' of message `Test' has not been declared
+
+protocol PBadArrayBase {
+child:
+  async Test(X[] x);
+  async __delete__();
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/error/PBadUniquePtrBase.ipdl
@@ -0,0 +1,7 @@
+//error: argument typename `UndeclaredType' of message `Test' has not been declared
+
+protocol PBadUniquePtrBase {
+child:
+  async Test(UniquePtr<UndeclaredType> x);
+  async __delete__();
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/error/PUniquePtrRecursive.ipdl
@@ -0,0 +1,5 @@
+//error: bad syntax near `UniquePtr'
+
+protocol PUniquePtrRecursive {
+child: async Msg( UniquePtr< UniquePtr<int> > aa);
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/error/PUniquePtrSelfRecStruct.ipdl
@@ -0,0 +1,9 @@
+//error: struct `X' is only partially defined
+
+struct X {
+    UniquePtr<X> x;
+};
+
+protocol PUniquePtrSelfRecStruct {
+child: async Msg(UniquePtr<X> aa);
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/error/PUniquePtrSelfRecUnion.ipdl
@@ -0,0 +1,7 @@
+//error: union `X' is only partially defined
+
+union X { UniquePtr<X>; };
+
+protocol PUniquePtrSelfRecUnion {
+child: async Msg(UniquePtr<X> aa);
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/PUniquePtrBasic.ipdl
@@ -0,0 +1,4 @@
+protocol PUniquePtrBasic {
+child:
+    async Msg(UniquePtr<int> maybe);
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/PUniquePtrOfActors.ipdl
@@ -0,0 +1,10 @@
+include protocol PUniquePtrOfActorsSub;
+
+protocol PUniquePtrOfActors {
+    manages PUniquePtrOfActorsSub;
+
+child:
+    async Msg(UniquePtr<PUniquePtrOfActorsSub> p);
+
+    async PUniquePtrOfActorsSub();
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/PUniquePtrOfActorsSub.ipdl
@@ -0,0 +1,7 @@
+include protocol PUniquePtrOfActors;
+
+protocol PUniquePtrOfActorsSub {
+    manager PUniquePtrOfActors;
+
+child: async __delete__();
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/PUniquePtrRecUnion.ipdl
@@ -0,0 +1,21 @@
+union X {
+    int;
+    Y[];
+    UniquePtr<Y>;
+};
+
+union Y {
+    X;
+    Z;
+};
+
+union Z {
+    double;
+    X;
+};
+
+protocol PUniquePtrRecUnion {
+child:
+    async Test(X x, Y y, Z z);
+    async __delete__();
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/PUniquePtrUnion.ipdl
@@ -0,0 +1,10 @@
+union UniquePtrUnion {
+    int[];
+    int;
+    double;
+};
+
+sync protocol PUniquePtrUnion {
+parent:
+    async Msg(UniquePtrUnion u, UniquePtr<UniquePtrUnion> au) returns (UniquePtrUnion r);
+};