| Re: [eigen] private copy ctors |
[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]
2009/12/13 mmoll <Markus.Moll@xxxxxxxxxxxxxxxx>:
> HiI agree that's not a convincing argument, for a simpler reason though:
>
> 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...)
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.
Dang, I have to admit something stupid: I never bothered to look at
>
> 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).
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/ |