[PATCH] D44954: [clangd] Add "member" symbols to the index

Marc-Andre Laperle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 28 11:54:46 PDT 2018


malaperle added inline comments.


================
Comment at: clangd/index/SymbolCollector.cpp:158
+      translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(),
+      enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(),
+      objcCategoryImplDecl(), objcImplementationDecl()));
----------------
ioeric wrote:
> (Disclaimer: I don't know obj-c...)
> 
> It seems that some objc contexts here are good for global code completion. If we want to support objc symbols, it might also make sense to properly set `SupportGlobalCompletion` for them.
Sounds good. Maybe it would be OK to do this in another (small) patch? I also know next to nothing about obj-c :)


================
Comment at: unittests/clangd/CodeCompleteTests.cpp:741
 
+TEST(CompletionTest, Enums) {
+  EXPECT_THAT(completions(R"cpp(
----------------
ioeric wrote:
> It's not clear to me what the following tests (`Enums`, `AnonymousNamespace`, `InMainFile`) are testing. Do they test code completion or  symbol collector? If these test symbol collection, they should be moved int SymbolCollectorTest.cpp
They are testing that code completion works as intended regardless of how symbol collector is implemented. It's similar to our previous discussion in D44882 about "black box testing". I can remove them but it seems odd to me to not have code completion level tests for all cases because we assume that it will behave a certain way at the symbol collection and querying levels.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44954





More information about the cfe-commits mailing list