aprantl added inline comments.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp:270
+ static const ArchSpec platform_arch(
+ HostInfo::GetArchitecture(HostInfo::eArchKind64));
+ arch = platform_arch;
----------------
I guess I should really know this... does this construct create a static
initializer? If yes, isn't that something we generally want to avoid in LLVM?
I checked: HostInfo::GetArchitecture() caches the result anyway, so there is no
need to cache it again here.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h:59
+ llvm::Triple::OSType m_os_type = llvm::Triple::UnknownOS;
+ llvm::ArrayRef<llvm::StringRef> m_supported_triples = {};
+
----------------
If the StringRef is supposed to be a triple, we might want to store an array of
llvm::Triple instead? They are basically std::strings.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp:81
+ case llvm::Triple::aarch64:
case llvm::Triple::x86_64: {
const llvm::Triple &triple = arch->GetTriple();
----------------
at some point we should factor out all the common code in the simulator
platforms ...
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformAppleTVSimulator.cpp:159
+ static const llvm::StringRef supported_triples[] = {
+ "x86_64-apple-tvos-simulator",
+ };
----------------
I'm probably missing something: Instead of declaring the static list of
supported triples here and then manually adding the host architecture, why not
create the list on-the-fly in GetSupportedArchitectureAtIndex()? Is
`m_supported_triples` used directly somewhere?
Something like:
```
switch (i) {
#ifdef __APPLE__
#if __arm64__
case 0: return "arm64-apple-tvos-simulator";
case 1: return "x86_64-apple-tvos-simulator";
case 2: return "x86_64h-apple-tvos-simulator",
};
#else
if (is_haswell)
switch (i) {
case 0: return "x86_64h-apple-tvos-simulator";
case 1: return "x86_64-apple-tvos-simulator";
}
else return "x86_64-apple-tvos-simulator"
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84537/new/
https://reviews.llvm.org/D84537
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
