jankratochvil created this revision.
jankratochvil added reviewers: labath, JDevlieghere, granata.enrico.
jankratochvil added a project: LLDB.
Herald added a subscriber: lldb-commits.
The sanity checking part
<https://reviews.llvm.org/D66398?id=216585#inline-597273> is improved here to
use some better error channel than original `llvm::errs()`.
With this patch (and without its `TestDataFormatterAdv.py` fix) one gets:
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]".>
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.).
With this alternative patch
<https://people.redhat.com/jkratoch/regexlistambiguouscheck-module.patch> one
gets:
runCmd: frame variable int_array
output: (int [5]) int_array = ([0] = -1, [1] = 2, [2] = 3, [3] = 4, [4] = 5)
= no formatter applied
and on console:
error: a.out Two regexes ("int \[[0-9]+\]" and "int \[[0-9]\]") ambiguously
match the same string "int [5]".
But I think the alternative patch is worse than this one - for example maybe
the variable does not come from a file/module (can it really happen?) and then
there would be no way to print the error.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D66654
Files:
lldb/include/lldb/Core/ValueObject.h
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
lldb/source/DataFormatters/TypeCategory.cpp
Index: lldb/source/DataFormatters/TypeCategory.cpp
===================================================================
--- lldb/source/DataFormatters/TypeCategory.cpp
+++ lldb/source/DataFormatters/TypeCategory.cpp
@@ -101,9 +101,10 @@
lldb::TypeFormatImplSP &entry, uint32_t *reason) {
if (!IsEnabled() || !IsApplicable(valobj))
return false;
- if (GetTypeFormatsContainer()->Get(candidates, entry, reason))
+ if (GetTypeFormatsContainer()->Get(candidates, entry, reason, &valobj))
return true;
- bool regex = GetRegexTypeFormatsContainer()->Get(candidates, entry, reason);
+ bool regex =
+ GetRegexTypeFormatsContainer()->Get(candidates, entry, reason, &valobj);
if (regex && reason)
*reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
return regex;
@@ -114,9 +115,10 @@
lldb::TypeSummaryImplSP &entry, uint32_t *reason) {
if (!IsEnabled() || !IsApplicable(valobj))
return false;
- if (GetTypeSummariesContainer()->Get(candidates, entry, reason))
+ if (GetTypeSummariesContainer()->Get(candidates, entry, reason, &valobj))
return true;
- bool regex = GetRegexTypeSummariesContainer()->Get(candidates, entry, reason);
+ bool regex =
+ GetRegexTypeSummariesContainer()->Get(candidates, entry, reason, &valobj);
if (regex && reason)
*reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
return regex;
@@ -132,17 +134,19 @@
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))
+ if (!GetTypeFiltersContainer()->Get(candidates, filter_sp, &reason_filter,
+ &valobj))
regex_filter = GetRegexTypeFiltersContainer()->Get(candidates, filter_sp,
- &reason_filter);
+ &reason_filter, &valobj);
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 (!GetTypeSyntheticsContainer()->Get(candidates, synth, &reason_synth,
+ &valobj))
+ regex_synth = GetRegexTypeSyntheticsContainer()->Get(
+ candidates, synth, &reason_synth, &valobj);
if (!filter_sp.get() && !synth.get())
return false;
else if (!filter_sp.get() && synth.get())
@@ -174,10 +178,10 @@
lldb::TypeValidatorImplSP &entry, uint32_t *reason) {
if (!IsEnabled())
return false;
- if (GetTypeValidatorsContainer()->Get(candidates, entry, reason))
+ if (GetTypeValidatorsContainer()->Get(candidates, entry, reason, &valobj))
return true;
- bool regex =
- GetRegexTypeValidatorsContainer()->Get(candidates, entry, reason);
+ bool regex = GetRegexTypeValidatorsContainer()->Get(candidates, entry, reason,
+ &valobj);
if (regex && reason)
*reason |= lldb_private::eFormatterChoiceCriterionRegularExpressionSummary;
return regex;
@@ -295,7 +299,8 @@
TypeValidatorImpl::SharedPointer validator_sp;
if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue) {
- if (GetTypeFormatsContainer()->Get(type_name, format_sp)) {
+ if (GetTypeFormatsContainer()->Get(type_name, format_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -305,7 +310,8 @@
}
if ((items & eFormatCategoryItemRegexValue) ==
eFormatCategoryItemRegexValue) {
- if (GetRegexTypeFormatsContainer()->Get(type_name, format_sp)) {
+ if (GetRegexTypeFormatsContainer()->Get(type_name, format_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -315,7 +321,8 @@
}
if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary) {
- if (GetTypeSummariesContainer()->Get(type_name, summary_sp)) {
+ if (GetTypeSummariesContainer()->Get(type_name, summary_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -325,7 +332,8 @@
}
if ((items & eFormatCategoryItemRegexSummary) ==
eFormatCategoryItemRegexSummary) {
- if (GetRegexTypeSummariesContainer()->Get(type_name, summary_sp)) {
+ if (GetRegexTypeSummariesContainer()->Get(type_name, summary_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -335,7 +343,8 @@
}
if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter) {
- if (GetTypeFiltersContainer()->Get(type_name, filter_sp)) {
+ if (GetTypeFiltersContainer()->Get(type_name, filter_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -345,7 +354,8 @@
}
if ((items & eFormatCategoryItemRegexFilter) ==
eFormatCategoryItemRegexFilter) {
- if (GetRegexTypeFiltersContainer()->Get(type_name, filter_sp)) {
+ if (GetRegexTypeFiltersContainer()->Get(type_name, filter_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -355,7 +365,8 @@
}
if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth) {
- if (GetTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
+ if (GetTypeSyntheticsContainer()->Get(type_name, synth_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -365,7 +376,8 @@
}
if ((items & eFormatCategoryItemRegexSynth) ==
eFormatCategoryItemRegexSynth) {
- if (GetRegexTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
+ if (GetRegexTypeSyntheticsContainer()->Get(type_name, synth_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -375,7 +387,8 @@
}
if ((items & eFormatCategoryItemValidator) == eFormatCategoryItemValidator) {
- if (GetTypeValidatorsContainer()->Get(type_name, validator_sp)) {
+ if (GetTypeValidatorsContainer()->Get(type_name, validator_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
@@ -385,7 +398,8 @@
}
if ((items & eFormatCategoryItemRegexValidator) ==
eFormatCategoryItemRegexValidator) {
- if (GetRegexTypeValidatorsContainer()->Get(type_name, validator_sp)) {
+ if (GetRegexTypeValidatorsContainer()->Get(type_name, validator_sp,
+ nullptr /*valobj*/)) {
if (matching_category)
*matching_category = m_name.GetCString();
if (matching_type)
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-advanced/TestDataFormatterAdv.py
@@ -130,6 +130,10 @@
self.expect("frame variable int_array",
substrs=['1,2'])
+ # "int []" gets converted by FixArrayTypeNameWithRegex into "int \[[0-9]+\]"
+ # which becomes ambiguous for some string against "int \[[0-9]\]".
+ self.runCmd("type summary clear")
+
self.runCmd(
'type summary add --summary-string \"${var[0-1]}\" "int []"')
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===================================================================
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -204,8 +204,8 @@
return ret;
}
- bool Get(ConstString type, MapValueType &entry) {
- return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
+ bool Get(ConstString type, MapValueType &entry, ValueObject *valobj) {
+ return Get_Impl(type, entry, valobj, static_cast<KeyType *>(nullptr));
}
bool GetExact(ConstString type, MapValueType &entry) {
@@ -262,13 +262,15 @@
return false;
}
- bool Get_Impl(ConstString type, MapValueType &entry, ConstString *dummy) {
+ bool Get_Impl(ConstString type, MapValueType &entry, ValueObject *valobj,
+ ConstString *dummy) {
return m_format_map.Get(type, entry);
}
bool GetExact_Impl(ConstString type, MapValueType &entry,
ConstString *dummy) {
- return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
+ return Get_Impl(type, entry, nullptr /*valobj*/,
+ static_cast<KeyType *>(nullptr));
}
lldb::TypeNameSpecifierImplSP
@@ -291,19 +293,31 @@
new TypeNameSpecifierImpl(regex->GetText().str().c_str(), true));
}
- bool Get_Impl(ConstString key, MapValueType &value,
+ bool Get_Impl(ConstString key, MapValueType &value, ValueObject *valobj,
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) {
+ if (valobj)
+ valobj->SetError(Status("Two regexes (\"%s\" and \"%s\") "
+ "ambiguously match the same string \"%s\".",
+ found->first->GetText().str().c_str(),
+ regex->GetText().str().c_str(),
+ key.GetCString()));
+ return false;
+ }
+ found = pos;
}
}
- return false;
+ if (found == end)
+ return false;
+ value = found->second;
+ return true;
}
bool GetExact_Impl(ConstString key, MapValueType &value,
@@ -321,9 +335,9 @@
}
bool Get(const FormattersMatchVector &candidates, MapValueType &entry,
- uint32_t *reason) {
+ uint32_t *reason, ValueObject *valobj) {
for (const FormattersMatchCandidate &candidate : candidates) {
- if (Get(candidate.GetTypeName(), entry)) {
+ if (Get(candidate.GetTypeName(), entry, valobj)) {
if (candidate.IsMatch(entry) == false) {
entry.reset();
continue;
Index: lldb/include/lldb/Core/ValueObject.h
===================================================================
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -456,6 +456,8 @@
// The functions below should NOT be modified by subclasses
const Status &GetError();
+ void SetError(Status &&error) { m_error = std::move(error); }
+
ConstString GetName() const;
virtual lldb::ValueObjectSP GetChildAtIndex(size_t idx, bool can_create);
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
