Bug 1336589 - Don't check struct decls from an included file in the context of the includee. r=billm draft
authorAndrew McCreight <continuation@gmail.com>
Wed, 08 Feb 2017 11:07:24 -0800
changeset 480698 1852753a1879a172e7cac410cb35597b1e5be3fb
parent 480619 5e17f9181c6cb0968966280d1c1d96e725702af1
child 480699 ef145592225f7a5b99cdc54f2c0eda6fa2687baa
push id44634
push userbmo:continuation@gmail.com
push dateWed, 08 Feb 2017 20:41:52 +0000
reviewersbillm
bugs1336589
milestone54.0a1
Bug 1336589 - Don't check struct decls from an included file in the context of the includee. r=billm See MutRecHeader1.ipdlh for a more detailed explanation. MozReview-Commit-ID: JHYd7qKSjrr
ipc/ipdl/ipdl/type.py
ipc/ipdl/test/ipdl/ok/MutRecHeader1.ipdlh
ipc/ipdl/test/ipdl/ok/MutRecHeader2.ipdlh
ipc/ipdl/test/ipdl/ok/MutRecHeader3.ipdlh
--- a/ipc/ipdl/ipdl/type.py
+++ b/ipc/ipdl/ipdl/type.py
@@ -617,20 +617,16 @@ class GatherDecls(TcheckVisitor):
         # first pass to "forward-declare" all structs and unions in
         # order to support recursive definitions
         for su in tu.structsAndUnions:
             self.declareStructOrUnion(su)
 
         # second pass to check each definition
         for su in tu.structsAndUnions:
             su.accept(self)
-        for inc in tu.includes:
-            if inc.tu.filetype == 'header':
-                for su in inc.tu.structsAndUnions:
-                    su.accept(self)
 
         if tu.protocol:
             # grab symbols in the protocol itself
             p.accept(self)
 
         self.symtab = savedSymtab
 
     def declareStructOrUnion(self, su):
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/MutRecHeader1.ipdlh
@@ -0,0 +1,34 @@
+include MutRecHeader2;
+
+/* MutRecHeader1 (H1) includes MutRecHeader2 (H2), and uses a struct from H2.
+   H2 includes MutRecHeader3 (H3).
+   H3 includes H1.
+
+When type checking H1, GatherDecls::visitInclude will recursively
+cause us to first check H2, which in turn will cause us to first check
+H3.
+
+H3 only includes H1, so when we check it, we do not have any
+declarations from H2 in the context. There used to be code in
+GatherDecls::visitTranslationUnit that would, as part of the "second
+pass", check the validity of all included structures. This would check
+Struct1, and fail, because Struct2 is not declared.
+
+Fundamentally, it doesn't make sense to check anything declared in an
+included file in the context of the file that included it.
+
+Note that this error did not show up when either H2 or H3 was
+checked. This is because in those cases we are not in the middle of
+checking H1 when we check H3, so we end up fully checking H1 before we
+get to the end of checking H3. This means the "visited" tag gets put
+on Struct1 before we get to the end of that troublesome block of code
+in visitTranslationUnit, and visitStructDecl doesn't do anything if
+that tag is set, so we don't end up actually checking H1 in the
+context of H3.
+
+*/
+
+struct Struct1
+{
+  Struct2 b;
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/MutRecHeader2.ipdlh
@@ -0,0 +1,8 @@
+include MutRecHeader3;
+
+// See MutRecHeader1.ipdlh for explanation.
+
+struct Struct2
+{
+  bool b;
+};
new file mode 100644
--- /dev/null
+++ b/ipc/ipdl/test/ipdl/ok/MutRecHeader3.ipdlh
@@ -0,0 +1,8 @@
+include MutRecHeader1;
+
+// See MutRecHeader1.ipdlh for explanation.
+
+struct Struct3
+{
+  bool b;
+};