ioeric added a comment.
PTAL
================
Comment at: clangd/Quality.cpp:215
void SymbolRelevanceSignals::merge(const CodeCompletionResult &SemaCCResult) {
if (SemaCCResult.Availability == CXAvailability_NotAvailable ||
----------------
ioeric wrote:
> sammccall wrote:
> > proximity path needs to be set here too
> Alternatively, I wonder if we could give sema result a fixed proximity score
> as they are symbols that are already included?
As discussed offline, sema symbols now have a fixed proximity score (not
entirely sure about the value though).
================
Comment at: clangd/Quality.h:74
+ // If set, this is used to compute proximity from symbol's declaring file.
+ llvm::StringRef ProximityPath;
/// Proximity between best declaration and the query. [0-1], 1 is closest.
----------------
ioeric wrote:
> ioeric wrote:
> > sammccall wrote:
> > > sammccall wrote:
> > > > It seems OK to have ProximityPath or ProximityScore, but we shouldn't
> > > > have both: drop proximityscore and calculate it during evaluate()?
> > > what's the plan for associated-header? should this be a smallvector<2>?
> > Just want to make sure I understand. We would copy the symbol URI to use in
> > `merge` right?
> I think it should be easy to change this to vector when it's actually needed?
Changed to vector anyway...
================
Comment at: clangd/Quality.h:74
+ // If set, this is used to compute proximity from symbol's declaring file.
+ llvm::StringRef ProximityPath;
/// Proximity between best declaration and the query. [0-1], 1 is closest.
----------------
ioeric wrote:
> ioeric wrote:
> > ioeric wrote:
> > > sammccall wrote:
> > > > sammccall wrote:
> > > > > It seems OK to have ProximityPath or ProximityScore, but we shouldn't
> > > > > have both: drop proximityscore and calculate it during evaluate()?
> > > > what's the plan for associated-header? should this be a smallvector<2>?
> > > Just want to make sure I understand. We would copy the symbol URI to use
> > > in `merge` right?
> > I think it should be easy to change this to vector when it's actually
> > needed?
> Changed to vector anyway...
Experimented with this a bit (removing ProximityScore). As we print the
proximity score for debugging, we would still want to keep the store.
Alternatively, I made the proximity paths a parameter of `merge` as we only use
them for index result anyway.
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
