Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature request] support for vscode-clangd #654

Closed
DoDoENT opened this issue Feb 21, 2019 · 18 comments
Closed

[feature request] support for vscode-clangd #654

DoDoENT opened this issue Feb 21, 2019 · 18 comments
Labels
enhancement an enhancement to the product that is either not present or an improvement to an existing feature

Comments

@DoDoENT
Copy link

DoDoENT commented Feb 21, 2019

vscode-clangd uses language server protocol and clangd daemon to provide automatic completion, clang fix-its and much more while editing C++ code. In my opinion, it provides superior performance than Intellisense from Microsoft.

Do you have any plans of supporting this extension in the same way you support official Microsoft's C++ tools extension?

@DoDoENT
Copy link
Author

DoDoENT commented Feb 27, 2019

Basically, this already works if I manually add the following to my settings.json:

    "clangd.arguments": [
        "--compile-commands-dir=/absolute/path/to/cmake/binary/directory"
    ]

But every time I switch cmake kit or build variant, I need to manually update this entry in settings.json and then reload the window to ensure clangd executable is restarted. It would be nice if CMakeTools extension did that automatically (I am not sure if vscode-clangd supports on-the-fly restart of the clangd with new parameters, but I am positive that they currently do not support expanding variables, so you cannot write something like "--compile-commands-dir=${cmake.buildDirectory}").

However, using clangd and vscode-clangd extension makes auto-complete suggestions much faster than CppTools' IntelliSense parser, especially when parsing complex template-heavy headers (i.e. Eigen) and works out of the box in the cross-compilation environment (tested with custom Android NDK kit), so it makes issues like #637 irrelevant.

@dcourtois
Copy link
Contributor

Hi,

Disclaimer: I'm not the author of the addon, so this is only my opinion: this addon should not try to support everything. C/C++ extension is the most fundamental extension required to start working with C/C++ projects so it makes sense to support it out of the box. But if the CMake Tools extension starts supporting clangd, then someone else will want it to support another one, then another, etc. and the author(s) can't possibly handle everything like this.

That being said ! The pull request #669 that I submitting a few days ago just got merged. This pull request adds support for the following command: cmake.launchTargetDirectory. What this means is that once this gets released, you'll be able to write this:

"clangd.arguments": [ "--compile-commands-dir=/${command:cmake.launchTargetDirectory}" ]

And it will always point to the correct target directory even when you change kit, build variant or even target.

@DoDoENT
Copy link
Author

DoDoENT commented Apr 5, 2019

Thank you for letting me know. I'll try this as soon as I get the update with that feature.

C/C++ extension is the most fundamental extension required to start working with C/C++ projects so it makes sense to support it out of the box. But if the CMake Tools extension starts supporting clangd, then someone else will want it to support another one, then another, etc. and the author(s) can't possibly handle everything like this.

I completely understand this. I just wanted to see if something like that would be an option for you. Currently, C/C++ extension does not support IntelliSense in android-clang-arm64 mode and at the time of my first post it was very slow (this improved a lot with IntelliSense caching introduced with the last update) and clangd simply worked out of the box (clangd is also backend for QtCreator's completition engine), while also giving some free features, such as squiggles on unused variables, implicit casts and similar (basically all warnings that are obtained from compile_commands.json) and support for clang fix-its (a very useful feature).

However, in cases of editing header files, Microsoft's IntelliSense was much better, as clangd couldn't provide good feedback for autocomplete of header files. Also, when developing android-specific c++ code, clangd was the only thing that was working.

Finally, I've ended up with using both, because none of them are perfect and each has their own pros and cons. Most importantly, those two do not clash - both C/C++ extension and clangd can work together.

@jnickborys
Copy link

jnickborys commented May 4, 2019

I am throwing in my support for this feature. I have installed the clangd extension and it kind of works as is.

My current configuration is: clangd 9 (dev release) specifically for clang-tidy support. Without clang-tidy clang8 is an official release and works pretty well.

"clangd.arguments": [
    "-clang-tidy",
    "-background-index",
    "-compile-commands-dir=build/RelWithDebInfo"
],

background index lets me index the project so things like find all references works well, unfortunately it is an experimental feature. With this I have pretty much replaced intellisense from my workflow and its just faster with more tooling and little work. Its also the one that gives me the most grief being an experimental feature it broke for a while and I had to wait for the developers to fix it.

The only thing that is not happy is switching build configurations, and CMake project file updates which requires a "Reload Window".

@bobbrow bobbrow added the enhancement an enhancement to the product that is either not present or an improvement to an existing feature label Jul 18, 2019
@bobbrow
Copy link
Member

bobbrow commented Jul 18, 2019

Hi @DoDoENT, integration with clangd is not something we plan to do in this extension. @dcourtois's solution should make the integration easier when the next version of the extension is released. It remains to be seen whether the clangd extension will be notified of the change though since from VS Code's standpoint it may not know that the return value of the command has changed. We can revisit this after the next version is released and see if there's anything we can do to make it automatic, but I suspect we may need a feature from VS Code to notify it that command outputs have changed and therefore a "settings change" message needs to be sent to the clangd extension.

Thank you for your feedback!

@bobbrow bobbrow closed this as completed Jul 18, 2019
@FranciscoPombal
Copy link

@dcourtois

Hi,

Disclaimer: I'm not the author of the addon, so this is only my opinion: this addon should not try to support everything. C/C++ extension is the most fundamental extension required to start working with C/C++ projects so it makes sense to support it out of the box. But if the CMake Tools extension starts supporting clangd, then someone else will want it to support another one, then another, etc. and the author(s) can't possibly handle everything like this.

That being said ! The pull request #669 that I submitting a few days ago just got merged. This pull request adds support for the following command: cmake.launchTargetDirectory. What this means is that once this gets released, you'll be able to write this:

"clangd.arguments": [ "--compile-commands-dir=/${command:cmake.launchTargetDirectory}" ]

And it will always point to the correct target directory even when you change kit, build variant or even target.

This is a nice idea, but unfortunately VSCode does not support such variable substitutions in the settings.json files, so this does not work.

@dcourtois
Copy link
Contributor

Oh ok I thought the ${command:...} was a standard substitution pattern. Maybe try to pock the maintainers of the clangd addon to ask them to support such variable substitution patterns ? It's really useful to support such generic commands because addons can then add new "variables". And you be able to fix your problem since the cmake.launchTargetDirectory command has been merged into the cmake-tools addons a while back (see #666) (I love that my PR was merged with this ID :D)

@FranciscoPombal
Copy link

FranciscoPombal commented Feb 24, 2020

Oh ok I thought the ${command:...} was a standard substitution pattern. Maybe try to pock the maintainers of the clangd addon to ask them to support such variable substitution patterns ? It's really useful to support such generic commands because addons can then add new "variables". And you be able to fix your problem since the cmake.launchTargetDirectory command has been merged into the cmake-tools addons a while back (see #666) (I love that my PR was merged with this ID :D)

Yeah I realize the ball is now on the clangd-vscode folks' court. Thanks for you contribution 👍

@DoDoENT
Copy link
Author

DoDoENT commented Feb 27, 2020

Does anyone know where this issue could be reported to clangd-vscode guys? This repository seems to be an automated mirror of this link, which opens a blank page for me.

@JonTheBurger
Copy link

If it's helpful, I've found a half workaround for this. The clangd extension mentions that it expects compile_commands.json to be at the project root, so no --compile-commands-dir is required if you can manage to place it there. Helpfully, the cmake extensions provides cmake.copyCompileCommands.

{
    "clangd.arguments": [ "-log=verbose", "-pretty", "--background-index" ],
    "cmake.buildDirectory": "${workspaceFolder}/build/${buildKit}/${buildType}",
    "cmake.copyCompileCommands": "${workspaceFolder}/compile_commands.json",
}

Unfortunately, clangd doesn't seem to substitute ${workspaceFolder} either, so add compile_commands.json to .gitignore and call it a day. The reason I saw this is a "half workaround" is because if you try and open a cpp file before a cmake build has completed, clangd will attempt to run without a compile_commands.json, and just fail. "clangd.syncFileEvents": true, did not seem to help. Running Ctrl+Shift+P Restart Extension Host, as you might expect, resets clangd and does the trick.

@FranciscoPombal
Copy link

If it's helpful, I've found a half workaround for this. The clangd extension mentions that it expects compile_commands.json to be at the project root, so no --compile-commands-dir is required if you can manage to place it there. Helpfully, the cmake extensions provides cmake.copyCompileCommands.

{
    "clangd.arguments": [ "-log=verbose", "-pretty", "--background-index" ],
    "cmake.buildDirectory": "${workspaceFolder}/build/${buildKit}/${buildType}",
    "cmake.copyCompileCommands": "${workspaceFolder}/compile_commands.json",
}

Unfortunately, clangd doesn't seem to substitute ${workspaceFolder} either, so add compile_commands.json to .gitignore and call it a day. The reason I saw this is a "half workaround" is because if you try and open a cpp file before a cmake build has completed, clangd will attempt to run without a compile_commands.json, and just fail. "clangd.syncFileEvents": true, did not seem to help. Running Ctrl+Shift+P Restart Extension Host, as you might expect, resets clangd and does the trick.

Thanks, this worked like a charm. I did not experience the limitation you mention (at least to such an extent). I just make sure the CMake configuring is done before launching cpp files, no need for a full build.

@sam-mccall
Copy link

@DoDoENT Hi, clangd dev here. The repo for issues is https://github.com/clangd/clangd/issues, sorry for the confusion.

I'd like to get these to work nicely together out of the box, at least in the common case.
It'd be great if we can find a simple mechanism to do this with zero config and little magical extension communication. I have a lot of sympathy for @bobbrow's wariness of lots of tricky integrations.

I'm missing a bit of context here, because I don't use this extension or VSCode regularly. But my understanding is:

  • this extension has a 2-dimensional configuration for builds (kit/type)
  • it puts the build directory (and thus compile_commands.json) under $workspace/build/$kit/$type by default
  • the common cmake convention is to make the build directory $src/build (which only allows for one, so I can see why you extend this)
  • it's unclear whether $workspace == $src in general, but it should at least be an ancestor of $src
  • clangd looks for compile_commands in each $ancestor/ of the source file, and as of a month ago in $ancestor/build as well (among other things, to work with the standard cmake convention)

So putting this all together, the only things missing are that

  1. clangd looks for compile_commands directly under build/, but it's actually further down $kit/$type
  2. clangd doesn't know which kit/type is active

To address these, would vscode-cmake-tools consider copying or symlinking of the compile commands from the most recent cmake invocation under $workspaceFolder/build/, at least in the case where the build directory is underneath $workspaceFolder/build as it is by default?

That way it would "just work" with (the next release of) clangd, as well as any other tools that understand the cmake convention of placing the build directory under $src/build. (This logic currently lives in clangd, but I think we can move it up into common libraries so clang-tidy and others understand it too)

@bobbrow
Copy link
Member

bobbrow commented May 8, 2020

I believe @JonTheBurger's solution should work as long as clangd is able to watch for changes in the file.

clangd could also consider adding a setting that allows developers to specify the location of the compile_commands.json file so that it doesn't have to be at the root. That way they won't have to copy it to the root and update .gitignore.

@sam-mccall
Copy link

@bobbrow Yeah both of those are viable, and I think always have been (clangd does have such a setting). But I think this bug/thread is some evidence that integrating this way isn't obvious, and is only accessible to people highly motivated to understand their tools. I'd really like to make it "just work".

That way they won't have to copy it to the root and update .gitignore.

Yeah, this is why I proposed copying into the build/ directory, presumably .gitignore already needs to cover that.

I would be interested in whether you think it makes sense to expose the current configuration in the file system in some way that's simple and tool-agnostic. Interop between build systems and source tools is not great today and it'd be great to have a mechanism that's accessible to other tools like clang-tidy and IWYU that don't necessarily run as a vscode plugin.

I like build/compile_commands.json because it's dead simple and aligns with an existing cmake convention. I'm happy to try other things, but I think tight coupling in either direction at the vscode level would be a lost opportunity.

@bobbrow
Copy link
Member

bobbrow commented May 8, 2020

The default folder is ${workspaceFolder}/build 😃

@DoDoENT
Copy link
Author

DoDoENT commented May 11, 2020

The solution from @JonTheBurger works OK for me, however, it requires reloading the extension host folder each time a build type or cmake kit is changed. Otherwise, indexer gets confused about what context it's in. This is especially painful when switching between different cross-build environments (default Intellisense also has lots of troubles with that).

Also, when I update my conan dependencies I need to delete the .conand folder because otherwise, the headers of the dependencies are found in wrong (old location). If the old package no longer exists on the system (because I deleted it for conserving the disk space), the indexer completely gets confused and stops working. In those scenarios, the default IntelliSense works much better, although is at least 10 times slower.

The default folder is ${workspaceFolder}/build

Personally, I think this is a bad default. If you commonly switch between different build types and kits, it is inefficient to always rebuild the code just to make the IDE work. This is especially important if your project generates some source files during the build. Therefore I instruct my developers to use build folders whose names depend on both selected cmake kit and build type.

Furthermore, if you would like to exclude cmake build folders from backup, as they are usually temporary and are very big, so it makes little sense to have them on backup and if you are using an operating system with a poor backup solution which does not give you the ability to exclude folders by regex or glob pattern (Apple's TimeMachine, 😘), then it's a good idea to put all your build folders into a single folder and exclude it from backup. For that matter, I instruct my developers to use something like ~/builds/${workspaceRootFolderName}/${buildKit}/${variant:buildOpt} for their cmake build folder in VSCode and then exclude ~/builds from TimeMachine backup.

AFAIK, the similar approach is used by default by Visual Studio when opening folder with a CMakeLists.txt - it generates a unique folder based on original source code path and VS toolset version and puts somewhere within user's home directory and configures all projects there. Maybe this idea could be something to think about also for VSCode?

Regardless of this, ideally, clangd should somehow also be aware of different contexts and should have an API which other extensions, like CMake Tools, could tell it to switch to a different context. Thus, it should not get confused when compile_commands.json it watches has suddenly changed so drastically that the entire index should be rebuilt (theoretically it could to it even now, but it doesn't).

@Trass3r
Copy link

Trass3r commented Jun 25, 2020

Does anyone know where this issue could be reported to clangd-vscode guys?

clangd/vscode-clangd#40
clangd/vscode-clangd#42 => clangd/vscode-clangd#33

They only support a small hard-coded list of variables (workspaceRoot, workspaceFolder, cwd, env:, config:):
clangd/vscode-clangd@6f66a43#diff-7e82dc7533620e594f349d3ea425b5b4R37

@abdes
Copy link

abdes commented Dec 7, 2021

This is still super ugly, but a workaround is to configure the compilation databse in clangd own configuration file not in vscode settings and have cmake generate the .clangd file at configuration stage.

That would basically look something like this:

  1. DO NOT configure the settings for --compile-commands-dir=xxxxx in vscode settings.json
  2. create a .clangd.in in in your project with:
CompileFlags:
  CompilationDatabase: "@CMAKE_BINARY_DIR@"
  1. Add this to the CmakeLists.txt
configure_file(.clangd.in ${CMAKE_SOURCE_DIR}/.clangd @ONLY)

Every time you change the cmake preset, just reconfigure and make sure to restart clangd.

This is still super ugly and unbelievably crazy and still no one from clangd devs is willing to consider to coordinate with the cmake-tools extension and get the preset build directory automatically from cmake tools.

NOTES

  • no, it's not acceptable to have a single build directory in a project with several presets for several target platforms,
  • vscode/cmake-tools do not provide the cmake preset name or the cmake build dir as a substitution variable,
  • it's not possible to reliably run any cmake action after generation is complete, for example to move the compiler_commands.json file to somewhere else. CMake people do everything they can so that we cannot do that :-)

@github-actions github-actions bot locked and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement an enhancement to the product that is either not present or an improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

9 participants