scott.smith added inline comments.

================
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");
----------------
labath wrote:
> 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.)
I don't think so.

git grep Timer::Category | grep -v LLVM_PRETTY_FUNCTION
shows one place (other than the unit test) where the name is explicitly set, 
and that's because that function already has its own scoped timer.  All the 
other places are unique (one per function).

The test might be testing for recursion-safe timers, in which case I can just 
change the declaration of t2:
Timer t2(tcat1a, "")

so it uses the same category object.


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