Issue getting incremental updates to compile_commands.json to play nicely.

[Icantjuddle]

Note: "Bug" is probably the wrong term for this, "question" is likely better but I couldn't find a better place to ask.

My situation

I work on a very large c++ monorepo where generating compile_commands.json takes a long time so to give our devs a nicer experience we have utilities setup for generating compile_commands.json incrementally (i.e. only for the files/libraries you're actively working with). I've built a small vscode extension that is able to, upon changing the build or opening new files, trigger these incremental updates to compile_commands.json.

The Issue

If I open a new file that doesn't yet have an entry in compile_commands.json it will show all kinds of errors (as expected); however, after the compile_commands.json file is generated clangd/this extension doesn't seem to realize it has new information about the open file and re-evaluated it. Right now I just have my extension use clangd.restart to fix this but I imagine there is a better way that is less disruptive.

I'm not an LSP expert so was hoping someone here who is more familiar could advise on the right place for this fix. I've seen here and upstream that clangd (after 12+) does watch compile_commands.json so do I just need to issue a rpc call to clangd to ask it to re-evaluate open files?

[sam-mccall]

It doesn't actually watch for changes in an event-driven way, but rather looks for updates when the compile command is fetched (e.g. because the file has changed and we're reparsing it).

So in effect, diagnostics will be updated the next time the file is edited, or when any file is saved (the latter in case the file you edited was a header included from other files).

If you want to trigger this externally, I don't think there's a well-supported way. Sending a spurious didSave notification would probably do it (though not guaranteed for the future). I don't know if you can do that from another extension?

A couple of caveats:

• in any case checks for changes to compile_commands are throttled to ~1/sec. So if you try to force a check, you can be unlucky: check + overwrite compile_commands + force attempt in quick succession will not see the change
• hopefully you're generating the new file into a new location and renaming: bad things may happen if we see a half-written file

[Icantjuddle]

Thanks so much for explaining, really appreciate it.

If I understand you correctly then the following should result in vscode displaying accurate diagnostics about the file:

Assume there is no entry for f.cc.

1. Open file f.cc
2. Extension auto-generates f.cc's entry in compile_commands.json after a second or two.
3. User edits the file (perhaps trivially) and saves it.
4. clangd reparses compile_comamnds.json finds the new/updated f.cc entry and uses to provide info?

If this is the case maybe I can just cause the file to be saved instead restarting, though as you mentioned maybe the throttling could have some weird interaction with this.

---

hopefully you're generating the new file into a new location and renaming: bad things may happen if we see a half-written file

Yep, good to know though.

[HighCommander4]

Perhaps it would make sense for clangd to support a "reload CDBs" message which an extension could invoke to trigger reloading CDBs without a restart?

[sam-mccall]

User edits the file (perhaps trivially) and saves it.

The save is not necessary - the edit will cause that file's diagnostics to be updated.
Saving any file will cause all files's diagnostics to be updated.

BTW I checked the cache time, and we'll only check once every five seconds for a changed file.

Perhaps it would make sense for clangd to support a "reload CDBs" message which an extension could invoke to trigger reloading CDBs without a restart?

Maybe, but I think we may need to see that it's a bit more widely applicable?

IIUC for this case we'd need to:

• add an LSP extension message
• add a vscode-clangd command primarily designed to be triggered by other extensions
• add cache invalidation - if we're committing to an extension, it probably needs to be robust

[WB2210]

Can we enable "clangd.onConfigChanged" back?
"description": "What to do when clangd configuration files are changed."

In file:
https://github.com/clangd/vscode-clangd/blob/master/src/config-file-watcher.ts

It seems that this.context.subscriptions.push(new ConfigFileWatcher(this.context))
Is never called

When I removed the condition:
" if ((capabilities as ClangdClientCapabilities)
.compilationDatabase?.automaticReload)
return;"

And in file https://github.com/clangd/vscode-clangd/blob/master/src/clangd-context.ts:
Moved configFileWatcher.activate(this); above this.client.start();

It worked fine for me, and the server refreshes all open files whenever compile_commands.json is changed

Can we enable this config back? Because it fixes the problem for me at least

Thanks

[HighCommander4]

Can we enable "clangd.onConfigChanged" back?
[...]

I've done a bit of poking around to understand the history here. Summarizing my understanding and some notes:

• Watching compile_commands.json for changes was first implemented on the client side, in the config-file-watcher.ts file you mentioned.
  • This feature actually watches the file on disk for changes, and when it detects a change, it restarts the server.
  • Restarting the server is kind of a crude way to react to a change, because it comes with a potentially non-trivial performance penalty:
    • the project's index has to be reloaded from disk, and its in-memory representation rebuilt
    • preambles for all open files have to be rebuilt, not just files whose compile command changed
• In clangd 12, "hot reload" of compile_commands.json was implemented on the server side.
  • This does not involve watching the file on disk for changes, just checking for changes whenever clangd needs to re-parse the file (whenever the file is edited, or any file is saved).
  • However, it avoids the performance penalty of a restart: the in-memory index, and preambles for files that did not change, are preserved.
• The server-side "hot reload" feature was considered a good-enough replacement for the client-side watcher, that when the server advertises the "hot reload" ability (which all servers since clangd 12 do; this is what the automaticReload server capability signifies), the client-side watcher is disabled.

So indeed the server-side functionality is better in some ways (when a change is detected, the server updates its state gracefully without going through the longer process of a restart), but also worse in some ways (detecting a change requires an edit or save of the file, it doesn't happen automatically if you're just reading the file).

This downside was actually recognized when the automaticReload server capability was added in llvm/llvm-project@213329d:

This is going to be a mild regression, as we do not actually watch for files on
disk and so new diagnostics need to wait until a rebuild is requested e.g. due
to file change (and the internal caches have expired).
However this is still a better tradeoff (and if it's important, we can request
the client to watch files for us in the future).

You're basically noticing this "mild regression".

---

Given that the config-file-watcher.ts code is still around, and some users prefer the behaviour of the client-side watcher, I don't see a great reason why not to have a config option to force-enable it even when the server supports automaticReload. (Not by default, of course.)

In the longer term, we should be able to get the best of both worlds by hooking up the server's graceful "hot reload" to be triggered by an actual file watcher.

[WB2210]

I agree enabling back config-file-watcher.ts would be a good idea (As an optional setting, the way it was before in the setting)
In my case the project isnt that large so when clangd LSP restarts everything returns to full functionality immediately and it would fix the problem
And it would take 1-2 lines change to bring it back so I think its a good solution for now 👍

[HighCommander4]

And it would take 1-2 lines change to bring it back so I think its a good solution for now 👍

Would you like to submit a PR?

[studgeek]

In the meantime, it seems there is no way to trigger clangd restart from the command line since it has to be done at clangd extension level, correct?

[HighCommander4]

In the meantime, it seems there is no way to trigger clangd restart from the command line since it has to be done at clangd extension level, correct?

Not that I can think of.

Note that there is a patch fixing this at #688. It needs only minor updates, though I haven't heard from the patch author in a while. If you're interested, you're welcome to take it across the finish line.