dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Tue, Mar 25, 2008 at 11:55:35AM +0100, Martin Sandve Alnæs wrote:
> A seriously bad side effect of replacing proper constructors with
> init(...) in the GenericTensor interfaces is that uninitialized
> vectors are inherently unsafe. It's unacceptable that all code that
> uses a Vector must check if it's initialized or potentially crash. A
> high level library is not any more user friendly than a low level
> library if it doesn't provide proper error messages wherever possible
> (on the contrary some might say), and therefore I consider all
> segmentation faults in the library to be critical bugs.

Yes, definitely!

> I see only one way we can avoid segmentation faults when it is
> possible to construct uninitialized objects, and that is placing
> initialization assertions (if/throw) at the top of every single member
> function in every single class in the GenericTensor hierarchy. For
> performance critical operations, this can be placed in #ifdef DEBUG
> guards, but for other operations it should be left in release code.

Sounds good.

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?

> We should also _require_ that all GenericTensor subclasses implement a
> proper set of constructors, in particular a copy constructor and
> constructor taking a sparsity pattern (this can't be enforced by the
> compiler):
> 
> explicit Foo::Foo(const Foo & foo) { ... }
> explicit Foo::Foo(const FooSparsityPattern & foo) { ... }
> 
> ("explicit" disables the potential use of a constructor with a single
> argument in assigments)

Sounds good to me.

-- 
Anders