[clangd] Support parsing comments without ASTContext #78491

tom-anders · 2024-01-17T19:26:35Z

This is in preparation for implementing doxygen parsing, see discussion in clangd/clangd#529.

(Old Phabricator review: https://reviews.llvm.org/D143112)

This is in preparation for implementing doxygen parsing, see discussion in clangd/clangd#529. Differential Revision: https://reviews.llvm.org/D143112

llvmbot · 2024-01-17T19:27:06Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangd

Author: Tom Praschan (tom-anders)

Changes

This is in preparation for implementing doxygen parsing, see discussion in clangd/clangd#529.

(Old Phabricator review: https://reviews.llvm.org/D143112)

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

3 Files Affected:

(modified) clang-tools-extra/clangd/CodeCompletionStrings.cpp (+24)
(modified) clang-tools-extra/clangd/CodeCompletionStrings.h (+11)
(modified) clang/include/clang/Basic/SourceManager.h (+1-1)

diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
index 2075e5965f181e..540eaa9a3eb6d7 100644
--- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -9,6 +9,9 @@
 #include "CodeCompletionStrings.h"
 #include "clang-c/Index.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/CommentLexer.h"
+#include "clang/AST/CommentParser.h"
+#include "clang/AST/CommentSema.h"
 #include "clang/AST/RawCommentList.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
@@ -316,5 +319,26 @@ std::string getReturnType(const CodeCompletionString &CCS) {
   return "";
 }
 
+comments::FullComment *parseComment(llvm::StringRef Comment,
+                                    llvm::BumpPtrAllocator &Allocator,
+                                    comments::CommandTraits &Traits) {
+  // The comment lexer expects markers, so add them back
+  auto CommentWithMarkers = "/*" + Comment.str() + "*/";
+
+  SourceManagerForFile SourceMgrForFile("mock_file.cpp", CommentWithMarkers);
+  SourceManager &SourceMgr = SourceMgrForFile.get();
+
+  comments::Lexer L(Allocator, SourceMgr.getDiagnostics(), Traits,
+                    SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID()),
+                    CommentWithMarkers.data(),
+                    CommentWithMarkers.data() + CommentWithMarkers.size());
+  comments::Sema S(Allocator, SourceMgr, SourceMgr.getDiagnostics(), Traits,
+                   nullptr);
+  comments::Parser P(L, S, Allocator, SourceMgr, SourceMgr.getDiagnostics(),
+                     Traits);
+
+  return P.parseFullComment();
+}
+
 } // namespace clangd
 } // namespace clang
diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h
index fa81ad64d406c3..b93420c68f32db 100644
--- a/clang-tools-extra/clangd/CodeCompletionStrings.h
+++ b/clang-tools-extra/clangd/CodeCompletionStrings.h
@@ -19,6 +19,11 @@
 namespace clang {
 class ASTContext;
 
+namespace comments {
+class CommandTraits;
+class FullComment;
+} // namespace comments
+
 namespace clangd {
 
 /// Gets a minimally formatted documentation comment of \p Result, with comment
@@ -67,6 +72,12 @@ std::string formatDocumentation(const CodeCompletionString &CCS,
 /// is usually the return type of a function.
 std::string getReturnType(const CodeCompletionString &CCS);
 
+/// Parse the \p Comment, storing the result in \p Allocator, assuming
+/// that comment markers have already been stripped (e.g. via getDocComment())
+comments::FullComment *parseComment(llvm::StringRef Comment,
+                                    llvm::BumpPtrAllocator &Allocator,
+                                    comments::CommandTraits &Traits);
+
 } // namespace clangd
 } // namespace clang
 
diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index d2ece14da0b11a..07e3fca1641c73 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -1971,7 +1971,7 @@ class BeforeThanCompare<SourceRange> {
 };
 
 /// SourceManager and necessary dependencies (e.g. VFS, FileManager) for a
-/// single in-memorty file.
+/// single in-memory file.
 class SourceManagerForFile {
 public:
   /// Creates SourceManager and necessary dependencies (e.g. VFS, FileManager).

trustytrojan · 2024-05-02T20:37:26Z

When will this ever get merged? Would really like to see vscode-clangd finally be able to render Doxygen comments and/or Markdown comments.

aaronliu0130 · 2024-05-02T20:40:08Z

Well, the last ready-to-go PR was from September (#65448), so probably a while.

trustytrojan · 2024-05-02T20:47:39Z

@aaronliu0130 Do you know if a patch for this exists?

aaronliu0130 · 2024-05-02T21:10:41Z

As mentioned in clangd/clangd#529 (comment), they're here. Hopefully you can follow the build instructions.

tfninjadoom · 2024-06-10T08:20:20Z

I'm new to the processes of open-source software, so pardon my confusion. I just read through this issue and am wondering:
What's preventing this PR from being merged right now? All checks seem to have passed.

aaronliu0130 · 2024-06-10T11:37:20Z

LLVM has 2.5k PRs to review while working on their own changes

cor3ntin · 2024-06-10T12:27:53Z

PRs need to be reviewed before they can be merged.
Feel free to ping PR once a week (at most) if no one gets to it.
This seem to have flown under the radar. sorry about that

aaronliu0130 · 2024-06-10T14:24:03Z

What's "ping PR"? Do you mean pinging the LLVM organization?

HighCommander4 · 2024-06-11T05:32:28Z

What's "ping PR"? Do you mean pinging the LLVM organization?

It just means posting a comment on the PR that says "ping", as a reminder for reviewers.

(But please only do that if you're the patch author, or, in the case of a patch author who's not active any more, if you're volunteering to take over the patch from the patch author and do the work of addressing any remaining review comments.)

What's preventing this PR from being merged right now? All checks seem to have passed.

PRs need to be reviewed before they can be merged.

Yes, code reviewer bandwidth is the bottleneck for clangd patches at the moment.

I have some other patches in my review queue which I've been trying to get to (and unfortunately haven't had much time to spend on them), so I'm hoping Kadir can review this one, especially as he has reviewed earlier versions of this and related patches and has more familiarity with the affected code areas.

But if no one else gets to this, I would like to at some point.

db7 · 2024-10-12T10:42:39Z

What's "ping PR"? Do you mean pinging the LLVM organization?

It just means posting a comment on the PR that says "ping", as a reminder for reviewers.

(But please only do that if you're the patch author, or, in the case of a patch author who's not active any more, if you're volunteering to take over the patch from the patch author and do the work of addressing any remaining review comments.)

What's preventing this PR from being merged right now? All checks seem to have passed.

PRs need to be reviewed before they can be merged.

Yes, code reviewer bandwidth is the bottleneck for clangd patches at the moment.

I have some other patches in my review queue which I've been trying to get to (and unfortunately haven't had much time to spend on them), so I'm hoping Kadir can review this one, especially as he has reviewed earlier versions of this and related patches and has more familiarity with the affected code areas.

But if no one else gets to this, I would like to at some point.

It seems this PR is still stuck.

erichkeane

I don't know much about the clang-d implementation, but this seems reasonable.

erichkeane · 2024-10-14T13:19:16Z

clang/include/clang/Basic/SourceManager.h

@@ -1971,7 +1971,7 @@ class BeforeThanCompare<SourceRange> {
 };

 /// SourceManager and necessary dependencies (e.g. VFS, FileManager) for a
-/// single in-memorty file.
+/// single in-memory file.


Unrelated chage, please put this in a different patch.

erichkeane · 2024-10-14T13:22:00Z

clang-tools-extra/clangd/CodeCompletionStrings.cpp

+  SourceManagerForFile SourceMgrForFile("mock_file.cpp", CommentWithMarkers);
+  SourceManager &SourceMgr = SourceMgrForFile.get();
+
+  comments::Lexer L(Allocator, SourceMgr.getDiagnostics(), Traits,


The order of these constructors seems odd. I would expect us to build these up in the 'correct' order of dependence/execution/etc. So I think you should swap the Lexer and Sema lines.

erichkeane · 2024-10-14T13:22:24Z

clang-tools-extra/clangd/CodeCompletionStrings.cpp

+                    CommentWithMarkers.data(),
+                    CommentWithMarkers.data() + CommentWithMarkers.size());
+  comments::Sema S(Allocator, SourceMgr, SourceMgr.getDiagnostics(), Traits,
+                   nullptr);


we need a /*stuff=*/ style comment for the nullptr here.

tom-anders · 2024-10-27T11:48:10Z

Hi everyone, I shifted focus to other projects and currently don't have time to work on this PR anymore. If somebody wants to take over, feel free to do so

cor3ntin · 2024-10-29T09:20:50Z

@AaronBallman @erichkeane what do you want to do here?

AaronBallman · 2024-10-29T11:53:50Z

@AaronBallman @erichkeane what do you want to do here?

Because the author is no longer available for the time being, I'd recommend we close the PR. Someone can always come along to commandeer it if they have the bandwidth to work on it. WDYT?

erichkeane · 2024-10-29T14:40:03Z

@AaronBallman @erichkeane what do you want to do here?

Because the author is no longer available for the time being, I'd recommend we close the PR. Someone can always come along to commandeer it if they have the bandwidth to work on it. WDYT?

Closing due to author nonavailability. Commandeering encouraged!

tom-anders requested a review from kadircet January 17, 2024 19:26

tom-anders changed the title ~~[clang] Support parsing comments without ASTContext~~ [clangd] Support parsing comments without ASTContext Jan 17, 2024

llvmbot added clang Clang issues not falling into any other category clangd clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 17, 2024

[clangd] Support parsing comments without ASTContext

640318f

This is in preparation for implementing doxygen parsing, see discussion in clangd/clangd#529. Differential Revision: https://reviews.llvm.org/D143112

tom-anders force-pushed the D143112 branch from b5b4ddc to 640318f Compare January 17, 2024 19:27

aaronliu0130 mentioned this pull request Feb 10, 2024

Doxygen parsing missing clangd/clangd#529

Open

cor3ntin requested a review from sam-mccall June 10, 2024 12:26

cor3ntin requested review from cor3ntin, AaronBallman and erichkeane October 13, 2024 15:30

erichkeane reviewed Oct 14, 2024

View reviewed changes

erichkeane closed this Oct 29, 2024

tcottin mentioned this pull request Feb 17, 2025

[clangd][WIP] Add doxygen parsing for Hover #127451

Closed

tcottin mentioned this pull request Feb 24, 2025

[clangd][WIP] Add doxygen parsing using standalone doxygen parser #128591

Closed

[clangd] Support parsing comments without ASTContext #78491

[clangd] Support parsing comments without ASTContext #78491

Uh oh!

Conversation

tom-anders commented Jan 17, 2024

Uh oh!

llvmbot commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trustytrojan commented May 2, 2024

Uh oh!

aaronliu0130 commented May 2, 2024

Uh oh!

trustytrojan commented May 2, 2024

Uh oh!

aaronliu0130 commented May 2, 2024

Uh oh!

tfninjadoom commented Jun 10, 2024

Uh oh!

aaronliu0130 commented Jun 10, 2024

Uh oh!

cor3ntin commented Jun 10, 2024

Uh oh!

aaronliu0130 commented Jun 10, 2024

Uh oh!

HighCommander4 commented Jun 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

db7 commented Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erichkeane left a comment

Choose a reason for hiding this comment

Uh oh!

erichkeane Oct 14, 2024

Choose a reason for hiding this comment

Uh oh!

erichkeane Oct 14, 2024

Choose a reason for hiding this comment

Uh oh!

erichkeane Oct 14, 2024

Choose a reason for hiding this comment

Uh oh!

tom-anders commented Oct 27, 2024

Uh oh!

cor3ntin commented Oct 29, 2024

Uh oh!

AaronBallman commented Oct 29, 2024

Uh oh!

erichkeane commented Oct 29, 2024

Uh oh!

Uh oh!

llvmbot commented Jan 17, 2024 •

edited

Loading

HighCommander4 commented Jun 11, 2024 •

edited

Loading

db7 commented Oct 12, 2024 •

edited

Loading