I think we should try to get rid of default arguments in all virtual functions. I have started looking at removing as much as I can.
-- Anders On Mon, Sep 07, 2009 at 12:03:51PM +0200, Johan Hake wrote: > On Monday 07 September 2009 10:01:26 Garth N. Wells wrote: > > Ola Skavhaug wrote: > > > On Mon, Sep 7, 2009 at 9:21 AM, Ola Skavhaug<[email protected]> wrote: > > >> We've run into problems since only GenericMatrix provides the default > > >> argument for norm type. Swig does not like this and omits generating a > > >> constructor for the derived types. According to > > >> https://www.securecoding.cert.org/confluence/display/cplusplus/OBJ04-CPP > > >>.+Prefer+not+to+give+virtual+functions+default+argument+initializers > > >> virtual function should not provide default arguments at it tends to > > >> "defeat polymorphism": > > >> > > >> > > >> class Thing { > > >> public: > > >> virtual ~Thing(); > > >> virtual void doItNTimes( int numTimes = 10 ); > > >> virtual void doItThisHigh( double howHigh = 1.0 ); > > >> // ... > > >> }; > > >> class MyThing : public Thing { > > >> public: > > >> void doItNTimes( int numTimes ); > > >> void doItThisHigh( double howHigh = 2.2 ); > > >> // ... > > >> }; > > >> > > >> MyThing *mt = new MyThing; > > >> Thing *t = mt; > > >> t->doItNTimes(); // default of 10 > > >> mt->doItNTimes(); // compile time error! > > >> t->doItThisHigh(); // default of 1.0! > > >> mt->doItThisHigh(); // default of 2.2 > > >> > > >> > > >> Is this the intended behaviour anyhow? > > >> > > >> This is the proposed solution: > > > > > > Ouch, this was another, bad solution actually :) > > > > > > The proposed solution (from the link above) is roughly like this: > > > > > > class Base > > > { > > > public: > > > void foo(double bar = 1.0) { foo_impl(bar); } > > > protected: > > > virtual void foo_impl(double bar); > > > }; > > > > > > class Sub: public Base > > > { > > > protected: > > > virtual void foo_impl(double bar); > > > }; > > > > This is pretty ugly and I know that I'll forget about it within a day or > > two. I suggest abandoning default arguments for norms if that fixes the > > problem. We have a default argument for str(..), but I think that > > PyDOLFIN has its own mechanism for this. > > Yes, this is correct. This was probably added to get around this issue. We can > add workarounds for this case too, but it becomes quite cumbersome to do it > for each method. Doing it for a fundamental base class as Variable maybe makes > more sense? > > However note that following the report from Ola this will not work: > > PETScMatrix *pm = new PETScMatrix(); > ... > pm.src(); // Compile error > > where this would: > > PETScMatrix *pm = new PETScMatrix(); > ... > Variable * v = pm; > pm.str(); > > This has probably not turned up as a problem as we are using str() indirectly > through info(). > > I can remove the default argument from > > PETScMatrix::norm > > > Note that we have a related Blueprint, > > > > https://blueprints.launchpad.net/dolfin/+spec/default-parameters-cpp > > I tried to link to a discussion for this blueprint but couldn't do that. We > need to first register a bug on it to be able to connect it to a blueprint. > > Johan > > > Garth > > > > > Sorry about that... > > > > > > Ola > > > > > >> static double const minimumHeight = 1.0; > > >> // ... > > >> namespace XYZ { > > >> class Thing { > > >> public: > > >> virtual ~Thing(); > > >> virtual void doItThisHigh( double howHigh = minimumHeight ); > > >> // ... > > >> }; > > >> } > > >> // ... > > >> namespace XYZ { > > >> static double const minimumHeight = 2.2; > > >> class MyThing : public Thing { > > >> public: > > >> void doItThisHigh( double howHigh = minimumHeight ); > > >> // ... > > >> }; > > >> } > > >> > > >> > > >> Resolving this issue also means getting the Python interface back on > > >> track. > > >> > > >> Ola > > >> > > >> > > > > _______________________________________________ > > DOLFIN-dev mailing list > > [email protected] > > http://www.fenics.org/mailman/listinfo/dolfin-dev > > > _______________________________________________ > DOLFIN-dev mailing list > [email protected] > http://www.fenics.org/mailman/listinfo/dolfin-dev
signature.asc
Description: Digital signature
_______________________________________________ DOLFIN-dev mailing list [email protected] http://www.fenics.org/mailman/listinfo/dolfin-dev
