Bug 1336589 - Don't check struct decls from an included file in the context of the includee. r=billm
authorAndrew McCreight <continuation@gmail.com>
Wed, 08 Feb 2017 11:07:24 -0800
changeset 342250 6e0ab51985e41c41aa476bb01505cb0774468ce0
parent 342249 59c0ce731318d210b56b349f1762cf6aeba5c686
child 342251 eee8f9b6791deff3db5fb7a3423ed04fdc4e6725
push id31346
push userkwierso@gmail.com
push dateFri, 10 Feb 2017 22:33:24 +0000
treeherdermozilla-central@7b9d9e4a82a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1336589
milestone54.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 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;
+};