[PATCH] D85774: [XCOFF][AIX] Enable tooling support for 64 bit symbol table parsing
Jason Liu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 9 13:04:56 PDT 2020
jasonliu added a comment.
In D85774#2217649 <https://reviews.llvm.org/D85774#2217649>, @jhenderson wrote:
> It seems to me like this should be using inheritance here. You have a base class that has the common members, and provides pure virtual declarations of the various getters, with the sub-classes defining them to do the right thing. Yes, it would introduce a number of getters, but I feel like it would make everything a bit cleaner from a usability standpoint. In most cases, you then don't need any `is64Bit` queries, because the getters hide that from you.
@jhenderson I tried to use inheritance as suggested. But inheritance would mean I need to use pointers to enable the runtime polymorphism. Then there is a life time issue that need to be managed when using pointers. The easier way to achieve that is to return via unique_ptr. But using unique_ptr introduced usability issue in the caller/user side, as we would see std::move, SymbolRef.get() before getting to the query we want. Also underneath of the unique_ptr, new/delete is not very efficient as well.
In the end, I tried to solve this in similar manner as COFF does, which is using a view class without inheritance. Although the downside of it is we basically have an if query in every call to the view class to differentiate which version (32/64) we are having right now, the good thing is that caller side is much more cleaner.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85774/new/
https://reviews.llvm.org/D85774
More information about the llvm-commits
mailing list