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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits