clangd-indexer builds its static index purely from compile_commands.json and ignores .clangd config files. So CompileFlags: Add/Remove overrides (commonly used to strip flags clangd’s parser chokes on) apply in the live server’s dynamic index but not in the static index. The two end up disagreeing about what was indexed and under which flags, which matters most for projects using remote index, where the static index is the primary source of truth for cross-file navigation.
Proposal
Add an opt-in flag to clangd-indexer. When enabled, resolve applicable .clangd fragments using the existing config::Provider machinery and apply CompileFlags.Add/Remove before invoking the compiler action.
Alternative
Patch compile_commands.json in the build system before invoking the indexer. This works today and is the current workaround, but every project ends up reimplementing flag resolution instead of sharing clangd’s.
Open questions
Is a single top-level .clangd enough for a first patch, or is path-aware resolution needed to be useful?
Is this a direction maintainers want at all, given the deliberate simplicity of clangd-indexer?
Looking at alternatives, the command line of clangd-indexer seems also a reasonable spot for passing the arguments.
In the config I only see 2 blocks of relevance for clangd-indexer: CompileFlags and Index. The other are irrelevant.
Compile flags is logical as you already indicated. Index points to subprojects and 3rd parties. From the perspective of clangd-indexer, this could be a point for ignoring specific files, maybe the background field could be used to distinct, though I wouldn’t be surprised to see use-cases where you want to create an index for a part that in clangd starts from an index file. So may be a separate field would be better.
In my head, there are 4 phases of completion:
Use .clangd without any path support
Use .clangd and understand but ignore everything but root
Use .clangd with paths to get other flags and other compile_commands.json
Use .clangd with paths to get other flags and other compile_commands.json with exclusion options
Out of these, I personally don’t think the first is an option. Mainly because it would break for people that use it adding a 3rd party which they don’t want to index.
The 3rd option also feels a bit strange as the delta with the 4th isn’t that big.
Though I’m OK with the 2nd and 4th options as I feel that 3rd parties (which you want to ignore) are a more common usage for the paths than adding/removing a specific option for a subtree which is not inside compile_commands.json
That said, @HighCommander4 is the defacto maintainer, so I would like to hear his opinion.
On the technical side, I think the code to read the .clangd file should be shared between .clangd and clangd-indexer such that when a field is added to one, the other doesn’t consider the yaml as invalid. I would be inclined to say the same for the code that gets a compile command of a file, though I’m not sure this is easily done.
I would like to see path-aware resolution, so that clangd-indexer’s behaviour is consistent with clangd’s.
Note that without path-awareness, it’s not just that .clangd files in subdirectories wouldn’t be respected, If conditions in the top-level .clangd file wouldn’t be resolved correctly either. Supporting one implies supporting the other.
Agreed, and this shouldn’t be too difficult. Most of the relevant logic is already in a reusable form.
I think an implementation of this would need two main pieces:
During indexer initialization, something similar to this code that creates the “config provider” that processes user and project config files.
When the indexer starts processing a source file, something similar to this code that tells the config provider that you’re now processing a file at a particular path
There is a bit of glue code here that translates between a “config provider” and a “context provider” that you’ll likely need to reimplement for the indexer. The main thing it does is handles diagnostics that occur while parsing a config file, which in the case of the indexer you probably just want to print to the terminal.
Everything else is taken care of by the config provider.
(@JVApen I’m not sure what you meant by “with exclusion options”, maybe you can elaborate on that.)
I’m mainly thinking about a boolean flag “include in clangd indexer index” (with a better name), such that 3rd party code that you could receive via (remote) index does not get duplicated in the generated index.
Thank you for your opinion. I’m glad that you are not opposed to the change.
I’m implementing a PR which adds support of .clangd files in clangd-indexer. The changes implement only flags manipulation. I reused as much as possible. I also added a test for the new behavior.
I already took a brief look and added a few comments based on that, would like to check it in more detail on the PC. Given that @HighCommander4 has more experience, I’ll leave the final judgement to him.
As I see in this PR, originally --enable-config flag was enabled by default, but after review it became disabled by default for backward compatibility.
Personally I prefer enabled it by default, if we agree that missing this functionality is an oversight.
My reasoning: I wouldn’t want more then it being enabled by default. Though to do so, we should be absolutely sure this would not break existing setups. Until we have analyzed that, false is the safer option. Maybe a follow-up PR van switch the flag and argue why it’s safe?
I think this problem is even worse if it is called --enable-somethingas that name alone suggests it’s disabled by default. Given it’s the same flag as in clangd, which does use ‘true’ as default, it’s reasonable to mimic if we think it should be enabled by default.
However, I’d also be fine with doing that in two separate PRs (1. implementation + default to false, 2. default to true) so that if the default causes any issues we can revert just the second PR.