[PATCH] D51525: [ADT] Add a new class OwnedOrBorrowed<T>

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

So this goes back to the PDB reading code.  IIRC you helped me design the interface of that library, so maybe you still remember some of it.  But here's a refresher.

The idea is to provide a platform-agnostic interface over DIA SDK.  Then we have one concrete implementation of this interface that is implemented via DIA, and we have another concrete implementation that we implement natively.  Furthermore, the way DIA works is that there is really only one main interface, called `IDiaSymbol`, that provides a monolithic interface to every possible operation for every possible symbol type, but a large portion of the methods simply return an error if you try to use them on the wrong symbol type.

So our solution was to provide two class hierarchies. The first is rooted at `IPDBRawSymbol` and provides this same monolithic interface.  Then we have, for example, `DIARawSymbol` which forwards calls to DIA, and wraps the results in a class that hides the implementation so there is no user dependency on DIA.  Then we could provide a native implementation that works the same way.

The second hierarchy is what the user actually operates on.  This hierarchy is rooted at `PDBSymbol` and there are concrete implementations for the different types of symbols.  There's `PDBSymbolFunc`, `PDBSymbolTypeEnum`, and many others.  The purpose of these is to expose only the set of the monolithic interface that makes sense for a given symbol type.  But there is **not** a parallel hierarchy for each implementation (DIA/Native).  There is just one hierarchy, rooted at `PDBSymbol`, and this `PDBSymbol` holds some kind of polymorphic instance of an `IPDBRawSymbol` so that it can forward calls.

To draw it out, it's something like this:

  IPDBRawSymbol
  |_ NativeRawSymbol
     |+ class member: NativePDBFile
  |_ DIARawSymbol
     |+ class member: CComPtr<IDiaSymbol>
  
  PDBSymbol
  |+ class member: unique_ptr<IPDBRawSymbol>
  |_ PDBSymbolFunc
  |_ PDBSymbolExe
  |_ PDBSymbolTypeEnum
  |_ PDBSymbolFunctionSignature
  |_ etc

Now all of the methods of `PDBSymbol` and subclasses return `unique_ptr<T>` where `T` is some class from the second hierarchy.  So if you say "give me the function signature of this function", you're going to get back a `unique_ptr<PDBSymbolFunctionSignature>`.  When it's destroyed, the class member of type `unique_ptr<IPDBRawSymbol>` will get destroyed, and in the case of DIA this means the `CComPtr<IDiaSymbol>` will get released and properly cleaned up.  All is good.

In the native implementation, I want to cache the `NativeRawSymbol` instances though.  It's expensive to create them, because it is actually required by the API in a way that they be "stable".  What I mean by this is that each symbol has a method called `getUniqueId()`.  In DIA, if you have an object, call `getUniqueId()`, then destroy the object, then call `getSymbolById(n)` where `n` is the previous id, you will get back the same object again.  Not the same pointer value obviously, since it was destroyed, but the COM interface is really just an opaque wrapper around something that *is* cached, and that's what you're getting back.  We need to do the same thing (both for efficiency and correctness).

So, when someone calls, for example, `getAllCompilands`, we return an enumerator for the compilands, and as you enumerate them, a unique id is assigned to each compiland returned.  The next time someone calls `getAllCompilands`, the enumerator should not assign new unique ids for any compilands which have already been enumerated.  It should simply return items from the cache.

so herein lies the problem, and the reason i want this class.  I want to change `PDBSymbol` to hold a `MaybeOwned<IPDBRawSymbol>`.  On the native implementation side, I can construct instances of them with references to the cached objects, and on the DIA side, I can construct instances of them with `unique_ptr<DIARawSymbol>`.

FWIW, if we don't think this is general purpose enough, I can just hack up the implementation of `PDBSymbol` to do this directly in the class.  This just seemed like a cleaner solution to me.


https://reviews.llvm.org/D51525