[Lldb-commits] [PATCH] D113608: [lldb] Simplify specifying of platform supported architectures

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/include/lldb/Target/Platform.h:330-331
+  /// NB: This implementation is mutually recursive with
+  /// GetSupportedArchitectureAtIndex. Subclasses should implement at least one
+  /// of them.
+  virtual std::vector<ArchSpec> GetSupportedArchitectures();
----------------
Do you expect any platform to implement both?


================
Comment at: lldb/source/Target/Platform.cpp:1217-1218
+std::vector<ArchSpec>
+Platform::CreateArchList(llvm::Triple::OSType os,
+                         llvm::ArrayRef<llvm::Triple::ArchType> archs) {
+  std::vector<ArchSpec> list;
----------------
In PlatformDarwin we set the vendor too. Could we have this take an optional OSType and Vendor maybe that sets the ones that are not None? That would also make migrating PlatformDarwin easier where we currently share the code and don't set the OS. Later we could move this out of PlatformDarwin into the child classes and have the OS set correctly. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113608