dolfin team mailing list archive

[Anders Logg <logg@xxxxxxxxx>]

On Thu, Apr 03, 2008 at 10:21:04PM +0200, kent-and@xxxxxxxxx wrote:
> > 2008/4/3, kent-and@xxxxxxxxx <kent-and@xxxxxxxxx>:
> >> > 2008/4/3, Anders Logg <logg@xxxxxxxxx>:
> >>  >> On Thu, Apr 03, 2008 at 07:47:13PM +0200, Martin Sandve Alnæs wrote:
> >>  >>  > We only talked about create() the other day, copy() can still be
> >>  >>  > useful! Or does the factory duplicate this functionality?
> >>  >>
> >>  >>
> >>  >> No, but Kent has added an assignment operator:
> >>  >>
> >>  >>   GenericVector* y = x.factory().createVector();
> >>  >>   *y = x;
> >>  >
> >>  > Ok, but the assignment operator doesn't exist in Python, so some
> >>  > variant of the copy function may still be needed. Could be just a
> >>  > wrapper for the two lines above.
> >>
> >>
> >>
> >> PyDolfin has assign:
> >>  dolfin_function_pre.i:%rename(assign) dolfin::Function::operator=;
> >>
> >>
> >>
> >>
> >>  >
> >>  > But the current default implementation of operator= (just return
> >>  > *this;) isn't good, it should be abstract or at least raise an error
> >>  > somehow.
> >>  >
> >>
> >>
> >> It did not compile when it was defined as an abstract function. But I
> >>  agree completely. You probably have a fix, I didn't manage.
> >
> > I didn't know virtual operators were allowed at all in C++, but simply
> > calling dolfin_error in there is one solution (I do this in Python
> > some times to "simulate" pure virtual functions).
> >
> 
> Then I get a warning:
> 
> cc1plus: warnings being treated as errors
> ./dolfin/la/GenericVector.h: In member function ‘virtual const
> dolfin::GenericVector& dolfin::GenericVector::operator=(const
> dolfin::GenericVector&)’:
> ./dolfin/la/GenericVector.h:111: warning: no return statement in function
> returning non-void
> scons: *** [dolfin/elements/ProjectionLibrary.os] Error 1
> scons: building terminated because of errors.

Normally, one can trick the compiler by doing something like

  return *(new GenericVector()); // code will never be reached

but this won't work here since GenericVector is not a concrete type so
it's not possible to instantiate it. And there are no other subclasses
to use since they should not be known to GenericVector...

-- 
Anders


> 
> 
> 
> >
> >>  > And I the uBlas implementation seems potentially dangerous too,
> >>  >
> >>  > const uBlasVector& uBlasVector::operator= (const GenericVector& x_)
> >>  > {
> >>  >   const uBlasVector* x = dynamic_cast<const uBlasVector*>(&x_);
> >>  >   if (!x) error("The vector should be of type PETScVector");
> >>  >
> >>  >   *this = (*x)*1.0;
> >>  >   return *this;
> >>  > }
> >>  >
> >>  > In particular, this line
> >>  >   *this = (*x)*1.0;
> >>  > depends on (*x)*1.0 being resolved by the ublas_vector subclass,
> >>  > triggering operator= in ublas_vector. Implementing operator* in
> >>  > GenericVector will break this and possibly make an infinite
> >> recursion.
> >>  > Unless I misunderstand something? (I don't know ublas).
> >>  >
> >>
> >>
> >> Please help here as well. The reason it looks like this is that I don't
> >>  know ublas.
> >>
> >>  Kent
> >
> > Plus the use of multiple inheritance... Maybe uBlasVector should be
> > changed to own an ublas_vector instead of being one? If operators are
> > added to GenericVector, multiple inheritance will be messy.
> 
> But GenericVector is abstract so I guess the operators should basically
> be wrappers on top the concrete vectors like the ublas vector.
> 
> However,  I agree that things are getting a bit messy.
> 
> Kent
> 
> 
> _______________________________________________
> DOLFIN-dev mailing list
> DOLFIN-dev@xxxxxxxxxx
> http://www.fenics.org/mailman/listinfo/dolfin-dev