[Lldb-commits] [PATCH] D68188: allow arbitrary python streams to be converted to SBFile

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna added a comment.

In D68188#1691129 <https://reviews.llvm.org/D68188#1691129>, @labath wrote:

> Hmm... I like your solution of the typemap problem, but this CRTP class seems to be way more complicated than needed. There shouldn't be any need for CRTP as we already have regular dynamic dispatch via virtual methods. Also, I don't think the forwarding should be needed if you're going the template route. How about something like this instead?
>
>   template<typename Base>
>   class OwningPythonFile : public Base {
>     Status Close() { /* close magic here*/; }
>     // no need to forward anything -- inheritance handles that
>   };
>  
>   class TextPythonFile: public OwningPythonFile<File> {
>     // override methods as usual
>   };
>   // same for BinaryPythonFile
>
>
> Then depending on what you need, you create either a NativeFile, OwningPythonFile<NativeFile>, or a Text/BinaryPythonFile. How does that sound?


Next step is add type maps to convert these things back to python.  Without the CRTP it can’t just check for a single OwnedPythonfile base class — it’ll have to check for all three.  Is that worse than using CRTP?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68188/new/

https://reviews.llvm.org/D68188