2/2: Process formatters in reverse-chronological order

[jankratochvil]

Summary

If one reverts D66398 then the TestDataFormatterStdList does fail - as the C++ formatters are initialized in the opposite order. But the current state of trunk does not mind the order for C++ formatters.

[JDevlieghere]

I understand one could pass some better abstracted error-reporting interface instead of ValueObject itself but that would require some more code (interface definition, inheritance etc.).

How much work would it be to return an llvm::Expected<bool> from Get_Impl? I'd really prefer that over the current approach.

lldb/include/lldb/Core/ValueObject.h

459

This allows overwriting the error without checking if it's already set. Maybe it's better to use GetError, check that it's not set, and then either set it or append to it?

lldb/include/lldb/DataFormatters/FormattersContainer.h

298–299

If returning an expected isn't feasible, we should pass at least a Status* instead of setting the error in the ValueObject directly.

lldb/source/DataFormatters/TypeCategory.cpp

350

It's really unfortunate that so many call sites now require a null pointer. Can we make it a default argument?

[granata.enrico]

I'm not working on LLDB anymore. Sorry about that.

[jankratochvil]

In D66654#1643082, @JDevlieghere wrote:

How much work would it be to return an llvm::Expected<bool> from Get_Impl? I'd really prefer that over the current approach.

Done. Although now if (Get(...)) became confusing which I have commented in lldb_private::FormatDropExpected().

BTW I haven't found how to format a table for Doxygen. Also by searching around I have found other tables in LLDB/LLVM are also not recognized by Doxygen as tables.

lldb/include/lldb/DataFormatters/FormattersContainer.h

298–299

With Expected<bool> this is no longer applicable.

lldb/source/DataFormatters/TypeCategory.cpp

350

With Expected<bool> this is no longer applicable.

[jankratochvil]

Testcase for the regex sanity check is failing - ValueObject::m_error remains set even after type summary clear. ValueObject::UpdateValueIfNeeded will find did_change_formats==true but it does not help it. I think there is some bug in ValueObject updating but maybe that setting of ValueObject::m_error is a wrong idea.

runCmd: type summary clear
output:

runCmd: type summary add --summary-string "${var[0-1]}" -x "int \[[0-9]\]"
output:

runCmd: frame variable int_array
output: (int [5]) int_array = [-1,2]


Expecting sub string: 1,2
Matched

runCmd: type summary add --summary-string "${var[0-1]}" "int []"
output:

runCmd: frame variable int_array
output: (int [5]) int_array = <Two regexes ("int \[[0-9]+\]" and "int \[[0-9]\]") ambiguously match the same string "int [5]".>

Expecting pattern: int_array = <Two regexes \(("int\ \\\[\[0\-9\]\\\]" and "int\ \\\[\[0\-9\]\+\\\]"|"int\ \\\[\[0\-9\]\+\\\]" and "int\ \\\[\[0\-9\]\\\]")\) ambiguously match the same string "int\ \[5\]".>
Matched

runCmd: type summary clear
output:

runCmd: type summary add --summary-string "${var[0-1]}" "int []"
output:

runCmd: frame variable int_array
output: (int [5]) int_array = <Two regexes ("int \[[0-9]+\]" and "int \[[0-9]\]") ambiguously match the same string "int [5]".>

Expecting sub string: 1,2
Not matched

FAIL

[labath]

+ @jingham for value object stuff

Though I agree with the general principle of this change, the new api which implements it seems extremely confusing. Unfortunately, it's not really clear to me how to improve that. Maybe it would be better if the Get methods returned Expected<lldb::TypeFormatImplSP>? Then an "error" would mean ambiguous match, valid pointer would be "ok", and a null pointer would be "no match"? Or maybe both "no match" and "ambiguous match" should be errors? Or maybe, since these objects claim to be containers, they shouldn't even treat multiple matches as an error, but instead provide an interface to get *all* matches, and then this can be converted to an error higher up?

lldb/include/lldb/Core/ValueObject.h

460

I didn't mention this in the previous patches, but I think you're overusing the rvalue references here. It's generally better to use regular value objects in cases like these because this enforces proper contracts between functions.

Using rvalue references leaves open the possibility for a bug (which you've actually made here) that the TakeError function will do nothing to the passed-in object (on some path of execution), which means that the object will continue to exist in the "untaken" state in the caller, and then probably assert some distance away from where it was meant to be used.

Passing the argument as a plain object means that this function *must* do something with it (really *take* it) or *it* will assert. The cost of that is a single object move, but that should generally be trivial for objects that implement moving properly.

lldb/include/lldb/DataFormatters/FormatClasses.h

176

std::string, please. :) ConstString is severely overused in lldb..

[clayborg]

What about having the first one that matched being the one that is used and avoid errors all together? We do this with the regex alias stuff already. With regular expressions people are sure to make ones that conflict with each other.

[jankratochvil]

In D66654#1645658, @clayborg wrote:

What about having the first one that matched being the one that is used and avoid errors all together?

This is how it worked so far before the fix D66398 (which made the regexes unambiguous - always matching at most 1 regex for any string). The previous version of the patch just sorted the regexes alphabetically because original LLDB did not sort them at all printing random results.

@labath did not like the alphabetical sorting as (IIUC) adding formatters from a new library can affect formatting for a different library.

We do this with the regex alias stuff already.

I haven't found the work alias in either grep -ri alias $(find -name DataFormatters) or in Variable Formatting doc.

With regular expressions people are sure to make ones that conflict with each other.

So which order to choose? It should not have different output during different runs of LLDB.

[labath]

In D66654#1645792, @jankratochvil wrote:

In D66654#1645658, @clayborg wrote:

What about having the first one that matched being the one that is used and avoid errors all together?

This is how it worked so far before the fix D66398 (which made the regexes unambiguous - always matching at most 1 regex for any string). The previous version of the patch just sorted the regexes alphabetically because original LLDB did not sort them at all printing random results.

@labath did not like the alphabetical sorting as (IIUC) adding formatters from a new library can affect formatting for a different library.

Kind of, yes. My concern was a bit more general, in that the alphabetical sorting just seems arbitrary and I don't believe anybody would expect that. It happens to do what we want here, but I can imagine that in some cases it might do the exact opposite, and then we'd have to massage the regular expressions to reverse their sort order, but retain meaning...

Personally, I'd go for the registration order, or possibly the reverse registration order, depending on what we want to give precedence to. It seems to me that this would give the most predictable results, and would make it easy to swap the precedence of two formatters if needed. But I don't feel strongly about that, so if you want to go with alphabetical, then I guess that's fine with me (it's definitely better than sorting on pointer values :P).

[jankratochvil]

lldb/include/lldb/Core/ValueObject.h

460

Thanks for this notification, I see I was automatically using the && parameter specifier excessively. rvalues still work properly with std::move() even without using this parameter specifier.
I also agree there was the bug with possibly untaken Error asserting later (I find that as an orthogonal bug to the && one).

[jankratochvil]

In D66654#1646544, @labath wrote:

Personally, I'd go for the registration order,

Then you cannot override any existing formatter. Then it would be better to rather assert/error in such case which I tried to do in this patch but nobody likes this patch much.

or possibly the reverse registration order,

That is what this new patch implements.

[labath]

Summarizing our discussion on IRC:
While I think that this version of the patch is definitely better than the previous ones, I still feel that the two container solution is unnecessarily complicated. I think using llvm::MapVector or even std::vector would be sufficient. While this may be optimal in terms of performance (not memory), I don't think that is really important in this case. The only place where the std::list solution is better than a std::vector is the removal of an arbitrary element in the list. However, I would stipulate that this is a very uncommon operation. The most common operations are lookup (which are linear in any case), and insertion (constant).

Therefore, I think we should optimize for readability/bug-freeness and choose the simplest solution.

[labath]

Hm... "summarizing" might not have been the best word here. I have "summarized" what *I* said over IRC, but I did not mean to imply that Jan agreed with my position (and he didn't, not yet anyway..)

[jankratochvil]

In D66654#1654713, @labath wrote:

The most common operations are lookup (which are linear in any case),

There are also lookups by name (GetExact) which are logarithmic now by that std::map.

I can try the MapVector. I just tried not to regress performance by using optimal data structures. If one was concerned about performance there can be done many other easy improvements of the existing code.

[jankratochvil]

I have tried the MapVector with RegularExpressionSP as its key as suggested by @labath but that would be even more code than the std::list I provided here, both for its custom shared-pointer sorting and for its EmptyKey+TombstoneKey if we should not put std::map there.
So I returned back to a single std::vector as originally suggested by @labath and after fixing missing Delete in the Add implementation for vector it I think that may be a good compromise from all the variants.

[labath]

I think this looks great. Some operations may get less efficient with this, but OTOH, some will get actually faster. And it is definitely the most understandable solution out of everything we had so far.

lldb/include/lldb/DataFormatters/FormattersContainer.h

83

m_map.emplace_back, maybe?

111

range-based for

130–134

lol

177

Is this still needed, or is that a leftover from the MapVector attempt?

307–308

consider: for(const auto &entry: llvm::reverse(m_format_map))

[jankratochvil]

lldb/include/lldb/DataFormatters/FormattersContainer.h

83

Yes, not sure why I forgot here.

111

OK, other existing loops updated in: rL371922

177

Still needed.

307–308

Nice, thanks, used.

[labath]

lgtm

lldb/include/lldb/Core/ValueObject.h

public:
   // a null pointer
   ValueObject *FollowParentChain(std::function<bool(ValueObject *)>);
 
   virtual bool GetDeclaration(Declaration &decl);
 
   // The functions below should NOT be modified by subclasses
   const Status &GetError();
 
+  // Store error from Expected<> parameter unless some error is already stored.
+  template <class T> void TakeError(llvm::Expected<T> &&e) {
+    if (!e && !GetError().Fail())
+      m_error = Status(e.takeError());
+  }
+
   ConstString GetName() const;
 
   virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create);
 
   // this will always create the children if necessary
   lldb::ValueObjectSP GetChildAtIndexPath(llvm::ArrayRef<size_t> idxs,
                                           size_t *index_of_error = nullptr);
 

lldb/include/lldb/DataFormatters/FormatClasses.h

struct TypeOrName {
     CompilerType m_compiler_type;
   };
   TypeOrName m_type;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(TypeNameSpecifierImpl);
 };
 
+class FormatDupError : public llvm::ErrorInfo<FormatDupError> {
+public:
+  static char ID;
+
+  FormatDupError(lldb::RegularExpressionSP regex1,
+                 lldb::RegularExpressionSP regex2, ConstString str);
+
+  void log(llvm::raw_ostream &OS) const override;
+
+  std::error_code convertToErrorCode() const override;
+
+private:
+  lldb::RegularExpressionSP m_regex1, m_regex2;
+  ConstString m_str;
+};
+
+bool FormatDropExpected(llvm::Expected<bool> &&e);
+
 } // namespace lldb_private
 
 #endif // lldb_FormatClasses_h_

lldb/include/lldb/DataFormatters/FormattersContainer.h

public:
   void Add(KeyType name, const ValueSP &entry) {
     if (listener)
       entry->GetRevision() = listener->GetCurrentRevision();
     else
       entry->GetRevision() = 0;
 
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     m_map[name] = entry;
     if (listener)
       listener->Changed();
   }
 
   bool Delete(KeyType name) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     MapIterator iter = m_map.find(name);
     if (iter == m_map.end())
       return false;
public:
   }
 
   bool Get(KeyType name, ValueSP &entry) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     MapIterator iter = m_map.find(name);
     if (iter == m_map.end())
       return false;
     entry = iter->second;
     return true;
   }
 
   void ForEach(ForEachCallback callback) {
     if (callback) {
       std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
       MapIterator pos, end = m_map.end();
       for (pos = m_map.begin(); pos != end; pos++) {
         KeyType type = pos->first;
         if (!callback(type, pos->second))
           break;
       }
     }
   }
 
   uint32_t GetCount() { return m_map.size(); }
 
   ValueSP GetValueAtIndex(size_t index) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     MapIterator iter = m_map.begin();
     MapIterator end = m_map.end();
     while (index > 0) {
       iter++;
       index--;
       if (end == iter)
         return ValueSP();
     }
     return iter->second;
   }
 
   KeyType GetKeyAtIndex(size_t index) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
protected:
   typedef FormatMap<KeyType, ValueType> BackEndType;
 
 public:
   typedef typename BackEndType::MapType MapType;
   typedef typename MapType::iterator MapIterator;
   typedef typename MapType::key_type MapKeyType;
   typedef typename MapType::mapped_type MapValueType;
   typedef typename BackEndType::ForEachCallback ForEachCallback;
   typedef typename std::shared_ptr<FormattersContainer<KeyType, ValueType>>
       SharedPointer;
 
   friend class TypeCategoryImpl;
 
   FormattersContainer(std::string name, IFormatChangeListener *lst)
       : m_format_map(lst), m_name(name) {}
 
   void Add(const MapKeyType &type, const MapValueType &entry) {
if (ret)
       entry = MapValueType(entry);
     else
       entry = MapValueType();
     if (why)
       *why = value;
     return ret;
   }
 
-  bool Get(ConstString type, MapValueType &entry) {
+  // See FormatDropExpected how to handle the return value.
+  llvm::Expected<bool> Get(ConstString type, MapValueType &entry) {
     return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
   }
 
-  bool GetExact(ConstString type, MapValueType &entry) {
+  // See FormatDropExpected how to handle the return value.
+  llvm::Expected<bool> GetExact(ConstString type, MapValueType &entry) {
     return GetExact_Impl(type, entry, static_cast<KeyType *>(nullptr));
   }
 
   MapValueType GetAtIndex(size_t index) {
     return m_format_map.GetValueAtIndex(index);
   }
 
   lldb::TypeNameSpecifierImplSP GetTypeNameSpecifierAtIndex(size_t index) {
for (pos = m_format_map.map().begin(); pos != end; pos++) {
         if (m_format_map.listener)
           m_format_map.listener->Changed();
         return true;
       }
     }
     return false;
   }
 
-  bool Get_Impl(ConstString type, MapValueType &entry, ConstString *dummy) {
+  // See FormatDropExpected how to handle the return value.
+  llvm::Expected<bool> Get_Impl(ConstString type, MapValueType &entry,
+                                ConstString *dummy) {
     return m_format_map.Get(type, entry);
   }
 
-  bool GetExact_Impl(ConstString type, MapValueType &entry,
+  // See FormatDropExpected how to handle the return value.
+  llvm::Expected<bool> GetExact_Impl(ConstString type, MapValueType &entry,
                                      ConstString *dummy) {
     return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
   }
 
   lldb::TypeNameSpecifierImplSP
   GetTypeNameSpecifierAtIndex_Impl(size_t index, ConstString *dummy) {
     ConstString key = m_format_map.GetKeyAtIndex(index);
     if (key)
       return lldb::TypeNameSpecifierImplSP(
           new TypeNameSpecifierImpl(key.AsCString(), false));
     else
       return lldb::TypeNameSpecifierImplSP();
   }
 
   lldb::TypeNameSpecifierImplSP
   GetTypeNameSpecifierAtIndex_Impl(size_t index,
                                    lldb::RegularExpressionSP *dummy) {
     lldb::RegularExpressionSP regex = m_format_map.GetKeyAtIndex(index);
     if (regex.get() == nullptr)
       return lldb::TypeNameSpecifierImplSP();
     return lldb::TypeNameSpecifierImplSP(
         new TypeNameSpecifierImpl(regex->GetText().str().c_str(), true));
   }
 
-  bool Get_Impl(ConstString key, MapValueType &value,
+  // See FormatDropExpected how to handle the return value.
+  llvm::Expected<bool> Get_Impl(ConstString key, MapValueType &value,
                                 lldb::RegularExpressionSP *dummy) {
     llvm::StringRef key_str = key.GetStringRef();
     std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
     MapIterator pos, end = m_format_map.map().end();
+    MapIterator found = end;
     for (pos = m_format_map.map().begin(); pos != end; pos++) {
       lldb::RegularExpressionSP regex = pos->first;
       if (regex->Execute(key_str)) {
-        value = pos->second;
-        return true;
+        if (found != end)
+          return llvm::make_error<FormatDupError>(found->first, regex, key);
+        found = pos;
       }
     }
+    if (found == end)
       return false;
+    value = found->second;
+    return true;
   }
 
   bool GetExact_Impl(ConstString key, MapValueType &value,
                      lldb::RegularExpressionSP *dummy) {
     std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
     MapIterator pos, end = m_format_map.map().end();
     for (pos = m_format_map.map().begin(); pos != end; pos++) {
       lldb::RegularExpressionSP regex = pos->first;
       if (regex->GetText() == key.GetStringRef()) {
         value = pos->second;
         return true;
       }
     }
     return false;
   }
 
-  bool Get(const FormattersMatchVector &candidates, MapValueType &entry,
-           uint32_t *reason) {
+  // See FormatDropExpected how to handle the return value.
+  llvm::Expected<bool> Get(const FormattersMatchVector &candidates,
+                           MapValueType &entry, uint32_t *reason) {
+    llvm::Expected<bool> retval(false);
     for (const FormattersMatchCandidate &candidate : candidates) {
-      if (Get(candidate.GetTypeName(), entry)) {
+      llvm::Expected<bool> got = Get(candidate.GetTypeName(), entry);
+      if (!got) {
+        FormatDropExpected(std::move(retval));
+        retval = std::move(got);
+      } else if (*got) {
         if (candidate.IsMatch(entry) == false) {
           entry.reset();
           continue;
         } else {
           if (reason)
             *reason = candidate.GetReason();
+          FormatDropExpected(std::move(retval));
           return true;
         }
       }
     }
-    return false;
+    return retval;
   }
 };
 
 } // namespace lldb_private
 
 #endif // lldb_FormattersContainer_h_

lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py

 """
 Test lldb data formatter subsystem.
 """
 
 from __future__ import print_function
 
 
 import lldb
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
+import re
 
 
 class AdvDataFormatterTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     def setUp(self):
         # Call super's setUp().
def test_with_run_command(self):
         self.runCmd("type summary clear")
 
         self.runCmd(
             'type summary add --summary-string \"${var[0-1]}\" -x \"int \[[0-9]\]\"')
 
         self.expect("frame variable int_array",
                     substrs=['1,2'])
 
+        # FixArrayTypeNameWithRegex converts "int []" into "int \[[0-9]+\]"
+        # which becomes ambiguous for some string against "int \[[0-9]\]".
         self.runCmd(
             'type summary add --summary-string \"${var[0-1]}\" "int []"')
+        self.expect("frame variable int_array",
+            patterns=['''int_array = <Two regexes \\(("{0}" and "{1}"|"{1}"'''
+                ''' and "{0}")\\) ambiguously match the same string "{2}".>'''
+                .format(re.escape('''int \\[[0-9]\\]'''),
+                    re.escape('''int \\[[0-9]+\\]'''),
+                    re.escape('''int [5]'''))])
+
+        self.runCmd("type summary clear")
 
+        self.runCmd(
+            'type summary add --summary-string \"${var[0-1]}\" "int []"')
         self.expect("frame variable int_array",
                     substrs=['1,2'])
 
         self.runCmd("type summary clear")
 
         self.runCmd("type summary add -c -x \"i_am_cool \[[0-9]\]\"")
         self.runCmd("type summary add -c i_am_cool")
 

lldb/source/API/SBTypeCategory.cpp

if (!IsValid())
     return LLDB_RECORD_RESULT(SBTypeFilter());
 
   if (!spec.IsValid())
     return LLDB_RECORD_RESULT(SBTypeFilter());
 
   lldb::TypeFilterImplSP children_sp;
 
   if (spec.IsRegex())
-    m_opaque_sp->GetRegexTypeFiltersContainer()->GetExact(
-        ConstString(spec.GetName()), children_sp);
+    FormatDropExpected(m_opaque_sp->GetRegexTypeFiltersContainer()->GetExact(
+        ConstString(spec.GetName()), children_sp));
   else
-    m_opaque_sp->GetTypeFiltersContainer()->GetExact(
-        ConstString(spec.GetName()), children_sp);
+    FormatDropExpected(m_opaque_sp->GetTypeFiltersContainer()->GetExact(
+        ConstString(spec.GetName()), children_sp));
 
   if (!children_sp)
     return LLDB_RECORD_RESULT(lldb::SBTypeFilter());
 
   TypeFilterImplSP filter_sp =
       std::static_pointer_cast<TypeFilterImpl>(children_sp);
 
   return LLDB_RECORD_RESULT(lldb::SBTypeFilter(filter_sp));
 }
 SBTypeFormat SBTypeCategory::GetFormatForType(SBTypeNameSpecifier spec) {
   LLDB_RECORD_METHOD(lldb::SBTypeFormat, SBTypeCategory, GetFormatForType,
                      (lldb::SBTypeNameSpecifier), spec);
 
   if (!IsValid())
     return LLDB_RECORD_RESULT(SBTypeFormat());
 
   if (!spec.IsValid())
     return LLDB_RECORD_RESULT(SBTypeFormat());
 
   lldb::TypeFormatImplSP format_sp;
 
   if (spec.IsRegex())
-    m_opaque_sp->GetRegexTypeFormatsContainer()->GetExact(
-        ConstString(spec.GetName()), format_sp);
+    FormatDropExpected(m_opaque_sp->GetRegexTypeFormatsContainer()->GetExact(
+        ConstString(spec.GetName()), format_sp));
   else
-    m_opaque_sp->GetTypeFormatsContainer()->GetExact(
-        ConstString(spec.GetName()), format_sp);
+    FormatDropExpected(m_opaque_sp->GetTypeFormatsContainer()->GetExact(
+        ConstString(spec.GetName()), format_sp));
 
   if (!format_sp)
     return LLDB_RECORD_RESULT(lldb::SBTypeFormat());
 
   return LLDB_RECORD_RESULT(lldb::SBTypeFormat(format_sp));
 }
 
 SBTypeSummary SBTypeCategory::GetSummaryForType(SBTypeNameSpecifier spec) {
   LLDB_RECORD_METHOD(lldb::SBTypeSummary, SBTypeCategory, GetSummaryForType,
                      (lldb::SBTypeNameSpecifier), spec);
 
   if (!IsValid())
     return LLDB_RECORD_RESULT(SBTypeSummary());
 
   if (!spec.IsValid())
     return LLDB_RECORD_RESULT(SBTypeSummary());
 
   lldb::TypeSummaryImplSP summary_sp;
 
   if (spec.IsRegex())
-    m_opaque_sp->GetRegexTypeSummariesContainer()->GetExact(
-        ConstString(spec.GetName()), summary_sp);
+    FormatDropExpected(m_opaque_sp->GetRegexTypeSummariesContainer()->GetExact(
+        ConstString(spec.GetName()), summary_sp));
   else
-    m_opaque_sp->GetTypeSummariesContainer()->GetExact(
-        ConstString(spec.GetName()), summary_sp);
+    FormatDropExpected(m_opaque_sp->GetTypeSummariesContainer()->GetExact(
+        ConstString(spec.GetName()), summary_sp));
 
   if (!summary_sp)
     return LLDB_RECORD_RESULT(lldb::SBTypeSummary());
 
   return LLDB_RECORD_RESULT(lldb::SBTypeSummary(summary_sp));
 }
 
 SBTypeSynthetic SBTypeCategory::GetSyntheticForType(SBTypeNameSpecifier spec) {
   LLDB_RECORD_METHOD(lldb::SBTypeSynthetic, SBTypeCategory, GetSyntheticForType,
                      (lldb::SBTypeNameSpecifier), spec);
 
   if (!IsValid())
     return LLDB_RECORD_RESULT(SBTypeSynthetic());
 
   if (!spec.IsValid())
     return LLDB_RECORD_RESULT(SBTypeSynthetic());
 
   lldb::SyntheticChildrenSP children_sp;
 
   if (spec.IsRegex())
-    m_opaque_sp->GetRegexTypeSyntheticsContainer()->GetExact(
-        ConstString(spec.GetName()), children_sp);
+    FormatDropExpected(m_opaque_sp->GetRegexTypeSyntheticsContainer()->GetExact(
+        ConstString(spec.GetName()), children_sp));
   else
-    m_opaque_sp->GetTypeSyntheticsContainer()->GetExact(
-        ConstString(spec.GetName()), children_sp);
+    FormatDropExpected(m_opaque_sp->GetTypeSyntheticsContainer()->GetExact(
+        ConstString(spec.GetName()), children_sp));
 
   if (!children_sp)
     return LLDB_RECORD_RESULT(lldb::SBTypeSynthetic());
 
   ScriptedSyntheticChildrenSP synth_sp =
       std::static_pointer_cast<ScriptedSyntheticChildren>(children_sp);
 
   return LLDB_RECORD_RESULT(lldb::SBTypeSynthetic(synth_sp));

lldb/source/DataFormatters/FormatClasses.cpp

CandidateLanguagesVector FormattersMatchData::GetCandidateLanguages() {
   return m_candidate_languages;
 }
 
 ValueObject &FormattersMatchData::GetValueObject() { return m_valobj; }
 
 lldb::DynamicValueType FormattersMatchData::GetDynamicValueType() {
   return m_dynamic_value_type;
 }
+
+char FormatDupError::ID = 0;
+
+FormatDupError::FormatDupError(RegularExpressionSP regex1,
+                               RegularExpressionSP regex2, ConstString str)
+    : m_regex1(std::move(regex1)), m_regex2(std::move(regex2)), m_str(str) {}
+
+void FormatDupError::log(llvm::raw_ostream &OS) const {
+  OS << llvm::formatv("Two regexes (\"{0}\" and \"{1}\") ambiguously match the "
+                      "same string \"{2}\".",
+                      m_regex1->GetText(), m_regex2->GetText(), m_str);
+}
+std::error_code FormatDupError::convertToErrorCode() const {
+  return llvm::inconvertibleErrorCode();
+}
+
+/// FormatDropExpected returns whether function succeeded, dropping the error
+/// code even if it failed. Call ValueObject::TakeError() if possible.
+///
+/// \param e confusingly evaluates as true even if it is a false return from
+//         function:
+///   |  retval &&  *retval | true return.                                     |
+///   |  retval && !*retval | false return with no error code reported.        |
+///   | !retval             | false return with error code retval.takeError(). |
+bool lldb_private::FormatDropExpected(llvm::Expected<bool> &&e) {
+  if (e)
+    return *e;
+  llvm::handleAllErrors(e.takeError(),
+                        [&](std::unique_ptr<FormatDupError> E) {});
+  return false;
+}

lldb/source/DataFormatters/TypeCategory.cpp

bool TypeCategoryImpl::HasLanguage(lldb::LanguageType lang) {
   return (iter != end);
 }
 
 bool TypeCategoryImpl::Get(ValueObject &valobj,
                            const FormattersMatchVector &candidates,
                            lldb::TypeFormatImplSP &entry, uint32_t *reason) {
   if (!IsEnabled() || !IsApplicable(valobj))
     return false;
-  if (GetTypeFormatsContainer()->Get(candidates, entry, reason))
+  llvm::Expected<bool> plain =
+      GetTypeFormatsContainer()->Get(candidates, entry, reason);
+  if (plain && *plain)
+    return true;
+  llvm::Expected<bool> regex =
+      GetRegexTypeFormatsContainer()->Get(candidates, entry, reason);
+  if (regex && *regex) {
+    FormatDropExpected(std::move(plain));
+    if (reason)
+      *reason |=
+          lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
     return true;
-  bool regex = GetRegexTypeFormatsContainer()->Get(candidates, entry, reason);
-  if (regex && reason)
-    *reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
-  return regex;
+  }
+  valobj.TakeError(std::move(plain));
+  valobj.TakeError(std::move(regex));
+  return false;
 }
 
 bool TypeCategoryImpl::Get(ValueObject &valobj,
                            const FormattersMatchVector &candidates,
                            lldb::TypeSummaryImplSP &entry, uint32_t *reason) {
   if (!IsEnabled() || !IsApplicable(valobj))
     return false;
-  if (GetTypeSummariesContainer()->Get(candidates, entry, reason))
+  llvm::Expected<bool> plain =
+      GetTypeSummariesContainer()->Get(candidates, entry, reason);
+  if (plain && *plain)
+    return true;
+  llvm::Expected<bool> regex =
+      GetRegexTypeSummariesContainer()->Get(candidates, entry, reason);
+  if (regex && *regex) {
+    FormatDropExpected(std::move(plain));
+    if (reason)
+      *reason |=
+          lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
     return true;
-  bool regex = GetRegexTypeSummariesContainer()->Get(candidates, entry, reason);
-  if (regex && reason)
-    *reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
-  return regex;
+  }
+  valobj.TakeError(std::move(plain));
+  valobj.TakeError(std::move(regex));
+  return false;
 }
 
 bool TypeCategoryImpl::Get(ValueObject &valobj,
                            const FormattersMatchVector &candidates,
                            lldb::SyntheticChildrenSP &entry, uint32_t *reason) {
   if (!IsEnabled() || !IsApplicable(valobj))
     return false;
   TypeFilterImpl::SharedPointer filter_sp;
   uint32_t reason_filter = 0;
   bool regex_filter = false;
   // first find both Filter and Synth, and then check which is most recent
 
-  if (!GetTypeFiltersContainer()->Get(candidates, filter_sp, &reason_filter))
-    regex_filter = GetRegexTypeFiltersContainer()->Get(candidates, filter_sp,
-                                                       &reason_filter);
+  llvm::Expected<bool> error_filter_regex(false);
+  llvm::Expected<bool> error_filter_plain =
+      GetTypeFiltersContainer()->Get(candidates, filter_sp, &reason_filter);
+  if (!error_filter_plain || !*error_filter_plain) {
+    FormatDropExpected(std::move(error_filter_regex));
+    error_filter_regex = GetRegexTypeFiltersContainer()->Get(
+        candidates, filter_sp, &reason_filter);
+    if (error_filter_regex && *error_filter_regex)
+      regex_filter = true;
+  }
 
   bool regex_synth = false;
   uint32_t reason_synth = 0;
   bool pick_synth = false;
   ScriptedSyntheticChildren::SharedPointer synth;
-  if (!GetTypeSyntheticsContainer()->Get(candidates, synth, &reason_synth))
-    regex_synth = GetRegexTypeSyntheticsContainer()->Get(candidates, synth,
-                                                         &reason_synth);
-  if (!filter_sp.get() && !synth.get())
+  llvm::Expected<bool> error_synth_regex(false);
+  llvm::Expected<bool> error_synth_plain =
+      GetTypeSyntheticsContainer()->Get(candidates, synth, &reason_synth);
+  if (!error_synth_plain || !*error_synth_plain) {
+    FormatDropExpected(std::move(error_synth_regex));
+    error_synth_regex = GetRegexTypeSyntheticsContainer()->Get(
+        candidates, synth, &reason_synth);
+    if (error_synth_regex && *error_synth_regex)
+      regex_synth = true;
+  }
+  if (!filter_sp.get() && !synth.get()) {
+    valobj.TakeError(std::move(error_filter_plain));
+    valobj.TakeError(std::move(error_filter_regex));
+    valobj.TakeError(std::move(error_synth_plain));
+    valobj.TakeError(std::move(error_synth_regex));
     return false;
-  else if (!filter_sp.get() && synth.get())
+  } else if (!filter_sp.get() && synth.get())
     pick_synth = true;
 
   else if (filter_sp.get() && !synth.get())
     pick_synth = false;
 
   else /*if (filter_sp.get() && synth.get())*/
   {
     pick_synth = filter_sp->GetRevision() <= synth->GetRevision();
   }
+
+  FormatDropExpected(std::move(error_filter_plain));
+  FormatDropExpected(std::move(error_filter_regex));
+  FormatDropExpected(std::move(error_synth_plain));
+  FormatDropExpected(std::move(error_synth_regex));
+
   if (pick_synth) {
     if (regex_synth && reason)
       *reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionFilter;
     entry = synth;
-    return true;
   } else {
     if (regex_filter && reason)
       *reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionFilter;
     entry = filter_sp;
-    return true;
   }
-  return false;
+  return true;
 }
 
 bool TypeCategoryImpl::Get(ValueObject &valobj,
                            const FormattersMatchVector &candidates,
                            lldb::TypeValidatorImplSP &entry, uint32_t *reason) {
   if (!IsEnabled())
     return false;
-  if (GetTypeValidatorsContainer()->Get(candidates, entry, reason))
+  llvm::Expected<bool> plain =
+      GetTypeValidatorsContainer()->Get(candidates, entry, reason);
+  if (plain && *plain)
     return true;
-  bool regex =
+  llvm::Expected<bool> regex =
       GetRegexTypeValidatorsContainer()->Get(candidates, entry, reason);
-  if (regex && reason)
-    *reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
-  return regex;
+  if (regex && *regex) {
+    FormatDropExpected(std::move(plain));
+    if (reason)
+      *reason |=
+          lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
+    return true;
+  }
+  valobj.TakeError(std::move(plain));
+  valobj.TakeError(std::move(regex));
+  return false;
 }
 
 void TypeCategoryImpl::Clear(FormatCategoryItems items) {
   if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue)
     GetTypeFormatsContainer()->Clear();
   if ((items & eFormatCategoryItemRegexValue) == eFormatCategoryItemRegexValue)
     GetRegexTypeFormatsContainer()->Clear();
 
bool TypeCategoryImpl::AnyMatches(ConstString type_name,
 
   lldb::TypeFormatImplSP format_sp;
   lldb::TypeSummaryImplSP summary_sp;
   TypeFilterImpl::SharedPointer filter_sp;
   ScriptedSyntheticChildren::SharedPointer synth_sp;
   TypeValidatorImpl::SharedPointer validator_sp;
 
   if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue) {
-    if (GetTypeFormatsContainer()->Get(type_name, format_sp)) {
+    if (FormatDropExpected(
+            GetTypeFormatsContainer()->Get(type_name, format_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemValue;
       return true;
     }
   }
   if ((items & eFormatCategoryItemRegexValue) ==
       eFormatCategoryItemRegexValue) {
-    if (GetRegexTypeFormatsContainer()->Get(type_name, format_sp)) {
+    if (FormatDropExpected(
+            GetRegexTypeFormatsContainer()->Get(type_name, format_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemRegexValue;
       return true;
     }
   }
 
   if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary) {
-    if (GetTypeSummariesContainer()->Get(type_name, summary_sp)) {
+    if (FormatDropExpected(
+            GetTypeSummariesContainer()->Get(type_name, summary_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemSummary;
       return true;
     }
   }
   if ((items & eFormatCategoryItemRegexSummary) ==
       eFormatCategoryItemRegexSummary) {
-    if (GetRegexTypeSummariesContainer()->Get(type_name, summary_sp)) {
+    if (FormatDropExpected(
+            GetRegexTypeSummariesContainer()->Get(type_name, summary_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemRegexSummary;
       return true;
     }
   }
 
   if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter) {
-    if (GetTypeFiltersContainer()->Get(type_name, filter_sp)) {
+    if (FormatDropExpected(
+            GetTypeFiltersContainer()->Get(type_name, filter_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemFilter;
       return true;
     }
   }
   if ((items & eFormatCategoryItemRegexFilter) ==
       eFormatCategoryItemRegexFilter) {
-    if (GetRegexTypeFiltersContainer()->Get(type_name, filter_sp)) {
+    if (FormatDropExpected(
+            GetRegexTypeFiltersContainer()->Get(type_name, filter_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemRegexFilter;
       return true;
     }
   }
 
   if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth) {
-    if (GetTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
+    if (FormatDropExpected(
+            GetTypeSyntheticsContainer()->Get(type_name, synth_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemSynth;
       return true;
     }
   }
   if ((items & eFormatCategoryItemRegexSynth) ==
       eFormatCategoryItemRegexSynth) {
-    if (GetRegexTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
+    if (FormatDropExpected(
+            GetRegexTypeSyntheticsContainer()->Get(type_name, synth_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemRegexSynth;
       return true;
     }
   }
 
   if ((items & eFormatCategoryItemValidator) == eFormatCategoryItemValidator) {
-    if (GetTypeValidatorsContainer()->Get(type_name, validator_sp)) {
+    if (FormatDropExpected(
+            GetTypeValidatorsContainer()->Get(type_name, validator_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemValidator;
       return true;
     }
   }
   if ((items & eFormatCategoryItemRegexValidator) ==
       eFormatCategoryItemRegexValidator) {
-    if (GetRegexTypeValidatorsContainer()->Get(type_name, validator_sp)) {
+    if (FormatDropExpected(
+            GetRegexTypeValidatorsContainer()->Get(type_name, validator_sp))) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
         *matching_type = eFormatCategoryItemRegexValidator;
       return true;
     }
   }
 
   return false;
 }
 
 TypeCategoryImpl::FormatContainer::MapValueType
 TypeCategoryImpl::GetFormatForType(lldb::TypeNameSpecifierImplSP type_sp) {
   FormatContainer::MapValueType retval;
 
   if (type_sp) {
     if (type_sp->IsRegex())
-      GetRegexTypeFormatsContainer()->GetExact(ConstString(type_sp->GetName()),
-                                               retval);
+      FormatDropExpected(GetRegexTypeFormatsContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
     else
-      GetTypeFormatsContainer()->GetExact(ConstString(type_sp->GetName()),
-                                          retval);
+      FormatDropExpected(GetTypeFormatsContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
   }
 
   return retval;
 }
 
 TypeCategoryImpl::SummaryContainer::MapValueType
 TypeCategoryImpl::GetSummaryForType(lldb::TypeNameSpecifierImplSP type_sp) {
   SummaryContainer::MapValueType retval;
 
   if (type_sp) {
     if (type_sp->IsRegex())
-      GetRegexTypeSummariesContainer()->GetExact(
-          ConstString(type_sp->GetName()), retval);
+      FormatDropExpected(GetRegexTypeSummariesContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
     else
-      GetTypeSummariesContainer()->GetExact(ConstString(type_sp->GetName()),
-                                            retval);
+      FormatDropExpected(GetTypeSummariesContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
   }
 
   return retval;
 }
 
 TypeCategoryImpl::FilterContainer::MapValueType
 TypeCategoryImpl::GetFilterForType(lldb::TypeNameSpecifierImplSP type_sp) {
   FilterContainer::MapValueType retval;
 
   if (type_sp) {
     if (type_sp->IsRegex())
-      GetRegexTypeFiltersContainer()->GetExact(ConstString(type_sp->GetName()),
-                                               retval);
+      FormatDropExpected(GetRegexTypeFiltersContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
     else
-      GetTypeFiltersContainer()->GetExact(ConstString(type_sp->GetName()),
-                                          retval);
+      FormatDropExpected(GetTypeFiltersContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
   }
 
   return retval;
 }
 
 TypeCategoryImpl::SynthContainer::MapValueType
 TypeCategoryImpl::GetSyntheticForType(lldb::TypeNameSpecifierImplSP type_sp) {
   SynthContainer::MapValueType retval;
 
   if (type_sp) {
     if (type_sp->IsRegex())
-      GetRegexTypeSyntheticsContainer()->GetExact(
-          ConstString(type_sp->GetName()), retval);
+      FormatDropExpected(GetRegexTypeSyntheticsContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
     else
-      GetTypeSyntheticsContainer()->GetExact(ConstString(type_sp->GetName()),
-                                             retval);
+      FormatDropExpected(GetTypeSyntheticsContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
   }
 
   return retval;
 }
 
 TypeCategoryImpl::ValidatorContainer::MapValueType
 TypeCategoryImpl::GetValidatorForType(lldb::TypeNameSpecifierImplSP type_sp) {
   ValidatorContainer::MapValueType retval;
 
   if (type_sp) {
     if (type_sp->IsRegex())
-      GetRegexTypeValidatorsContainer()->GetExact(
-          ConstString(type_sp->GetName()), retval);
+      FormatDropExpected(GetRegexTypeValidatorsContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
     else
-      GetTypeValidatorsContainer()->GetExact(ConstString(type_sp->GetName()),
-                                             retval);
+      FormatDropExpected(GetTypeValidatorsContainer()->GetExact(
+          ConstString(type_sp->GetName()), retval));
   }
 
   return retval;
 }
 
 lldb::TypeNameSpecifierImplSP
 TypeCategoryImpl::GetTypeNameSpecifierForSummaryAtIndex(size_t index) {
   if (index < GetTypeSummariesContainer()->GetCount())