sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
I think the actual rules implemented here are probably *too* relaxed, and we
should require the first character to match the start of a word segment.
But I'm not sure and this was easier to write, will try it out.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D61495
Files:
clangd/CodeComplete.cpp
clangd/FuzzyMatch.cpp
clangd/FuzzyMatch.h
clangd/unittests/CodeCompleteTests.cpp
clangd/unittests/FuzzyMatchTests.cpp
Index: clangd/unittests/FuzzyMatchTests.cpp
===================================================================
--- clangd/unittests/FuzzyMatchTests.cpp
+++ clangd/unittests/FuzzyMatchTests.cpp
@@ -45,9 +45,11 @@
struct MatchesMatcher : public testing::MatcherInterface<llvm::StringRef> {
ExpectedMatch Candidate;
+ FuzzyMatcher::MatchStyle Style;
llvm::Optional<float> Score;
- MatchesMatcher(ExpectedMatch Candidate, llvm::Optional<float> Score)
- : Candidate(std::move(Candidate)), Score(Score) {}
+ MatchesMatcher(ExpectedMatch Candidate, FuzzyMatcher::MatchStyle Style,
+ llvm::Optional<float> Score)
+ : Candidate(std::move(Candidate)), Style(Style), Score(Score) {}
void DescribeTo(::std::ostream *OS) const override {
llvm::raw_os_ostream(*OS) << "Matches " << Candidate;
@@ -61,7 +63,7 @@
L->stream()
? (llvm::raw_ostream *)(new llvm::raw_os_ostream(*L->stream()))
: new llvm::raw_null_ostream());
- FuzzyMatcher Matcher(Pattern);
+ FuzzyMatcher Matcher(Pattern, Style);
auto Result = Matcher.match(Candidate.Word);
auto AnnotatedMatch = Matcher.dumpLast(*OS << "\n");
return Result && Candidate.accepts(AnnotatedMatch) &&
@@ -71,9 +73,12 @@
// Accepts patterns that match a given word, optionally requiring a score.
// Dumps the debug tables on match failure.
-testing::Matcher<llvm::StringRef> matches(llvm::StringRef M,
- llvm::Optional<float> Score = {}) {
- return testing::MakeMatcher<llvm::StringRef>(new MatchesMatcher(M, Score));
+testing::Matcher<llvm::StringRef>
+matches(llvm::StringRef M,
+ FuzzyMatcher::MatchStyle Style = FuzzyMatcher::Strict,
+ llvm::Optional<float> Score = {}) {
+ return testing::MakeMatcher<llvm::StringRef>(
+ new MatchesMatcher(M, Style, Score));
}
TEST(FuzzyMatch, Matches) {
@@ -179,6 +184,15 @@
EXPECT_THAT("std", Not(matches("pthread_condattr_setpshared")));
}
+TEST(FuzzyMatch, MatchesLax) {
+ EXPECT_THAT("", matches("unique_ptr", FuzzyMatcher::Lax));
+ EXPECT_THAT("u_p", matches("[u]nique[_p]tr", FuzzyMatcher::Lax));
+ EXPECT_THAT("up", matches("[u]nique_[p]tr", FuzzyMatcher::Lax));
+ EXPECT_THAT("uq", matches("[u]ni[q]ue_ptr", FuzzyMatcher::Lax));
+ EXPECT_THAT("qp", matches("uni[q]ue_[p]tr", FuzzyMatcher::Lax));
+ EXPECT_THAT("log", matches("SVGFEMorpho[log]yElement", FuzzyMatcher::Lax));
+}
+
struct RankMatcher : public testing::MatcherInterface<llvm::StringRef> {
std::vector<ExpectedMatch> RankedStrings;
RankMatcher(std::initializer_list<ExpectedMatch> RankedStrings)
@@ -272,16 +286,17 @@
// Verify some bounds so we know scores fall in the right range.
// Testing exact scores is fragile, so we prefer Ranking tests.
TEST(FuzzyMatch, Scoring) {
- EXPECT_THAT("abs", matches("[a]w[B]xYz[S]", 7.f / 12.f));
- EXPECT_THAT("abs", matches("[abs]l", 1.f));
- EXPECT_THAT("abs", matches("[abs]", 2.f));
- EXPECT_THAT("Abs", matches("[abs]", 2.f));
+ EXPECT_THAT("abs",
+ matches("[a]w[B]xYz[S]", FuzzyMatcher::Strict, 7.f / 12.f));
+ EXPECT_THAT("abs", matches("[abs]l", FuzzyMatcher::Strict, 1.f));
+ EXPECT_THAT("abs", matches("[abs]", FuzzyMatcher::Strict, 2.f));
+ EXPECT_THAT("Abs", matches("[abs]", FuzzyMatcher::Strict, 2.f));
}
TEST(FuzzyMatch, InitialismAndPrefix) {
// We want these scores to be roughly the same.
- EXPECT_THAT("up", matches("[u]nique_[p]tr", 3.f / 4.f));
- EXPECT_THAT("up", matches("[up]per_bound", 1.f));
+ EXPECT_THAT("up", matches("[u]nique_[p]tr", FuzzyMatcher::Strict, 3.f / 4.f));
+ EXPECT_THAT("up", matches("[up]per_bound", FuzzyMatcher::Strict, 1.f));
}
// Returns pretty-printed segmentation of Text.
Index: clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clangd/unittests/CodeCompleteTests.cpp
+++ clangd/unittests/CodeCompleteTests.cpp
@@ -202,6 +202,37 @@
AllOf(Has("Car"), Not(Has("MotorCar"))));
}
+TEST(CompletionTest, FilterStrictLax) {
+ // qry matches "query" in lax mode, but not in strict mode.
+ std::string Body = R"cpp(
+ #define QUERY_MACRO
+ int queryVariable;
+ void queryFunction();
+ struct QueryClass {
+ void queryBaseMethod();
+ };
+ class QueryEnclosingClass : QueryClass {
+ int QueryField = 42;
+ void queryMethod();
+ void queryEnclosingMethod() {
+ int queryLocalVariable;
+ qry^
+ }
+ };
+ )cpp";
+ auto Results = completions(Body).Completions;
+ EXPECT_THAT(Results, Not(Has("QUERY_MACRO")));
+ EXPECT_THAT(Results, Not(Has("queryVariable")));
+ EXPECT_THAT(Results, Not(Has("queryFunction")));
+ EXPECT_THAT(Results, Not(Has("QueryClass")));
+ EXPECT_THAT(Results, Not(Has("queryBaseMethod")));
+ EXPECT_THAT(Results, Not(Has("QueryEnclosingClass")));
+ EXPECT_THAT(Results, Has("QueryField"));
+ EXPECT_THAT(Results, Has("queryMethod"));
+ EXPECT_THAT(Results, Has("queryEnclosingMethod"));
+ EXPECT_THAT(Results, Has("queryLocalVariable"));
+}
+
void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
auto Results = completions(
R"cpp(
Index: clangd/FuzzyMatch.h
===================================================================
--- clangd/FuzzyMatch.h
+++ clangd/FuzzyMatch.h
@@ -70,8 +70,13 @@
// It's optimized for matching against many strings - match() does not allocate.
class FuzzyMatcher {
public:
+ // Controls which matches are accepted (but not how they're scored).
+ enum MatchStyle {
+ Strict, // Word boundaries matter. [pat] matches PartTwo but not PopTart.
+ Lax, // Case-insensitive substring match. [sv] matches IsEven.
+ };
// Characters beyond MaxPat are ignored.
- FuzzyMatcher(llvm::StringRef Pattern);
+ FuzzyMatcher(llvm::StringRef Pattern, MatchStyle Style = Strict);
// If Word matches the pattern, return a score indicating the quality match.
// Scores usually fall in a [0,1] range, with 1 being a very good score.
@@ -111,6 +116,7 @@
CharRole PatRole[MaxPat]; // Pattern segmentation info
CharTypeSet PatTypeSet; // Bitmask of 1<<CharType for all Pattern characters
float ScoreScale; // Normalizes scores for the pattern length.
+ bool StrictMatch; // Poor substring matches are reject.
// Word data is initialized on each call to match(), mostly by init().
char Word[MaxWord]; // Word data
Index: clangd/FuzzyMatch.cpp
===================================================================
--- clangd/FuzzyMatch.cpp
+++ clangd/FuzzyMatch.cpp
@@ -73,9 +73,10 @@
static bool isAwful(int S) { return S < AwfulScore / 2; }
static constexpr int PerfectBonus = 4; // Perfect per-pattern-char score.
-FuzzyMatcher::FuzzyMatcher(llvm::StringRef Pattern)
+FuzzyMatcher::FuzzyMatcher(llvm::StringRef Pattern, MatchStyle Style)
: PatN(std::min<int>(MaxPat, Pattern.size())),
- ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0), WordN(0) {
+ ScoreScale(PatN ? float{1} / (PerfectBonus * PatN) : 0),
+ StrictMatch(Style == Strict), WordN(0) {
std::copy(Pattern.begin(), Pattern.begin() + PatN, Pat);
for (int I = 0; I < PatN; ++I)
LowPat[I] = lower(Pat[I]);
@@ -254,7 +255,7 @@
// We require a "strong" match:
// - for the first pattern character. [foo] !~ "barefoot"
// - after a gap. [pat] !~ "patnther"
- if (Last == Miss) {
+ if (StrictMatch && Last == Miss) {
// We're banning matches outright, so conservatively accept some other cases
// where our segmentation might be wrong:
// - allow matching B in ABCDef (but not in NDEBUG)
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -144,6 +144,12 @@
llvm_unreachable("Unhandled CodeCompletionResult::ResultKind.");
}
+bool isConstructor(const NamedDecl *ND) {
+ if (const auto *Tmpl = dyn_cast<FunctionTemplateDecl>(ND))
+ ND = Tmpl->getTemplatedDecl();
+ return isa<CXXConstructorDecl>(ND);
+}
+
/// Get the optional chunk as a string. This function is possibly recursive.
///
/// The parameter info for each parameter is appended to the Parameters.
@@ -259,6 +265,21 @@
return RankedIncludeHeaders[0];
}
+ // Most results can be spammy, so we apply pretty strict matching rules.
+ // A few are always good though, like local variables.
+ bool couldBeSpam() const {
+ if (SemaResult && SemaResult->Declaration) {
+ if (const NamedDecl *ND = dyn_cast<NamedDecl>(SemaResult->Declaration))
+ if (ND->isCXXClassMember() && !SemaResult->InBaseClass &&
+ !isConstructor(ND))
+ return false;
+ if (const VarDecl* VD = dyn_cast<VarDecl>(SemaResult->Declaration))
+ if (VD->isLocalVarDeclOrParm())
+ return false;
+ }
+ return true;
+ }
+
using Bundle = llvm::SmallVector<CompletionCandidate, 4>;
};
using ScoredBundle =
@@ -1208,6 +1229,7 @@
bool Incomplete = false; // Would more be available with a higher limit?
CompletionPrefix HeuristicPrefix;
llvm::Optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
+ llvm::Optional<FuzzyMatcher> FilterLax;
Range ReplacedRange;
std::vector<std::string> QueryScopes; // Initialized once Sema runs.
// Initialized once QueryScopes is initialized, if there are scopes.
@@ -1401,6 +1423,7 @@
}
Filter = FuzzyMatcher(
Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
+ FilterLax = FuzzyMatcher(Filter->pattern(), FuzzyMatcher::Lax);
std::tie(QueryScopes, AllScopes) = getQueryScopes(
Recorder->CCContext, *Recorder->CCSema, HeuristicPrefix, Opts);
if (!QueryScopes.empty())
@@ -1554,6 +1577,8 @@
if (C.SemaResult && C.SemaResult->Kind == CodeCompletionResult::RK_Macro &&
!C.Name.startswith_lower(Filter->pattern()))
return None;
+ if (FilterLax && !C.couldBeSpam())
+ return FilterLax->match(C.Name);
return Filter->match(C.Name);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
