JDevlieghere added inline comments.
================ Comment at: lldb/source/Target/Platform.cpp:1252 + if (selected_platform_sp) { + if (selected_platform_sp->IsCompatibleArchitecture( + arch, process_host_arch, false, nullptr)) { ---------------- JDevlieghere wrote: > jingham wrote: > > jingham wrote: > > > Why are you passing process_host_arch here? This is the > > > "selected_platform" so you have no way of knowing a priori that this is > > > the host platform or has the same architecture as the host system. In > > > the old version, this selected platform part of the processing passed {} > > > instead of the process_host_arch, which seems more correct. > > Note, the old code made what seems like the opposite mistake, and DIDN'T > > pass process_host_arch in the Host Platform section of the code. > The `process_host_arch` argument was added to > `Platform::IsCompatibleArchitecture` for > https://reviews.llvm.org/rGc22c7a61b6d9c90d5d4292205c63cd576f4fd05b. You're > correct that we don't have a process host arch from where this function is > being called. So I could have omitted it, but I decided not to for > consistency with `Platform::GetPlatformForArchitecture` just above. The new code in TargetList.cpp:179 still passes `{}` to GetPlatformForArchitectures as the process host arch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122684/new/ https://reviews.llvm.org/D122684 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits