[clang] Support parsing comments without ASTContext

[tom-anders]

Summary

This is in preparation for implementing doxygen parsing, see discussion in https://github.com/clangd/clangd/issues/529.

[tom-anders]

clang/include/clang/AST/RawCommentList.h

159 ↗

(On Diff #494076)

Not sure if this is the right place for this? Could be moved to FullComment or made a free function instead?

clang/lib/AST/RawCommentList.cpp

227 ↗

(On Diff #494076)

I wasn't sure if it's worth making the Diagnostics parameter optional here - Our SourceMgr already has it anyway, does it impact performance significantly to use it, even though we don't need it (yet)?

[kadircet]

Thanks, this looks like a good start. I left some comments about the details of implementation, as for placing and APIs in general, i think it's useful to see how things will be built on top inside clangd to make the right call here.

clang/include/clang/AST/RawCommentList.h

159 ↗

(On Diff #494076)

i think this would make more sense inside clangd as a free function when we're processing comments. it's nice to get to re-use some Lexer/Sema/Parser creation logic, but i don't think it's big enough to justify a new public interfaces that's solely used by clangd.
if we can figure out more patterns that we can migrate (the unittest in this patch is a good example, but still just a unittest and doesn't benefit much from code reuse) we can consider lifting this logic up to a common place. the other users should hopefully make it easier to decide where this code should live.

clang/lib/AST/RawCommentList.cpp

227 ↗

(On Diff #494076)

as things stand, Lexer, Sema and Parser expect a non-null diagnosticsengine so we have to pass something (i guess that's what you meant by making it optional).

the performance implications comes from two places:
1- extra analysis being performed in the face of malformed comments to produce "useful" diagnostics
2- creation cost of diagnostic itself, as all the data about ranges, decls etc. are copied around when constructing a diagnostic.

i don't know how costly the first action is today, it's unfortunately the case that would require a lot of code changes, if it's happening quite often in practice (especially when we're in codebases without proper doxygen comments)
the latter is somewhat easier to address by providing our own "mock" diagnostics engine, that'll just drop all of these extra diagnostic data on the floor instead of copying around.

i think taking a look at the codebase for the first (searching for Diag(...) << patterns in Comment{Lexer,Sema,Parser}.cpp should give some ideas) and assessing whether we're performing any extra complicated analysis would be a good start. i would expect us not to, and if that's the case i think we can leave it at that and don't try to make code changes.
the latter we can address easily and we should.

[tom-anders]

• Move to free function in CodeCompletionStrings.h
• Add back comment markers before parsing (Index stores comments without markers, but the comment lexer expects them)

[tom-anders]

clang/lib/AST/RawCommentList.cpp

227 ↗

(On Diff #494076)

1 - At first glance it indeed seems like there's no significant extra analysis performed, so I think we're good for now.
2 - I think this is already taken care of by SourceManagerForFile which just passes nullptr as the DiagnosticConsumer, right?

[tom-anders]

(ping) Does this make sense or are more adjustments needed?

clang-tools-extra/clangd/CodeCompletionStrings.h

 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETIONSTRINGS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETIONSTRINGS_H
 
 #include "clang/Sema/CodeCompleteConsumer.h"
 
 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
 /// markers stripped. See clang::RawComment::getFormattedText() for the detailed
 /// explanation of how the comment text is transformed.
 /// Returns empty string when no comment is available.
 /// If \p CommentsFromHeaders parameter is set, only comments from the main
 /// file will be returned. It is used to workaround crashes when parsing

 ///        it should be obtained via getDocComment or getParameterDocComment.
 std::string formatDocumentation(const CodeCompletionString &CCS,
                                 llvm::StringRef DocComment);
 
 /// Gets detail to be used as the detail field in an LSP completion item. This
 /// 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
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_CODECOMPLETIONSTRINGS_H

clang-tools-extra/clangd/CodeCompletionStrings.cpp

 //===--- CodeCompletionStrings.cpp -------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "CodeCompletionStrings.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"
 #include "llvm/Support/JSON.h"
 #include <limits>
 #include <utility>
 
 namespace clang {

 
 std::string getReturnType(const CodeCompletionString &CCS) {
   for (const auto &Chunk : CCS)
     if (Chunk.Kind == CodeCompletionString::CK_ResultType)
       return Chunk.Text;
   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

clang/include/clang/Basic/SourceManager.h

public:
   explicit BeforeThanCompare(SourceManager &SM) : SM(SM) {}
 
   bool operator()(SourceRange LHS, SourceRange RHS) const {
     return SM.isBeforeInTranslationUnit(LHS.getBegin(), RHS.getBegin());
   }
 };
 
 /// 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).
   /// The main file in the SourceManager will be \p FileName with \p Content.
   SourceManagerForFile(StringRef FileName, StringRef Content);
 
   SourceManager &get() {
     assert(SourceMgr);