ioeric added inline comments.
================
Comment at: clangd/Quality.h:71
+class SymbolRelevanceContext {
+ public:
----------------
sammccall wrote:
> This is ambiguously a couple of different (and good!) things:
> - an encapsulation of the proximitypaths state and logic
> - a grouping together of the "query-dependent, symbol-invariant" inputs to
> the relevance calculation.
>
> There's a place for both of these, but I'd argue for separating them (and
> only doing the second in this patch). Reasons:
> - the former doesn't need to be in this file if it gets complex
> (FuzzyMatch.h is a similar case), while the latter does
> - easier to understand/name if this hierarchy is expressed explicitly
> - I suspect we may want the context to be a separate struct, passed to
> SymbolRelevanceSignals::evaluate(), rather than a member of
> SymbolRelevanceSignals. That would add more churn than needs to be in this
> patch though.
>
> If this makes sense to you then I think this class looks great but should be
> called something specific like `FileProximityMatcher`.
Sounds good. Thanks for the explanation!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47935
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
