ilya-biryukov updated this revision to Diff 149765. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment.
- Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46943 Files: clangd/Quality.cpp clangd/Quality.h unittests/clangd/QualityTests.cpp
Index: unittests/clangd/QualityTests.cpp =================================================================== --- unittests/clangd/QualityTests.cpp +++ unittests/clangd/QualityTests.cpp @@ -58,17 +58,46 @@ } TEST(QualityTests, SymbolRelevanceSignalExtraction) { - auto AST = TestTU::withHeaderCode(R"cpp( + TestTU Test; + Test.HeaderCode = R"cpp( + int test_func_in_header(); + int test_func_in_header_and_cpp(); + )cpp"; + Test.Code = R"cpp( + int ::test_func_in_header_and_cpp() { + } + int test_func_in_cpp(); + [[deprecated]] - int f() { return 0; } - )cpp") - .build(); - - SymbolRelevanceSignals Relevance; - Relevance.merge(CodeCompletionResult(&findDecl(AST, "f"), /*Priority=*/42, - nullptr, false, /*Accessible=*/false)); - EXPECT_EQ(Relevance.NameMatch, SymbolRelevanceSignals().NameMatch); - EXPECT_TRUE(Relevance.Forbidden); + int test_deprecated() { return 0; } + )cpp"; + auto AST = Test.build(); + + SymbolRelevanceSignals Deprecated; + Deprecated.merge(CodeCompletionResult(&findDecl(AST, "test_deprecated"), + /*Priority=*/42, nullptr, false, + /*Accessible=*/false)); + EXPECT_EQ(Deprecated.NameMatch, SymbolRelevanceSignals().NameMatch); + EXPECT_TRUE(Deprecated.Forbidden); + + // Test proximity scores. + SymbolRelevanceSignals FuncInCpp; + FuncInCpp.merge(CodeCompletionResult(&findDecl(AST, "test_func_in_cpp"), + CCP_Declaration)); + /// Decls in the current file should get a proximity score of 1.0. + EXPECT_FLOAT_EQ(FuncInCpp.ProximityScore, 1.0); + + SymbolRelevanceSignals FuncInHeader; + FuncInHeader.merge(CodeCompletionResult(&findDecl(AST, "test_func_in_header"), + CCP_Declaration)); + /// Decls outside current file currently don't get a proximity score boost. + EXPECT_FLOAT_EQ(FuncInHeader.ProximityScore, 0.0); + + SymbolRelevanceSignals FuncInHeaderAndCpp; + FuncInHeaderAndCpp.merge(CodeCompletionResult( + &findDecl(AST, "test_func_in_header_and_cpp"), CCP_Declaration)); + /// Decls in both header **and** the main file get the same boost. + EXPECT_FLOAT_EQ(FuncInHeaderAndCpp.ProximityScore, 1.0); } // Do the signals move the scores in the direction we expect? @@ -104,6 +133,10 @@ SymbolRelevanceSignals PoorNameMatch; PoorNameMatch.NameMatch = 0.2f; EXPECT_LT(PoorNameMatch.evaluate(), Default.evaluate()); + + SymbolRelevanceSignals WithProximity; + WithProximity.ProximityScore = 0.2; + EXPECT_LT(Default.evaluate(), WithProximity.evaluate()); } TEST(QualityTests, SortText) { Index: clangd/Quality.h =================================================================== --- clangd/Quality.h +++ clangd/Quality.h @@ -64,6 +64,9 @@ // 0-1 fuzzy-match score for unqualified name. Must be explicitly assigned. float NameMatch = 1; bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private). + /// Proximity between the best declaration and the query location. [0-1] score + /// where 1 is closest + float ProximityScore = 0; void merge(const CodeCompletionResult &SemaResult); Index: clangd/Quality.cpp =================================================================== --- clangd/Quality.cpp +++ clangd/Quality.cpp @@ -8,6 +8,8 @@ //===---------------------------------------------------------------------===// #include "Quality.h" #include "index/Index.h" +#include "clang/AST/ASTContext.h" +#include "clang/Basic/SourceManager.h" #include "clang/Sema/CodeCompleteConsumer.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/MathExtras.h" @@ -17,9 +19,18 @@ namespace clangd { using namespace llvm; +static bool hasDeclInMainFile(const Decl &D) { + auto &SourceMgr = D.getASTContext().getSourceManager(); + for (auto *Redecl : D.redecls()) { + auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation()); + if (SourceMgr.isWrittenInMainFile(Loc)) + return true; + } + return false; +} + void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) { SemaCCPriority = SemaCCResult.Priority; - if (SemaCCResult.Availability == CXAvailability_Deprecated) Deprecated = true; } @@ -60,12 +71,25 @@ if (SemaCCResult.Availability == CXAvailability_NotAvailable || SemaCCResult.Availability == CXAvailability_NotAccessible) Forbidden = true; + + if (SemaCCResult.Declaration) { + // We boost things that have decls in the main file. + // The real proximity scores would be more general when we have them. + float DeclProximity = + hasDeclInMainFile(*SemaCCResult.Declaration) ? 1.0 : 0.0; + ProximityScore = std::max(DeclProximity, ProximityScore); + } } float SymbolRelevanceSignals::evaluate() const { if (Forbidden) return 0; - return NameMatch; + + float Score = NameMatch; + // Proximity scores are [0,1] and we translate them into a multiplier in the + // range from 1 to 2. + Score *= 1 + ProximityScore; + return Score; } raw_ostream &operator<<(raw_ostream &OS, const SymbolRelevanceSignals &S) { OS << formatv("=== Symbol relevance: {0}\n", S.evaluate());
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits