ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: clangd/index/dex/dexp/Dexp.cpp:64
+// add the global scope to the request.
+Request.Scopes = {""};
ioeric wrote:
> I think the old
ioeric added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:64
+// add the global scope to the request.
+Request.Scopes = {""};
I think the old behavior used `AnyScope` for a unqualified name. Do we want to
keep that?
Repository:
rCTE
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/ASTMatchers/ASTMatchFinder.cpp:674
+ // Nodes may have no parents if:
+ // a) the node is the TranslationUnitDecl
+ // a) there is a bug
ioeric added a subscriber: thakis.
ioeric added a comment.
The new API and the refactoring look good to me. Just a nit and a question.
Comment at: lib/AST/ASTContext.cpp:10292
+ if (!Parents)
// We always need to run over the whole translation unit, as
// hasAncestor
ioeric added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:55
FuzzyFindRequest Request;
+ Request.AnyScope = true;
// Remove leading "::" qualifier as FuzzyFind doesn't need leading "::"
I don't think you would want AnyScope here. For example
ioeric added inline comments.
Comment at: clangd/ExpectedTypes.cpp:27
+return llvm::None;
+ auto *VD = llvm::dyn_cast(R.Declaration);
+ if (!VD)
maybe add a comment what `ValueDecl` covers roughly? E.g. functions, classes,
variables etc.
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:217
+bool shouldIndexFile(const Decl& D, const SymbolCollector::Options &Opts,
+ llvm::DenseMap *FilesToIndexCache) {
nit: this is very easily confused with the callb
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:555
+ auto Loc = findNameLoc(&ND);
+ if (!shouldIndexFile(SM, SM.getFileID(Loc), Opts, &FilesToIndexCache))
+return nullptr;
Should we use `getTokenLocation` like what we do below?
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346224: [clangd] Deduplicate query scopes. (authored by
ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D54105
Files:
clang-tools-extra/
ioeric added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:166
+ cl::init(10),
+ cl::desc("Max results to display. This flag is only meaningful when
-name"
+ " is set."),
Maybe `The max number of symbols with the same name b
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE346223: [clangd] Get rid of QueryScopes.empty() ==
AnyScope special case. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53933?vs=172735&id=172737#toc
Reposit
ioeric updated this revision to Diff 172735.
ioeric added a comment.
- rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53933
Files:
clangd/FindSymbols.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/dex/Dex.cpp
unittests/clangd/DexTests.cpp
uni
This revision was automatically updated to reflect the committed changes.
Closed by commit rL346221: [clangd] auto-index stores symbols per-file instead
of per-TU. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53433
ioeric updated this revision to Diff 172732.
ioeric added a comment.
- Rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/SymbolColle
ioeric added a comment.
In https://reviews.llvm.org/D54077#1288387, @sammccall wrote:
> Someone mentioned to me that the interaction-between-features argument wasn't
> clear here:
>
> - we **don't** currently update diagnostics for `A.cc` when `A.h` is edited
> - we should, this seems more obvio
ioeric added inline comments.
Comment at: clangd/index/Background.cpp:235
+IndexedFileDigests[Path] = FilesToUpdate.lookup(Path);
+IndexedSymbols.update(Path,
+ make_unique(std::move(Syms).build()),
kadircet wrote:
> This call is
ioeric added inline comments.
Comment at: clangd/index/dex/dexp/Dexp.cpp:128
};
cl::opt Limit{
"limit",
I think we should make it configurable like what we do here.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54106
__
ioeric updated this revision to Diff 172591.
ioeric marked an inline comment as done.
ioeric added a comment.
- Remove addressed FIXME.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53933
Files:
clangd/FindSymbols.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
clan
ioeric abandoned this revision.
ioeric marked an inline comment as done.
ioeric added inline comments.
Comment at: clangd/CodeComplete.cpp:563
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleS
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
For example, when anonymous namespace is present, duplicated namespaces might be
generated for the enclosing namespace.
Repository:
rCTE Clang Tool
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Tooling/AllTUsExecution.cpp:103
+if (!RegexFilter.match(File))
+ continue;
+Files.push_back(File);
nit: `if (match) then pus
ioeric added a comment.
maybe add a test?
Repository:
rC Clang
https://reviews.llvm.org/D54092
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: lib/Tooling/AllTUsExecution.cpp:120
for (std::string File : Files) {
+ if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File))
+con
ioeric added inline comments.
Comment at: lib/Tooling/AllTUsExecution.cpp:120
for (std::string File : Files) {
+ if (Filter.getNumOccurrences() != 0 && !RegexFilter.match(File))
+continue;
hokein wrote:
> ioeric wrote:
> > > `Filter.getNumOccurr
ioeric added inline comments.
Comment at: lib/Tooling/AllTUsExecution.cpp:58
+"filter",
+llvm::cl::desc("Only process files that match this filter"),
+llvm::cl::init(".*"));
Please also mention that this only applies to all-TUs.
Com
ioeric planned changes to this revision.
ioeric added a comment.
I got the behavior of `printNamespaceScope` wrong in this patch. Will update.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
___
cfe-commits mailing list
cfe-com
ioeric updated this revision to Diff 171928.
ioeric added a comment.
- revert wrong comment
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
Files:
clangd/AST.h
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.
ioeric updated this revision to Diff 171924.
ioeric added a comment.
- Clarify documentation for printNamespaceScope
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53926
Files:
clangd/AST.h
clangd/CodeComplete.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/
ioeric added inline comments.
Comment at: clangd/CodeComplete.cpp:563
for (auto *Context : CCContext.getVisitedContexts()) {
- if (isa(Context))
+ if (isa(Context)) {
Info.AccessibleScopes.push_back(""); // global namespace
ilya-biryukov w
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53933
Files:
clangd/FindSymbols.cpp
clangd/index/Index.h
clangd/inde
ioeric added a comment.
Made some more changes to make the code work in multiple threads (add mutex for
digests, take snapshot of digests for each run etc). PTAL
Comment at: clangd/index/Background.cpp:311
SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
- // FIXME: parti
ioeric updated this revision to Diff 171901.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- Merged with multi-threading changes. Added mutex for file digests.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
This avoids duplicated scopes in the query e.g. when anonymous namespace
is present.
Repository:
rCTE Clang Tools Extra
https://review
ioeric added inline comments.
Comment at: clangd/index/Background.cpp:311
SPAN_ATTACH(Tracer, "refs", int(Refs.numRefs()));
- // FIXME: partition the symbols by file rather than TU, to avoid duplication.
- IndexedSymbols.update(AbsolutePath,
-llvm::ma
ioeric updated this revision to Diff 171684.
ioeric marked 11 inline comments as done.
ioeric added a comment.
- address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd/index/FileInd
ioeric updated this revision to Diff 171467.
ioeric added a comment.
- minor cleanup and a friendly ping.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd/index/FileIndex.cpp
clangd/index/FileIndex.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345140: [clangd] Downrank members from base class (authored
by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53638
Files:
clang-tools
ioeric added inline comments.
Comment at: clangd/Quality.cpp:380
+ if (InBaseClass)
+Score *= 0.7;
+
sammccall wrote:
> This seems like a pretty light penalty to me, I'd consider 0.5...
0.5 sounds reasonable. I think we should penalize the non-instance membe
ioeric updated this revision to Diff 170870.
ioeric marked an inline comment as done.
ioeric added a comment.
- adjust parameter
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53638
Files:
clangd/Quality.cpp
clangd/Quality.h
unittests/clangd/QualityTests.cpp
Index: unit
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
The diff seems to be wrong. Please rebase.
Comment at: clangd/CodeComplete.cpp:732
+ // Class members that are shadowed by subclasses are usually noise.
+ if (Resul
ioeric updated this revision to Diff 170862.
ioeric marked an inline comment as done.
ioeric added a comment.
- restore accidentally removed test.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53638
Files:
clangd/Quality.cpp
clangd/Quality.h
unittests/clangd/QualityTests
ioeric marked an inline comment as done.
ioeric added inline comments.
Comment at: unittests/clangd/QualityTests.cpp:187
Relevance.merge(CodeCompletionResult(&findDecl(AST, "S::S"), 42));
- EXPECT_EQ(Relevance.Scope, SymbolRelevanceSignals::GlobalScope);
}
i
ioeric added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:112
+SPAN_ATTACH(Tracer, "Params", Params);
+ReplyOnce Reply(ID, Method, &Server, Tracer.Args);
log("<-- {0}({1})", Method, ID);
Do we have guarantee that `Tracer.Args` outlives `Re
ioeric added a comment.
> Keep required quailifier machinery around though, for cross-ns completion.
Do we have cross-ns completion in sema?
Comment at: clangd/CodeComplete.cpp:732
+ // Class members that are shadowed by subclasses are usually noise.
+ if (Result.Hid
This revision was automatically updated to reflect the committed changes.
Closed by commit rL345135: [CodeComplete] Expose InBaseClass signal in code
completion results. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D
ioeric added inline comments.
Comment at: lib/Sema/CodeCompleteConsumer.cpp:557
+ if (!Tags.empty())
+OS << " (" << llvm::join(Tags, ",") << ")";
+ if (CodeCompletionString *CCS = Results[I].CreateCodeCompletionString(
ilya-biryukov wrote:
> NIT
ioeric updated this revision to Diff 170855.
ioeric added a comment.
- Rebase
Repository:
rC Clang
https://reviews.llvm.org/D53635
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/CodeCompleteConsumer.cpp
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion/member-access.cpp
ioeric updated this revision to Diff 170854.
ioeric marked an inline comment as done.
ioeric added a comment.
- move tags into case.
Repository:
rC Clang
https://reviews.llvm.org/D53635
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/CodeCompleteConsumer.cpp
lib/Sema/SemaCode
ioeric created this revision.
ioeric added reviewers: sammccall, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
ioeric added a dependency: D53635: [CodeComplete] Expose InBaseClass signal in
code completion results..
Repository:
rCTE Clang Tools Extr
ioeric added a comment.
In https://reviews.llvm.org/D53635#1273904, @ilya-biryukov wrote:
> LG, but could we add a test for the new flag it by printing it in
> `PrintingCodeCompleteConsumer::ProcessCodeCompleteResults()` and adding
> corresponding tests to `clang/test/CodeCompletion`?
>
> Simil
ioeric updated this revision to Diff 170838.
ioeric added a comment.
- Add tests for the new flag.
Repository:
rC Clang
https://reviews.llvm.org/D53635
Files:
include/clang/Sema/CodeCompleteConsumer.h
lib/Sema/CodeCompleteConsumer.cpp
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added a subscriber: cfe-commits.
No new tests as the existing tests for result priority should give us
coverage. Also as the new flag is trivial enough, I'm reluctant to plumb the
flag to c-index-test output.
Repository
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
We should watch out for the downstream backward compatibility.
Comment at: clangd/index/Index.h:92
// As USRs (Unified Symbol Resolution) could be large, especially for func
ioeric added inline comments.
Comment at: clangd/index/IndexAction.h:33
+std::function RefsCallback,
+std::function FileDigestsCallback);
sammccall wrote:
> thinking about what we eventually want here:
> - the index action needs to tell the auto-indexe
ioeric updated this revision to Diff 170664.
ioeric marked 5 inline comments as done.
ioeric added a comment.
- address review comments
- merged with origin/master
- Cleanup
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index
ioeric updated this revision to Diff 170440.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- address review comments
- minor cleanup
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53433
Files:
clangd/index/Background.cpp
clangd/index/Background.h
clangd
ioeric added inline comments.
Comment at: clangd/index/FileIndex.h:49
+/// rename the existing FileSymbols to something else e.g. TUSymbols?
+class SymbolsGroupedByFiles {
+public:
sammccall wrote:
> `FileSymbols` isn't actually that opinionated about the data it
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344912: [clangd] Support URISchemes configuration in
BackgroundIndex. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53503
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53503
Files:
clangd/index/Background.cpp
clangd/index/Background.h
I
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344897: [change-namespace] Enhance detection of conflicting
namespaces. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53489
ioeric created this revision.
ioeric added a reviewer: hokein.
Herald added a subscriber: cfe-commits.
For example:
namespace util { class Base; }
namespace new {
namespace util { class Internal; }
}
namespace old {
util::Base b1;
}
When changing `old::` to `new::`, `util::`
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344889: [CodeComplete] Fix accessibility of protected
members when accessing members… (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53369?vs=169995&id=170377#to
ioeric created this revision.
ioeric added reviewers: sammccall, hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
This allows us to deduplicate header symbols across TUs. File digests
are collects when collecting symbols/refs. And the index store
ioeric added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:182
+ // - if there were multiple replies, only the first is sent
+ class ReplyOnce {
+struct State {
As discussed offline, we could potentially make this non-copyable to make
harder to
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
Looks good!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53398
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344736: [clangd] Names that are not spelled in source code
are reserved. (authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D5337
ioeric updated this revision to Diff 170068.
ioeric marked an inline comment as done.
ioeric added a comment.
- add isImplementationDetail helper
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53374
Files:
clangd/AST.cpp
clangd/AST.h
clangd/Quality.cpp
clangd/Quality.h
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clangd/ClangdLSPServer.cpp:148
+ if (Trace)
+(*Trace)["Reply"] = *Result;
+ Server.reply(ID, json::Value(std::move(*Result)));
ioeric added inline comments.
Comment at: clangd/Quality.cpp:182
if (auto *ID = SemaCCResult.Declaration->getIdentifier())
- ReservedName = ReservedName || isReserved(ID->getName());
+ ReservedName = ReservedName || isReserved(ID->getName()) ||
+
ioeric updated this revision to Diff 170046.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- address comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53374
Files:
clangd/AST.cpp
clangd/AST.h
clangd/Quality.cpp
clangd/Quality.h
clangd/index/In
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
These are often not expected to be used directly e.g.
TEST_F(Fixture, X) {
^ // "Fixture_X_Test" expanded in the macro should be do
ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, arphaman.
Repository:
rC Clang
https://reviews.llvm.org/D53369
Files:
lib/Sema/SemaCodeComplete.cpp
test/Index/complete-access-checks.cpp
Index: test/Index/complete-access-checks.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344688: [clangd] Support scope proximity in code completion.
(authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D53131
Files:
ioeric updated this revision to Diff 169983.
ioeric added a comment.
- rebase
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53131
Files:
clangd/AST.cpp
clangd/AST.h
clangd/CodeComplete.cpp
clangd/FileDistance.cpp
clangd/FileDistance.h
clangd/Quality.cpp
clangd/Qu
ioeric updated this revision to Diff 169982.
ioeric marked an inline comment as done.
ioeric added a comment.
- address comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53131
Files:
clangd/AST.cpp
clangd/AST.h
clangd/CodeComplete.cpp
clangd/FileDistance.cpp
cla
ioeric updated this revision to Diff 169962.
ioeric marked 18 inline comments as done.
ioeric added a comment.
- Addressed review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53131
Files:
clangd/AST.cpp
clangd/AST.h
clangd/CodeComplete.cpp
clangd/FileDistanc
ioeric added a comment.
Thanks for the suggestions! After taking a closer look at boosting factors for
other signals, I think I was being too conservative about boosting preferred
scopes and penalizing non-preferred ones. I have tuned the parameters to make
these more aggressive now according t
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: clangd/JSONRPCDispatcher.h:70
/// A handler responds to requests for a particular method name.
+ /// It returns true if the server should now shut down.
ioeric added a comment.
Nice! The code looks much clearer. Just some nits
Comment at: clangd/ClangdLSPServer.cpp:156
+void ClangdLSPServer::onExit(ExitParams &Params) {
+ // XXX handler should return true.
+}
?
Comment at: clangd/ClangdLSPSe
ioeric updated this revision to Diff 169814.
ioeric marked 3 inline comments as done.
ioeric added a comment.
- refactor to use FileDistance and address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53131
Files:
clangd/AST.cpp
clangd/AST.h
clangd/CodeCom
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE344604: [clangd] Allow disble down traversals from root.
(authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53317?vs=169804&id=169805#toc
Repository:
rCTE Clang
ioeric updated this revision to Diff 169804.
ioeric marked an inline comment as done.
ioeric added a comment.
- Simplify according to review suggestion.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53317
Files:
clangd/FileDistance.cpp
clangd/FileDistance.h
unittests/cla
ioeric abandoned this revision.
ioeric added a comment.
As discussed offline, the semantics is getting a bit complicated. As what we
really want for scope proximity is just disallow down traversals from root, we
can go with special-casing the root path for now:
https://reviews.llvm.org/D53317
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
This is useful for symbo scope proximity, where down traversals from
the global scope if not desired.
Repository:
rCTE Clang Tools Extr
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
This is useful for symbo scope proximity, where down traversals from
the global scope if not desired. The limitation in the current impleme
ioeric accepted this revision.
ioeric added a comment.
still lgtm
Comment at: clangd/Compiler.cpp:83
+std::string getStandardResourceDir() {
+ static int Dummy; // Just an address in this process.
Maybe also revert this change?
Repository:
rCTE Clang Too
This revision was automatically updated to reflect the committed changes.
Closed by commit rC344509: [CodeComplete] Make sure keyword
'template' is added even when code pattern is… (authored by ioeric,
committed by ).
Changed prior to commit:
https://reviews.llvm.org/D53284?vs=169681&id=169685
ioeric created this revision.
ioeric added reviewers: sammccall, hokein.
Herald added subscribers: cfe-commits, arphaman.
Repository:
rC Clang
https://reviews.llvm.org/D53284
Files:
lib/Sema/SemaCodeComplete.cpp
test/Index/complete-template-keywords.cpp
Index: test/Index/complete-templat
ioeric added inline comments.
Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+ MockFSProvider FS;
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > ioeric wrote:
> > > > Also add a test for `enqu
ioeric added inline comments.
Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:89
llvm::cl::init(false), llvm::cl::cat(ClangDocCategory));
+static llvm::cl::opt ClangDocExecutorName(
This flag can also be removed. Users can still use `--execut
ioeric added inline comments.
Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+ MockFSProvider FS;
sammccall wrote:
> ioeric wrote:
> > Also add a test for `enqueueAll` with multiple TUs ?
> Is it important
ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.
Comment at: clangd/index/Background.cpp:51
+ std::unique_lock Lock(QueueMu);
+ assert(!HasActiveTask);
+ QueueCV.wait(Lock, [&] { return ShouldStop || !Que
ioeric added a comment.
Looks good in general.
Comment at: clangd/ClangdLSPServer.h:117
+llvm::Optional CompileCommandsDir,
+std::function);
please document what the callback is for and how often it's called.
Comment at: clang
ioeric requested changes to this revision.
ioeric added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:203
+llvm::Expected>
+createClangDocExecutor(int &argc, const char **argv,
+
ioeric added inline comments.
Comment at: clangd/CodeComplete.cpp:558
+if (const auto *NS = dyn_cast(Ctx))
+ return NS->getQualifiedNameAsString() + "::";
+ return llvm::None;
sammccall wrote:
> does this do the right thing if it's anonymous or inline,
ioeric updated this revision to Diff 169231.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53131
Files:
clangd/CodeComplete.cpp
clangd/Quality.cpp
clangd/Quality.h
unittests/clan
ioeric added inline comments.
Comment at: clangd/index/FileIndex.h:78
+ /// Remove all index data associated with the file \p Path.
+ void removeFile(PathRef Path);
+
hokein wrote:
> ioeric wrote:
> > should we use this somewhere? E.g. when file is closed in Cl
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
LGTM
> Of course the question arises: *should* we set one
Agree with your points. If the cleanup is unsafe, it might end up biting us
harder in the future.
Repository:
rCTE Clang Tools Ex
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ilya-biryukov.
This should make all-scope completion more usable. Scope proximity for
indexes will be added in followup patch.
Measurements showed a slig
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344024: [clangd] Avoid cache main file status in preamble.
(authored by ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D52991
Files:
cl
201 - 300 of 1511 matches
Mail list logo