myhdl-list

[myhdl-list] Adding Error Messages to MyHDL source

[Christopher L. F. <chr...@gm...>]

Is there any general method for adding error messages?  In the MyHDL 
base there seems to be a couple different methods used?  Assertions, 
Exceptions, prints, etc.

I have added messages over time to my local copy.  Some of the messages 
might be useful to the users as well.  Maybe we could add a section on 
the wiki (maybe under open tasks) that outlines the best approach for 
adding error, warning, and informative messages.

Thanks
.chris

Re: [myhdl-list] Adding Error Messages to MyHDL source

[Christopher L. F. <chr...@gm...>]

Is there any general method for adding error messages?  In the MyHDL 
base there seems to be a couple different methods used?  Assertions, 
Exceptions, prints, etc.
>
> I have added messages over time to my local copy.  Some of the 
> messages might be useful to the users as well.  Maybe we could add a 
> section on the wiki (maybe under open tasks) that outlines the best 
> approach for adding error, warning, and informative messages.
>
> Thanks
> .chris

Re: [myhdl-list] Adding Error Messages to MyHDL source

[Jan D. <ja...@ja...>]

Christopher L. Felton wrote:
> Is there any general method for adding error messages?  In the MyHDL 
> base there seems to be a couple different methods used?  Assertions, 
> Exceptions, prints, etc.
> 
> I have added messages over time to my local copy.  Some of the messages 
> might be useful to the users as well.  Maybe we could add a section on 
> the wiki (maybe under open tasks) that outlines the best approach for 
> adding error, warning, and informative messages.

I often struggle with this and I have no golden rules.

Some observations:

I often use "naked" print statements for debugging. However, these
should never make it to a production release, if they do it's a bug.

assert statements and AssertionError exceptions are safety nets or
temporary placeholders. They mark something that either should never
happen (but I'm not sure), or something for which no unit test has
been written to check it. When they escape to a user, it should be
considered a bug to be fixed.

For warnings I sometimes try to use the warn module, but it's not
that user-friendly to use.

For errors I use exceptions, potentially accompanied by hopefully
informative error messages. (Sometimes I use the message string to
distinguish between idential exceptions in unit tests, e.g.
in the conversion stuff).

If applicable, I use prefined Python exceptions.
Otherwise, I define an exception per module or feature that
inherits from myhdl.Error. I'm reasonably satisfied with this
exception strategy. 

Jan

Re: [myhdl-list] Adding Error Messages to MyHDL source

[Ben <ben...@gm...>]

Hi there,

Thanks for reminding me that I had such a patch lying on my shelve also.

Here it is:

# HG changeset patch
# User Benoit Allard <be...@ae...>
# Date 1260897892 -3600
# Node ID 4a33c0707e2268147ec8cc68330b1002772dbad4
# Parent  1342b9d95452771835589c77af56d873b2704ac6
Improve Error messages

diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_always_comb.py
--- a/myhdl/_always_comb.py	Tue Dec 15 18:45:10 2009 +0100
+++ b/myhdl/_always_comb.py	Tue Dec 15 18:24:52 2009 +0100
@@ -37,7 +37,7 @@
 _error.ArgType = "always_comb argument should be a classic function"
 _error.NrOfArgs = "always_comb argument should be a function without arguments"
 _error.Scope = "always_comb argument should be a local function"
-_error.SignalAsInout = "signal used as inout in always_comb function argument"
+_error.SignalAsInout = "signal (%s) used as inout in always_comb
function argument"
 _error.EmbeddedFunction = "embedded functions in always_comb function
argument not supported"
 _error.EmptySensitivityList= "sensitivity list is empty"

@@ -56,8 +56,11 @@
     # handle free variables
     if func.func_code.co_freevars:
         for n, c in zip(func.func_code.co_freevars, func.func_closure):
-            obj = _cell_deref(c)
-            symdict[n] = obj
+            try:
+                obj = _cell_deref(c)
+                symdict[n] = obj
+            except NameError:
+                raise NameError(n)
     c = _AlwaysComb(func, symdict)
     return c

@@ -163,7 +166,7 @@
             self.visit(n)
         for n in inputs:
             if n in outputs:
-                raise AlwaysCombError(_error.SignalAsInout)
+                raise AlwaysCombError(_error.SignalAsInout % n)

     def visit_FunctionDef(self, node):
         if self.toplevel:
@@ -191,7 +194,7 @@
             elif self.context == OUTPUT:
                 self.outputs.add(id)
             elif self.context == INOUT:
-                raise AlwaysCombError(_error.SignalAsInout)
+                raise AlwaysCombError(_error.SignalAsInout % id)
             else:
                 raise AssertionError("bug in always_comb")

diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_concat.py
--- a/myhdl/_concat.py	Tue Dec 15 18:45:10 2009 +0100
+++ b/myhdl/_concat.py	Tue Dec 15 18:24:52 2009 +0100
@@ -63,7 +63,7 @@
             raise TypeError("concat: inappropriate argument type: %s" \
                             % type(arg))
         if not w:
-            raise TypeError, "concat: arg to concat should have length"
+            raise TypeError, "concat: arg  %d to concat should have
length" % arg
         width += w
         val = val*(2**w) + v

diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_extractHierarchy.py
--- a/myhdl/_extractHierarchy.py	Tue Dec 15 18:45:10 2009 +0100
+++ b/myhdl/_extractHierarchy.py	Tue Dec 15 18:24:52 2009 +0100
@@ -41,7 +41,7 @@
 class _error:
     pass
 _error.NoInstances = "No instances found"
-_error.InconsistentHierarchy = "Inconsistent hierarchy - are all
instances returned?"
+_error.InconsistentHierarchy = "Inconsistent hierarchy inside %s -
are all instances returned ?"


 class _Instance(object):
@@ -170,11 +170,11 @@
         names[id(obj)] = name
         absnames[id(obj)] = name
         if not top_inst.level == 1:
-                raise ExtractHierarchyError(_error.InconsistentHierarchy)
+            raise ExtractHierarchyError(_error.InconsistentHierarchy % name)
         for inst in hierarchy:
             obj, subs = inst.obj, inst.subs
             if id(obj) not in names:
-                raise ExtractHierarchyError(_error.InconsistentHierarchy)
+                raise
ExtractHierarchyError(_error.InconsistentHierarchy % inst.name)
             inst.name = names[id(obj)]
             tn = absnames[id(obj)]
             for sn, so in subs:
diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_intbv.py
--- a/myhdl/_intbv.py	Tue Dec 15 18:45:10 2009 +0100
+++ b/myhdl/_intbv.py	Tue Dec 15 18:24:52 2009 +0100
@@ -154,7 +154,11 @@
                 self._val &= ~(1L << i)
             self._checkBounds()
         elif isinstance(key, slice):
+            if val == None:
+                raise ValueError, "cannot attribute None to a slice"
             i, j = key.start, key.stop
+            if (self._val == None) and (i != None) and (j != None):
+                raise ValueError, "cannot slice value None"
             if j is None: # default
                 if i is None and self._val is None:
                     self._val = val
diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_traceSignals.py
--- a/myhdl/_traceSignals.py	Tue Dec 15 18:45:10 2009 +0100
+++ b/myhdl/_traceSignals.py	Tue Dec 15 18:24:52 2009 +0100
@@ -140,6 +140,8 @@
                 print >> f, "$upscope $end"
         print >> f, "$scope module %s $end" % name
         for n, s in sigdict.items():
+            if s._val == None:
+                raise ValueError("%s of module %s has no initial
value" % (n, name))
             if not s._tracing:
                 s._tracing = 1
                 s._code = namegen.next()
diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/test/core/test_always_comb.py
--- a/myhdl/test/core/test_always_comb.py	Tue Dec 15 18:45:10 2009 +0100
+++ b/myhdl/test/core/test_always_comb.py	Tue Dec 15 18:24:52 2009 +0100
@@ -136,7 +136,7 @@
         try:
             g = always_comb(h).gen
         except AlwaysCombError, e:
-            self.assertEqual(e.kind, _error.SignalAsInout)
+            self.assertEqual(e.kind, _error.SignalAsInout % "c")
         else:
             self.fail()

@@ -148,7 +148,7 @@
         try:
             g = always_comb(h).gen
         except AlwaysCombError, e:
-            self.assertEqual(e.kind, _error.SignalAsInout)
+            self.assertEqual(e.kind, _error.SignalAsInout % "c")
         else:
             self.fail()

diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/test/core/test_traceSignals.py
--- a/myhdl/test/core/test_traceSignals.py	Tue Dec 15 18:45:10 2009 +0100
+++ b/myhdl/test/core/test_traceSignals.py	Tue Dec 15 18:24:52 2009 +0100
@@ -122,7 +122,7 @@
         try:
             dut = traceSignals(dummy)
         except ExtractHierarchyError, e:
-            self.assertEqual(e.kind, _error.InconsistentHierarchy)
+            self.assertEqual(e.kind, _error.InconsistentHierarchy % "dummy")
         else:
             self.fail()

Re: [myhdl-list] Adding Error Messages to MyHDL source

[Jan D. <ja...@ja...>]

Thanks for using mercurial for patches!
I have applied an pushed it.

Note:

- It think the idiomatic way to check for None should
  use 'is', not '=='
- your patch reminds that I have to check whether it really
makes sense to support 'None' as an intbv value. I once thought
this was necessary to support 'X' and 'Z', but I now think
this is not true: we can use special signals for that and
leave the intbv simpler and more consistent. Now the 'None'
support looks clumsy and half-baked (my bad).

Jan


Ben wrote:
> Hi there,
> 
> Thanks for reminding me that I had such a patch lying on my shelve also.
> 
> Here it is:
> 
> # HG changeset patch
> # User Benoit Allard <be...@ae...>
> # Date 1260897892 -3600
> # Node ID 4a33c0707e2268147ec8cc68330b1002772dbad4
> # Parent  1342b9d95452771835589c77af56d873b2704ac6
> Improve Error messages
> 
> diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_always_comb.py
> --- a/myhdl/_always_comb.py	Tue Dec 15 18:45:10 2009 +0100
> +++ b/myhdl/_always_comb.py	Tue Dec 15 18:24:52 2009 +0100
> @@ -37,7 +37,7 @@
>  _error.ArgType = "always_comb argument should be a classic function"
>  _error.NrOfArgs = "always_comb argument should be a function without arguments"
>  _error.Scope = "always_comb argument should be a local function"
> -_error.SignalAsInout = "signal used as inout in always_comb function argument"
> +_error.SignalAsInout = "signal (%s) used as inout in always_comb
> function argument"
>  _error.EmbeddedFunction = "embedded functions in always_comb function
> argument not supported"
>  _error.EmptySensitivityList= "sensitivity list is empty"
> 
> @@ -56,8 +56,11 @@
>      # handle free variables
>      if func.func_code.co_freevars:
>          for n, c in zip(func.func_code.co_freevars, func.func_closure):
> -            obj = _cell_deref(c)
> -            symdict[n] = obj
> +            try:
> +                obj = _cell_deref(c)
> +                symdict[n] = obj
> +            except NameError:
> +                raise NameError(n)
>      c = _AlwaysComb(func, symdict)
>      return c
> 
> @@ -163,7 +166,7 @@
>              self.visit(n)
>          for n in inputs:
>              if n in outputs:
> -                raise AlwaysCombError(_error.SignalAsInout)
> +                raise AlwaysCombError(_error.SignalAsInout % n)
> 
>      def visit_FunctionDef(self, node):
>          if self.toplevel:
> @@ -191,7 +194,7 @@
>              elif self.context == OUTPUT:
>                  self.outputs.add(id)
>              elif self.context == INOUT:
> -                raise AlwaysCombError(_error.SignalAsInout)
> +                raise AlwaysCombError(_error.SignalAsInout % id)
>              else:
>                  raise AssertionError("bug in always_comb")
> 
> diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_concat.py
> --- a/myhdl/_concat.py	Tue Dec 15 18:45:10 2009 +0100
> +++ b/myhdl/_concat.py	Tue Dec 15 18:24:52 2009 +0100
> @@ -63,7 +63,7 @@
>              raise TypeError("concat: inappropriate argument type: %s" \
>                              % type(arg))
>          if not w:
> -            raise TypeError, "concat: arg to concat should have length"
> +            raise TypeError, "concat: arg  %d to concat should have
> length" % arg
>          width += w
>          val = val*(2**w) + v
> 
> diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_extractHierarchy.py
> --- a/myhdl/_extractHierarchy.py	Tue Dec 15 18:45:10 2009 +0100
> +++ b/myhdl/_extractHierarchy.py	Tue Dec 15 18:24:52 2009 +0100
> @@ -41,7 +41,7 @@
>  class _error:
>      pass
>  _error.NoInstances = "No instances found"
> -_error.InconsistentHierarchy = "Inconsistent hierarchy - are all
> instances returned?"
> +_error.InconsistentHierarchy = "Inconsistent hierarchy inside %s -
> are all instances returned ?"
> 
> 
>  class _Instance(object):
> @@ -170,11 +170,11 @@
>          names[id(obj)] = name
>          absnames[id(obj)] = name
>          if not top_inst.level == 1:
> -                raise ExtractHierarchyError(_error.InconsistentHierarchy)
> +            raise ExtractHierarchyError(_error.InconsistentHierarchy % name)
>          for inst in hierarchy:
>              obj, subs = inst.obj, inst.subs
>              if id(obj) not in names:
> -                raise ExtractHierarchyError(_error.InconsistentHierarchy)
> +                raise
> ExtractHierarchyError(_error.InconsistentHierarchy % inst.name)
>              inst.name = names[id(obj)]
>              tn = absnames[id(obj)]
>              for sn, so in subs:
> diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_intbv.py
> --- a/myhdl/_intbv.py	Tue Dec 15 18:45:10 2009 +0100
> +++ b/myhdl/_intbv.py	Tue Dec 15 18:24:52 2009 +0100
> @@ -154,7 +154,11 @@
>                  self._val &= ~(1L << i)
>              self._checkBounds()
>          elif isinstance(key, slice):
> +            if val == None:
> +                raise ValueError, "cannot attribute None to a slice"
>              i, j = key.start, key.stop
> +            if (self._val == None) and (i != None) and (j != None):
> +                raise ValueError, "cannot slice value None"
>              if j is None: # default
>                  if i is None and self._val is None:
>                      self._val = val
> diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/_traceSignals.py
> --- a/myhdl/_traceSignals.py	Tue Dec 15 18:45:10 2009 +0100
> +++ b/myhdl/_traceSignals.py	Tue Dec 15 18:24:52 2009 +0100
> @@ -140,6 +140,8 @@
>                  print >> f, "$upscope $end"
>          print >> f, "$scope module %s $end" % name
>          for n, s in sigdict.items():
> +            if s._val == None:
> +                raise ValueError("%s of module %s has no initial
> value" % (n, name))
>              if not s._tracing:
>                  s._tracing = 1
>                  s._code = namegen.next()
> diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/test/core/test_always_comb.py
> --- a/myhdl/test/core/test_always_comb.py	Tue Dec 15 18:45:10 2009 +0100
> +++ b/myhdl/test/core/test_always_comb.py	Tue Dec 15 18:24:52 2009 +0100
> @@ -136,7 +136,7 @@
>          try:
>              g = always_comb(h).gen
>          except AlwaysCombError, e:
> -            self.assertEqual(e.kind, _error.SignalAsInout)
> +            self.assertEqual(e.kind, _error.SignalAsInout % "c")
>          else:
>              self.fail()
> 
> @@ -148,7 +148,7 @@
>          try:
>              g = always_comb(h).gen
>          except AlwaysCombError, e:
> -            self.assertEqual(e.kind, _error.SignalAsInout)
> +            self.assertEqual(e.kind, _error.SignalAsInout % "c")
>          else:
>              self.fail()
> 
> diff -r 1342b9d95452 -r 4a33c0707e22 myhdl/test/core/test_traceSignals.py
> --- a/myhdl/test/core/test_traceSignals.py	Tue Dec 15 18:45:10 2009 +0100
> +++ b/myhdl/test/core/test_traceSignals.py	Tue Dec 15 18:24:52 2009 +0100
> @@ -122,7 +122,7 @@
>          try:
>              dut = traceSignals(dummy)
>          except ExtractHierarchyError, e:
> -            self.assertEqual(e.kind, _error.InconsistentHierarchy)
> +            self.assertEqual(e.kind, _error.InconsistentHierarchy % "dummy")
>          else:
>              self.fail()
> 
> 
> ------------------------------------------------------------------------
> 
> ------------------------------------------------------------------------------
> This SF.Net email is sponsored by the Verizon Developer Community
> Take advantage of Verizon's best-in-class app development support
> A streamlined, 14 day to market process makes app distribution fast and easy
> Join now and get one step closer to millions of Verizon customers
> http://p.sf.net/sfu/verizon-dev2dev 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> myhdl-list mailing list
> myh...@li...
> https://lists.sourceforge.net/lists/listinfo/myhdl-list


-- 
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
    Python as a HDL: http://www.myhdl.org
    VHDL development, the modern way: http://www.sigasi.com
    Analog design automation: http://www.mephisto-da.com
    World-class digital design: http://www.easics.com

Re: [myhdl-list] Adding Error Messages to MyHDL source

[Ben <ben...@gm...>]

On Thu, Dec 17, 2009 at 16:20, Jan Decaluwe <ja...@ja...> wrote:
> Thanks for using mercurial for patches!
> I have applied an pushed it.

Thanks, thank you for using Mercurial as VCS !

>
> Note:
>
> - It think the idiomatic way to check for None should
>  use 'is', not '=='

Make sense, my bad, want a patch that correct it ?

> - your patch reminds that I have to check whether it really
> makes sense to support 'None' as an intbv value. I once thought
> this was necessary to support 'X' and 'Z', but I now think
> this is not true: we can use special signals for that and
> leave the intbv simpler and more consistent. Now the 'None'
> support looks clumsy and half-baked (my bad).

The main trouble is Signal declaration and the initialisation that
comes with it, that taken apart, I don't use None as signal value. and
that's anyway being reported by the traceVCD step. So no trouble for
me to remove anything None related in the intbv type.

Regards,
Benoit

Re: [myhdl-list] Adding Error Messages to MyHDL source

[Jan D. <ja...@ja...>]

Ben wrote:
> On Thu, Dec 17, 2009 at 16:20, Jan Decaluwe <ja...@ja...> wrote:
>> Thanks for using mercurial for patches!
>> I have applied an pushed it.
> 
> Thanks, thank you for using Mercurial as VCS !
> 
>> Note:
>>
>> - It think the idiomatic way to check for None should
>>  use 'is', not '=='
> 
> Make sense, my bad, want a patch that correct it ?
> 
>> - your patch reminds that I have to check whether it really
>> makes sense to support 'None' as an intbv value. I once thought
>> this was necessary to support 'X' and 'Z', but I now think
>> this is not true: we can use special signals for that and
>> leave the intbv simpler and more consistent. Now the 'None'
>> support looks clumsy and half-baked (my bad).
> 
> The main trouble is Signal declaration and the initialisation that
> comes with it, that taken apart, I don't use None as signal value. and
> that's anyway being reported by the traceVCD step. So no trouble for
> me to remove anything None related in the intbv type.

The whole question of tristate modelling is now handled better
with dedicated special signals. So I'm indeed inclined to redo
intbv and remove the "None" value related stuff alltogether.

-- 
Jan Decaluwe - Resources bvba - http://www.jandecaluwe.com
    Python as a HDL: http://www.myhdl.org
    VHDL development, the modern way: http://www.sigasi.com
    Analog design automation: http://www.mephisto-da.com
    World-class digital design: http://www.easics.com