← Back to team overview

dolfin team mailing list archive

Re: [HG DOLFIN] Remove functions copy() and create() (use factory instead)

 

2008/4/3, kent-and@xxxxxxxxx <kent-and@xxxxxxxxx>:
> > 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:
>  > 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.

You don't have to delete the return statement.

Another solution is to implement virtual operators in GenericVector
but make them call abstract functions in the interface, which can be
reimplemented in subclasses.


>  >>  > 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

Having a virtual operator= like you did is useful everywhere in C++ code
that only has GenericVector &. That code wouldn't otherwise have access
to f.ex. the uBlasVector::operator=.

Depends on what we want GenericVector to do.

--
Martin


References