Skip to content

[clangd] Support parsing comments without ASTContext #78491

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions clang-tools-extra/clangd/CodeCompletionStrings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID()),
CommentWithMarkers.data(),
CommentWithMarkers.data() + CommentWithMarkers.size());
comments::Sema S(Allocator, SourceMgr, SourceMgr.getDiagnostics(), Traits,
nullptr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

comments::Parser P(L, S, Allocator, SourceMgr, SourceMgr.getDiagnostics(),
Traits);

return P.parseFullComment();
}

} // namespace clangd
} // namespace clang
11 changes: 11 additions & 0 deletions clang-tools-extra/clangd/CodeCompletionStrings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/SourceManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated chage, please put this in a different patch.

class SourceManagerForFile {
public:
/// Creates SourceManager and necessary dependencies (e.g. VFS, FileManager).
Expand Down