[Lldb-commits] [lldb] r255603 - Fix a bug where language categories would hold on to their caches even after changes
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 15 11:17:44 PST 2015
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 <egranata at 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 <zturner at 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 <egranata at 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 <zturner at 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 at 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 at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>>
>>
>>
>> Thanks,
>> *- Enrico*
>> 📩 egranata@.com ☎️ 27683
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151215/27f19a91/attachment-0001.html>
More information about the lldb-commits
mailing list