JDevlieghere marked an inline comment as done.
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)) {
----------------
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.


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

Reply via email to