clangd ignores symbols with double leading underscores __like_this_one

[johnhubbard]

I'm using clangd on Linux kernel code (git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git), where a lot, A LOT of the functions (and other symbols) start with double underscores. clangd refuses to find the implementation of such functions in a .c file, until the file is actually loaded (at which point it is forced to see them, and then it works). Normal lookup of such symbols fails until the .c file is loaded. clangd can find the declaration of the function in the .h file, but that's all.

I've isolated this to clangd, by running in standalone mode, as well as trying it out in VS Code.

Logs

I am attaching the clangd stderr log, with --log=verbose, from opening up the header file (gph.h), and trying to jump from the declaration there of __free_pages(), to the definition. This fails because the definition is in another .c file that, while in the project (and in the compile_commands.json), has not yet been opened, and this bug here prevents that function from being found.

System information

Output of clangd --version:
clangd version 15.0.7
Features: linux
Platform: x86_64-pc-linux-gnu

Editor/LSP plugin:
VS Code
cland_logs.json.zip

Operating system:
Linux

[HighCommander4]

The relevant code is here:

  // We want stdlib implementation details in the index only if we've opened the
  // file in question. This does means xrefs won't work, though.
  CollectorOpts.CollectReserved = IsIndexMainAST;

I expect that if you were to change that to true in a local build, things would work.

The "reserved" in the variable names refers to the C++ standard's definition of reserved identifiers (there is a similar definition in the C standard); generally speaking, in a standards-conforming program these should only appear in the implementation of the standard library (which is why the comment refers to them as "stdlib implementation details").

I guess the kernel is a bit of a special case here? Or it's technically non-conforming?

In any case, I guess we should probably expose this as a config option.

[johnhubbard]

Yes, the kernel is a very special case. And it has very enthusiastically embraced the use of leading double underscores for symbols; there are 10's or 100's of thousands of them.

As a result, clangd on kernel code is a strange experience: clangd is mind-blowing, but in every editing session I have to fall back to grep in order to find some of the symbols. I would LOVE to have a config option or whatever I can get.

[johnhubbard]

I built clangd with this change, as recommended, and re-tested, but it didn't seem to change the behavior:

diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp
index dc5c83433ffd..691801749522 100644
--- a/clang-tools-extra/clangd/index/FileIndex.cpp
+++ b/clang-tools-extra/clangd/index/FileIndex.cpp
@@ -57,7 +57,7 @@ SlabTuple indexSymbols(ASTContext &AST, Preprocessor &PP,
   CollectorOpts.CollectMainFileRefs = CollectMainFileRefs;
   // We want stdlib implementation details in the index only if we've opened the
   // file in question. This does means xrefs won't work, though.
-  CollectorOpts.CollectReserved = IsIndexMainAST;
+  CollectorOpts.CollectReserved = true;
 
   index::IndexingOptions IndexOpts;
   // We only need declarations, because we don't count references.

[HighCommander4]

You likely need to delete <project>/.cache/clangd/index and restart clangd to trigger re-indexing of the project.

[johnhubbard]

Oh yes, I'm very familiar with the .cache and the associated re-indexing steps. But just to be sure, I've spent some time just now on two different machines, very carefully verifying that I'm the latest llvm commit (c32c0e14d694 "[RISCV] Add i32 as a legal type for GPR register class. (Mon, 26 Jun 2023)"), plus my local commit above, and that is the version of clangd that the editor is launching:

clangd version 17.0.0 (https://github.com/llvm/llvm-project.git 3833bd30f31fe92c73804df9d52ef3fb6639d391)
Features: linux
Platform: x86_64-unknown-linux-gnu

And of course, that the entire .cache directory is deleted before opening the editor.

I just can't seem to get this to work, though. It behaves exactly as before. It's possible that I'm just Doing It Wrong, or that there is something else that I'm missing, but I also suspect that the above code diff may not be quite enough to get the desired behavior.

Any thoughts or hints?

[HighCommander4]

I also suspect that the above code diff may not be quite enough to get the desired behavior.

You're right; I suggested that diff based on code reading and hadn't actually tried it. Now that I have, I see there is another place in the code where we don't explicitly set CollectReserved (we just rely on its default value being false), where we now need to set it to true. My bad.

The following makes indexing of double-underscore decls work for me in a minimal test project:

diff --git a/clang-tools-extra/clangd/index/Background.cpp b/clang-tools-extra/clangd/index/Background.cpp
index 73330b3ae6a8..ced02ff097c6 100644
--- a/clang-tools-extra/clangd/index/Background.cpp
+++ b/clang-tools-extra/clangd/index/Background.cpp
@@ -304,6 +304,7 @@ llvm::Error BackgroundIndex::index(tooling::CompileCommand Cmd) {
     return true;
   };
   IndexOpts.CollectMainFileRefs = true;
+  IndexOpts.CollectReserved = true;

   IndexFileIn Index;
   auto Action = createStaticIndexingAction(
diff --git a/clang-tools-extra/clangd/index/FileIndex.cpp b/clang-tools-extra/clangd/index/FileIndex.cpp
index dc5c83433ffd..691801749522 100644
--- a/clang-tools-extra/clangd/index/FileIndex.cpp
+++ b/clang-tools-extra/clangd/index/FileIndex.cpp
@@ -57,7 +57,7 @@ SlabTuple indexSymbols(ASTContext &AST, Preprocessor &PP,
   CollectorOpts.CollectMainFileRefs = CollectMainFileRefs;
   // We want stdlib implementation details in the index only if we've opened the
   // file in question. This does means xrefs won't work, though.
-  CollectorOpts.CollectReserved = IsIndexMainAST;
+  CollectorOpts.CollectReserved = true;

   index::IndexingOptions IndexOpts;
   // We only need declarations, because we don't count references.

Does that work for you?

[johnhubbard]

Yes! It is working for me. Oh this is amazing, I really should not have waited so long to ask about this. Huge huge improvement in browsing the kernel code. Thanks so much for the answers.

It would definitely be good to get this turned into a clangd command line option.

[HighCommander4]

Glad to hear you got it working!

It would definitely be good to get this turned into a clangd command line option.

I put up a draft patch that does this for feedback at https://reviews.llvm.org/D153946. The way to enable it would be to add:

Index:
  ReservedIdentifiers: true

to <project>/.clangd.

[weiyshay]

I met the same issue and got here from:
llvm/llvm-project#63862

Little knowledge on clang and clangd. I tried to built it but it seems no cpp in clangd.
It seems that more to do. Anyway to share a binary for me and I can give it a quick try?

root@sgjur-pod001-3:/localdata/tmp/clangd# git clone https://github.com/clangd/clangd.git 
root@sgjur-pod001-3:/localdata/tmp/clangd# find . -name *.cpp

[sam-mccall]

Clangd is part of llvm/llvm-project, which is a different (big) repo. https://clangd.llvm.org/faq#how-do-i-build-clangd-from-sources

(I can't help with a binary right now I'm afraid, only the CI knows how to produce useful portable binaries...)

I had a look at @HighCommander4's patch (sorry for the delay!). I'm not sure our config mechanism is well-suited for this job.
But I did have another idea: this heuristic is mostly targeted at hiding details of "system libraries" like the C++ standard library. These tend to be marked as system headers with -isystem flags, #pragma GCC system_header pragmas, etc, and clang keeps track of this. Maybe we can allow indexing of reserved-name symbols if they're not in system headers...

[weiyshay]

Sam,
Thank you for sharing the link. I compiled clangd 16.0.2 with the patch and it works perfectly on neovim v0.9.1.

Commands to build it in case someone needs:

	git clone https://github.com/llvm/llvm-project.git
	cd llvm-project
	git checkout llvmorg-16.0.2
	patch -p1 < index-reserved.diff 

	export LLVM_ROOT=$(pwd)
	mkdir $LLVM_ROOT/build
	cd $LLVM_ROOT/build
	
	cmake $LLVM_ROOT/llvm/ -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra"
	cmake --build $LLVM_ROOT/build --target clangd -j
	./bin/clangd --version
	~/.local/share/nvim/mason/bin/clangd -version
	cp ./bin/clangd ~/.local/share/nvim/mason/bin/clangd 

[HighCommander4]

@johnhubbard, when you get a chance could you try Sam's alternative fix at https://reviews.llvm.org/D155381, and see if it works for you just as well?

[johnhubbard]

Yes, Sam's alternative fix at https://reviews.llvm.org/D155381 is working perfectly for me. It finds and cross-references the Linux kernel's symbols such as __alloc_pages(), whereas stock clangd cannot do that.

Looks good!

[HighCommander4]

Thanks for checking, and thank you Sam for the improved (no configuration required!) fix.