labath added a comment.
Ok, then let's keep them. I don't mind changing all call sites -- having a
separate category object is the cleanest solution with least magic. However see
my comments about category naming and merging.
================
Comment at: unittests/Core/TimerTest.cpp:39
std::this_thread::sleep_for(std::chrono::milliseconds(10));
- Timer t2("CAT1", "");
+ // Explicitly testing the same category as above.
+ static Timer::Category tcat1b("CAT1");
----------------
Do we actually need to support that? Since we already have a nice Category
object then I think we should use that as a unique key for everything
(including printing). There no reason why the category needs to have local
scope. If it really needs to be used from multiple places, we can expose the
object at a higher scope. For the tests, this could be done by having a fixture
which creates the categories in the SetUpTestCase static function.
Alternatively, we could have Category object be based on llvm::ManagedStatic,
so it would go away when you call llvm_shutdown, which would enable us to have
truly isolated and leak-free tests. (Right now we don't call llvm_shutdown in
the main app as we cannot shutdown cleanly, but it would be great if one day we
could.)
================
Comment at: unittests/Core/TimerTest.cpp:48
+ // It should only appear once.
+ ASSERT_EQ(nullptr, strstr(strstr(ss.GetData(), "CAT1") + 1, "CAT1"));
ASSERT_EQ(1, sscanf(ss.GetData(), "%lf sec for CAT1", &seconds));
----------------
ss.GetStringRef().count("CAT1") == 1
Repository:
rL LLVM
https://reviews.llvm.org/D32823
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
