[lldb][Breakpoint] Fix setting breakpoints on templates by basename

This patch fixes a regression with setting breakpoints on template
functions by name. E.g.,:
```
$ cat main.cpp
template<typename T>
struct Foo {
  template<typename U>
  void func() {}
};

int main() {
  Foo<int> f;
  f.func<double>();
}

(lldb) br se -n func
```

This has regressed since `3339000e0bda696c2e29173d15958c0a4978a143`
where we started using the `CPlusPlusNameParser` for getting the
basename of the function symbol and match it exactly against
the name in the breakpoint command. The parser will include template
parameters in the basename, so the exact match will always fail

**Testing**

* Added API tests
* Added unit-tests

Differential Revision: https://reviews.llvm.org/D135921

diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 0339d38..d4dfc4c 100644 (file)
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -269,9 +269,21 @@ std::string CPlusPlusLanguage::MethodName::GetScopeQualifiedName() {
   return res;
 }
 
+llvm::StringRef
+CPlusPlusLanguage::MethodName::GetBasenameNoTemplateParameters() {
+  llvm::StringRef basename = GetBasename();
+  size_t arg_start, arg_end;
+  llvm::StringRef parens("<>", 2);
+  if (ReverseFindMatchingChars(basename, parens, arg_start, arg_end))
+    return basename.substr(0, arg_start);
+
+  return basename;
+}
+
 bool CPlusPlusLanguage::MethodName::ContainsPath(llvm::StringRef path) {
   if (!m_parsed)
     Parse();
+
   // If we can't parse the incoming name, then just check that it contains path.
   if (m_parse_error)
     return m_full.GetStringRef().contains(path);
@@ -286,8 +298,23 @@ bool CPlusPlusLanguage::MethodName::ContainsPath(llvm::StringRef path) {
   if (!success)
     return m_full.GetStringRef().contains(path);
 
-  if (identifier != GetBasename())
+  // Basename may include template arguments.
+  // E.g.,
+  // GetBaseName(): func<int>
+  // identifier   : func
+  //
+  // ...but we still want to account for identifiers with template parameter
+  // lists, e.g., when users set breakpoints on template specializations.
+  //
+  // E.g.,
+  // GetBaseName(): func<uint32_t>
+  // identifier   : func<int32_t*>
+  //
+  // Try to match the basename with or without template parameters.
+  if (GetBasename() != identifier &&
+      GetBasenameNoTemplateParameters() != identifier)
     return false;
+
   // Incoming path only had an identifier, so we match.
   if (context.empty())
     return true;

diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
index 53a01cf..4d4226a 100644 (file)
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -58,6 +58,21 @@ public:
     
     bool ContainsPath(llvm::StringRef path);
 
+  private:
+    /// Returns the Basename of this method without a template parameter
+    /// list, if any.
+    ///
+    // Examples:
+    //
+    //   +--------------------------------+---------+
+    //   | MethodName                     | Returns |
+    //   +--------------------------------+---------+
+    //   | void func()                    | func    |
+    //   | void func<int>()               | func    |
+    //   | void func<std::vector<int>>()  | func    |
+    //   +--------------------------------+---------+
+    llvm::StringRef GetBasenameNoTemplateParameters();
+
   protected:
     void Parse();
     bool TrySimplifiedParse();

diff --git a/lldb/test/API/functionalities/breakpoint/cpp/Makefile b/lldb/test/API/functionalities/breakpoint/cpp/Makefile
index 2c00681..66108b7 100644 (file)
--- a/lldb/test/API/functionalities/breakpoint/cpp/Makefile
+++ b/lldb/test/API/functionalities/breakpoint/cpp/Makefile
@@ -1,4 +1,5 @@
 CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++14
 
 ifneq (,$(findstring icc,$(CC)))
     CXXFLAGS_EXTRAS := -debug inline-debug-info

diff --git a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
index 8296ae6..6c86f50 100644 (file)
--- a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
+++ b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
@@ -54,6 +54,23 @@ class TestCPPBreakpointLocations(TestBase):
             {'name': 'a::c::func1()', 'loc_names': ['a::c::func1()']},
             {'name': 'b::c::func1()', 'loc_names': ['b::c::func1()']},
             {'name': 'c::d::func2()', 'loc_names': ['c::d::func2()']},
+
+            # Template cases
+            {'name': 'func<float>', 'loc_names': []},
+            {'name': 'func<int>', 'loc_names': ['auto ns::Foo<double>::func<int>()']},
+            {'name': 'func', 'loc_names': ['auto ns::Foo<double>::func<int>()',
+                                           'auto ns::Foo<double>::func<ns::Foo<int>>()']},
+
+            {'name': 'operator', 'loc_names': []},
+            {'name': 'ns::Foo<double>::operator bool', 'loc_names': ['ns::Foo<double>::operator bool()']},
+
+            {'name': 'operator a::c', 'loc_names': ['ns::Foo<double>::operator a::c<a::c>()']},
+            {'name': 'operator ns::Foo<int>', 'loc_names': ['ns::Foo<double>::operator ns::Foo<int><ns::Foo<int>>()']},
+
+            {'name': 'operator<<<a::c>', 'loc_names': []},
+            {'name': 'operator<<<int>', 'loc_names': ['void ns::Foo<double>::operator<<<int>(int)']},
+            {'name': 'ns::Foo<double>::operator<<', 'loc_names': ['void ns::Foo<double>::operator<<<int>(int)',
+                                                                  'void ns::Foo<double>::operator<<<ns::Foo<int>>(ns::Foo<int>)']},
         ]
 
         for bp_dict in bp_dicts:

diff --git a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp
index 088e33c..7ee61e9 100644 (file)
--- a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp
+++ b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp
@@ -82,6 +82,20 @@ namespace c {
     };
 }
 
+namespace ns {
+template <typename Type> struct Foo {
+  template <typename T> void import() {}
+
+  template <typename T> auto func() {}
+
+  operator bool() { return true; }
+
+  template <typename T> operator T() { return {}; }
+
+  template <typename T> void operator<<(T t) {}
+};
+} // namespace ns
+
 int main (int argc, char const *argv[])
 {
     a::c ac;
@@ -98,5 +112,16 @@ int main (int argc, char const *argv[])
     bc.func3();
     cd.func2();
     cd.func3();
+
+    ns::Foo<double> f;
+    f.import <int>();
+    f.func<int>();
+    f.func<ns::Foo<int>>();
+    f.operator bool();
+    f.operator a::c();
+    f.operator ns::Foo<int>();
+    f.operator<<(5);
+    f.operator<< <ns::Foo<int>>({});
+
     return 0;
 }

diff --git a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
index 9a7e9a7..85cf808 100644 (file)
--- a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -143,7 +143,12 @@ TEST(CPlusPlusLanguage, ContainsPath) {
   CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()"));
   CPlusPlusLanguage::MethodName 
       reference_4(ConstString("bar::baz::operator bool()"));
-  
+  CPlusPlusLanguage::MethodName reference_5(
+      ConstString("bar::baz::operator bool<int, Type<double>>()"));
+  CPlusPlusLanguage::MethodName reference_6(ConstString(
+      "bar::baz::operator<<<Type<double>, Type<std::vector<double>>>()"));
+
+  EXPECT_TRUE(reference_1.ContainsPath(""));
   EXPECT_TRUE(reference_1.ContainsPath("func01"));
   EXPECT_TRUE(reference_1.ContainsPath("bar::func01"));
   EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01"));
@@ -153,17 +158,35 @@ TEST(CPlusPlusLanguage, ContainsPath) {
   EXPECT_FALSE(reference_1.ContainsPath("::foo::baz::func01"));
   EXPECT_FALSE(reference_1.ContainsPath("foo::bar::baz::func01"));
   
+  EXPECT_TRUE(reference_2.ContainsPath(""));
   EXPECT_TRUE(reference_2.ContainsPath("foofoo::bar::func01"));
   EXPECT_FALSE(reference_2.ContainsPath("foo::bar::func01"));
   
+  EXPECT_TRUE(reference_3.ContainsPath(""));
   EXPECT_TRUE(reference_3.ContainsPath("func01"));
   EXPECT_FALSE(reference_3.ContainsPath("func"));
   EXPECT_FALSE(reference_3.ContainsPath("bar::func01"));
 
+  EXPECT_TRUE(reference_4.ContainsPath(""));
+  EXPECT_TRUE(reference_4.ContainsPath("operator"));
   EXPECT_TRUE(reference_4.ContainsPath("operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("baz::operator bool"));
   EXPECT_TRUE(reference_4.ContainsPath("bar::baz::operator bool"));
   EXPECT_FALSE(reference_4.ContainsPath("az::operator bool"));
+
+  EXPECT_TRUE(reference_5.ContainsPath(""));
+  EXPECT_TRUE(reference_5.ContainsPath("operator"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool"));
+  EXPECT_TRUE(reference_5.ContainsPath("operator bool<int, Type<double>>"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool<int, double>"));
+  EXPECT_FALSE(reference_5.ContainsPath("operator bool<int, Type<int>>"));
+
+  EXPECT_TRUE(reference_6.ContainsPath(""));
+  EXPECT_TRUE(reference_6.ContainsPath("operator"));
+  EXPECT_TRUE(reference_6.ContainsPath("operator<<"));
+  EXPECT_TRUE(reference_6.ContainsPath(
+      "bar::baz::operator<<<Type<double>, Type<std::vector<double>>>()"));
+  EXPECT_FALSE(reference_6.ContainsPath("operator<<<Type<double>>"));
 }
 
 TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {