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

[cmake] Auto-complete via clangd auto-setup #29313

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ntrrgc
Copy link
Contributor

@ntrrgc ntrrgc commented May 30, 2024

b506202

[cmake] Auto-complete via clangd auto-setup
https://bugs.webkit.org/show_bug.cgi?id=274909

Reviewed by NOBODY (OOPS!).

 # Goals

This patch aims to bring out-of-the box completion with clangd for
WebKit developers using CMake builds.

clangd is an open source language server for providing code-completion,
go-to-definition and similar smart features for C++ code by analyzing it
with the same backend clang uses. **You do not need to compile WebKit
with clang to take advantage of this**, as clang strives to be flag
compatible with GCC, and even MSVC through `clang-cl`.

Using clangd with WebKit was already possible before, but required
manual intervention in the project files to set it up.

This patch attempts to clear some of the existing pitfalls,
particularly for CMake builds, so that more people take advantage of
Clangd without having to put conscious effort into troubleshooting.

This patch also makes WebKit stock clangd setup compatible with both
Mac and non-Mac platforms and allows developers to customize clangd
configuration without dirtying their repository.

---

Getting clangd right takes a lot of context and moving pieces, so this
is one of those very long commit messages. Sections with (NEW) in the
title explain changes introduced by this patch, whereas the rest hope to
explain the background and why those changes are necessary or useful.

 # Architecture (NEW)

Several scripts have been added in Tools/clangd. A diagram describing
the build system targets, dependencies and products is also included.

The clangd auto-setup needs to generate the following artifacts:
 * .clangd configuration file
 * compile_commands.json file/symlink in the WebKit repo root.

Starting with this patch `.clangd` is no longer committed in the
repository it is now .gitignore'd and auto-generated during the build.

 # Compatibility with XCode builds (NEW)

This patch doesn't improve or worsen the status of clangd support in
XCode builds.

XCode build users are still expected to generate compile_commands.json
manually as per the wiki states:
https://trac.webkit.org/wiki/Clangd#macOS1

    make release EXPORT_COMPILE_COMMANDS=YES
    generate-compile-commands WebKitBuild/Release

The generation of the `.clangd` file is also included in the XCode
build, as part of the WTF XCode project.

 # clangd config generation (NEW)

The script `update-clangd-config` autogenerates a `.clangd` file with an
«autogenerated» marker at the top. The file will only be updated as long
as that marker exists, therefore allowing end-user customization if
needed.

The `.clangd` file is generated from a Jinja template, which will produce
slightly different options for Mac and non-Mac platforms, fixing the
regression introduced in #23273

`update-clangd-config` is made part of the build by default. For XCode
builds, it is generated as part of WTF. For CMake builds, it is enabled
as a default target as long as CLANGD_AUTO_SETUP is set, which is the
default when building inside the WebKitBuild directory.

 # clangd configuration changes (NEW)

This patch also makes several changes to the clangd configuration file
to fix issues detected during testing:

 * UnifiedSource files are no longer indexed.
 * The `-fcoroutines` flag is stripped, as it is not recognized by some
   versions of clang, and by extension, clangd.
 * `-xobjective-c++` is now only passed in macOS to avoid depending on
   macOS-only headers in other systems.
 * `config.h` is no longer prepended to files within the ThirdParty
   directory.

 # compile_commands.json generation: context

CMake already generates a compile_commands.json file when the Ninja or
Makefile file is generated as long as CMAKE_EXPORT_COMPILE_COMMANDS is
ON, which is already the default in DEVELOPER_MODE.

When using unified builds (which is the default), that
`compile_commands.json` is somewhat subpar, as the language server is
unaware of most of the source .cpp files and hence has to require to
heuristics to match them to other build commands.

In XCode builds, the `generate-compile-commands` scripts already includes
logic to resolve unified builds.

On the CMake side, a RewriteCompileCommands target exists that parses
that file along with unified sources to generate an improved file at
`<BUILD_DIR>/DeveloperTools/compile_commands.json`. Unfortunately, this
may not be obvious to many developers, which may be unaware of its
existence.

 # Same compile_commands.json path for non-unified builds (NEW)

The first change this patch makes is solving the unified builds vs
non-unified builds dichotomy by introducing a new build target for
non-unified builds named SymlinkCompileCommandsInDeveloperTools.

This way, **no matter the type of build**,
`<BUILD_DIR>/DeveloperTools/compile_commands.json` is the path to be
relied upon.

 # compile_commands.json vs builds

For a `compile_commands.json` file to be used, it needs to exist in a
parent directory of the sources being worked on.

Since any generated `compile_commands.json` is specific for a given build
(e.g. Debug vs Release), with build-specific flags, developers need to
make a choice of what build to use for the `compile_commands.json` in
their WebKit checkout.

In XCode builds, this is passed explicitly to the
generate-compile-commands command.

In CMake builds, this is usually accomplished by symlinking the file
into the root of the WebKit repository.

Developers who frequently switch and remove builds need to be careful to
not let the compile_commands.json symlink become broken, as that would
cause the language server to rely on much worsened heuristics.

 # compile_commands.json symlink autogeneration and management (NEW)

This patch introduces automatic compile_commands.json symlink handling.
A new target is introduced that intentionally runs with every build:
`UpdateCompileCommandsSymlink`, which relies on the
`update-compile-commands-symlink` script, also introduced by this patch.
The script is meant to be fast enough to not introduce any perceivable
delay in builds.

`update-compile-commands-symlink` receives as arguments a symlink path and
a YAML file listing a series of candidate paths. The symlink is updated
to point to the first candidate path that exists.

This list of candidate paths is declared in a new .gitignore'd
`compile_commands.conf` file that is generated early during any CMake
build that uses `CLANGD_AUTO_SETUP`.

`compile_commands.conf` will be automatically updated from
`Tools/clangd/compile_commands.conf.example`; but similarly to `.clangd`,
a user can disable this and manage the file on their own, in this case
by setting the `user_managed` property inside the file to True.

 # Testing

clangd auto-setup has been tested with a Linux x86_64 machine using
webkit-container-sdk and Visual Studio Code, both with the default GCC
and with clang 16, with a fresh installation of VS Code (no settings).

It is possible to confirm if a file passes under clangd by using the
`clangd --check` command along with the compile lines used during
compilation, which can be queried from compile_commands.json.

A couple barebones scripts showing this usage have been provided in
Tools/clangd as documentation: `clangd-check-file` and
`clangd-check-codebase`.

I was able to run clangd-check-codebase with very few .cpp files
failing, and the failing files had good reasons for failing, like
missing includes that were obscured by unified builds.

b506202

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
⏳ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
⏳ 🧪 vision-wk2 ❌ 🛠 jsc-armv7
✅ 🛠 tv ❌ 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@ntrrgc ntrrgc requested a review from JonWBedard as a code owner May 30, 2024 17:12
@ntrrgc ntrrgc self-assigned this May 30, 2024
@ntrrgc ntrrgc added the Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases label May 30, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 30, 2024
@ntrrgc ntrrgc removed the merging-blocked Applied to prevent a change from being merged label May 30, 2024
@ntrrgc ntrrgc force-pushed the 2024-04-26-clangd-auto-setup branch from 05612f8 to 845e6b2 Compare May 30, 2024 17:22
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 30, 2024
@ntrrgc ntrrgc removed the merging-blocked Applied to prevent a change from being merged label May 30, 2024
@ntrrgc ntrrgc force-pushed the 2024-04-26-clangd-auto-setup branch from 845e6b2 to 06a7bb0 Compare May 30, 2024 18:44
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 30, 2024

set(webkitbuild_dir ${CMAKE_SOURCE_DIR}/WebKitBuild)
cmake_path(IS_PREFIX webkitbuild_dir ${CMAKE_BINARY_DIR} NORMALIZE clangd_auto_setup_default)
set(CLANGD_AUTO_SETUP ${clangd_auto_setup_default} CACHE BOOL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

option() is just a specific case of set(). It might be possible but there would be no difference whatsoever.

endif ()

set(webkitbuild_dir ${CMAKE_SOURCE_DIR}/WebKitBuild)
cmake_path(IS_PREFIX webkitbuild_dir ${CMAKE_BINARY_DIR} NORMALIZE clangd_auto_setup_default)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you check this here? How about checking DEVELOPER_MODE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would definitely be an option. This was my train of thought behind this default: "writing things in the source tree instead of the build tree is generally frowned upon, but we need to do it for clangd. However, if the user is building inside WebKitBuild, they're already doing a build inside their source tree, so it's not dirtying that much more.

However, I could use DEVELOPER_MODE, or even boolean OR of DEVELOPER_MODE or building inside WebKitBuild. What do you think makes the most sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. I have no preference. It's up to you.

# Create a simple symlink in DeveloperTools/compile_commands.json so that
# it can be relied upon regardless of unified or non-unified builds.
add_custom_command(
OUTPUT ${CMAKE_BINARY_DIR}/DeveloperTools/compile_commands.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can generate DeveloperTools/compile_commands.json symlink at CMake time rather than build time for non-unified builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but when using unified builds it would still be generated at build time, so I see no reason to do things differently for non-unified builds.

Ideally I'd like both to be done at CMake generation time. Unfortunately CMake doesn't support that. I looked even into the CMake source code to look for somewhere to hook it into, but compile_commands.json is only written at the end of generation, long after all configure scripts have run.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Update) I ended up changing this to create the symlink in configure time, as it didn't add much complexity.

# it can be relied upon regardless of unified or non-unified builds.
add_custom_command(
OUTPUT ${CMAKE_BINARY_DIR}/DeveloperTools/compile_commands.json
DEPENDS ${CMAKE_BINARY_DIR}/compile_commands.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to regenerate DeveloperTools/compile_commands.json every time compile_commands.json is updated?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be accurate yes, I would assume the main compile_commands.json only changes when cmake is invoked with changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compile_commands.json is generated along with the Ninja file, so I would expect both to be dirtied simultaneously.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeveloperTools/compile_commands.json is just a symlink constantly refering to ../compile_commands.json in non-unified build case.

${CMAKE_BINARY_DIR}/DeveloperTools/compile_commands.json
VERBATIM
)
add_custom_target(SymlinkCompileCommandsInDeveloperTools
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are using a pair of add_custom_command and add_custom_target here. But, above section RewriteCompileCommands is using only add_custom_target. How different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the confusion... it comes from CMake being very janky.

From add_custom_target() documentation:

The target has no output file and is always considered out of date even if the commands try to create a file with the name of the target. Use the add_custom_command() command to generate a file with dependencies.

You may be thinking "wait, does that mean RewriteCompileCommands is running every time you build?" Yes, the answer is yes. That could very much be fixed and I was tempted on patching that too, but I ultimately considered it outside of the scope of this patch.

You may think that just using add_custom_command() instead would solve it, but the jank strikes again: add_custom_command() adds a rule to generate the files passed as output, but it does not on itself add a target! If those output files are not used by any targets, then they won't be built. Here is a minimal case showing the problem:

cmake_minimum_required(VERSION 3.16)
project(proof_of_jank)

add_custom_command(
    OUTPUT autogenerated.txt
    COMMAND echo Hello | tee autogenerated.txt
)

add_executable(foo main.c)
$ ninja all
[1/1] Linking C executable foo
$ ls autogenerated.txt
ls: cannot access 'autogenerated.txt': No such file or directory

So what do you do if, like here, you want a command to run to generate a file, but that file is not part of the sources of an executable? You define a rule with add_custom_command() and then depend on it in add_custom_target(). It's not a problem that add_custom_target() "runs" every time if it doesn't have any command but only dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may be thinking "wait, does that mean RewriteCompileCommands is running every time you build?" Yes, the answer is yes. That could very much be fixed and I was tempted on patching that too, but I ultimately considered it outside of the scope of this patch.

(update) I went ahead and fixed this too in the latest version of the patch.

set(CLANGD_AUTO_SETUP ${clangd_auto_setup_default} CACHE BOOL
"Install a .clangd configuration file and a compile_commands.json symlink "
"in the root of the source tree to have out-of-the-box code completion "
"in editors.")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does clangd work with GCC?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang is the only backend for clangd, however you can use gcc to compile and still use clangd. The compile arguments just may differ a bit leading to inaccurate results.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes as long as clang can parse all the GCC flags being used. In the case of WebKit, this is the case, I tested it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about SVG rather than PNG?

Copy link
Contributor Author

@ntrrgc ntrrgc Jun 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose PNG for compatibility reasons (text rendering in SVG is quite inconsistent in practice), but I can upload SVG if you prefer, or both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Update) I uploaded now versions in both PNG and SVG.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a suggestion. WDYT?

  • Rename update-symlink-from-candidates to update-compile-commands-symlink.
  • update-compile-commands-symlink takes no arguments
  • update-compile-commands-symlink reads $top/Tools/clangd/update-compile-commands-symlink.conf and $top/.update-compile-commands-symlink.conf if exists.
  • update-compile-commands-symlink generates $top/compile_commands.json symlink.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename update-symlink-from-candidates to update-compile-commands-symlink.

I wouldn't mind.

update-compile-commands-symlink takes no arguments

I initially did it that way and ended up adding arguments. I don't remember what was the reason I switched, maybe simply because it was more honest with the build systems (since you still have to code those paths for CMake and XCode so they know when it needs to be run again. If the command takes no arguments, then it's very easy to end up with a typo or outdated file name in the dependencies of the rule which would cause the command to be ran too many times instead of immediately reporting the error.

update-compile-commands-symlink reads $top/Tools/clangd/update-compile-commands-symlink.conf and $top/.update-compile-commands-symlink.conf if exists.

That feels a bit more elegant, but I think the current approach is better for discoverability actually, and that's much more important in my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename update-symlink-from-candidates to update-compile-commands-symlink.

(Update) Done. The other suggestions I didn't follow for the reasons explained in the previous comment.

@fujii fujii requested a review from emw-apple June 3, 2024 21:49
${TOOLS_DIR}/clangd/compile_commands.conf.example
COMMAND ${Python_EXECUTABLE}
${TOOLS_DIR}/clangd/update-compile-commands-conf
${CMAKE_SOURCE_DIR}/compile_commands.conf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to pass the path of compile_commands.conf.example as an argument?
#29313 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would make sense indeed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Update) This is done in the new version.

Copy link
Contributor

@fujii fujii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch doesn't work on Windows. It seems that symlinks have to use backslashes on Windows. Could you fix.
https://webkit.slack.com/archives/CU84Q46JZ/p1717377736652069?thread_ts=1717142017.022589&cid=CU84Q46JZ

https://bugs.webkit.org/show_bug.cgi?id=274909

Reviewed by NOBODY (OOPS!).

 # Goals

This patch aims to bring out-of-the box completion with clangd for
WebKit developers using CMake builds.

clangd is an open source language server for providing code-completion,
go-to-definition and similar smart features for C++ code by analyzing it
with the same backend clang uses. **You do not need to compile WebKit
with clang to take advantage of this**, as clang strives to be flag
compatible with GCC, and even MSVC through `clang-cl`.

Using clangd with WebKit was already possible before, but required
manual intervention in the project files to set it up.

This patch attempts to clear some of the existing pitfalls,
particularly for CMake builds, so that more people take advantage of
Clangd without having to put conscious effort into troubleshooting.

This patch also makes WebKit stock clangd setup compatible with both
Mac and non-Mac platforms and allows developers to customize clangd
configuration without dirtying their repository.

---

Getting clangd right takes a lot of context and moving pieces, so this
is one of those very long commit messages. Sections with (NEW) in the
title explain changes introduced by this patch, whereas the rest hope to
explain the background and why those changes are necessary or useful.

 # Architecture (NEW)

Several scripts have been added in Tools/clangd. A diagram describing
the build system targets, dependencies and products is also included.

The clangd auto-setup needs to generate the following artifacts:
 * .clangd configuration file
 * compile_commands.json file/symlink in the WebKit repo root.

Starting with this patch `.clangd` is no longer committed in the
repository it is now .gitignore'd and auto-generated during the build.

 # Compatibility with XCode builds (NEW)

This patch doesn't improve or worsen the status of clangd support in
XCode builds.

XCode build users are still expected to generate compile_commands.json
manually as per the wiki states:
https://trac.webkit.org/wiki/Clangd#macOS1

    make release EXPORT_COMPILE_COMMANDS=YES
    generate-compile-commands WebKitBuild/Release

The generation of the `.clangd` file is also included in the XCode
build, as part of the WTF XCode project.

 # clangd config generation (NEW)

The script `update-clangd-config` autogenerates a `.clangd` file with an
«autogenerated» marker at the top. The file will only be updated as long
as that marker exists, therefore allowing end-user customization if
needed.

The `.clangd` file is generated from a Jinja template, which will produce
slightly different options for Mac and non-Mac platforms, fixing the
regression introduced in WebKit#23273.

`update-clangd-config` is made part of the build by default. For XCode
builds, it is generated as part of WTF. For CMake builds, it is enabled
as a default target as long as CLANGD_AUTO_SETUP is set, which is the
default when building inside the WebKitBuild directory.

 # clangd configuration changes (NEW)

This patch also makes several changes to the clangd configuration file
to fix issues detected during testing:

 * UnifiedSource files are no longer indexed.
 * The `-fcoroutines` flag is stripped, as it is not recognized by some
   versions of clang, and by extension, clangd.
 * `-xobjective-c++` is now only passed in macOS to avoid depending on
   macOS-only headers in other systems.
 * `config.h` is no longer prepended to files within the ThirdParty
   directory.

 # compile_commands.json generation: context

CMake already generates a compile_commands.json file when the Ninja or
Makefile file is generated as long as CMAKE_EXPORT_COMPILE_COMMANDS is
ON, which is already the default in DEVELOPER_MODE.

When using unified builds (which is the default), that
`compile_commands.json` is somewhat subpar, as the language server is
unaware of most of the source .cpp files and hence has to require to
heuristics to match them to other build commands.

In XCode builds, the `generate-compile-commands` scripts already includes
logic to resolve unified builds.

On the CMake side, a RewriteCompileCommands target exists that parses
that file along with unified sources to generate an improved file at
`<BUILD_DIR>/DeveloperTools/compile_commands.json`. Unfortunately, this
may not be obvious to many developers, which may be unaware of its
existence.

 # Same compile_commands.json path for non-unified builds (NEW)

The first change this patch makes is solving the unified builds vs
non-unified builds dichotomy by introducing a new build target for
non-unified builds named SymlinkCompileCommandsInDeveloperTools.

This way, **no matter the type of build**,
`<BUILD_DIR>/DeveloperTools/compile_commands.json` is the path to be
relied upon.

 # compile_commands.json vs builds

For a `compile_commands.json` file to be used, it needs to exist in a
parent directory of the sources being worked on.

Since any generated `compile_commands.json` is specific for a given build
(e.g. Debug vs Release), with build-specific flags, developers need to
make a choice of what build to use for the `compile_commands.json` in
their WebKit checkout.

In XCode builds, this is passed explicitly to the
generate-compile-commands command.

In CMake builds, this is usually accomplished by symlinking the file
into the root of the WebKit repository.

Developers who frequently switch and remove builds need to be careful to
not let the compile_commands.json symlink become broken, as that would
cause the language server to rely on much worsened heuristics.

 # compile_commands.json symlink autogeneration and management (NEW)

This patch introduces automatic compile_commands.json symlink handling.
A new target is introduced that intentionally runs with every build:
`UpdateCompileCommandsSymlink`, which relies on the
`update-compile-commands-symlink` script, also introduced by this patch.
The script is meant to be fast enough to not introduce any perceivable
delay in builds.

`update-compile-commands-symlink` receives as arguments a symlink path and
a YAML file listing a series of candidate paths. The symlink is updated
to point to the first candidate path that exists.

This list of candidate paths is declared in a new .gitignore'd
`compile_commands.conf` file that is generated early during any CMake
build that uses `CLANGD_AUTO_SETUP`.

`compile_commands.conf` will be automatically updated from
`Tools/clangd/compile_commands.conf.example`; but similarly to `.clangd`,
a user can disable this and manage the file on their own, in this case
by setting the `user_managed` property inside the file to True.

 # Testing

clangd auto-setup has been tested with a Linux x86_64 machine using
webkit-container-sdk and Visual Studio Code, both with the default GCC
and with clang 16, with a fresh installation of VS Code (no settings).

It is possible to confirm if a file passes under clangd by using the
`clangd --check` command along with the compile lines used during
compilation, which can be queried from compile_commands.json.

A couple barebones scripts showing this usage have been provided in
Tools/clangd as documentation: `clangd-check-file` and
`clangd-check-codebase`.

I was able to run clangd-check-codebase with very few .cpp files
failing, and the failing files had good reasons for failing, like
missing includes that were obscured by unified builds.
@ntrrgc ntrrgc removed the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024
@ntrrgc ntrrgc changed the title [cmake] clangd auto-setup [cmake] Auto-complete via clangd auto-setup Jun 18, 2024
@ntrrgc ntrrgc force-pushed the 2024-04-26-clangd-auto-setup branch from 06a7bb0 to b506202 Compare June 18, 2024 18:24
@ntrrgc
Copy link
Contributor Author

ntrrgc commented Jun 18, 2024

This patch doesn't work on Windows. It seems that symlinks have to use backslashes on Windows. Could you fix. https://webkit.slack.com/archives/CU84Q46JZ/p1717377736652069?thread_ts=1717142017.022589&cid=CU84Q46JZ

(Update) This should now be fixed.

@ntrrgc
Copy link
Contributor Author

ntrrgc commented Jun 18, 2024

Summary of the changes in this version:

  • Made it more clear that clangd is about code-completion and that this is GCC compatible.
  • Added backslash conversions for symlinks in Windows (from @fujii's suggested changes in Slack)
  • CLANGD_AUTO_SETUP will default to ON if building inside WebKitBuild/ or if using DEVELOPER_MODE (previously it defaulted to ON only if building inside WebKitBuild/.
  • Renamed update-symlink-from-candidates -> update-compile-commands-symlink.
  • Added JSCOnly builds to the search list in compile_commands.conf.example.
  • Unified builds: Drive-by fix: Make RewriteCompileCommands run only when compile_commands.json is dirtied (e.g. when re-configuring the project).
  • Non-unified builds: The DeveloperTools/compile_commands.json -> ../compile_commands.json symlink is created now on configure time, and the DeveloperTools directory is created if it doesn't exist (this was missing earlier, which would make the symlink creation fail on clean builds).
  • update-compile-commands-conf now also receives compile_commands.conf.example as an argument, for consistency.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merging-blocked Applied to prevent a change from being merged Tools / Tests Tools in the Tools directory, build issues, test infrastructure, and bugs in test cases
Projects
None yet
5 participants