[clang] [clang] Don't add documentation comments to the AST if not requested (PR #206363)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Anonmiraj (AnonMiraj)

<details>
<summary>Changes</summary>

Don't collect documentation comments unless they are requested (-Wdocumentation, -fparse-all-comments, code completion, PCH serialization, or libclang)

closes #<!-- -->165515 

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


5 Files Affected:

- (modified) clang/include/clang/Lex/PreprocessorOptions.h (+5) 
- (modified) clang/include/clang/Sema/Sema.h (+5) 
- (modified) clang/lib/Frontend/ASTUnit.cpp (+6) 
- (modified) clang/lib/Parse/Parser.cpp (+8-3) 
- (modified) clang/lib/Sema/Sema.cpp (+29) 


``````````diff
diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h
index 4d1a30712836f..5d695e5f0e875 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -157,6 +157,11 @@ class PreprocessorOptions {
   /// clients don't use them.
   bool WriteCommentListToPCH = true;
 
+  /// Force the front end to retain all documentation comments in the AST, even
+  /// when no comment consuming diagnostic or language option is enabled. Tools
+  /// that query comments after parsing set this
+  bool RetainComments = false;
+
   /// When enabled, preprocessor is in a mode for parsing a single file only.
   ///
   /// Disables #includes of other files and if there are unresolved identifiers
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2cf16fac83282..b818f4c4c4a60 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1130,6 +1130,11 @@ class Sema final : public SemaBase {
 
   void ActOnComment(SourceRange Comment);
 
+  /// Returns true if a comment at \p Loc should be retained in the AST
+  /// (some consumer such as -Wdocumentation, -fparse-all-comments, code
+  /// completion, or AST-file serialization may read it back).
+  bool shouldRetainCommentsFromLexer(SourceLocation Loc) const;
+
   /// Retrieve the parser's current scope.
   ///
   /// This routine must only be used when it is certain that semantic analysis
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 2974cf2660184..404761c4488f7 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -1533,6 +1533,9 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction(
 
   // We'll manage file buffers ourselves.
   CI->getPreprocessorOpts().RetainRemappedFileBuffers = true;
+  // libclang and other ASTUnit clients query documentation comments after
+  // parsing, so keep them in the AST.
+  CI->getPreprocessorOpts().RetainComments = true;
   CI->getFrontendOpts().DisableFree = false;
   ProcessWarningOptions(AST->getDiagnostics(), CI->getDiagnosticOpts(),
                         AST->getFileManager().getVirtualFileSystem());
@@ -1641,6 +1644,9 @@ bool ASTUnit::LoadFromCompilerInvocation(
 
   // We'll manage file buffers ourselves.
   Invocation->getPreprocessorOpts().RetainRemappedFileBuffers = true;
+  // libclang and other ASTUnit clients query documentation comments after
+  // parsing, so keep them in the AST.
+  Invocation->getPreprocessorOpts().RetainComments = true;
   Invocation->getFrontendOpts().DisableFree = false;
   getDiagnostics().Reset();
   ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(),
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 5e1fd4df1a3f0..6b7c4bce6645c 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -74,8 +74,12 @@ Parser::Parser(Preprocessor &pp, Sema &actions, bool skipFunctionBodies)
   // destructor.
   initializePragmaHandlers();
 
-  CommentSemaHandler.reset(new ActionCommentHandler(actions));
-  PP.addCommentHandler(CommentSemaHandler.get());
+  // Only install the comment handler when some consumer may read documentation
+  // comments back.
+  if (actions.shouldRetainCommentsFromLexer(SourceLocation())) {
+    CommentSemaHandler.reset(new ActionCommentHandler(actions));
+    PP.addCommentHandler(CommentSemaHandler.get());
+  }
 
   PP.setCodeCompletionHandler(*this);
 
@@ -481,7 +485,8 @@ Parser::~Parser() {
 
   resetPragmaHandlers();
 
-  PP.removeCommentHandler(CommentSemaHandler.get());
+  if (CommentSemaHandler)
+    PP.removeCommentHandler(CommentSemaHandler.get());
 
   PP.clearCodeCompletionHandler();
 
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 78fbc9e31842d..7a7329c271716 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/TypeOrdering.h"
 #include "clang/Basic/DarwinSDKInfo.h"
+#include "clang/Basic/DiagnosticComment.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
@@ -31,6 +32,7 @@
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CXXFieldCollector.h"
 #include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/ExternalSemaSource.h"
@@ -2704,10 +2706,37 @@ LambdaScopeInfo *Sema::getCurGenericLambda() {
 }
 
 
+bool Sema::shouldRetainCommentsFromLexer(SourceLocation Loc) const {
+  if (LangOpts.CommentOpts.ParseAllComments)
+    return true;
+
+  if (PP.getPreprocessorOpts().RetainComments)
+    return true;
+
+  // When building a PCH the comments are serialized into the AST file
+  // so downstream consumers like clangd) can retrieve documentation, and the
+  // incremental/REPL front end may query them interactively.
+  if (TUKind != TU_Complete)
+    return true;
+
+  if (PP.isCodeCompletionEnabled())
+    return true;
+
+  // Keep the comment if -Wdocumentation is enabled at its location (checking
+  // the location handles warnings turned on by `#pragma clang diagnostic`).
+  if (!Diags.isIgnored(diag::warn_doc_param_not_found, Loc) ||
+      !Diags.isIgnored(diag::warn_unknown_comment_command_name, Loc))
+    return true;
+
+  return false;
+}
+
 void Sema::ActOnComment(SourceRange Comment) {
   if (!LangOpts.RetainCommentsFromSystemHeaders &&
       SourceMgr.isInSystemHeader(Comment.getBegin()))
     return;
+  if (!shouldRetainCommentsFromLexer(Comment.getBegin()))
+    return;
   RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false);
   if (RC.isAlmostTrailingComment() || RC.hasUnsupportedSplice(SourceMgr)) {
     SourceRange MagicMarkerRange(Comment.getBegin(),

``````````

</details>


https://github.com/llvm/llvm-project/pull/206363