[PATCH] D79500: [clangd] Refactor code completion signal's utility properties.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang-tools-extra/clangd/FindSymbols.cpp:112
     SymbolRelevanceSignals Relevance;
-    Relevance.Name = Sym.Name;
     Relevance.Query = SymbolRelevanceSignals::Generic;
----------------
why this change?


================
Comment at: clang-tools-extra/clangd/Quality.h:129
 
+  // Properties and utilites used to compute derived signals. These are ignored
+  // by a scoring function. Must be explicitly assigned.
----------------
Why is it better to group the fields acconding to how they're used in the scoring function, rather than by what they mean?
(I find the new grouping harder to follow)


================
Comment at: clang-tools-extra/clangd/Quality.h:153
+    unsigned ScopeProximityDistance = FileDistance::Unreachable;
+  } Derived;
+
----------------
Can we make this Optional, so we can verify it gets computed? In fact, does it need to be a member at all, or can it just be transiently created while calling evaluate?


================
Comment at: clang-tools-extra/clangd/Quality.h:155
+
+  void calculateDerivedSignals();
+
----------------
why must this be called explicitly rather than being computed by Evaluate?


================
Comment at: clang-tools-extra/clangd/Quality.h.rej:1
+--- third_party/llvm/llvm-project/clang-tools-extra/clangd/Quality.h
++++ third_party/llvm/llvm-project/clang-tools-extra/clangd/Quality.h
----------------
Bad merge?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79500