I'm more hardline than most, but personally I would say "making it testable" is in and of itself a driver :) Of course even the most robust test suite will never catch everything, but it makes me sleep easier at night when I know that I will never encounter the same bug more than once.
On Mon, Dec 14, 2015 at 7:58 PM Enrico Granata <egran...@apple.com> wrote: > All of these things are theoretically possible (i.e. They can be done by > writing enough code :-), but I would love if there were a real feature > driver for them before I sit down and write said code. > > The most interesting avenue is making the FormatManager be a per-debugger > entity instead of a shared singleton, but given my last try it is quite a > bit of work. Possible to do, unlikely to happen anytime soon though. > > The easier thing to do might be allowing the addition of formatters to > language categories via the command line/API. Then one could have an ad-hoc > formatter added just for testing. > > Sent from my iPhone > > On Dec 14, 2015, at 7:07 PM, Zachary Turner <ztur...@google.com> wrote: > > Ahh. Is there any way to change that behavior so that you can add back > deleted formatters? Or make it so that FormatManager itself can be > created and then destroyed. i.e. we could still use it as a singleton / > global state in LLDB, but we could create and destroy them with each test > in the test suite. > > On Mon, Dec 14, 2015 at 6:48 PM Enrico Granata <egran...@apple.com> wrote: > >> It’s a hard unit test to write though >> >> I caught this because I deleted a formatter in a language category and >> saw it still applied >> >> Problem is that you can’t add new formatters or add back previously >> deleted formatters to a language category, and the FormatManager is global >> state, so a deletion is forever, which worries me as potentially affecting >> downstream tests >> >> I’ll keep this in mind though, if I can come up with a good way to cons a >> test up, I’ll definitely make it happen >> >> On Dec 14, 2015, at 6:45 PM, Zachary Turner <ztur...@google.com> wrote: >> >> A unit test would be good at catching bugs like this. >> >> On Mon, Dec 14, 2015 at 6:23 PM Enrico Granata via lldb-commits < >> lldb-commits@lists.llvm.org> wrote: >> >>> Author: enrico >>> Date: Mon Dec 14 20:20:48 2015 >>> New Revision: 255603 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=255603&view=rev >>> Log: >>> Fix a bug where language categories would hold on to their caches even >>> after changes >>> >>> >>> Modified: >>> lldb/trunk/include/lldb/DataFormatters/FormatManager.h >>> lldb/trunk/include/lldb/DataFormatters/LanguageCategory.h >>> lldb/trunk/source/DataFormatters/FormatManager.cpp >>> lldb/trunk/source/DataFormatters/LanguageCategory.cpp >>> >>> Modified: lldb/trunk/include/lldb/DataFormatters/FormatManager.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/FormatManager.h?rev=255603&r1=255602&r2=255603&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/include/lldb/DataFormatters/FormatManager.h (original) >>> +++ lldb/trunk/include/lldb/DataFormatters/FormatManager.h Mon Dec 14 >>> 20:20:48 2015 >>> @@ -238,11 +238,7 @@ public: >>> ShouldPrintAsOneLiner (ValueObject& valobj); >>> >>> void >>> - Changed () override >>> - { >>> - ++m_last_revision; >>> - m_format_cache.Clear (); >>> - } >>> + Changed () override; >>> >>> uint32_t >>> GetCurrentRevision () override >>> @@ -290,13 +286,13 @@ private: >>> bool did_strip_ref, >>> bool did_strip_typedef, >>> bool root_level = false); >>> - >>> + >>> + std::atomic<uint32_t> m_last_revision; >>> FormatCache m_format_cache; >>> + Mutex m_language_categories_mutex; >>> + LanguageCategories m_language_categories_map; >>> NamedSummariesMap m_named_summaries_map; >>> - std::atomic<uint32_t> m_last_revision; >>> TypeCategoryMap m_categories_map; >>> - LanguageCategories m_language_categories_map; >>> - Mutex m_language_categories_mutex; >>> >>> ConstString m_default_category_name; >>> ConstString m_system_category_name; >>> >>> Modified: lldb/trunk/include/lldb/DataFormatters/LanguageCategory.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/LanguageCategory.h?rev=255603&r1=255602&r2=255603&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/include/lldb/DataFormatters/LanguageCategory.h (original) >>> +++ lldb/trunk/include/lldb/DataFormatters/LanguageCategory.h Mon Dec 14 >>> 20:20:48 2015 >>> @@ -69,6 +69,9 @@ public: >>> lldb::TypeCategoryImplSP >>> GetCategory () const; >>> >>> + FormatCache& >>> + GetFormatCache (); >>> + >>> void >>> Enable (); >>> >>> >>> Modified: lldb/trunk/source/DataFormatters/FormatManager.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/FormatManager.cpp?rev=255603&r1=255602&r2=255603&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/DataFormatters/FormatManager.cpp (original) >>> +++ lldb/trunk/source/DataFormatters/FormatManager.cpp Mon Dec 14 >>> 20:20:48 2015 >>> @@ -123,6 +123,19 @@ GetFormatFromFormatName (const char *for >>> return false; >>> } >>> >>> +void >>> +FormatManager::Changed () >>> +{ >>> + ++m_last_revision; >>> + m_format_cache.Clear (); >>> + Mutex::Locker lang_locker(m_language_categories_mutex); >>> + for (auto& iter : m_language_categories_map) >>> + { >>> + if (iter.second) >>> + iter.second->GetFormatCache().Clear(); >>> + } >>> +} >>> + >>> bool >>> FormatManager::GetFormatFromCString (const char *format_cstr, >>> bool partial_match_ok, >>> @@ -1043,12 +1056,12 @@ FormatManager::GetHardcodedValidator (Fo >>> } >>> >>> FormatManager::FormatManager() : >>> + m_last_revision(0), >>> m_format_cache(), >>> + m_language_categories_mutex(Mutex::eMutexTypeRecursive), >>> + m_language_categories_map(), >>> m_named_summaries_map(this), >>> - m_last_revision(0), >>> m_categories_map(this), >>> - m_language_categories_map(), >>> - m_language_categories_mutex(Mutex::eMutexTypeRecursive), >>> m_default_category_name(ConstString("default")), >>> m_system_category_name(ConstString("system")), >>> m_vectortypes_category_name(ConstString("VectorTypes")) >>> @@ -1063,7 +1076,6 @@ FormatManager::FormatManager() : >>> void >>> FormatManager::LoadSystemFormatters() >>> { >>> - >>> TypeSummaryImpl::Flags string_flags; >>> string_flags.SetCascades(true) >>> .SetSkipPointers(true) >>> >>> Modified: lldb/trunk/source/DataFormatters/LanguageCategory.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/LanguageCategory.cpp?rev=255603&r1=255602&r2=255603&view=diff >>> >>> ============================================================================== >>> --- lldb/trunk/source/DataFormatters/LanguageCategory.cpp (original) >>> +++ lldb/trunk/source/DataFormatters/LanguageCategory.cpp Mon Dec 14 >>> 20:20:48 2015 >>> @@ -242,6 +242,12 @@ LanguageCategory::GetCategory () const >>> return m_category_sp; >>> } >>> >>> +FormatCache& >>> +LanguageCategory::GetFormatCache () >>> +{ >>> + return m_format_cache; >>> +} >>> + >>> void >>> LanguageCategory::Enable () >>> { >>> >>> >>> _______________________________________________ >>> lldb-commits mailing list >>> lldb-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >>> >> >> >> Thanks, >> *- Enrico* >> 📩 egranata@.com ☎️ 27683 >> >>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
