Re: [eigen] private copy ctors

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]


2009/12/13 mmoll <Markus.Moll@xxxxxxxxxxxxxxxx>:
> Hi
>
> Quoting Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>:
>
>> Another argument pro the empty and private implementation is that
>> even in cases where we potentially need the assignment in the future,
>> we will have to implement it anyways manually since VS.net fails to
>> generate the code automatically and probably the resulting compiler
>> errors are nicer too.
>
> That's not a convincing argument. Doesn't it just make it more difficult
> to find out for which classes an assignment operator has already been
> defined? (no compiler errors (but linker errors), not as easily
> greppable...)

I agree that's not a convincing argument, for a simpler reason though:
if we didn't need it so far, we might easily never need it, and if we
do, that will likely be at only a few places.

>
> Furthermore, VS doesn't really fail to create an assignment operator. It
> just can't, and isn't even allowed to, create one in these
> circumstances. (Member-wise assignment is impossible to do in the
> presence of const members). GCC doesn't create one either (it simply
> doesn't warn).

Dang, I have to admit something stupid: I never bothered to look at
the specifics of that warning and at the forum thread, I was just
hoping silently that other people would take care of it, as it is
about MSVC!

Now here's what bothers me with this commit: it adds boilerplate code
everywhere, and it implicitly adds a new coding rule (that one should
add private assignment ops in such classes). This is a big concern, as
Eigen coding rules are already complex enough!

Looking at the specifics of that warning, it seems to be a stupid
warning, not a warning about something that's actually wrong in our
code. I wouldnt mind addressing it if it were just a few lines of
code, but now that I see how intrusive the change was... I hate to say
this so late, but it seems to me that the better solution is to add
this warning to DisableMSVCWarnings.h. Is that acceptable?

Benoit



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/