asmith marked 2 inline comments as done.
asmith added a comment.
I don't see anything else to address in this review. Comments?
================
Comment at:
lldb/source/Plugins/Process/Utility/RegisterContextWindows_x86_64.cpp:141
+GetRegisterInfo_WoW64(const lldb_private::ArchSpec &arch) {
+ // A WoW64 register info is the same as the i386's.
+ std::vector<lldb_private::RegisterInfo> &g_register_infos =
----------------
labath wrote:
> Hui wrote:
> > labath wrote:
> > > Why is all of this complexity necessary? Couldn't you just switch on the
> > > target architecture in `CreateRegisterInfoInterface` in
> > > NativeRegisterContextWindows_x86_64.cpp and create either
> > > `RegisterContextWindows_WoW64` or `RegisterContextWindows_x86_64` ?
> > >
> > > In fact, if RegisterContextWindows_WoW64 is identical to
> > > RegisterContextWindows_i386, then why do you even need the _WoW64 version
> > > of the class in the first place? FWIW, linux also does not have a
> > > RegisterContextLinux_32_bit_process_on_64_bit_kernel class...
> > I think WoW64 is i686 that shall deserve a separate arch specific
> > implementation?
> I am sorry, but I don't follow. Can you repeat the question?
>
> (FWIW, I don't believe that the fact that two things are different from a
> certain point of view justifies copy-pasting (at least) 150 LOC. If you think
> it's confusing to use something called "i386" in things dealing with WoW64,
> you can always typedef the WOW64 context to it, or rename the i386 context to
> something more generic.)
I don't want this to block the review and have removed the code.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63165/new/
https://reviews.llvm.org/D63165
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
