[PATCH 1 of 5] rust-cpython: put leaked reference in PyLeakedRef

[Martin von Zweigbergk]

On Sun, Oct 13, 2019 at 8:52 PM Yuya Nishihara <yuya at tcha.org> wrote:

> On Sun, 13 Oct 2019 07:46:14 -0700, Martin von Zweigbergk wrote:
> > On Sat, Oct 12, 2019, 01:06 Yuya Nishihara <yuya at tcha.org> wrote:
> > > # HG changeset patch
> > > # User Yuya Nishihara <yuya at tcha.org>
> > > # Date 1568552779 -32400
> > > #      Sun Sep 15 22:06:19 2019 +0900
> > > # Node ID 458c6598a13caee640294d88af4e93783fc36476
> > > # Parent  ce20b870041fc4d6ba6989acbb9373797ce9b3d6
> > > rust-cpython: put leaked reference in PyLeakedRef
>
> > > -    /// Returns a leaked reference and its management object.
> > > +    /// Returns a leaked reference.
> > >      ///
> > >      /// # Safety
> > >      ///
> > >      /// It's up to you to make sure that the management object lives
> > >      /// longer than the leaked reference. Otherwise, you'll get a
> > >      /// dangling reference.
> > >
> >
> > This seems inaccurate, or at least unclear, now.
>
> Yeah, it could be rephrased as "Returns a leaked reference temporarily
> held by its management object." I don't care much about the document
> accuracy of intermediate patches.
>

Sorry, I had not realized the comment was changed in a later patch.


>
> > > @@ -318,12 +319,13 @@ macro_rules! py_shared_ref {
> > >  ///
> > >  /// In truth, this does not represent leaked references themselves;
> > >  /// it is instead useful alongside them to manage them.
> >
> > So does this.
>
> This one is still true in this patch. PyLeakedRef doesn't provide any
> smart-pointer-like functions yet.
>

Oh, I guess I misunderstood what this comment was about. I had thought that
storing the reference in the management object (as we do after this patch)
would count as "represent".

I'll queue this mostly based on Georges' review, thanks.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20191013/18221107/attachment.html>