This revision was automatically updated to reflect the committed changes.
Closed by commit rL334017: [clangd] Add "member" symbols to the index
(authored by malaperle, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D44954
Files:
clang-t
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE334017: [clangd] Add "member" symbols to the
index (authored by malaperle, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D44954?vs=149532&id=149970#toc
Repository:
rCTE Clang To
malaperle updated this revision to Diff 149532.
malaperle marked 4 inline comments as done.
malaperle added a comment.
Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd
malaperle added inline comments.
Comment at: unittests/clangd/CodeCompleteTests.cpp:741
+TEST(CompletionTest, Enums) {
+ EXPECT_THAT(completions(R"cpp(
ioeric wrote:
> malaperle wrote:
> > ioeric wrote:
> > > It's not clear to me what the following tests (`Enu
sammccall accepted this revision.
sammccall added a comment.
Thanks, LG!
Comment at: clangd/CodeComplete.h:86
+// For index-based completion, we only want:
+// * symbols in namespaces or translation unit scopes (e.g. no class
nit: want -> consider?
==
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: clangd/index/Index.h:158
unsigned References = 0;
-
+ /// Whether or not this is symbol is meant to be used for the global
+ /// completion.
---
malaperle updated this revision to Diff 149206.
malaperle added a comment.
Update with suggestions.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
Files:
clangd/CodeComplete.cpp
clangd/CodeComplete.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/Sy
malaperle marked an inline comment as done.
malaperle added inline comments.
Comment at: clangd/index/Index.h:158
unsigned References = 0;
-
+ /// Whether or not this is symbol is meant to be used for the global
+ /// completion.
sammccall wrote:
> ioeric wr
sammccall added a subscriber: hokein.
sammccall added inline comments.
Comment at: clangd/index/Index.h:158
unsigned References = 0;
-
+ /// Whether or not this is symbol is meant to be used for the global
+ /// completion.
ioeric wrote:
> s/this is symbol/t
malaperle added inline comments.
Comment at: unittests/clangd/SymbolCollectorTests.cpp:141
Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
+Args.push_back(TestFileName);
+
This allows to override the "-xc++" with something else, i.e. -xobjec
malaperle added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:158
+ translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(),
+ enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(),
+ objcCategoryImplDecl(), objcImp
malaperle updated this revision to Diff 148834.
malaperle marked 6 inline comments as done.
malaperle added a comment.
Address comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
Files:
clangd/CodeComplete.cpp
clangd/index/Index.h
clangd/index/MemIndex.cpp
cl
ioeric added a comment.
The change looks mostly good. Some nits and questions about the testing.
Comment at: clangd/index/Index.h:158
unsigned References = 0;
-
+ /// Whether or not this is symbol is meant to be used for the global
+ /// completion.
s/this
malaperle updated this revision to Diff 148329.
malaperle added a comment.
Use "SupportGlobalCompletion", white-list decl contexts, add more tests
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
Files:
clangd/CodeComplete.cpp
clangd/index/Index.h
clangd/index/MemInde
malaperle added a comment.
In https://reviews.llvm.org/D44954#1104497, @malaperle wrote:
> >>> What scopes will non-scoped enum members have?
> >>
> >> Hmm. I think all of them, since you can refer them like that in code too.
> >> Case #1 doesn't work but that was the case before this patch so
malaperle added a comment.
In https://reviews.llvm.org/D44954#1104178, @ilya-biryukov wrote:
> In https://reviews.llvm.org/D44954#1103664, @malaperle wrote:
>
> > It's probably better to consider this in a future patch. Maybe something
> > like the first suggestion: vector::push_back and match b
ilya-biryukov added a comment.
In https://reviews.llvm.org/D44954#1103664, @malaperle wrote:
> It's probably better to consider this in a future patch. Maybe something like
> the first suggestion: vector::push_back and match both. Otherwise, I would
> think it might be a bit too verbose to have
ioeric added a comment.
> It's also for textDocument/documentSymbol. For this, we technically don't
> need them in the static index since we could collect symbols when the
> document is opened, but we also want them for workspaceSymbols so we might as
> well use the same symbol collector, etc.
malaperle added a comment.
In https://reviews.llvm.org/D44954#1102807, @ilya-biryukov wrote:
> A few questions regarding class members. To pinpoint some interesting cases
> and agree on how we want those to behave in the long run.
>
> How do we handle template specializations? What will the qual
ioeric added inline comments.
Comment at: clangd/index/SymbolCollector.cpp:113
- // We only want:
- // * symbols in namespaces or translation unit scopes (e.g. no class
- // members)
- // * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
- auto I
ioeric added a comment.
In https://reviews.llvm.org/D44954#1101922, @malaperle wrote:
> @ioeric You mentioned in https://reviews.llvm.org/D46751 that it would make
> sense to add a flag to disable indexing members. Could you comment on that?
> What kind of granularity were you thinking? Would a
ilya-biryukov added a comment.
A few questions regarding class members. To pinpoint some interesting cases and
agree on how we want those to behave in the long run.
How do we handle template specializations? What will the qualified names of
those instantiations be?
I.e. how do I query for `push
malaperle added a comment.
@ioeric You mentioned in https://reviews.llvm.org/D46751 that it would make
sense to add a flag to disable indexing members. Could you comment on that?
What kind of granularity were you thinking? Would a "member" flag cover both
class members (member vars and function
malaperle added inline comments.
Comment at: clangd/CodeComplete.cpp:932
Req.Query = Filter->pattern();
+Req.DeclContexts = {Decl::Kind::Namespace, Decl::Kind::TranslationUnit,
+Decl::Kind::LinkageSpec, Decl::Kind::Enum};
I want t
24 matches
Mail list logo
