This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341542: [clangd] Implement proximity path boosting for Dex
(authored by omtcyfz, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51481?vs=164176&id=164195#toc
Repository:
rCTE Cl
kbobyrev updated this revision to Diff 164176.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
Resolve the issue with `SymbolSlab::Builder` and make sure passed vector has
correct lifetime.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/FileDistance.h
kbobyrev updated this revision to Diff 164172.
kbobyrev marked 10 inline comments as done.
kbobyrev added a comment.
This should address the last round of comments.
There was still some hassle with the `SymbolSlab::Builder` in the unit tests
and I decided to leave it as it is due to some weird o
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/URI.cpp:200
+ return make_string_error("Couldn't convert " + AbsolutePath +
+ " to any given scheme.");
+}
kbobyrev added inline comments.
Comment at: clang-tools-extra/unittests/clangd/DexIndexTests.cpp:623
+
+ auto I = DexIndex::build(std::move(Builder).build(), URISchemes);
+
ioeric wrote:
> We could use the constructor that doesn't take ownership e.g.
> `DexInde
kbobyrev updated this revision to Diff 164062.
kbobyrev marked 18 inline comments as done.
kbobyrev added a comment.
Address a round of comments
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/FileDistance.h
clang-tools-extra/clangd/URI.cpp
clang-tools-extra/clangd/URI.h
ioeric requested changes to this revision.
ioeric added a comment.
This revision now requires changes to proceed.
This should be the last round! ;)
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:45
+std::vector>
+createPathIterators(llvm::ArrayRef ProximityPaths,
+
kbobyrev updated this revision to Diff 164024.
kbobyrev added a comment.
Keep 2 minor refactorings out of the scope of this patch.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/FileDistance.h
clang-tools-extra/clangd/index/SymbolYA
kbobyrev updated this revision to Diff 164023.
kbobyrev marked 5 inline comments as done.
kbobyrev added a comment.
Address another round of comments.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/FileDistance.h
clang-tools-extra/c
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:59
LookupTable[Sym->ID] = Sym;
-SymbolQuality[Sym] = quality(*Sym);
+SymbolAndScores[I] = {Sym, static_cast(quality(*Sym))};
}
ioeric wrote:
> ioeric wrote:
kbobyrev updated this revision to Diff 163987.
kbobyrev marked 14 inline comments as done.
kbobyrev added a comment.
- Rebase on top of master
- Address suggestions Eric wrote here
- Address suggestions from the offline discussion by reusing `Quality.cpp`
infrastructure for path proximity boostin
ioeric added a comment.
There are a few changes/refactorings which I would suggest splitting from this
patch, as they would require more thoughts and seem unrelated in this patch.
Please see the inline comments :)
Comment at: clang-tools-extra/clangd/Quality.h:130
+/// direct
ioeric requested changes to this revision.
ioeric added a comment.
This revision now requires changes to proceed.
(and forgot to request changes ;)
https://reviews.llvm.org/D51481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.
kbobyrev updated this revision to Diff 163773.
kbobyrev marked 12 inline comments as done.
kbobyrev added a comment.
- Rebase on top of the parent patch
- Apply many refactorings where appropriate
- Write more comments and documentation
- Abstract pieces of code which are shared by multiple pieces
ioeric added inline comments.
Comment at: clang-tools-extra/;:1
+//===--- Token.cpp - Symbol Search primitive *- C++
-*-===//
+// The LLVM Compiler Infrastructure
Unintended change?
Comment at: clang-too
kbobyrev updated this revision to Diff 163680.
kbobyrev marked 17 inline comments as done.
kbobyrev added a comment.
Address a round of comments, refactor code.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/;
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/inde
kbobyrev updated this revision to Diff 163667.
kbobyrev marked 2 inline comments as done.
kbobyrev edited the summary of this revision.
kbobyrev added a comment.
Rebase on top of `master`, s/Path/PathURI/g to be more explicit about the token
contents and origin.
https://reviews.llvm.org/D51481
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:36
+Result.push_back(PathToken);
+ }
return Result;
nit: no need for braces. [llvm-style]
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Token.h:54
Scope,
+/// Path to symbol declaration.
+///
ioeric wrote:
> As this is called `Path`, I'd try to decouple it from URIs, so that we don't
> need special handli
kbobyrev updated this revision to Diff 163538.
kbobyrev marked an inline comment as not done.
kbobyrev added a comment.
Fix tests
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/clangd/index/d
kbobyrev updated this revision to Diff 163537.
kbobyrev marked 14 inline comments as done.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/clangd/index/dex/DexIndex.h
clang-tools-extra/clangd/
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.cpp:137
+ BoostingIterators.push_back(
+ createBoost(create(It->second), P.second + 10));
+ }
Could you comment on `P.second + 10` here? It sounds lik
kbobyrev updated this revision to Diff 163507.
kbobyrev added a comment.
Canonicalize URIs, slightly simplify code structure.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/clangd/index/dex/D
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+ void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
sammccall wrote:
> ioeric wrote:
> > URI schemes are property of `Symbols`. We shouldn
kbobyrev updated this revision to Diff 163332.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.
Address couple of comments.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/
ioeric added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+ void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
sammccall wrote:
> ioeric wrote:
> > URI schemes are property of `Symbols`. We shouldn't
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+ void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
ioeric wrote:
> URI schemes are property of `Symbols`. We shouldn't need to pass spec
ioeric added a comment.
Some high-level comments :)
Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+ void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
URI schemes are property of `Symbols`. We shouldn't need to pass
kbobyrev updated this revision to Diff 163294.
kbobyrev added a comment.
Stop query generation as soon as one valid URI scheme was found.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/DexIndex.cpp
clang-tools-extra/clangd
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, mgrang, jkorous, MaskRay, mgorny.
https://reviews.llvm.org/D51481
Files:
clang-tools-extra/clangd/CMakeLists.txt
30 matches
Mail list logo
