[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting when reading memory in Scripted Process

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added a comment.

In D134033#3933936 <https://reviews.llvm.org/D134033#3933936>, @labath wrote:

> I kinda like it. One thing that I think would help with the readability is if the "transformation" methods were grouped according to the type of the object being transformed, rather than according to the direction. So something like:
>
>   // general transform
>   // general reverse
>   // Status transform
>   // Status reverse
>
> instead of
>
>   // general transform
>   // Status transform
>   // general reverse
>   // Status reverse
>
> Also I don't think that these structs (`transformation`, `reverse_transformation`) wrapping the transformation functions are really necessary. It's true that one cannot partially specialize functions, but one of the reasons for that is this is normally not necessary -- regular function overloading <https://godbolt.org/z/114KeeK3f> can do most of that as well.

Thanks for the feedback! I really appreciate it :) For now I reordered the structs as you suggested but I'm thinking of moving all the transformation related code to an anonymous namespace above the class. I'll do that in a follow-up patch :)



================
Comment at: lldb/include/lldb/API/SBError.h:95
 private:
-  std::unique_ptr<lldb_private::Status> m_opaque_up;
+  std::shared_ptr<lldb_private::Status> m_opaque_sp;
 
----------------
labath wrote:
> This is technically an ABI break (changes `sizeof(SBError)`). I don't care, but someone might.
Luckily, I didn't have to change this.


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

https://reviews.llvm.org/D134033