[PATCH] D94755: [clangd] Fix division by zero when computing scores

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

NameMatch could be a float close to zero, in such cases we were
dividing by zero and moreover propogating a "NaN" to clients, which is invalid
per JSON.

This fixes the issue by only using Quality scores whenever the NameMatch is low,
as we do in CodeCompletion ranking.

Fixes https://github.com/clangd/clangd/issues/648.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94755

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/FindSymbols.cpp


Index: clang-tools-extra/clangd/FindSymbols.cpp
===================================================================
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -25,6 +25,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include <limits>
 #include <tuple>
 
 #define DEBUG_TYPE "FindSymbols"
@@ -146,8 +147,9 @@
       return;
     }
     Relevance.merge(Sym);
-    auto Score = evaluateSymbolAndRelevance(Quality.evaluateHeuristics(),
-                                            Relevance.evaluateHeuristics());
+    auto QualScore = Quality.evaluateHeuristics();
+    auto RelScore = Relevance.evaluateHeuristics();
+    auto Score = evaluateSymbolAndRelevance(QualScore, RelScore);
     dlog("FindSymbols: {0}{1} = {2}\n{3}{4}\n", Sym.Scope, Sym.Name, Score,
          Quality, Relevance);
 
@@ -159,7 +161,9 @@
     Info.containerName = Scope.str();
 
     // Exposed score excludes fuzzy-match component, for client-side 
re-ranking.
-    Info.score = Score / Relevance.NameMatch;
+    Info.score = Relevance.NameMatch > std::numeric_limits<float>::epsilon()
+                     ? Score / Relevance.NameMatch
+                     : QualScore;
     Top.push({Score, std::move(Info)});
   });
   for (auto &R : std::move(Top).items())
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -70,6 +70,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include <algorithm>
 #include <iterator>
+#include <limits>
 
 // We log detailed candidate here if you run with -debug-only=codecomplete.
 #define DEBUG_TYPE "CodeComplete"
@@ -1655,9 +1656,10 @@
           evaluateSymbolAndRelevance(Scores.Quality, Scores.Relevance);
       // NameMatch is in fact a multiplier on total score, so rescoring is
       // sound.
-      Scores.ExcludingName = Relevance.NameMatch
-                                 ? Scores.Total / Relevance.NameMatch
-                                 : Scores.Quality;
+      Scores.ExcludingName =
+          Relevance.NameMatch > std::numeric_limits<float>::epsilon()
+              ? Scores.Total / Relevance.NameMatch
+              : Scores.Quality;
       return Scores;
 
     case RM::DecisionForest:



Index: clang-tools-extra/clangd/FindSymbols.cpp
===================================================================
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -25,6 +25,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include <limits>
 #include <tuple>
 
 #define DEBUG_TYPE "FindSymbols"
@@ -146,8 +147,9 @@
       return;
     }
     Relevance.merge(Sym);
-    auto Score = evaluateSymbolAndRelevance(Quality.evaluateHeuristics(),
-                                            Relevance.evaluateHeuristics());
+    auto QualScore = Quality.evaluateHeuristics();
+    auto RelScore = Relevance.evaluateHeuristics();
+    auto Score = evaluateSymbolAndRelevance(QualScore, RelScore);
     dlog("FindSymbols: {0}{1} = {2}\n{3}{4}\n", Sym.Scope, Sym.Name, Score,
          Quality, Relevance);
 
@@ -159,7 +161,9 @@
     Info.containerName = Scope.str();
 
     // Exposed score excludes fuzzy-match component, for client-side re-ranking.
-    Info.score = Score / Relevance.NameMatch;
+    Info.score = Relevance.NameMatch > std::numeric_limits<float>::epsilon()
+                     ? Score / Relevance.NameMatch
+                     : QualScore;
     Top.push({Score, std::move(Info)});
   });
   for (auto &R : std::move(Top).items())
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -70,6 +70,7 @@
 #include "llvm/Support/ScopedPrinter.h"
 #include <algorithm>
 #include <iterator>
+#include <limits>
 
 // We log detailed candidate here if you run with -debug-only=codecomplete.
 #define DEBUG_TYPE "CodeComplete"
@@ -1655,9 +1656,10 @@
           evaluateSymbolAndRelevance(Scores.Quality, Scores.Relevance);
       // NameMatch is in fact a multiplier on total score, so rescoring is
       // sound.
-      Scores.ExcludingName = Relevance.NameMatch
-                                 ? Scores.Total / Relevance.NameMatch
-                                 : Scores.Quality;
+      Scores.ExcludingName =
+          Relevance.NameMatch > std::numeric_limits<float>::epsilon()
+              ? Scores.Total / Relevance.NameMatch
+              : Scores.Quality;
       return Scores;
 
     case RM::DecisionForest:

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits