clayborg added a comment.

So this will fix LLDB complaining when a binary contains arm64 and x86_64 when 
debugging on either platform? Great, we ran into this quite a bit lately!



================
Comment at: lldb/source/Target/Platform.cpp:1277-1279
+  // Prefer the selected platform if it matches all architectures.
+  if (selected_platform_matches_all_archs)
+    return selected_platform_sp;
----------------
Shouldn't we just check if the selected platform matches one architecture here 
and return the selected platform? Why do all architectures need to match. If I 
select a platform, and then do "file a.out" and "a.out" contained any number of 
architectures, it should just use the selected platform, no?


================
Comment at: lldb/source/Target/Platform.cpp:1281-1283
+  // Prefer the host platform if it matches all architectures.
+  if (host_platform_matches_all_archs)
+    return host_platform_sp;
----------------
Again, why would we not just return the host platform if one arch matches? same 
kind of thing as above?


================
Comment at: lldb/source/Target/Platform.cpp:1285-1286
+
+  // If there's only one platform left then that means that means that it
+  // supports all architectures.
+  if (candidates.size() == archs.size()) {
----------------
Is this comment out of date? We are iterating through more than one platform 
here.


================
Comment at: lldb/source/Target/Platform.cpp:1287-1294
+  if (candidates.size() == archs.size()) {
+    if (std::all_of(candidates.begin(), candidates.end(),
+                    [&](const PlatformSP &p) -> bool {
+                      return p->GetName() == candidates.front()->GetName();
+                    })) {
+      return candidates.front();
+    }
----------------
What is this doing? Comparing the first entry in candidates to each of the 
entries and if the name matches, returns the first entry? When would this ever 
not return true?


================
Comment at: lldb/source/Target/Platform.cpp:1296
+
+  // Prefer the selected platform if it matches at least on architecture.
+  if (selected_platform_matches_one_arch)
----------------



================
Comment at: lldb/source/Target/Platform.cpp:1300
+
+  // Prefer the host platform if it matches at least on architecture.
+  if (host_platform_matches_one_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

Reply via email to