Re: Issue 4156: Define Smob<> constructors. (issue 152370043 by address@hidden)

[nine . fierce . ballads]

On 2014/10/08 13:54:07, dak wrote:
> "Not defined" is quite definitely not a statement of intent.  Nor is
it of
> purpose.  It is of fact.  And it is quite confusing since it is
immediately
> adjacent to a declaration.

> It's more like
> // declared, do not define!  Prevents default copy constructor.

I assumed one level of familiarity with this convention, and you assumed
another.  To me, the presence of a comment signals intent.  (Not
arguing, just saying.)

Maybe you would prefer something like this:
http://www.boost.org/doc/libs/1_56_0/libs/core/doc/html/core/noncopyable.html

> Well, but those should not be using a Smob copy constructor but rather
the Smob
> default constructor because a Smob is not copyable.

I agree to the extent that copying is not part of a Smob's public
interface, however I respectfully disagree with your conclusion.
Generally speaking, it is contradictory to copy a derived object without
copying the members it inherits.  In the current implementation, the set
of inherited members worth copying is empty, but that should not be
reflected in the code of the derived class.

> I'll see whether I can wrap up a patch for that.

Please clarify what that means for my patch.  Is there anything to keep,
or should I abandon it?  Thanks.

https://codereview.appspot.com/152370043/