lenary added inline comments.
================ Comment at: llvm/unittests/Support/Host.cpp:33 -class HostTest : public testing::Test { - Triple Host; - -protected: - bool isSupportedArchAndOS() { - // Initially this is only testing detection of the number of - // physical cores, which is currently only supported/tested on - // some systems. - return (Host.isOSWindows() && llvm_is_multithreaded()) || - Host.isOSDarwin() || (Host.isX86() && Host.isOSLinux()) || - (Host.isOSLinux() && !Host.isAndroid()) || - (Host.isSystemZ() && Host.isOSzOS()); - } - - HostTest() : Host(Triple::normalize(sys::getProcessTriple())) {} -}; - -TEST_F(HostTest, NumPhysicalCoresSupported) { - if (!isSupportedArchAndOS()) - GTEST_SKIP(); - int Num = sys::getHostNumPhysicalCores(); - ASSERT_GT(Num, 0); -} - -TEST_F(HostTest, NumPhysicalCoresUnsupported) { - if (isSupportedArchAndOS()) - GTEST_SKIP(); - int Num = sys::getHostNumPhysicalCores(); - ASSERT_EQ(Num, -1); -} +class HostTest : public testing::Test {}; ---------------- fpetrogalli wrote: > lenary wrote: > > fpetrogalli wrote: > > > Is this needed? > > My original intention was to do the minimal changes required to just move > > the code. > > > > It's not technically needed, so I will remove it. > Yeah, I totally get the idea of doing minimal changes. Juts mention in the > commit message that you have some extra cleanup that is not strictly needed > for the major changes of the patch, so that people blaming the change can > understand why that was done. So, I forgot to submit this comment *before* I submitted the changes. If you're happy with how the patch looks now, +2 and I'll land it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137836/new/ https://reviews.llvm.org/D137836 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits