JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

It's always great to see changes that enable more testing. I left one inline 
comment/nit but besides that this LGTM.



================
Comment at: lldb/source/Target/Platform.cpp:1891
+
+PlatformSP PlatformList::GetOrCreate(const ArchSpec &arch,
+                                     ArchSpec *platform_arch_ptr,
----------------
[Nit/pedantic] We have a few calls to `IsCompatibleArchitecture` in this 
function. Based on the existing comments you can infer that that's what the 
second argument is for. Normally I'd suggest an inline comment, but given the 
that there's a bunch of calls, could we have either have two constants:

```
enum ArchMatch : bool {
  ExactArchMatch = true,
  CompatibleArchMatch = false,
};
```

Or maybe even better, can we change the signature of that function to take an 
enum with those values? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120810/new/

https://reviews.llvm.org/D120810

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to