Adding         
packages/Python/lldbsuite/test/functionalities/data-formatter/language_category_updates
Adding         
packages/Python/lldbsuite/test/functionalities/data-formatter/language_category_updates/Makefile
Adding         
packages/Python/lldbsuite/test/functionalities/data-formatter/language_category_updates/TestDataFormatterLanguageCategoryUpdates.py
Adding         
packages/Python/lldbsuite/test/functionalities/data-formatter/language_category_updates/main.cpp
Transmitting file data ...
Committed revision 256034.

> On Dec 15, 2015, at 11:17 AM, Zachary Turner <ztur...@google.com> wrote:
> 
> 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 
> <mailto: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 
> <mailto: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 
>> <mailto: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 
>>> <mailto: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 <mailto: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 
>>> <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
>>>  
>>> <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
>>>  
>>> <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
>>>  
>>> <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
>>>  
>>> <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 <mailto:lldb-commits@lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits 
>>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits>
>> 
>> 
>> Thanks,
>> - Enrico
>> 📩 egranata@.com ☎️ 27683
>> 


Thanks,
- Enrico
📩 egranata@.com ☎️ 27683

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to