Bug 1318110 - Correctly handle parse errors in inner IPDL parsers. r=billm draft
authorAndrew McCreight <continuation@gmail.com>
Thu, 17 Nov 2016 14:18:06 -0800
changeset 440632 7b469c5c5da628acc0e0599af6a8190b0fe5ed01
parent 439734 51750761f2c61c64cf0553f6cb5fefd4999d3bc0
child 537422 b135caf4670f80af9b1416905c8f0cdf53860997
push id36278
push userbmo:continuation@gmail.com
push dateThu, 17 Nov 2016 22:47:04 +0000
reviewersbillm
bugs1318110
milestone53.0a1
Bug 1318110 - Correctly handle parse errors in inner IPDL parsers. r=billm The IPDL parser handles include statements by using a stack of parsers. If an inner parser encounters a parsing error, it will print out an error message, which is maybe okay, but then it makes two mistakes: 1. It does not pop the current parser off of the parser stack. This means that the file that included the syntactically invalid file will be parsed as though it were the invalid file. In bkelly's case, an .ipdl file included an invalid .ipdlh file, so he got a bizarre error message about how you can't define a protocol in a header, because the parser was treating the protocol in the .ipdl file as though it were in the .ipdlh file. I fixed this by using a "finally" clause to pop the parser stack, ensuring that it is correct even in case of error. 2. A parse error in the include should cause the entire parse to fail, but instead it will keep going. inc.tu will get set to None, which eventually causes an error later in type checking, when it attempts to examine inc.tu. I fixed this by only catching the parse error where we invoke the outermost parser. This has the drawback that later errors in other files will not be reported. An alternate fix would set a global flag to indicate that a parse error had occured, and somehow report that to the caller. I think this bug was introduced in 2009 by commit cb8189926a69872c73508fba50830f0d07af341f. MozReview-Commit-ID: DhbDUO7MXGB
ipc/ipdl/ipdl/__init__.py
ipc/ipdl/ipdl/parser.py
--- a/ipc/ipdl/ipdl/__init__.py
+++ b/ipc/ipdl/ipdl/__init__.py
@@ -4,35 +4,39 @@
 
 __all__ = [ 'gencxx', 'genipdl', 'parse', 'typecheck', 'writeifmodified' ]
 
 import os, sys
 from cStringIO import StringIO
 
 from ipdl.cgen import IPDLCodeGen
 from ipdl.lower import LowerToCxx, msgenums
-from ipdl.parser import Parser
+from ipdl.parser import Parser, ParseError
 from ipdl.type import TypeCheck
 
 from ipdl.cxx.cgen import CxxCodeGen
 
 
 def parse(specstring, filename='/stdin', includedirs=[ ], errout=sys.stderr):
     '''Return an IPDL AST if parsing was successful.  Print errors to |errout|
     if it is not.'''
     # The file type and name are later enforced by the type checker.
     # This is just a hint to the parser.
     prefix, ext = os.path.splitext(filename)
     name = os.path.basename(prefix)
     if ext == '.ipdlh':
         type = 'header'
     else:
         type = 'protocol'
-    return Parser(type, name).parse(specstring, os.path.abspath(filename), includedirs, errout)
 
+    try:
+        return Parser(type, name).parse(specstring, os.path.abspath(filename), includedirs, errout)
+    except ParseError as p:
+        print >>errout, p
+        return None
 
 def typecheck(ast, errout=sys.stderr):
     '''Return True iff |ast| is well typed.  Print errors to |errout| if
     it is not.'''
     return TypeCheck().check(ast, errout)
 
 
 def gencxx(ipdlfilename, ast, outheadersdir, outcppdir):
--- a/ipc/ipdl/ipdl/parser.py
+++ b/ipc/ipdl/ipdl/parser.py
@@ -73,21 +73,19 @@ class Parser:
 
         Parser.parsed[filename] = self
         Parser.parseStack.append(Parser.current)
         Parser.current = self
 
         try:
             ast = self.parser.parse(input=input, lexer=self.lexer,
                                     debug=self.debug)
-        except ParseError, p:
-            print >>errout, p
-            return None
+        finally:
+            Parser.current = Parser.parseStack.pop()
 
-        Parser.current = Parser.parseStack.pop()
         return ast
 
     def resolveIncludePath(self, filepath):
         '''Return the absolute path from which the possibly partial
 |filepath| should be read, or |None| if |filepath| cannot be located.'''
         for incdir in self.includedirs +[ '' ]:
             realpath = os.path.join(incdir, filepath)
             if os.path.isfile(realpath):