Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make array_view::operator= non-const #3813

Merged
merged 1 commit into from Nov 19, 2014

Conversation

mdboom
Copy link
Member

@mdboom mdboom commented Nov 18, 2014

Here's another C++ pedantry issue.

The operator= in array_view returned a const reference. I think this means that when the compiler needs a non-const reference it is free to use an auto-generated operator=, which of course would be wrong because it doesn't do the reference counting.

This is purely theoretical, of course, since I haven't seen a problem related to this, but I think this change makes things more correct.

@WeatherGod
Copy link
Member

Would be funny if this is the underlying reason that agg changed all of
their functions.

That being said, back in my C++ days, I was a heavy user of const&. gcc
would always catch me when I would show even the slightest sign of calling
a non-const method of a const& object. I am doubtful that gcc would
autogenerate a non-const version of the copy constructor, but "explicit is
better than implicit", right?

On Tue, Nov 18, 2014 at 2:41 PM, Michael Droettboom <
notifications@github.com> wrote:

Here's another C++ pedantry issue.

The operator= in array_view returned a const reference. I think this
means that when the compiler needs a non-const reference it is free to use
an auto-generated operator=, which of course would be wrong because it
doesn't do the reference counting.

This is purely theoretical, of course, since I haven't seen a problem

related to this, but I think this change makes things more correct.

You can merge this Pull Request by running

git pull https://github.com/mdboom/matplotlib non-const-operator=

Or view, comment on, or merge it at:

#3813
Commit Summary

  • Make array_view::operator= non-const

File Changes

Patch Links:


Reply to this email directly or view it on GitHub
#3813.

@jenshnielsen
Copy link
Member

Which compilers do we support? With C++11 you can explicitly delete default constructors https://en.wikipedia.org/wiki/C%2B%2B11#Explicitly_defaulted_and_deleted_special_member_functions
which could be useful in cases like this.

@jenshnielsen
Copy link
Member

If nothing else it could be used to debug if the default constructor is initialised.

@ianthomas23
Copy link
Member

A C++ compiler will only autogenerate a default assignment operator if one has not been explicitly declared. Hence the existing code is fine. If the assignment operator returns a const reference and a non-const reference is required, the code will not compile.

Having said that, I have no objections to the change.

mdboom added a commit that referenced this pull request Nov 19, 2014
Make array_view::operator= non-const
@mdboom mdboom merged commit 20425e2 into matplotlib:master Nov 19, 2014
@mdboom mdboom deleted the non-const-operator= branch March 3, 2015 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants