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