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
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to