[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: dak
Subject: Re: Issue 4156: Define Smob<> constructors. (issue 152370043 by address@hidden)
Date: Thu, 09 Oct 2014 05:13:48 +0000

On 2014/10/08 23:48:16, Dan Eble wrote:
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
> 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
another.  To me, the presence of a comment signals intent.  (Not
arguing, just

Maybe you would prefer something like this:

That makes some sense in a library framework like Boost with its own
conventions.  But basically anything you have to look up is a reading
obstacle in an application.  The comment should be definite enough
that one does not bother looking.

> 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
however I respectfully disagree with your conclusion.  Generally
speaking, it is
contradictory to copy a derived object without copying the members
it inherits.

When the base class is solely responsible for managing per-copy
semantics, it is not "contradictory" to _not_ transfer it to a copy,
neither conceptually nor factually.

The copy constructor calls smobify_self () and that is called on a
smob not yet in use.  Initializing it from the source does not make

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.

Smob is a base class used for memory management.  Before templates, it
was common to use a base class for creating linked lists and other
data structures.  In that case, I'd consider it also contradictory to
copy the per-copy data by default just to overwrite it afterwards.
And with things like separate garbage collection threads doing
conservative stack and heap scanning (which we have in GUILEv2) we
don't _want_ an initialization with legit-looking material.

Your code did not actually copy any data, but one had to dig up the
respective definitions to make sure.

> 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.

Rietveld does not allow patches from more than one author in one
Rietveld review.  So that means if you want to work incrementally from
a proposed patch, you have the option of either making comments to the
patch, or of uploading a version of the patch in a review of your own.
The last upload "wins" in the Google code issue in that it is tested
and commented on by default.

In this particular case I uploaded a "competing" patch since
accommodating my propositions was a considerable amount of work and
I care enough about it that I would see the approaches compared on
discussed on their respective merits rather than have them ride on the
"but somebody would need to do it" theme.

For reviewing, that's worse than working on the same Rietveld review
since you cannot compare versions.  Incidentally, do you use git cl
for uploading?  Your reviews are remarkable in that they do not allow
comparing/viewing the various versions of your uploads.  That should
not usually happen so it would be interesting to know what causes this

So for single changes, it makes more sense to comment on the last
uploaded variant of a patch.  For sweeping changes or competing
proposals, it may make more sense to upload to a Rietveld issue you
own yourself.

When you don't do anything, usually the last uploaded patch eventually
makes it into countdown and code.  Unless the discussion around it
seems to indicate a lack of workable consensus among developers.

So in a nutshell: you don't _need_ to do anything more if you consider
the last proposed state acceptable.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]