dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Tue, Mar 25, 2008 at 01:38:22PM +0100, Martin Sandve Alnæs wrote:
> 2008/3/25, Anders Logg <logg@xxxxxxxxx>:
> > On Tue, Mar 25, 2008 at 01:18:39PM +0100, Martin Sandve Alnæs wrote:
> >  > 2008/3/25, Anders Logg <logg@xxxxxxxxx>:
> >  > >  There are quite a few of these already (but they are missing in many
> >  > >  places). For example, here is PETScMatrix::get:
> >  > >
> >  > >   void PETScMatrix::get(real* block,
> >  > >                         uint m, const uint* rows,
> >  > >                         uint n, const uint* cols) const
> >  > >   {
> >  > >     dolfin_assert(A);
> >  > >     MatGetValues(A,
> >  > >                  static_cast<int>(m),
> >  > >                  reinterpret_cast<int*>(const_cast<uint*>(rows)),
> >  > >                  static_cast<int>(n),
> >  > >                  reinterpret_cast<int*>(const_cast<uint*>(cols)),
> >  > >                  block);
> >  > >   }
> >  > >
> >  > >  Is this what you have in mind?
> >  >
> >  > Looks good. Will dolfin_assert result in an exception or just kill the
> >  > application? An informative error message and stack trace is nice to
> >  > have.
> >
> >
> > It will throw a runtime error. (I think Ola or you fixed this.)
> >  Check in dolfin/log/log.h where dolfin_assert is defined. It will
> >  ultimately call Logger::__assert:
> >
> >  void Logger::__assert(std::string msg)
> >  {
> >   std::string s = std::string("*** Assertion ") + msg;
> >   throw std::runtime_error(s);
> >
> > }
> >
> >  > Will the code be removed in release builds or not?
> >
> >
> > Depends on how the builds are made, but dolfin_assert() is only active
> >  when DEBUG is defined:
> >
> >  #ifdef DEBUG
> >  #define dolfin_assert(check) do { if ( !(check) )
> >  #dolfin::__dolfin_assert(__FILE__, __LINE__, __FUNCTION__, "(" #check ")"); } while (false)
> >  #else
> >  #define dolfin_assert(check)
> >  #endif
> >
> >
> 
> Looks good. But a lot of error checks / assertions should always be
> kept though, there's no reason to remove them where they don't impact
> performance. Maybe there should be a release version of dolfin_assert
> with a different name, dolfin_assert_release or dolfin_check or
> something?

I think we can use dolfin::error for that. We could do something like
this (which is what we've tried to do in the past but perhaps we've
been a little sloppy):

1. Use dolfin_assert() for things that should *never* go wrong,
meaning that it should only be called when something is wrong in
the library code (not user code).

2. Use dolfin::error when a user might do something wrong.

This is difficult to enforce strictly, especially for operations that
should be fast (like vector element access), but we can try.

So for all non-performance-critical operations, we should have an if
check and then error("Informative message, like matrix not
initialized").

?

> But what's the point of the "do { ... } while(false);" ??
> { ... } or just ... should be the same thing as far as I can see.

I don't know. It might be a leftover from old times when this was
needed by some compiler. Remove it and see what happens. Tell me if it
works and then we can remove it.

-- 
Anders