ilya-biryukov added inline comments.
Comment at: unittests/clangd/QualityTests.cpp:129
+ Test.Code = R"cpp(
+#include "foo.h"
+int ::test_func_in_header_and_cpp() {
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > this is not needed,
sammccall added inline comments.
Comment at: unittests/clangd/QualityTests.cpp:129
+ Test.Code = R"cpp(
+#include "foo.h"
+int ::test_func_in_header_and_cpp() {
ilya-biryukov wrote:
> sammccall wrote:
> > this is not needed, the `#include` is implicit
sammccall added a comment.
oops, just saw this is closed - will fix these nits as I'm touching this file
soon
Comment at: clangd/Quality.h:68
+ /// Proximity between the best declaration and the query location. [0-1]
score
+ /// where 1 is closest
+ float ProximityScore =
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE333906: [clangd] Boost scores for decls from current file
in completion (authored by ibiryukov, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46943?vs=149765=149771#toc
ilya-biryukov added inline comments.
Comment at: clangd/Quality.h:52
unsigned References = 0;
+ float ProximityScore = 0.0; /// Proximity score, in a [0,1] interval.
sammccall wrote:
> this belongs in `SymbolRelevanceSignals` rather than this struct. In
>
ilya-biryukov updated this revision to Diff 149765.
ilya-biryukov marked 5 inline comments as done.
ilya-biryukov added a comment.
- Address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46943
Files:
clangd/Quality.cpp
clangd/Quality.h
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Nice, simple and will admit refinements later.
Just test nits and a trivial organizational thing.
Comment at: clangd/Quality.cpp:22
+namespace {
+bool
ilya-biryukov updated this revision to Diff 148381.
ilya-biryukov added a comment.
- Simplify the change by boosting any Decls that have a redecl in the current
file
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46943
Files:
clangd/Quality.cpp
clangd/Quality.h
ilya-biryukov added a comment.
After an offline chat: we decided to boost sema results that have **any** decls
in the main file. Even though it introduces some unwanted inconsistencies in
some cases (e.g. see the comment), most of us agree that's a very simple
implementation that does boost
ilya-biryukov added a comment.
In https://reviews.llvm.org/D46943#1109199, @ioeric wrote:
> > - Symbols store their paths as URIs ⇒ we need to parse them in order to
> > apply heuristics. We could avoid that by writing a version of
> > header-matching that also works on URIs, but that would
ioeric added a comment.
In https://reviews.llvm.org/D46943#1107880, @ilya-biryukov wrote:
> I've added an initial version of testing for the matching header and wanted
> to get feedback before proceeding further with tests and other changes.
>
> A few things that bug me so far:
>
> - We need to
ilya-biryukov added a comment.
I've added an initial version of testing for the matching header and wanted to
get feedback before proceeding further with tests and other changes.
A few things that bug me so far:
- We need to match headers of items from the index, not only from the Sema
ilya-biryukov updated this revision to Diff 148006.
ilya-biryukov added a comment.
Herald added a subscriber: mgorny.
- Move helpers tha switch header and source into separate files.
- Also uprank items coming from the matching headers
Repository:
rCTE Clang Tools Extra
ilya-biryukov added inline comments.
Comment at: clangd/Quality.cpp:24
+ if (SemaCCResult.Declaration)
+AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration);
if (SemaCCResult.Availability == CXAvailability_Deprecated)
ioeric wrote:
> ioeric
ioeric added inline comments.
Comment at: clangd/Quality.cpp:24
+ if (SemaCCResult.Declaration)
+AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration);
if (SemaCCResult.Availability == CXAvailability_Deprecated)
ioeric wrote:
> ilya-biryukov
ioeric added inline comments.
Comment at: clangd/Quality.cpp:24
+ if (SemaCCResult.Declaration)
+AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration);
if (SemaCCResult.Availability == CXAvailability_Deprecated)
ilya-biryukov wrote:
>
ilya-biryukov added inline comments.
Comment at: clangd/AST.h:32
+/// Returns true iff all redecls of \p D are in the main file.
+bool allDeclsInMainFile(const Decl *D);
sammccall wrote:
> Do you expect this to be reused?
> If so, unit test? Otherwise this
ilya-biryukov updated this revision to Diff 147301.
ilya-biryukov added a comment.
- Move tests to QualityTests.cpp
- Fix findDecl() assertion on redecls of the same thing
- Fix file comment of TestTU.cpp
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46943
Files:
ilya-biryukov added inline comments.
Comment at: clangd/Quality.cpp:24
+ if (SemaCCResult.Declaration)
+AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration);
if (SemaCCResult.Availability == CXAvailability_Deprecated)
sammccall wrote:
>
sammccall added inline comments.
Comment at: clangd/AST.h:32
+/// Returns true iff all redecls of \p D are in the main file.
+bool allDeclsInMainFile(const Decl *D);
Do you expect this to be reused?
If so, unit test? Otherwise this seems small enough to move
ioeric added inline comments.
Comment at: clangd/Quality.cpp:24
+ if (SemaCCResult.Declaration)
+AllDeclsInMainFile = allDeclsInMainFile(SemaCCResult.Declaration);
if (SemaCCResult.Availability == CXAvailability_Deprecated)
Could you explain why
ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall.
Herald added subscribers: mgrang, jkorous, MaskRay, klimek.
This should, arguably, give better ranking.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46943
Files:
clangd/AST.cpp
22 matches
Mail list logo
