[bind10-dev] what to do for MSVC 4512 warnings?

[Francis Dupont]

> On Sun, Jul 01, 2012 at 09:18:56AM +0000, Francis Dupont wrote:
> > You can resolve the C4512 warning for this code in one of three ways:
> >  - Explicitly define an assignment operator for the class.
> 
> I think this is not exactly what we want. If we didn't have a
> problem with this yet, it means we don't want to assign to the
> class. Anyway, I think it would make little sense anyway, because,
> what would the operator do? The class is meant to be imutable.

=> it is not we don't want to assign instances of the class, just
the automatic assignment operator is not provided by the compiler
because of cont fields (BTW this is the correct behavior but
MSVC adds a warning...).

> >  - Remove const from the data item in the class.
> 
> That is probably a bad idea.

=> it is listed but was never a real candidate.

> > I tried the first but it is very painful (a lot of spurious changes
> > in far too many files), the second is not available, the third works
> > well (I added a '#pragma warning(disable: 4512)' at the end of config.h)
> > but IMHO it is a bad idea to disable a warning by laziness (BTW there
> > is no way to disable a warning by compiler config/argument as it is
> > done, including in case it should not, for Unix compilers).
>
> I'm not sure it is laziness. I don't see other reasonable
>  solution. I mean, such class does act little bit strangely, but if
>  it is the behaviour we want.

=> so this should be made more explicit.

> And it shouldn't cause any kind of runtime breakage, if anyone hits
> a problem with this, it would be a compile-time problem.

=> do you mean you rely on all compilers to raise an error when
someone will try to use a not defined and not auto-generated
assignment? With the trivial bugs I saw going through g++ without
notice I am afraid you ask too much.

> > I propose a fourth way, IMHO far better: to make all impacted
> > class (and struct) declarations derived from boost::noncopyable

=> in fact it is a bit too strong: we only need to protect the
assignment operator using the same style than for boost::noncopyable.

> That is, IMO, a bad idea that would probably not work in many
> cases. This does disallow a copy constructor as well (which works,
> even when there are the const members). That would make it not
> possible to return an object of the class = from any method, as the
> copy constructor is used on the return value.

=> so I change my proposal:
 - create a nonassignable include file which defines a class without
  assignment operator. Likely in src/lib/util but in isc:: name space.
 - derived all classes and structures (public as one can want to
  write the corresponding code) with a const field from this class.
 - remove the 4512 disable.

Regards

Francis Dupont <fdupont at isc.org>

PS: something like:

namespace isc {

namespace nonassignable_ {

class nonassignable {
protected:
 nonassignable() {}
 ~nonassignable() {}
private:
 const nonassignable& operator=(const nonassignable&);
};

};

typedef nonassignable_::nonassignable nonassignable;

}