Add document outline symbols from unnamed contexts, e.g. extern "C".

[ilya-golovenko]

Summary

It is necessary to traverse children of unnamed declaration contexts
to get symbols which are currently missing in document outline, e.g.:

extern "C" {
void foo();
}

[ilya-golovenko]

Trigger new build

[kadircet]

clang-tools-extra/clangd/FindSymbols.cpp

200–202

this will result in traversal of other declcontexts like block/capturedecls.

i think we should rather change the VisitKind to {No, Decl, Children} and then make shouldVisit return a VisitKindSet and take a Decl* instead of a NamedDecl.
Later on we should first check for LinkageSpecDecl and return Children only while keeping the rest of the logic the same (i.e. try to cast to nameddecl and return No if it fails.
Finally logic in here should also be adjusted accordingly to visit children and the decl itself separately.

Do you have other cases apart from extern'd symbols?

[ilya-golovenko]

clang-tools-extra/clangd/FindSymbols.cpp

200–202

I have only C++20 ExportDecl in mind. Initially I wanted to make changes similar like you propose, but tried to minimize the diff :-) I will re-work the fix according to your proposal.

[ilya-golovenko]

Changes afte code review, add test for export context

[ilya-golovenko]

@kadircet I decided to add OnlyChildren to enum VisitKind instead of making a VisitKindSet - this helped to minimize the changes in shouldVisit(). What do you think?

[ilya-golovenko]

Remove invalid comment

[kadircet]

In D84839#2187222, @ilya-golovenko wrote:

@kadircet I decided to add OnlyChildren to enum VisitKind instead of making a VisitKindSet - this helped to minimize the changes in shouldVisit(). What do you think?

SGTM.

clang-tools-extra/clangd/FindSymbols.cpp

213

ah this is complicated now, but at least for the sake of nestedness:

if (Visit == OnlyChildren)
  return traverseChildren;

// We must have the Decl in the result set.
auto *ND = llvm::cast<NamedDecl>(D);
auto Sym = declToSym(AST.getASTContext(), *ND);
if (!Sym) return;
Results.push_back(std::move(*Sym));

if(Visit == OnlyDecl) return;

assert(Visit == DeclAndChildren && "Unexpected VisitKind");
traverseChildren(Result.back()->children);

[ilya-golovenko]

Simplify logic in traverseDecl method

[kadircet]

thanks, lgtm!

let me know if i should land this for you

[ilya-golovenko]

In D84839#2189120, @kadircet wrote:

thanks, lgtm!

let me know if i should land this for you

Thank you much for the review.

I don't have commit access so could you please land it for me:
Ilya Golovenko <[e-mail]>

clang-tools-extra/clangd/FindSymbols.cpp

private:
 
   void traverseDecl(Decl *D, std::vector<DocumentSymbol> &Results) {
     if (auto *Templ = llvm::dyn_cast<TemplateDecl>(D)) {
       // TemplatedDecl might be null, e.g. concepts.
       if (auto *TD = Templ->getTemplatedDecl())
         D = TD;
     }
     auto *ND = llvm::dyn_cast<NamedDecl>(D);
-    if (!ND)
+    if (!ND) {
+      // Traverse children of unnamed contexts, e.g. extern "C".
+      traverseChildren(D, Results);
       return;
+    }
     VisitKind Visit = shouldVisit(ND);
     if (Visit == VisitKind::No)
       return;
     llvm::Optional<DocumentSymbol> Sym = declToSym(AST.getASTContext(), *ND);
     if (!Sym)
       return;
     if (Visit == VisitKind::DeclAndChildren)
       traverseChildren(D, Sym->children);
     Results.push_back(std::move(*Sym));
   }
 
   void traverseChildren(Decl *D, std::vector<DocumentSymbol> &Results) {
     auto *Scope = llvm::dyn_cast<DeclContext>(D);
     if (!Scope)
       return;
     for (auto *C : Scope->decls())
       traverseDecl(C, Results);

clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp

TU.AdditionalFiles["foo.h"] = R"cpp(
       )cpp";
   TU.Code = R"cpp(
       #include "foo.h"
       )cpp";
 
   EXPECT_THAT(getSymbols(TU.build()), IsEmpty());
 }
 
+TEST(DocumentSymbols, ExternContext) {
+  TestTU TU;
+  TU.Code = R"cpp(
+      extern "C" {
+      void foo();
+      class Foo {};
+      }
+      namespace ns {
+        extern "C" {
+        void bar();
+        class Bar {};
+        }
+      })cpp";
+
+  EXPECT_THAT(getSymbols(TU.build()),
+              ElementsAre(WithName("foo"), WithName("Foo"),
+                          AllOf(WithName("ns"),
+                                Children(WithName("bar"), WithName("Bar")))));
+}
+
 TEST(DocumentSymbols, NoLocals) {
   TestTU TU;
   TU.Code = R"cpp(
       void test(int FirstParam, int SecondParam) {
         struct LocalClass {};
         int local_var;
       })cpp";
   EXPECT_THAT(getSymbols(TU.build()), ElementsAre(WithName("test")));