[bazel] Add clangd as a library support #81556

josh11b · 2024-02-13T00:09:19Z

Creates a BUILD.bazel file for the clangd directory in the project overlay.

This upstreams the patch that allows https://github.com/carbon-language/carbon-lang/tree/trunk/language_server to use clangd as a library. This was created as part of a Summer of Code project building a prototype Carbon language server. If this is not an appropriate architecture, I'm very open to alternative paths forward.

Thanks!

github-actions · 2024-02-13T00:09:38Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-02-13T00:10:05Z

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clangd

Author: None (josh11b)

Changes

Both creates a BUILD.bazel file for the clangd directory in the project overlay, and removes an unnecessary #include "Feature.h" to reduce the dependencies from Transport.h.

This upstreams the patch that allows https://github.com/carbon-language/carbon-lang/tree/trunk/language_server to use clangd as a library. This was created as part of a Summer of Code project building a prototype Carbon language server. If this is not an appropriate architecture, I'm very open to alternative paths forward.

Thanks!

Full diff: https://github.com/llvm/llvm-project/pull/81556.diff

2 Files Affected:

(modified) clang-tools-extra/clangd/Transport.h (-1)
(added) utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel (+45)

diff --git a/clang-tools-extra/clangd/Transport.h b/clang-tools-extra/clangd/Transport.h
index 4e80ea95b8537d..f17441cfc1ef22 100644
--- a/clang-tools-extra/clangd/Transport.h
+++ b/clang-tools-extra/clangd/Transport.h
@@ -18,7 +18,6 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H
 
-#include "Feature.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
diff --git a/utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel b/utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel
new file mode 100644
index 00000000000000..0e25dd1c166560
--- /dev/null
+++ b/utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel
@@ -0,0 +1,45 @@
+# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+package(default_visibility = ["//visibility:public"])
+
+licenses(["notice"])
+
+cc_library(
+    name = "clangd_library",
+    srcs = [
+        "JSONTransport.cpp",
+        "Protocol.cpp",
+        "URI.cpp",
+        "index/SymbolID.cpp",
+        "support/Logger.cpp",
+        "support/Trace.cpp",
+        "support/MemoryTree.cpp",
+        "support/Context.cpp",
+        "support/Cancellation.cpp",
+        "support/ThreadCrashReporter.cpp",
+        "support/Shutdown.cpp",
+    ],
+    hdrs = [
+        "Transport.h",
+        "Protocol.h",
+        "URI.h",
+        "LSPBinder.h",
+        "index/SymbolID.h",
+        "support/Function.h",
+        "support/Cancellation.h",
+        "support/ThreadCrashReporter.h",
+        "support/Logger.h",
+        "support/Trace.h",
+        "support/MemoryTree.h",
+        "support/Context.h",
+        "support/Shutdown.h",
+    ],
+    includes = ["."],
+    deps = [
+        "//llvm:Support",
+        "//clang:basic",
+        "//clang:index",
+    ],
+)

github-actions · 2024-02-13T00:10:19Z

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

josh11b · 2024-02-14T21:43:26Z

I've made my email address public, as requested.

josh11b · 2024-02-29T00:38:33Z

Ping!

josh11b · 2024-03-14T20:55:31Z

Ping!

MaskRay · 2024-03-14T20:55:24Z

utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel

+licenses(["notice"])
+
+cc_library(
+    name = "clangd_library",


clangDaemon would match the library name in clang-tools-extra/clangd/CMakeLists.txt

Small nit: IIUC the convention in the llvm bzl tree is to use casing like ClangDaemon.

MaskRay · 2024-03-14T20:56:11Z

utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel

+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+package(default_visibility = ["//visibility:public"])


Add features = ["layering_check"],

MaskRay · 2024-03-14T21:06:13Z

Ping!

llvm-project email notifications are extremely high-volume and nobody can notice every change even if they are subscribed to the relevant labels...
(Now, I noticed this patch because I noticed a message in an IM I am subscribed to... not that I actively check clang-tools-extra patches/issues)

If you don't have write/triage role (https://docs.github.com/en/organizations/managing-user-access-to-your-organizations-repositories/managing-repository-roles/repository-roles-for-an-organization#repository-roles-for-organizations), the best is to CC relevant people (git log -- directory and finding recent reviewers is a good way).

Feel free to ping the patch once a week when no feedback is received per https://llvm.org/docs/Contributing.html#how-to-submit-a-patch

If you have received no comments on your patch for a week, you can request a review by ‘ping’ing the GitHub PR with “Ping”. The common courtesy ‘ping’ rate is once a week. Please remember that you are asking for valuable time from other professional developers.

MaskRay

bazel-specific changes often have a title [bazel] Add clangd support. Please consider following suit.

It would be nice to have a more complete support for clangd, but I guess having this cc_library first can make your downstream changes easier, so LGTM.

MaskRay · 2024-03-14T21:07:40Z

utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel

+# See https://llvm.org/LICENSE.txt for license information.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+package(features = ["layering_check"],


llvm/BUILD.bazel places default_visibility before features. Best to follow its convention.

MaskRay · 2024-03-14T21:08:04Z

clang-tools-extra/clangd/Transport.h

@@ -18,7 +18,6 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRANSPORT_H

-#include "Feature.h"


This change should be removed.

rupprecht · 2024-03-14T21:05:22Z

utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel

+    srcs = [
+        "JSONTransport.cpp",
+        "Protocol.cpp",
+        "URI.cpp",
+        "index/SymbolID.cpp",
+        "support/Logger.cpp",
+        "support/Trace.cpp",
+        "support/MemoryTree.cpp",
+        "support/Context.cpp",
+        "support/Cancellation.cpp",
+        "support/ThreadCrashReporter.cpp",
+        "support/Shutdown.cpp",
+    ],
+    hdrs = [
+        "Transport.h",
+        "Protocol.h",
+        "URI.h",
+        "LSPBinder.h",
+        "index/SymbolID.h",
+        "support/Function.h",
+        "support/Cancellation.h",
+        "support/ThreadCrashReporter.h",
+        "support/Logger.h",
+        "support/Trace.h",
+        "support/MemoryTree.h",
+        "support/Context.h",
+        "support/Shutdown.h",
+    ],


clangd/CMakeLists.txt lists many other files needed by the clangDaemon target.

I think this is totally fine to submit as-is, as maybe this is the minimal set of files that one needs for basic clangd functionality, but we should probably have a TODO to pick up other files (e.g. glob(["*.cpp", "dir/**/*.cpp", ...])) for more complete functionality. (I'm not a clangd expert, so I'm not sure what the implication of leaving out all those other files is).

I added a # TODO comment.

rupprecht · 2024-03-14T21:09:38Z

utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel

+licenses(["notice"])
+
+cc_library(
+    name = "clangd_library",


Small nit: IIUC the convention in the llvm bzl tree is to use casing like ClangDaemon.

josh11b

Thank you for the feedback!

josh11b · 2024-03-14T21:26:54Z

utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel

+    srcs = [
+        "JSONTransport.cpp",
+        "Protocol.cpp",
+        "URI.cpp",
+        "index/SymbolID.cpp",
+        "support/Logger.cpp",
+        "support/Trace.cpp",
+        "support/MemoryTree.cpp",
+        "support/Context.cpp",
+        "support/Cancellation.cpp",
+        "support/ThreadCrashReporter.cpp",
+        "support/Shutdown.cpp",
+    ],
+    hdrs = [
+        "Transport.h",
+        "Protocol.h",
+        "URI.h",
+        "LSPBinder.h",
+        "index/SymbolID.h",
+        "support/Function.h",
+        "support/Cancellation.h",
+        "support/ThreadCrashReporter.h",
+        "support/Logger.h",
+        "support/Trace.h",
+        "support/MemoryTree.h",
+        "support/Context.h",
+        "support/Shutdown.h",
+    ],


I added a # TODO comment.

josh11b · 2024-03-14T21:27:13Z

utils/bazel/llvm-project-overlay/clang-tools-extra/clangd/BUILD.bazel

+licenses(["notice"])
+
+cc_library(
+    name = "clangd_library",


github-actions · 2024-03-15T00:54:09Z

@josh11b Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

rupprecht · 2024-03-15T00:56:35Z

Thanks! Sorry about the delay, we should get the bzl tree setup for automatic assignment to reviewers.

llvmbot added clang-tools-extra clangd labels Feb 13, 2024

Add bazel support for clangd as a library.

e2b83f0

josh11b force-pushed the main branch from 25c2f4f to e2b83f0 Compare February 13, 2024 00:27

MaskRay reviewed Mar 14, 2024

View reviewed changes

josh11b added 2 commits March 14, 2024 21:02

Implement suggestions from review

9dc8c8e

Merge branch 'main' into main

3fa3242

MaskRay approved these changes Mar 14, 2024

View reviewed changes

rupprecht reviewed Mar 14, 2024

View reviewed changes

josh11b changed the title ~~Add bazel support for clangd as a library.~~ [bazel] Add clangd as a library support Mar 14, 2024

josh11b added 2 commits March 14, 2024 21:17

Checkpoint progress.

ca0ab60

Incorporate more feedback

86b82c8

josh11b commented Mar 14, 2024

View reviewed changes

Merge branch 'main' into main

8b70021

rupprecht approved these changes Mar 15, 2024

View reviewed changes

rupprecht merged commit bff8755 into llvm:main Mar 15, 2024
4 checks passed

josh11b mentioned this pull request Mar 20, 2024

Remove clangd BUILD patch carbon-language/carbon-lang#3136

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[bazel] Add clangd as a library support #81556

[bazel] Add clangd as a library support #81556

josh11b commented Feb 13, 2024 •

edited

github-actions bot commented Feb 13, 2024

llvmbot commented Feb 13, 2024 •

edited

github-actions bot commented Feb 13, 2024

josh11b commented Feb 14, 2024

josh11b commented Feb 29, 2024

josh11b commented Mar 14, 2024

MaskRay Mar 14, 2024

josh11b Mar 14, 2024

rupprecht Mar 14, 2024

josh11b Mar 14, 2024

MaskRay Mar 14, 2024

josh11b Mar 14, 2024

MaskRay commented Mar 14, 2024 •

edited

MaskRay left a comment •

edited

MaskRay Mar 14, 2024

josh11b Mar 14, 2024

MaskRay Mar 14, 2024

josh11b Mar 14, 2024

rupprecht Mar 14, 2024

josh11b Mar 14, 2024

rupprecht Mar 14, 2024

josh11b left a comment

josh11b Mar 14, 2024

josh11b Mar 14, 2024

github-actions bot commented Mar 15, 2024

rupprecht commented Mar 15, 2024

[bazel] Add clangd as a library support #81556

[bazel] Add clangd as a library support #81556

Conversation

josh11b commented Feb 13, 2024 • edited

github-actions bot commented Feb 13, 2024

llvmbot commented Feb 13, 2024 • edited

github-actions bot commented Feb 13, 2024

josh11b commented Feb 14, 2024

josh11b commented Feb 29, 2024

josh11b commented Mar 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaskRay commented Mar 14, 2024 • edited

MaskRay left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josh11b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 15, 2024

rupprecht commented Mar 15, 2024

josh11b commented Feb 13, 2024 •

edited

llvmbot commented Feb 13, 2024 •

edited

MaskRay commented Mar 14, 2024 •

edited

MaskRay left a comment •

edited