]> hydra-www.ietfng.org Git - dyna2/commitdiff
Better error messages:
authortimv <tim.f.vieira@gmail.com>
Wed, 5 Jun 2013 22:14:56 +0000 (18:14 -0400)
committertimv <tim.f.vieira@gmail.com>
Wed, 5 Jun 2013 22:14:56 +0000 (18:14 -0400)
 - handle errors in rule initializers.

 - include rule source in error message

 - no need to run delete-updates when an item "was" causing an error because
   these updates don't land on the agenda.

REPL uses correct rule indexes

 - Parser state maintained across calls.

 - colon equals is no longer buggy in the REPL.

Codegen minor aesthetics

examples/errors.dyna
src/Dyna/Backend/Python/Backend.hs
src/Dyna/Backend/Python/interpreter.py
src/Dyna/Backend/Python/utils.py

index 3d06b012a9ed0bdce8acc9a393faa6effb6a030f..e44fa3b6be54b0742f74c09684d6c3535c20238a 100644 (file)
@@ -5,6 +5,11 @@ a += 1/b.
 
 c += "" + b.
 
+% initializer exceptions.
+%p += 1+null.
+%q max= 3/0.
+
+    b := b/0. a += b/0.
 
 d += null.
 d += 1.
index 26a4eb0257420c9276b4643ba13a88b5416d43e3..380919fbeca6fb9852ddee65777aa95cee30a2dd 100644 (file)
@@ -203,13 +203,13 @@ pdope_ (OPCheq v val) = return $ "if" <+> pretty v <+> "!="
 pdope_ (OPCkne v val) = return $ "if" <+> pretty v <+> "=="
                                       <+> pretty val <> ": continue"
 pdope_ (OPPeel vs i f) = return $
-    "try:" `above` (indent 4 $
+    --"try:" `above` (indent 4 $
            tupledOrUnderscore vs
             <+> equals
                 <+> "peel" <> (parens $ pfas f vs <> comma <> pretty i)
-     )
+    --)
     -- you'll get a "TypeError: 'NoneType' is not iterable."
-    `above` "except (TypeError, AssertionError): continue"
+    --`above` "except (TypeError, AssertionError): continue"
 pdope_ (OPWrap v vs f) = return $ pretty v
                            <+> equals
                            <+> "build"
@@ -268,7 +268,7 @@ printPlanHeader r c mn = do
 printInitializer :: Handle -> Rule -> Cost -> Actions PyDopeBS -> IO ()
 printInitializer fh rule@(Rule _ h _ r _ _ ucruxes _) cost dope = do
   displayIO fh $ renderPretty 1.0 100
-                 $ "@initializer" <> parens (uncurry pfa $ MA.fromJust $ findHeadFA h ucruxes)
+                 $ "@initializer" <> (uncurry pfa $ MA.fromJust $ findHeadFA h ucruxes)
                    `above` "def" <+> char '_' <> tupled ["emit"] <> colon
                    `above` (indent 4 $ printPlanHeader rule cost Nothing)
                    `above` pdope dope
@@ -278,7 +278,7 @@ printInitializer fh rule@(Rule _ h _ r _ _ ucruxes _) cost dope = do
 printUpdate :: Handle -> Rule -> Cost -> Int -> Maybe DFunctAr -> (DVar, DVar) -> Actions PyDopeBS -> IO ()
 printUpdate fh rule@(Rule _ h _ r _ _ _ _) cost evalix (Just (f,a)) (hv,v) dope = do
   displayIO fh $ renderPretty 1.0 100
-                 $ "@register" <> parens (pfa f a)
+                 $ "@register" <> (pfa f a)
                    `above` "def" <+> char '_' <> tupled (map pretty [hv,v,"emit"]) <> colon
                    `above` (indent 4 $ printPlanHeader rule cost (Just evalix))
                    `above` pdope dope
@@ -297,7 +297,7 @@ driver am um {-qm-} is pr fh = do
 
   -- Aggregation mapping
   forM_ (M.toList am) $ \((f,a),v) -> do
-     hPutStrLn fh $ show $    "agg_decl"
+     hPutStrLn fh $ show $    "_agg_decl"
                            <> brackets (dquotes $ pretty f <> "/" <> pretty a)
                            <+> equals <+> (dquotes $ pretty v)
 
index 16e630b56d6af10eef9a0da7195e317be17c537a..b941ecf79c9aad3fb068946d0b944901d28f8545 100644 (file)
@@ -1,9 +1,23 @@
 #!/usr/bin/env python
 
 """
+
+This error message is unhelpful
+
+    :-dispos_def dyna.
+    :-ruleix 27.
+    rewrite("VP", "V", "NP") -= 100.
+
+    FATAL: Encountered error in input program:
+     Parser error
+     /tmp/tmp.dyna:1:1: error: expected: end of input
+     :-dispos_def dyna.
+
 MISC
 ====
 
+ - TODO: filter and bulk loader
+
  - TODO: make sure interpreter uses the right exceptions. The codegen catches a
    few things -- I think assertionerror is one them... we should probably do
    whatever this is doing with a custom exception.
@@ -90,16 +104,16 @@ REPL
  - TODO: (Aggregator conflicts)
 
    We throw and AggregatorConflict exception if newly loaded code tries to
-   overwrite an aggregator in `agg_decl`.
+   overwrite an aggregator in `_agg_decl`.
 
    However, we need to make sure that we don't load subsequent code pertaining
    to this rule that we should reject altogether.
 
-   timv: At the moment I believe we're safe because agg_decl is set before any
-   of the registers (i.e. it's at the top of the generated code). This obviously
-   isn't the best way to do this, but we're going to have to overhaul this
-   entire infrastructure soon to handle rule-retraction.. So we can fix this
-   later.
+   timv: At the moment I believe we're safe because `_agg_decl` is set before
+   any of the registers (i.e. it's at the top of the generated code). This
+   obviously isn't the best way to do this, but we're going to have to overhaul
+   this entire infrastructure soon to handle rule-retraction.. So we can fix
+   this later.
 
 """
 
@@ -118,7 +132,19 @@ from defn import aggregator
 
 
 class AggregatorConflict(Exception):
-    pass
+    def __init__(self, key, expected, got):
+        msg = "Aggregator conflict %r was %r trying to set to %r." \
+            % (key, expected, got)
+        super(AggregatorConflict, self).__init__(msg)
+
+
+class DynaInitializerException(Exception):
+    def __init__(self, exception, init):
+        msg = '%r in ininitializer for rule\n  %s\n        %s' % \
+            (exception,
+             init.dyna_attrs['Span'],
+             init.dyna_attrs['rule'])
+        super(DynaInitializerException, self).__init__(msg)
 
 
 # TODO: as soon as we have safe names for these things we can get rid of this.
@@ -133,11 +159,10 @@ class chart_indirect(dict):
 class aggregator_declaration(object):
     def __init__(self):
         self.map = {}
-    def __setitem__(self, key, value):
-        if key in self.map and self.map[key] != value:
-            raise AggregatorConflict("Aggregator conflict %s was %r trying to "
-                                     "set to %r." % (key, self.map[key], value))
-        self.map[key] = value
+    def __setitem__(self, key, val):
+        if key in self.map and self.map[key] != val:
+            raise AggregatorConflict(key, self.map[key], val)
+        self.map[key] = val
     def __getitem__(self, key):
         try:
             return self.map[key]
@@ -145,14 +170,22 @@ class aggregator_declaration(object):
             return None
 
 
+# options
 error_suppression = True
 trace = None
-agg_decl = aggregator_declaration()
+
 agenda = prioritydict()
 chart = chart_indirect()
 errors = {}
 changed = {}
 
+# declarations
+_agg_decl = aggregator_declaration()
+_edges = defaultdict(set)
+_updaters = defaultdict(list)
+_initializers = []
+_rules = {}
+
 
 def dump_charts(out=sys.stdout):
     print >> out
@@ -213,14 +246,13 @@ class Term(object):
     __add__ = __sub__ = __mul__ = notimplemented
 
 
-_edges = defaultdict(set)
 def edges():
     def _emit(item, val, ruleix, variables):
         b = variables['nodes']
         b.sort()
         b = tuple(b)
         _edges[item].add((ruleix, b))
-    for init in initializer.handlers:
+    for init in _initializers:
         init(emit=_emit)
 
 
@@ -290,7 +322,7 @@ class Chart(object):
 
         self.intern[args] = term = Term(self.name, args)
         term.value = val
-        term.aggregator = aggregator(agg_decl[self.name])
+        term.aggregator = aggregator(_agg_decl[self.name])
 
         # indexes new term
         for i, x in enumerate(args):
@@ -319,45 +351,33 @@ def register(fn):
     Decorator for registering update handlers. Used by update dispatcher.
 
     Note: registration is with a global/mutable table.
-
-    For testing purposes clear current handlers
-    >>> register.handlers.clear()
-
-    >>> @register("test")
-    ... def _(H, V):
-    ...     return 'OK', H, V
-    ...
-    >>> [h] = register.handlers['test']
-    >>> h("head", "val")
-    ('OK', 'head', 'val')
-
     """
 
     def wrap(handler):
-        register.handlers[fn].append(handler)
+        handler.dyna_attrs = parse_attrs(handler)
+        _updaters[fn].append(handler)
         # you can't call these guys directly. Must go thru handler
         # indirection table
         return None
 
     return wrap
 
-register.handlers = defaultdict(list)
-
 
 # - "initializers" aren't just initializers -- They are fully-naive bottom-up
 #   inference routines. At the moment we only use them to initialize the chart.
 
+from utils import parse_attrs
+
 def initializer(_):
     "Implementation idea is very similar to register."
 
     def wrap(handler):
-        initializer.handlers.append(handler)
+        handler.dyna_attrs = parse_attrs(handler)
+        _initializers.append(handler)
         return None
 
     return wrap
 
-initializer.handlers = []
-
 
 def update_dispatcher(item, val, delete):
     """
@@ -367,7 +387,7 @@ def update_dispatcher(item, val, delete):
     if val is None:
         return
 
-    for handler in register.handlers[item.fn]:
+    for handler in _updaters[item.fn]:
 
         emittiers = []
         _emit = lambda item, val, ruleix, variables: \
@@ -383,8 +403,10 @@ def update_dispatcher(item, val, delete):
                 if item not in errors:
                     errors[item] = (val, [])
 
-                errors[item][1].append('%s\n        in rule %s' % \
-                                           (e, re.findall('Span:\s*(.*?)\n', handler.__doc__)[0]))
+                errors[item][1].append('%s\n        in rule %s\n            %s' % \
+                                           (e,
+                                            handler.dyna_attrs['Span'],
+                                            handler.dyna_attrs['rule']))
 
             else:
                 raise e
@@ -441,9 +463,6 @@ def _go():
         was = item.value
         print >> trace, '(was: %s,' % (was,),
 
-        if item in errors:    # clear the error
-            del errors[item]
-
         try:
             now = item.aggregator.fold()
         except (ZeroDivisionError, TypeError) as e:
@@ -457,8 +476,17 @@ def _go():
             print >> trace, yellow % 'unchanged'
             continue
 
+        was_error = False
+        if item in errors:    # clear the error
+            was_error = True
+            del errors[item]
+
         # TODO: handle `was` and `now` at the same time to avoid the two passes.
-        if was is not None:
+        if was is not None and not was_error:
+
+            # if `was` resulted in an error we know it didn't propagate so we
+            # can skip running the update dispatcher in delete mode.
+
             update_dispatcher(item, was, delete=True)
 
         item.value = now
@@ -488,6 +516,7 @@ def dynac_code(code, debug=False, run=True):
     out = '%s.plan.py' % dyna
 
     with file(dyna, 'wb') as f:
+        f.write(globals().get('parser_state', ''))  # include parser state if any.
         f.write(code)
 
     if dynac(dyna, out):   # stop if the compiler failed.
@@ -507,6 +536,9 @@ def load(f):
         print >> trace, magenta % 'Loading new code'
         print >> trace, yellow % h.read()
 
+    # TODO: loading new code should be atomic. if we fail for some reason we
+    # need to revert.
+
     # load generated code.
     execfile(f, globals())     # if we want to isolate side-effects of new code
                                # we can pass in something insead of globals()
@@ -524,27 +556,24 @@ def do(filename):
 
     assert os.path.exists(filename)
 
-    initializer.handlers = []    # XXX: do we really want to clear?
+    global _initializers
+    _initializers = []     # XXX: do we really want to clear?
 
     load(filename)
 
-    for init in initializer.handlers:   # assumes we have cleared
+    for init in _initializers:   # assumes we have cleared
 
-        _emit = partial(emit, delete=False)
+        def _emit(head, val, *args, **kw):
+            return emit(head, val, *args, delete=False, **kw)
 
         try:
             init(emit=_emit)
         except (TypeError, ZeroDivisionError) as e:
-            if error_suppression:
-                #print >> trace,
-                print e, 'in initializer.'
-            else:
-                raise e
+            raise DynaInitializerException(e, init)
 
     go()
 
 
-
 import cmd, readline
 
 class REPL(cmd.Cmd, object):
@@ -592,6 +621,7 @@ class REPL(cmd.Cmd, object):
         for x, v in sorted(changed.items()):
             print '%s := %r' % (x, v)
         print
+        dump_errors()
 
     def do_chart(self, args):
         if not args:
@@ -606,6 +636,7 @@ class REPL(cmd.Cmd, object):
             for f in args.split():
                 print chart[f]
                 print
+            dump_errors()
 
     def emptyline(self):
         """Do nothing on empty input line"""
index ad9cf09b6991e6081c43b9f9bde52e2f6733aa0e..5f2922f72bc844db55444a848685d6d645511bb1 100644 (file)
@@ -143,3 +143,39 @@ class prioritydict(dict):
 
     setdefault = None
     update = None
+
+
+def parse_attrs(fn):
+    attrs = dict(re.findall('\s*(\S+):\s*(.*)\s*\n', fn.__doc__.strip()))
+    if 'Span' in attrs:
+        attrs['rule'] = rule_source(attrs['Span']).strip()
+    return attrs
+
+
+def rule_source(span):
+    """
+    Utility for retrieving source code for Parsec error message.
+    """
+    [(filename, bl, bc, el, ec)] = re.findall(r'(.*):(\d+):(\d+)-\1:(\d+):(\d+)', span)
+    (bl, bc, el, ec) = map(int, [bl, bc, el, ec])
+
+    with file(filename) as f:
+        src = f.read()
+
+    lines = [l + '\n' for l in src.split('\n')]
+
+    rlines = lines[bl-1: el]
+
+    if len(rlines) > 1:
+        s = rlines[0][bc-1:]
+        m = rlines[1:-1]
+        e = rlines[-1][:ec]
+        return s + ''.join(m) + e
+
+    else:
+        [line] = rlines
+        return line[bc-1:ec]
+
+
+if __name__ == '__main__':
+    rule_source('examples/papa.dyna:4:1-examples/papa.dyna:4:47')