Re: [lmi] Non-null smart pointer

[Greg Chicares]

On 13/10/2018 22.54, Vadim Zeitlin wrote:
> On Sat, 13 Oct 2018 22:08:48 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Does lmi need exactly one kind of smart pointer, or more than one?
> 
>  Right now I can't point to any precise place where we need either
> shared_ptr<> or scoped_ptr<> or even unique_ptr<> which can be null. But
> this doesn't mean there any no such places.

We can't demonstrate that there are no such places now, or that none will
be introduced in the future. That's a problem.

> GC> Do all our smart pointers need to be returnable from a function,
> GC> or only some of them? Do any actually need reference counting?
> 
>  I can try to find the definitive answers to these questions, but it's
> going to take some time and, to be honest, I'm not sure why are they
> important. What matters, IMO, is that we want to have a never null owning
> smart pointer which can be returned from a function and that right now we
> don't have such a thing.

If we were to design such a thing now, before figuring out what we
actually need, we might later find that our real needs haven't been met.

> GC> If we sort all of lmi's smart pointers into buckets, I'm not
> GC> sure how many buckets we'd need, but I'm pretty sure that
> GC> the "no smart pointer needed at all" bucket is nonempty,
> 
>  Yes, all pointers used by/passed to wxWidgets, for example, would fall in
> this bucket. I've experimented with defining a gui_ptr<> template for such
> pointers in some of my other programs, but I'm not really sure if it really
> helps that much. In lmi it might be perhaps more useful because it could
> provide a perfectly-forwarding ctor that would always use the right ("wx")
> operator new for allocating the object. In any case, such gui_ptr<> is not
> really a smart pointer, even if it looks like one, because it doesn't do
> anything in its dtor.

That's a well-defined bucket, independent of lmi's present use of standard
smart pointers. Is gui_ptr<> small enough to show its implementation here?

> GC> and I imagine that the "reference counting needed" bucket is relatively
> GC> small if not quite empty.
> 
>  I guess so...

Our std::shared_ptr usage is dominated by a few cases:

- BasicValues members, which should just be const non-pointer members.

- Class Ledger: I suspect we could just make class IllusVal own any
  Ledger we use, and std::move() it thither from class AccountValue.

- Inside class Ledger, we have:
    std::shared_ptr<ledger_map_holder> ledger_map_;
    std::shared_ptr<LedgerInvariant>   ledger_invariant_;
  which, AFAICT, is some sort of trickery to avoid including the
  headers for class Ledger{Variant,Invariant}. I suspect we should
  purge the trickery and have non-pointer members instead:
    std::map<mcenum_run_basis,LedgerVariant> ledger_map;
    LedgerInvariant   ledger_invariant_;

- 'rate_table.?pp', where AIUI the purpose is to extend the lifetime
  of a std::istream. IIRC, this code was written for C++98 plus the
  old boost smart pointers; maybe now it doesn't need std::shared_ptr.

- Various old product-editor files: here, perhaps we just switch to
  unique_ptr and test whether that works. I'm not inclined to try
  changing the design beyond that.

As you recently remarked, std::unique_ptr suffices for the group
quote callback, and the same is true of the progress_meter callback.
(I've tested them both with such a change.)

Then we're left with a small handful of cases that might really have
a need for std::shared_ptr. Perhaps there's only one such case,
class cache_file_reads, where it seems that std::shared_ptr really
may be best.

>  So, what's the current state of discussion? Do we decide that we
> 
> 1. Want to use not_null<T*>? And, if so, do we want to take some existing
>    implementation or write our own version (which should be simple enough
>    but OTOH why reinvent even a simple wheel?)?
> 
> 2. Want to use not_null_unique_ptr<T*> which would be used for returning 
>    new objects from functions?
> 
> 3. Or need more time to review all the existing (smart) pointer usage in
>    lmi before making the decision?

The third. We have several problems:
 - Dereferencing pointers that might be null.
 - Using shared_ptr where unique_ptr would be better.
 - Using smart pointers where concrete references would be better.