llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: nerix (Nerixyz) <details> <summary>Changes</summary> To be able to have multiple formatters that match on the same type name but for different types, this adds a callback that can be specified, which is called with the match candidate. As [mentlerd pointed out](https://discourse.llvm.org/t/formatting-msvc-stl-types/86667/12?u=nerixyz), it's similar to passing `--recognizer-function`. This isn't implemented as another match type, because the `TypeMatcher` still needs to have _some_ name for [`GetMatchString`](https://github.com/llvm/llvm-project/blob/f1575de4c5de9268f92eea1641af755a477e4ee4/lldb/include/lldb/DataFormatters/FormattersContainer.h#L120-L126). The only use I currently see here is for the C++ STL, specifically libstdc++ and MSVC's STL. In https://github.com/llvm/llvm-project/pull/143177, I used this to add formatters for `std::string` and friends from MSVC's STL. One potential issue of adding the callback here is that the summary/synthetic will be cached for a type name - disregarding the callback. The scenario here would be two shared libraries that use a different type with the same name and two formatters (e.g. they use two different standard libraries). Once the first type was formatted, its formatter is cached. Now, the second type will always be formatted with the wrong formatter (the cached one), even though the callback for the cached one wouldn't match. For one, `--recognizer-function` has the same issue and secondly, I don't think this is really realistic. --- Full diff: https://github.com/llvm/llvm-project/pull/143748.diff 3 Files Affected: - (modified) lldb/include/lldb/DataFormatters/FormatClasses.h (+8-2) - (modified) lldb/include/lldb/DataFormatters/FormattersContainer.h (+37-21) - (modified) lldb/unittests/DataFormatter/FormattersContainerTest.cpp (+64) ``````````diff diff --git a/lldb/include/lldb/DataFormatters/FormatClasses.h b/lldb/include/lldb/DataFormatters/FormatClasses.h index 89d74649ecc64..16685594db663 100644 --- a/lldb/include/lldb/DataFormatters/FormatClasses.h +++ b/lldb/include/lldb/DataFormatters/FormatClasses.h @@ -149,13 +149,16 @@ class FormattersMatchData { CandidateLanguagesVector m_candidate_languages; }; +using CxxTypeCandidateMatchFn = bool(const FormattersMatchCandidate &); + class TypeNameSpecifierImpl { public: TypeNameSpecifierImpl() = default; TypeNameSpecifierImpl(llvm::StringRef name, - lldb::FormatterMatchType match_type) - : m_match_type(match_type) { + lldb::FormatterMatchType match_type, + CxxTypeCandidateMatchFn *match_fn = nullptr) + : m_match_type(match_type), m_match_fn(match_fn) { m_type.m_type_name = std::string(name); } @@ -188,6 +191,8 @@ class TypeNameSpecifierImpl { return CompilerType(); } + CxxTypeCandidateMatchFn *GetCxxMatchFunction() const { return m_match_fn; } + lldb::FormatterMatchType GetMatchType() { return m_match_type; } bool IsRegex() { return m_match_type == lldb::eFormatterMatchRegex; } @@ -200,6 +205,7 @@ class TypeNameSpecifierImpl { CompilerType m_compiler_type; }; TypeOrName m_type; + CxxTypeCandidateMatchFn *m_match_fn = nullptr; TypeNameSpecifierImpl(const TypeNameSpecifierImpl &) = delete; const TypeNameSpecifierImpl & diff --git a/lldb/include/lldb/DataFormatters/FormattersContainer.h b/lldb/include/lldb/DataFormatters/FormattersContainer.h index f7465fc65538d..2cf33347b3c53 100644 --- a/lldb/include/lldb/DataFormatters/FormattersContainer.h +++ b/lldb/include/lldb/DataFormatters/FormattersContainer.h @@ -50,6 +50,11 @@ class TypeMatcher { /// - eFormatterMatchCallback: run the function in m_name to decide if a type /// matches or not. lldb::FormatterMatchType m_match_type; + /// An optional additional check for a FormattersMatchCandidate. + /// + /// If set, the name/regex is still checked first and this function will be + /// called if the name matches. + CxxTypeCandidateMatchFn *m_match_fn = nullptr; // if the user tries to add formatters for, say, "struct Foo" those will not // match any type because of the way we strip qualifiers from typenames this @@ -86,32 +91,19 @@ class TypeMatcher { /// name specifier. TypeMatcher(lldb::TypeNameSpecifierImplSP type_specifier) : m_name(type_specifier->GetName()), - m_match_type(type_specifier->GetMatchType()) { + m_match_type(type_specifier->GetMatchType()), + m_match_fn(type_specifier->GetCxxMatchFunction()) { if (m_match_type == lldb::eFormatterMatchRegex) m_type_name_regex = RegularExpression(type_specifier->GetName()); } /// True iff this matches the given type. bool Matches(FormattersMatchCandidate candidate_type) const { - ConstString type_name = candidate_type.GetTypeName(); - switch (m_match_type) { - case lldb::eFormatterMatchExact: - return m_name == type_name || - StripTypeName(m_name) == StripTypeName(type_name); - case lldb::eFormatterMatchRegex: - return m_type_name_regex.Execute(type_name.GetStringRef()); - case lldb::eFormatterMatchCallback: - // CommandObjectType{Synth,Filter}Add tries to prevent the user from - // creating both a synthetic child provider and a filter for the same type - // in the same category, but we don't have a type object at that point, so - // it creates a dummy candidate without type or script interpreter. - // Skip callback matching in these cases. - if (candidate_type.GetScriptInterpreter()) - return candidate_type.GetScriptInterpreter()->FormatterCallbackFunction( - m_name.AsCString(), - std::make_shared<TypeImpl>(candidate_type.GetType())); - } - return false; + if (!MatchesName(candidate_type)) + return false; + if (m_match_fn && !m_match_fn(candidate_type)) + return false; + return true; } lldb::FormatterMatchType GetMatchType() const { return m_match_type; } @@ -134,7 +126,31 @@ class TypeMatcher { /// (lldb) type summary add --summary-string \"A\" -x TypeName /// (lldb) type summary delete TypeName bool CreatedBySameMatchString(TypeMatcher other) const { - return GetMatchString() == other.GetMatchString(); + return GetMatchString() == other.GetMatchString() && + m_match_fn == other.m_match_fn; + } + +private: + bool MatchesName(FormattersMatchCandidate candidate_type) const { + ConstString type_name = candidate_type.GetTypeName(); + switch (m_match_type) { + case lldb::eFormatterMatchExact: + return m_name == type_name || + StripTypeName(m_name) == StripTypeName(type_name); + case lldb::eFormatterMatchRegex: + return m_type_name_regex.Execute(type_name.GetStringRef()); + case lldb::eFormatterMatchCallback: + // CommandObjectType{Synth,Filter}Add tries to prevent the user from + // creating both a synthetic child provider and a filter for the same type + // in the same category, but we don't have a type object at that point, so + // it creates a dummy candidate without type or script interpreter. + // Skip callback matching in these cases. + if (candidate_type.GetScriptInterpreter()) + return candidate_type.GetScriptInterpreter()->FormatterCallbackFunction( + m_name.AsCString(), + std::make_shared<TypeImpl>(candidate_type.GetType())); + } + return false; } }; diff --git a/lldb/unittests/DataFormatter/FormattersContainerTest.cpp b/lldb/unittests/DataFormatter/FormattersContainerTest.cpp index 41b01adfb9ecd..a8e9c1a1cccc0 100644 --- a/lldb/unittests/DataFormatter/FormattersContainerTest.cpp +++ b/lldb/unittests/DataFormatter/FormattersContainerTest.cpp @@ -165,3 +165,67 @@ TEST(TypeMatcherTests, CreatedBySameMatchStringExactNamePrefixes) { EXPECT_TRUE(without_prefix.CreatedBySameMatchString(without_prefix)); } } + +TEST(TypeMatcherTests, CxxTypeCandidateMatchTest) { + auto alwaysTrue = +[](const FormattersMatchCandidate &) { return true; }; + auto alwaysFalse = +[](const FormattersMatchCandidate &) { return false; }; + + TypeMatcher regex_matcher_true(std::make_shared<TypeNameSpecifierImpl>( + "^a[a-z]c$", eFormatterMatchRegex, alwaysTrue)); + EXPECT_TRUE(regex_matcher_true.Matches(CandidateFromTypeName("abc"))); + EXPECT_TRUE(regex_matcher_true.Matches(CandidateFromTypeName("azc"))); + EXPECT_FALSE(regex_matcher_true.Matches(CandidateFromTypeName("a"))); + + TypeMatcher regex_matcher_false(std::make_shared<TypeNameSpecifierImpl>( + "^a[a-z]c$", eFormatterMatchRegex, alwaysFalse)); + EXPECT_FALSE(regex_matcher_false.Matches(CandidateFromTypeName("abc"))); + EXPECT_FALSE(regex_matcher_false.Matches(CandidateFromTypeName("azc"))); + EXPECT_FALSE(regex_matcher_false.Matches(CandidateFromTypeName("a"))); + + TypeMatcher string_matcher_true(std::make_shared<TypeNameSpecifierImpl>( + "abc", eFormatterMatchExact, alwaysTrue)); + EXPECT_TRUE(string_matcher_true.Matches(CandidateFromTypeName("abc"))); + EXPECT_FALSE(string_matcher_true.Matches(CandidateFromTypeName("azc"))); + EXPECT_FALSE(string_matcher_true.Matches(CandidateFromTypeName("a"))); + + TypeMatcher string_matcher_false(std::make_shared<TypeNameSpecifierImpl>( + "abc", eFormatterMatchExact, alwaysFalse)); + EXPECT_FALSE(string_matcher_false.Matches(CandidateFromTypeName("abc"))); + EXPECT_FALSE(string_matcher_false.Matches(CandidateFromTypeName("azc"))); + EXPECT_FALSE(string_matcher_false.Matches(CandidateFromTypeName("a"))); + + EXPECT_TRUE(regex_matcher_true.CreatedBySameMatchString(regex_matcher_true)); + EXPECT_FALSE( + regex_matcher_true.CreatedBySameMatchString(regex_matcher_false)); + EXPECT_FALSE( + regex_matcher_true.CreatedBySameMatchString(string_matcher_true)); + EXPECT_FALSE( + regex_matcher_true.CreatedBySameMatchString(string_matcher_false)); + + EXPECT_FALSE( + regex_matcher_false.CreatedBySameMatchString(regex_matcher_true)); + EXPECT_TRUE( + regex_matcher_false.CreatedBySameMatchString(regex_matcher_false)); + EXPECT_FALSE( + regex_matcher_false.CreatedBySameMatchString(string_matcher_true)); + EXPECT_FALSE( + regex_matcher_false.CreatedBySameMatchString(string_matcher_false)); + + EXPECT_FALSE( + string_matcher_true.CreatedBySameMatchString(regex_matcher_true)); + EXPECT_FALSE( + string_matcher_true.CreatedBySameMatchString(regex_matcher_false)); + EXPECT_TRUE( + string_matcher_true.CreatedBySameMatchString(string_matcher_true)); + EXPECT_FALSE( + string_matcher_true.CreatedBySameMatchString(string_matcher_false)); + + EXPECT_FALSE( + string_matcher_false.CreatedBySameMatchString(regex_matcher_true)); + EXPECT_FALSE( + string_matcher_false.CreatedBySameMatchString(regex_matcher_false)); + EXPECT_FALSE( + string_matcher_false.CreatedBySameMatchString(string_matcher_true)); + EXPECT_TRUE( + string_matcher_false.CreatedBySameMatchString(string_matcher_false)); +} `````````` </details> https://github.com/llvm/llvm-project/pull/143748 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
