labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
lgtm, thank you.
================
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");
----------------
scott.smith wrote:
> 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.
Yes, the test was testing recursive timers (I wrote it).
Repository:
rL LLVM
https://reviews.llvm.org/D32823
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
