[Lldb-commits] [PATCH] D124579: Make partial function name matching match on context boundaries

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yeah, this was very surprising.

The fact that we're still doing substring matches on the context is suspicious, and might cause false positives, but given that we first verify that the context is valid, I was not able to come up with counterexamples.



================
Comment at: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp:293-309
+  // Incoming path has context but this method does not, no match.
+  if (m_context.empty())
+    return false;
+
+  // Make sure the incoming path's context is found in the method context:
+  size_t path_pos = m_context.rfind(context);
+  if (path_pos == llvm::StringRef::npos)
----------------
```
StringRef haystack = m_context;
if (!haystack.consume_back(context))
  return false;
if (haystack.empty() || !isalnum(haystack.back()))
  return true;
return false;
```


================
Comment at: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp:151-155
+  TestCase test_cases_3[] {
+    {"func01", true},
+    {"func", false},
+    {"bar::func01", false},
+  };
----------------
what about these?


================
Comment at: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp:157
+
+  for (auto test : test_cases_1) {
+    EXPECT_EQ(reference_1.ContainsPath(test.path), test.result);
----------------
This saves very little compared to spelling out the individual checks (`EXPECT_TRUE/FALSE(reference_1.ContainsPath("whatever"))`), and makes the error messages a lot more cryptic.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124579/new/

https://reviews.llvm.org/D124579