labath added a comment.
I like how this has turned out. Some small requests inline.
================
Comment at: lldb/include/lldb/Host/HostInfoBase.h:107
+ static SharedCacheImageInfo
+ GetSharedCacheImageInfo(llvm::StringRef image_name) {
----------------
Some doxygen here? "Try to find a module with the given name in the address
space of the current process?"
================
Comment at: lldb/source/Host/macosx/objcxx/HostInfoMacOSX.mm:493-494
+
+ SharedCacheInfo();
+ friend class HostInfoMacOSX;
+};
----------------
The class is already in an implementation file (you could empasize that by
making that an anonymous namespace:
<http://llvm.org/docs/CodingStandards.html#anonymous-namespaces>). I don't
think you need to go through all that trouble to avoid someone instantiating
it...
================
Comment at: lldb/unittests/Host/HostInfoTest.cpp:81-117
+ ASSERT_TRUE(llvm::isa<ObjectFileMachO>(OF));
+ EXPECT_TRUE(
+ OF->GetArchitecture().IsCompatibleMatch(HostInfo::GetArchitecture()));
+ Symtab *symtab = OF->GetSymtab();
+ ASSERT_NE(symtab, nullptr);
+ void *libobjc = dlopen("/usr/lib/libobjc.A.dylib", RTLD_LAZY);
+ ASSERT_NE(libobjc, nullptr);
----------------
This is mostly about checking the ObjectFileMachO functionality, is it not?
Could you make that a ObjectFileMachO unittest?
I'm mainly trying to avoid pulling in lots of libraries into the host unittest
binary. The unittest binary is a good way to ensure that that the host module
does not grow external dependencies (as the unittest wouldn't link), but this
would pull in pretty much everything into that binary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83023/new/
https://reviews.llvm.org/D83023
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
