https://github.com/AnonMiraj updated https://github.com/llvm/llvm-project/pull/206363
>From 959899a60e675694ab37bbebad73be8b3ee65f2c Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Sun, 28 Jun 2026 22:27:23 +0300 Subject: [PATCH 01/12] [clang] Don't add comments to the AST if not requested --- clang/include/clang/Lex/PreprocessorOptions.h | 5 ++++ clang/include/clang/Sema/Sema.h | 5 ++++ clang/lib/Frontend/ASTUnit.cpp | 6 ++++ clang/lib/Parse/Parser.cpp | 11 +++++-- clang/lib/Sema/Sema.cpp | 29 +++++++++++++++++++ 5 files changed, 53 insertions(+), 3 deletions(-) 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 d43b6954196a2..a1d07bdcef872 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1132,6 +1132,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 75e4f7772f47c..0fc9625a6fae8 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 6a21acd9dc4ef..e8794ad9f44c9 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); @@ -480,7 +484,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 9f962912148ab..70f5c95623135 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" @@ -2727,10 +2729,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(), >From eb2eb83d577ee89421a6cdb305b0895f144f1bdd Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Sun, 28 Jun 2026 22:41:05 +0300 Subject: [PATCH 02/12] fix formatting --- clang/lib/Sema/Sema.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 70f5c95623135..668c9032939e7 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2728,7 +2728,6 @@ LambdaScopeInfo *Sema::getCurGenericLambda() { return nullptr; } - bool Sema::shouldRetainCommentsFromLexer(SourceLocation Loc) const { if (LangOpts.CommentOpts.ParseAllComments) return true; >From 65cd5e1e127f65b5c7c9da0c7ba7496d39e25c5c Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Sun, 28 Jun 2026 23:38:28 +0300 Subject: [PATCH 03/12] retain comments with -fretain-comments so --doxygen works --- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp | 3 ++- clang/include/clang/Options/Options.td | 6 ++++++ clang/lib/Driver/ToolChains/Clang.cpp | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp index 00290d7cdc74b..0b81a3cb20351 100644 --- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp +++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp @@ -285,7 +285,8 @@ Example usage for a project using a compile commands database: llvm::outs() << "Emiting docs in " << Format << " format.\n"; auto G = ExitOnErr(doc::findGeneratorByName(Format)); - ArgumentsAdjuster ArgAdjuster; + ArgumentsAdjuster ArgAdjuster = getInsertArgumentAdjuster( + "-fretain-comments", tooling::ArgumentInsertPosition::END); if (!DoxygenOnly) ArgAdjuster = combineAdjusters( getInsertArgumentAdjuster("-fparse-all-comments", diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index 4afb089e8a51f..e36d1f39d9c9e 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -2173,6 +2173,12 @@ defm define_target_os_macros : OptInCC1FFlag<"define-target-os-macros", def fparse_all_comments : Flag<["-"], "fparse-all-comments">, Group<f_clang_Group>, Visibility<[ClangOption, CC1Option]>, MarshallingInfoFlag<LangOpts<"CommentOpts.ParseAllComments">>; +def fretain_comments : Flag<["-"], "fretain-comments">, Group<f_clang_Group>, + Visibility<[ClangOption, CC1Option]>, + HelpText<"Retain documentation comments in the AST even when no diagnostic or " + "language option would otherwise require them (e.g. for tools that " + "query comments after parsing)">, + MarshallingInfoFlag<PreprocessorOpts<"RetainComments">>; def frecord_command_line : Flag<["-"], "frecord-command-line">, DocBrief<[{Generate a section named ".GCC.command.line" containing the driver command-line. After linking, the section may contain multiple command diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 8c9b98795b194..7ea76f34da4a9 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -8113,6 +8113,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, Args.AddAllArgs(CmdArgs, options::OPT_fcomment_block_commands); // Forward -fparse-all-comments to -cc1. Args.AddAllArgs(CmdArgs, options::OPT_fparse_all_comments); + // Forward -fretain-comments to -cc1. + Args.AddAllArgs(CmdArgs, options::OPT_fretain_comments); // Turn -fplugin=name.so into -load name.so for (const Arg *A : Args.filtered(options::OPT_fplugin_EQ)) { >From 5124a5119d43c0e314d80c7dc325dcd45b8089fa Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Mon, 29 Jun 2026 09:40:32 +0300 Subject: [PATCH 04/12] fix ci --- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp | 3 +++ clang/lib/Frontend/FrontendActions.cpp | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp index 85da480fb67a6..6c6076e889f5a 100644 --- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp +++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp @@ -448,6 +448,9 @@ ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { } bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) { + // ExtractAPI reads documentation comments off the AST + CI.getPreprocessorOpts().RetainComments = true; + auto &Inputs = CI.getFrontendOpts().Inputs; if (Inputs.empty()) return true; diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 4ef94c1991ee2..9c762706f7f40 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -85,6 +85,8 @@ ASTPrintAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { std::unique_ptr<ASTConsumer> ASTDumpAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { + // Dumping the AST shows documentation comments + CI.getPreprocessorOpts().RetainComments = true; const FrontendOptions &Opts = CI.getFrontendOpts(); return CreateASTDumper(nullptr /*Dump to stdout.*/, Opts.ASTDumpFilter, Opts.ASTDumpDecls, Opts.ASTDumpAll, @@ -249,6 +251,10 @@ GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance &CI, bool GenerateModuleInterfaceAction::PrepareToExecuteAction( CompilerInstance &CI) { + // Documentation comments must still be serialized into the BMI + // so importers can query them + CI.getPreprocessorOpts().RetainComments = true; + for (const auto &FIF : CI.getFrontendOpts().Inputs) { if (const auto InputFormat = FIF.getKind().getFormat(); InputFormat != InputKind::Format::Source) { >From 5c6724f2ed2e08accf3da05a3057e3060f09decf Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Mon, 29 Jun 2026 21:56:53 +0300 Subject: [PATCH 05/12] Move RetainComments and RetainCommentsFromSystemHeaders into CommentOptions --- clang-tools-extra/clangd/Compiler.cpp | 2 +- clang-tools-extra/clangd/index/IndexAction.cpp | 2 +- clang/include/clang/Basic/CommentOptions.h | 8 ++++++++ clang/include/clang/Basic/LangOptions.def | 2 -- clang/include/clang/Lex/PreprocessorOptions.h | 5 ----- clang/include/clang/Options/Options.td | 4 ++-- clang/lib/AST/ASTContext.cpp | 2 +- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp | 4 ++-- clang/lib/Frontend/ASTUnit.cpp | 4 ++-- clang/lib/Frontend/FrontendActions.cpp | 8 ++++---- clang/lib/Sema/Sema.cpp | 4 ++-- 11 files changed, 23 insertions(+), 22 deletions(-) diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp index 4644cd75c0833..095c765e518dd 100644 --- a/clang-tools-extra/clangd/Compiler.cpp +++ b/clang-tools-extra/clangd/Compiler.cpp @@ -121,7 +121,7 @@ buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D, // createInvocationFromCommandLine sets DisableFree. CI->getFrontendOpts().DisableFree = false; CI->getLangOpts().CommentOpts.ParseAllComments = true; - CI->getLangOpts().RetainCommentsFromSystemHeaders = true; + CI->getLangOpts().CommentOpts.RetainCommentsFromSystemHeaders = true; disableUnsupportedOptions(*CI); return CI; diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp index 489c61f1ff424..bd662e3c90b1f 100644 --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -167,7 +167,7 @@ class IndexAction : public ASTFrontendAction { bool BeginInvocation(CompilerInstance &CI) override { // We want all comments, not just the doxygen ones. CI.getLangOpts().CommentOpts.ParseAllComments = true; - CI.getLangOpts().RetainCommentsFromSystemHeaders = true; + CI.getLangOpts().CommentOpts.RetainCommentsFromSystemHeaders = true; // Index the whole file even if there are warnings and -Werror is set. // Avoids some analyses too. Set in two places as we're late to the party. CI.getDiagnosticOpts().IgnoreWarnings = true; diff --git a/clang/include/clang/Basic/CommentOptions.h b/clang/include/clang/Basic/CommentOptions.h index 7d142fc32f511..73e7cba91cca8 100644 --- a/clang/include/clang/Basic/CommentOptions.h +++ b/clang/include/clang/Basic/CommentOptions.h @@ -30,6 +30,14 @@ struct CommentOptions { /// Treat ordinary comments as documentation comments. bool ParseAllComments = false; + /// 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; + + /// Retain documentation comments from system headers in the AST. + bool RetainCommentsFromSystemHeaders = false; + CommentOptions() = default; }; diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 1fb491a54a278..8c265090f0238 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -401,8 +401,6 @@ LANGOPT(ApplePragmaPack, 1, 0, NotCompatible, "Apple gcc-compatible #pragma pack LANGOPT(XLPragmaPack, 1, 0, NotCompatible, "IBM XL #pragma pack handling") -LANGOPT(RetainCommentsFromSystemHeaders, 1, 0, Compatible, "retain documentation comments from system headers in the AST") - LANGOPT(APINotes, 1, 0, NotCompatible, "use external API notes") LANGOPT(APINotesModules, 1, 0, NotCompatible, "use module-based external API notes") LANGOPT(SwiftVersionIndependentAPINotes, 1, 0, NotCompatible, "use external API notes capturing all versions") diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h index 5d695e5f0e875..4d1a30712836f 100644 --- a/clang/include/clang/Lex/PreprocessorOptions.h +++ b/clang/include/clang/Lex/PreprocessorOptions.h @@ -157,11 +157,6 @@ 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/Options/Options.td b/clang/include/clang/Options/Options.td index e36d1f39d9c9e..2a8fc61383ca2 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -2178,7 +2178,7 @@ def fretain_comments : Flag<["-"], "fretain-comments">, Group<f_clang_Group>, HelpText<"Retain documentation comments in the AST even when no diagnostic or " "language option would otherwise require them (e.g. for tools that " "query comments after parsing)">, - MarshallingInfoFlag<PreprocessorOpts<"RetainComments">>; + MarshallingInfoFlag<LangOpts<"CommentOpts.RetainComments">>; def frecord_command_line : Flag<["-"], "frecord-command-line">, DocBrief<[{Generate a section named ".GCC.command.line" containing the driver command-line. After linking, the section may contain multiple command @@ -3890,7 +3890,7 @@ defm implicit_modules : BoolFOption<"implicit-modules", [NoXarchOption], [ClangOption, CLOption]>>; def fretain_comments_from_system_headers : Flag<["-"], "fretain-comments-from-system-headers">, Group<f_Group>, Visibility<[ClangOption, CC1Option]>, - MarshallingInfoFlag<LangOpts<"RetainCommentsFromSystemHeaders">>; + MarshallingInfoFlag<LangOpts<"CommentOpts.RetainCommentsFromSystemHeaders">>; def fmodule_header : Flag <["-"], "fmodule-header">, Group<f_Group>, Visibility<[ClangOption, CLOption]>, HelpText<"Build a C++20 Header Unit from a header">; diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 2228811546c0f..57f271eb4e775 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -357,7 +357,7 @@ RawComment *ASTContext::getRawCommentNoCache(RawCommentLookupKey Key) const { } void ASTContext::addComment(const RawComment &RC) { - assert(LangOpts.RetainCommentsFromSystemHeaders || + assert(LangOpts.CommentOpts.RetainCommentsFromSystemHeaders || !SourceMgr.isInSystemHeader(RC.getSourceRange().getBegin())); Comments.addComment(RC, LangOpts.CommentOpts, BumpAlloc); } diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp index 6c6076e889f5a..569e600d091cc 100644 --- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp +++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp @@ -448,8 +448,8 @@ ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { } bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) { - // ExtractAPI reads documentation comments off the AST - CI.getPreprocessorOpts().RetainComments = true; + // ExtractAPI reads documentation comments off the AST. + CI.getLangOpts().CommentOpts.RetainComments = true; auto &Inputs = CI.getFrontendOpts().Inputs; if (Inputs.empty()) diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 0fc9625a6fae8..738fe99cef46b 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1535,7 +1535,7 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction( 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->getLangOpts().CommentOpts.RetainComments = true; CI->getFrontendOpts().DisableFree = false; ProcessWarningOptions(AST->getDiagnostics(), CI->getDiagnosticOpts(), AST->getFileManager().getVirtualFileSystem()); @@ -1646,7 +1646,7 @@ bool ASTUnit::LoadFromCompilerInvocation( 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->getLangOpts().CommentOpts.RetainComments = true; Invocation->getFrontendOpts().DisableFree = false; getDiagnostics().Reset(); ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 9c762706f7f40..93e68dc1fcf51 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -85,8 +85,8 @@ ASTPrintAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { std::unique_ptr<ASTConsumer> ASTDumpAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { - // Dumping the AST shows documentation comments - CI.getPreprocessorOpts().RetainComments = true; + // Dumping the AST shows documentation comments. + CI.getLangOpts().CommentOpts.RetainComments = true; const FrontendOptions &Opts = CI.getFrontendOpts(); return CreateASTDumper(nullptr /*Dump to stdout.*/, Opts.ASTDumpFilter, Opts.ASTDumpDecls, Opts.ASTDumpAll, @@ -252,8 +252,8 @@ GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance &CI, bool GenerateModuleInterfaceAction::PrepareToExecuteAction( CompilerInstance &CI) { // Documentation comments must still be serialized into the BMI - // so importers can query them - CI.getPreprocessorOpts().RetainComments = true; + // so importers can query them. + CI.getLangOpts().CommentOpts.RetainComments = true; for (const auto &FIF : CI.getFrontendOpts().Inputs) { if (const auto InputFormat = FIF.getKind().getFormat(); diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 668c9032939e7..b8854beeda565 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2732,7 +2732,7 @@ bool Sema::shouldRetainCommentsFromLexer(SourceLocation Loc) const { if (LangOpts.CommentOpts.ParseAllComments) return true; - if (PP.getPreprocessorOpts().RetainComments) + if (LangOpts.CommentOpts.RetainComments) return true; // When building a PCH the comments are serialized into the AST file @@ -2754,7 +2754,7 @@ bool Sema::shouldRetainCommentsFromLexer(SourceLocation Loc) const { } void Sema::ActOnComment(SourceRange Comment) { - if (!LangOpts.RetainCommentsFromSystemHeaders && + if (!LangOpts.CommentOpts.RetainCommentsFromSystemHeaders && SourceMgr.isInSystemHeader(Comment.getBegin())) return; if (!shouldRetainCommentsFromLexer(Comment.getBegin())) >From 55b62e3bb498d2bafc0247bbeef6101bdaccbd64 Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Tue, 30 Jun 2026 00:09:09 +0300 Subject: [PATCH 06/12] fix ci --- clang/lib/Frontend/CompilerInvocation.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 56362e758a78b..41200a7626ab1 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -5245,6 +5245,7 @@ std::string CompilerInvocation::computeContextHash() const { HBuilder.add(getLangOpts().ObjCRuntime); HBuilder.addRange(getLangOpts().CommentOpts.BlockCommandNames); + HBuilder.add(getLangOpts().CommentOpts.RetainCommentsFromSystemHeaders); // Extend the signature with the target options. HBuilder.add(getTargetOpts().Triple, getTargetOpts().CPU, >From f578ec8be32d5cd4558e2ae574f901a8cf165887 Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Wed, 1 Jul 2026 14:09:18 +0300 Subject: [PATCH 07/12] address reviews --- clang/include/clang/Sema/Sema.h | 2 +- clang/lib/Parse/Parser.cpp | 11 +++-------- clang/lib/Sema/Sema.cpp | 4 ++-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index a1d07bdcef872..2235676602c27 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1135,7 +1135,7 @@ class Sema final : public SemaBase { /// 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; + bool shouldRetainCommentsInAST(SourceLocation Loc) const; /// Retrieve the parser's current scope. /// diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index e8794ad9f44c9..6a21acd9dc4ef 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -74,12 +74,8 @@ Parser::Parser(Preprocessor &pp, Sema &actions, bool skipFunctionBodies) // destructor. initializePragmaHandlers(); - // 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()); - } + CommentSemaHandler.reset(new ActionCommentHandler(actions)); + PP.addCommentHandler(CommentSemaHandler.get()); PP.setCodeCompletionHandler(*this); @@ -484,8 +480,7 @@ Parser::~Parser() { resetPragmaHandlers(); - if (CommentSemaHandler) - PP.removeCommentHandler(CommentSemaHandler.get()); + PP.removeCommentHandler(CommentSemaHandler.get()); PP.clearCodeCompletionHandler(); diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index b8854beeda565..c0eb531335fdf 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2728,7 +2728,7 @@ LambdaScopeInfo *Sema::getCurGenericLambda() { return nullptr; } -bool Sema::shouldRetainCommentsFromLexer(SourceLocation Loc) const { +bool Sema::shouldRetainCommentsInAST(SourceLocation Loc) const { if (LangOpts.CommentOpts.ParseAllComments) return true; @@ -2757,7 +2757,7 @@ void Sema::ActOnComment(SourceRange Comment) { if (!LangOpts.CommentOpts.RetainCommentsFromSystemHeaders && SourceMgr.isInSystemHeader(Comment.getBegin())) return; - if (!shouldRetainCommentsFromLexer(Comment.getBegin())) + if (!shouldRetainCommentsInAST(Comment.getBegin())) return; RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false); if (RC.isAlmostTrailingComment() || RC.hasUnsupportedSplice(SourceMgr)) { >From caa48b0276110a339c3b142c303a462c552f706f Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Wed, 1 Jul 2026 14:19:38 +0300 Subject: [PATCH 08/12] add a release note --- clang/docs/ReleaseNotes.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 9301745b9628e..312ce1905e749 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -285,6 +285,12 @@ latest release, please see the [Clang Web Site](https://clang.llvm.org) or the - Improved how Unicode characters are displayed in diagnostic messages. +- Clang no longer retains source comments in the AST when nothing will read them + back. Comments are now collected only when they may be consumed (e.g. with + ``-fparse-all-comments``, when ``-Wdocumentation`` is enabled, when emitting a + PCH/module, or during code completion), reducing memory overhead for typical + compilations. + - `-Wtautological-pointer-compare` and `-Wpointer-bool-conversion` now diagnose a reference to a function (e.g. of type `void (&)()`) compared against or converted to a null pointer, the same as a bare function name. >From 11d4c47f19a7cae3dfd86ef29a3900c054ac6553 Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Mon, 20 Jul 2026 06:50:29 +0300 Subject: [PATCH 09/12] add DiagnosticsEngine::areAllIgnored to check a whole warning group Co-authored-by: Erich Keane <[email protected]> --- clang/include/clang/Basic/Diagnostic.h | 10 ++ clang/include/clang/Basic/DiagnosticIDs.h | 6 + clang/lib/Basic/DiagnosticIDs.cpp | 196 ++++++++++++---------- 3 files changed, 128 insertions(+), 84 deletions(-) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 66e79e3b4300b..95ea477502ea7 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -963,6 +963,16 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { diag::Severity::Ignored; } + bool areAllIgnored(StringRef Group, SourceLocation Loc) const { + llvm::SmallVector<diag::kind> diagsInGroup; + bool Failed = Diags->getDiagnosticsInGroup(diag::Flavor::WarningOrError, + Group, diagsInGroup); + assert(!Failed && "Incorrect group name?"); + (void)Failed; + return Diags->getDiagnosticListHighestSeverity(diagsInGroup, Loc, *this) == + diag::Severity::Ignored; + } + /// Based on the way the client configured the DiagnosticsEngine /// object, classify the specified diagnostic ID into a Level, consumable by /// the DiagnosticConsumer. diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h index 8e714efac46ae..cbd7677cd7108 100644 --- a/clang/include/clang/Basic/DiagnosticIDs.h +++ b/clang/include/clang/Basic/DiagnosticIDs.h @@ -514,6 +514,12 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> { getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, const DiagnosticsEngine &Diag) const LLVM_READONLY; + /// Given a collection of diagnostic IDs, get the 'highest' severity of them + /// at the provided location for this DiagnosticsEngine. + diag::Severity getDiagnosticListHighestSeverity( + llvm::ArrayRef<diag::kind> DiagIDs, SourceLocation Loc, + const DiagnosticsEngine &Diag) const LLVM_READONLY; + Class getDiagClass(unsigned DiagID) const; /// Whether the diagnostic may leave the AST in a state where some diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index 92f96d1b9ad90..e9ce73863cb17 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -538,103 +538,131 @@ DiagnosticIDs::getDiagnosticLevel(unsigned DiagID, SourceLocation Loc, diag::Severity DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, const DiagnosticsEngine &Diag) const { - bool IsCustomDiag = DiagnosticIDs::IsCustomDiag(DiagID); - assert(getDiagClass(DiagID) != CLASS_NOTE); - - // Specific non-error diagnostics may be mapped to various levels from ignored - // to error. Errors can only be mapped to fatal. - diag::Severity Result = diag::Severity::Fatal; + return getDiagnosticListHighestSeverity({DiagID}, Loc, Diag); +} - // Get the mapping information, or compute it lazily. +diag::Severity DiagnosticIDs::getDiagnosticListHighestSeverity( + llvm::ArrayRef<diag::kind> DiagIDs, SourceLocation Loc, + const DiagnosticsEngine &Diag) const { DiagnosticsEngine::DiagState *State = Diag.GetDiagStateForLoc(Loc); - DiagnosticMapping Mapping = State->getOrAddMapping((diag::kind)DiagID); - - // TODO: Can a null severity really get here? - if (Mapping.getSeverity() != diag::Severity()) - Result = Mapping.getSeverity(); - - // Upgrade ignored diagnostics if -Weverything is enabled. - if (State->EnableAllWarnings && Result == diag::Severity::Ignored && - !Mapping.isUser() && - (IsCustomDiag || getDiagClass(DiagID) != CLASS_REMARK)) - Result = diag::Severity::Warning; - - // Ignore -pedantic diagnostics inside __extension__ blocks. - // (The diagnostics controlled by -pedantic are the extension diagnostics - // that are not enabled by default.) - bool EnabledByDefault = false; - bool IsExtensionDiag = isExtensionDiag(DiagID, EnabledByDefault); - if (Diag.AllExtensionsSilenced && IsExtensionDiag && !EnabledByDefault) - return diag::Severity::Ignored; - - // For extension diagnostics that haven't been explicitly mapped, check if we - // should upgrade the diagnostic. Skip if the user explicitly suppressed it - // (e.g. -Wno-foo). - if (IsExtensionDiag && - !(Mapping.isUser() && Result == diag::Severity::Ignored)) { - if (Mapping.hasNoWarningAsError()) - Result = std::max(Result, - std::min(State->ExtBehavior, diag::Severity::Warning)); - else - Result = std::max(Result, State->ExtBehavior); - } - - // At this point, ignored errors can no longer be upgraded. - if (Result == diag::Severity::Ignored) - return Result; - // Honor -w: this disables all messages which are not Error/Fatal by - // default (disregarding attempts to upgrade severity from Warning to Error), - // as well as disabling all messages which are currently mapped to Warning - // (whether by default or downgraded from Error via e.g. -Wno-error or #pragma - // diagnostic.) - // FIXME: Should -w be ignored for custom warnings without a group? - if (State->IgnoreAllWarnings) { - if ((!IsCustomDiag || CustomDiagInfo->getDescription(DiagID).GetGroup()) && - (Result == diag::Severity::Warning || - (Result >= diag::Severity::Error && - !isDefaultMappingAsError((diag::kind)DiagID)))) + auto checkSingleDiag = [&](diag::kind DiagID) -> diag::Severity { + bool IsCustomDiag = DiagnosticIDs::IsCustomDiag(DiagID); + assert(getDiagClass(DiagID) != CLASS_NOTE); + + // Specific non-error diagnostics may be mapped to various levels from + // ignored to error. Errors can only be mapped to fatal. + diag::Severity Result = diag::Severity::Fatal; + + // Get the mapping information, or compute it lazily. + DiagnosticMapping Mapping = State->getOrAddMapping((diag::kind)DiagID); + + // TODO: Can a null severity really get here? + if (Mapping.getSeverity() != diag::Severity()) + Result = Mapping.getSeverity(); + + // Upgrade ignored diagnostics if -Weverything is enabled. + if (State->EnableAllWarnings && Result == diag::Severity::Ignored && + !Mapping.isUser() && + (IsCustomDiag || getDiagClass(DiagID) != CLASS_REMARK)) + Result = diag::Severity::Warning; + + // Ignore -pedantic diagnostics inside __extension__ blocks. + // (The diagnostics controlled by -pedantic are the extension diagnostics + // that are not enabled by default.) + bool EnabledByDefault = false; + bool IsExtensionDiag = isExtensionDiag(DiagID, EnabledByDefault); + if (Diag.AllExtensionsSilenced && IsExtensionDiag && !EnabledByDefault) return diag::Severity::Ignored; - } - // If -Werror is enabled, map warnings to errors unless explicitly disabled. - if (Result == diag::Severity::Warning) { - if (State->WarningsAsErrors && !Mapping.hasNoWarningAsError()) + // For extension diagnostics that haven't been explicitly mapped, check if + // we should upgrade the diagnostic. Skip if the user explicitly + // suppressed it (e.g. -Wno-foo). + if (IsExtensionDiag && + !(Mapping.isUser() && Result == diag::Severity::Ignored)) { + if (Mapping.hasNoWarningAsError()) + Result = std::max( + Result, std::min(State->ExtBehavior, diag::Severity::Warning)); + else + Result = std::max(Result, State->ExtBehavior); + } + + // At this point, ignored errors can no longer be upgraded. + if (Result == diag::Severity::Ignored) + return Result; + + // Honor -w: this disables all messages which are not Error/Fatal by + // default (disregarding attempts to upgrade severity from Warning to + // Error), as well as disabling all messages which are currently mapped to + // Warning (whether by default or downgraded from Error via e.g. + // -Wno-error or #pragma diagnostic.) + // FIXME: Should -w be ignored for custom warnings without a group? + if (State->IgnoreAllWarnings) { + if ((!IsCustomDiag || + CustomDiagInfo->getDescription(DiagID).GetGroup()) && + (Result == diag::Severity::Warning || + (Result >= diag::Severity::Error && + !isDefaultMappingAsError((diag::kind)DiagID)))) + return diag::Severity::Ignored; + } + + // If -Werror is enabled, map warnings to errors unless explicitly + // disabled. + if (Result == diag::Severity::Warning) { + if (State->WarningsAsErrors && !Mapping.hasNoWarningAsError()) + Result = diag::Severity::Error; + } + + // If -Wfatal-errors is enabled, map errors to fatal unless explicitly + // disabled. + if (Result == diag::Severity::Error) { + if (State->ErrorsAsFatal && !Mapping.hasNoErrorAsFatal()) + Result = diag::Severity::Fatal; + } + + // If explicitly requested, map fatal errors to errors. + if (Result == diag::Severity::Fatal && + DiagID != diag::fatal_too_many_errors && Diag.FatalsAsError) Result = diag::Severity::Error; - } - // If -Wfatal-errors is enabled, map errors to fatal unless explicitly - // disabled. - if (Result == diag::Severity::Error) { - if (State->ErrorsAsFatal && !Mapping.hasNoErrorAsFatal()) - Result = diag::Severity::Fatal; - } + // Rest of the mappings are only applicable for diagnostics associated + // with a SourceLocation, bail out early for others. + if (!Diag.hasSourceManager()) + return Result; + + // We check both the location-specific state and the ForceSystemWarnings + // override. In some cases (like template instantiations from system + // modules), the location-specific state might have suppression enabled, + // but the engine might have an override (e.g. + // AllowWarningInSystemHeaders) to show the warning. + if (State->SuppressSystemWarnings && !Diag.getForceSystemWarnings() && + shouldSuppressAsSystemWarning(DiagID, Loc, Diag)) { + return diag::Severity::Ignored; + } - // If explicitly requested, map fatal errors to errors. - if (Result == diag::Severity::Fatal && - DiagID != diag::fatal_too_many_errors && Diag.FatalsAsError) - Result = diag::Severity::Error; + // Clang-diagnostics pragmas always take precedence over suppression + // mapping. + if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc)) + return diag::Severity::Ignored; - // Rest of the mappings are only applicable for diagnostics associated with a - // SourceLocation, bail out early for others. - if (!Diag.hasSourceManager()) return Result; + }; - // We check both the location-specific state and the ForceSystemWarnings - // override. In some cases (like template instantiations from system modules), - // the location-specific state might have suppression enabled, but the - // engine might have an override (e.g. AllowWarningInSystemHeaders) to show - // the warning. - if (State->SuppressSystemWarnings && !Diag.getForceSystemWarnings() && - shouldSuppressAsSystemWarning(DiagID, Loc, Diag)) { - return diag::Severity::Ignored; + diag::Severity CompositeResult = diag::Severity::Ignored; + for (diag::kind DiagID : DiagIDs) { + CompositeResult = std::max(CompositeResult, checkSingleDiag(DiagID)); + + // If we already hit 'fatal', we can't get any higher! So just return that. + // We could potentially short-cut this by taking a parameter for "return + // first greater than", but since our uses of this are fairly small, and + // that only optimizes for the "we are about to do something expensive + // anyway" variant (that is, when everything is NOT ignored), it doesn't + // seem particularly valuable. + if (CompositeResult == diag::Severity::Fatal) + break; } - // Clang-diagnostics pragmas always take precedence over suppression mapping. - if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc)) - return diag::Severity::Ignored; - - return Result; + return CompositeResult; } bool DiagnosticIDs::shouldSuppressAsSystemWarning( >From 69d517df6d4703d05fdbb5a7468afa84fb7bda17 Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Mon, 20 Jul 2026 06:50:29 +0300 Subject: [PATCH 10/12] use areAllIgnored to check the whole -Wdocumentation warning groups --- clang/lib/Sema/Sema.cpp | 12 ++++++------ clang/lib/Sema/SemaDecl.cpp | 7 ++----- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index c0eb531335fdf..a2aadebb7de34 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -24,7 +24,6 @@ #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" @@ -32,7 +31,6 @@ #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" @@ -2744,10 +2742,12 @@ bool Sema::shouldRetainCommentsInAST(SourceLocation Loc) const { 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)) + // Keep the comment if any of the -Wdocumentation warnings is enabled at + // its location (checking the location handles warnings turned on by + // `#pragma clang diagnostic`). -Wdocumentation-pedantic is checked + // separately because it is not a subgroup of -Wdocumentation. + if (!Diags.areAllIgnored("documentation", Loc) || + !Diags.areAllIgnored("documentation-pedantic", Loc)) return true; return false; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 9b2efd96cb62d..4629d9b3c636f 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -30,7 +30,6 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/Type.h" #include "clang/Basic/Builtins.h" -#include "clang/Basic/DiagnosticComment.h" #include "clang/Basic/HLSLRuntime.h" #include "clang/Basic/PartialDiagnostic.h" #include "clang/Basic/SourceManager.h" @@ -15620,10 +15619,8 @@ void Sema::ActOnDocumentableDecls(ArrayRef<Decl *> Group) { if (Group.empty() || !Group[0]) return; - if (Diags.isIgnored(diag::warn_doc_param_not_found, - Group[0]->getLocation()) && - Diags.isIgnored(diag::warn_unknown_comment_command_name, - Group[0]->getLocation())) + if (Diags.areAllIgnored("documentation", Group[0]->getLocation()) && + Diags.areAllIgnored("documentation-pedantic", Group[0]->getLocation())) return; if (Group.size() >= 2) { >From a08b2ec9d19a8726c022adc0fc688af44029599e Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Mon, 20 Jul 2026 07:56:37 +0300 Subject: [PATCH 11/12] add tests --- clang/test/AST/ast-dump-comment-retention.cpp | 28 ++++++++++++++ .../warn-documentation-comment-retention.cpp | 38 +++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 clang/test/AST/ast-dump-comment-retention.cpp create mode 100644 clang/test/Sema/warn-documentation-comment-retention.cpp diff --git a/clang/test/AST/ast-dump-comment-retention.cpp b/clang/test/AST/ast-dump-comment-retention.cpp new file mode 100644 index 0000000000000..e2a68e2f3221d --- /dev/null +++ b/clang/test/AST/ast-dump-comment-retention.cpp @@ -0,0 +1,28 @@ +// Comments are only collected into the AST when a consumer may read them back +// (see Sema::shouldRetainCommentsInAST). -ast-dump is such a consumer: it +// force-enables comment retention, so documentation comments remain visible in +// its output even without -Wdocumentation. An ordinary comment is only turned +// into an AST comment node when -fparse-all-comments is passed. + +// RUN: %clang_cc1 -ast-dump -ast-dump-filter Test %s \ +// RUN: | FileCheck -strict-whitespace %s --check-prefixes=CHECK,DEFAULT +// RUN: %clang_cc1 -fparse-all-comments -ast-dump -ast-dump-filter Test %s \ +// RUN: | FileCheck -strict-whitespace %s --check-prefixes=CHECK,ALL + +/// Doc +int Test_DocComment; +// A documentation comment is retained for -ast-dump in both modes. +// CHECK: VarDecl{{.*}}Test_DocComment +// CHECK-NEXT: FullComment +// CHECK-NEXT: ParagraphComment +// CHECK-NEXT: TextComment{{.*}} Text=" Doc" + +// Ordinary +int Test_OrdinaryComment; +// An ordinary comment becomes an AST comment node only with +// -fparse-all-comments; by default it is dropped. +// CHECK: VarDecl{{.*}}Test_OrdinaryComment +// ALL-NEXT: FullComment +// ALL-NEXT: ParagraphComment +// ALL-NEXT: TextComment{{.*}} Text=" Ordinary" +// DEFAULT-NOT: FullComment diff --git a/clang/test/Sema/warn-documentation-comment-retention.cpp b/clang/test/Sema/warn-documentation-comment-retention.cpp new file mode 100644 index 0000000000000..5d7a50b0bb970 --- /dev/null +++ b/clang/test/Sema/warn-documentation-comment-retention.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// The comment-retention optimization (Sema::shouldRetainCommentsInAST) must +// still parse a documentation comment when -Wdocumentation is enabled at the +// comment's location -- including when it is turned on by a #pragma clang +// diagnostic rather than on the command line. The check is done at the +// comment's location precisely so pragma regions are honored. + +/// \returns Aaa +void outside(); +// -Wdocumentation is off at this location, so the comment is not checked and +// no diagnostic is produced. -verify fails on any unexpected diagnostic, so +// this line asserts the comment is *not* diagnosed here. + +#pragma clang diagnostic push +#pragma clang diagnostic warning "-Wdocumentation" +/// \returns Aaa +void inside(); +// expected-warning@-2 {{'\returns' command used in a comment that is attached to a function returning void}} +#pragma clang diagnostic pop + +// Any warning in the -Wdocumentation group must keep the comment, not just a +// hard-coded subset: -Wdocumentation-html is a subgroup of -Wdocumentation. +#pragma clang diagnostic push +#pragma clang diagnostic warning "-Wdocumentation-html" +/// Aaa <br></br> +void html_inside(); +// expected-warning@-2 {{HTML end tag 'br' is forbidden}} +#pragma clang diagnostic pop + +// -Wdocumentation-unknown-command is under -Wdocumentation-pedantic, which is +// not a subgroup of -Wdocumentation and must be checked separately. +#pragma clang diagnostic push +#pragma clang diagnostic warning "-Wdocumentation-unknown-command" +/// \unknowncommand Aaa +void unknown_inside(); +// expected-warning@-2 {{unknown command tag name}} +#pragma clang diagnostic pop >From c75d82582cbe7322d92a32b8dba1f645d5ed8788 Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Tue, 21 Jul 2026 13:04:42 +0300 Subject: [PATCH 12/12] address reviews --- clang-tools-extra/clangd/Compiler.cpp | 1 + clang-tools-extra/clangd/index/IndexAction.cpp | 1 + clang/lib/Sema/Sema.cpp | 7 ++++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp index 095c765e518dd..aaeaab30b96db 100644 --- a/clang-tools-extra/clangd/Compiler.cpp +++ b/clang-tools-extra/clangd/Compiler.cpp @@ -121,6 +121,7 @@ buildCompilerInvocation(const ParseInputs &Inputs, clang::DiagnosticConsumer &D, // createInvocationFromCommandLine sets DisableFree. CI->getFrontendOpts().DisableFree = false; CI->getLangOpts().CommentOpts.ParseAllComments = true; + CI->getLangOpts().CommentOpts.RetainComments = true; CI->getLangOpts().CommentOpts.RetainCommentsFromSystemHeaders = true; disableUnsupportedOptions(*CI); diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp index bd662e3c90b1f..21e055b82d722 100644 --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -167,6 +167,7 @@ class IndexAction : public ASTFrontendAction { bool BeginInvocation(CompilerInstance &CI) override { // We want all comments, not just the doxygen ones. CI.getLangOpts().CommentOpts.ParseAllComments = true; + CI.getLangOpts().CommentOpts.RetainComments = true; CI.getLangOpts().CommentOpts.RetainCommentsFromSystemHeaders = true; // Index the whole file even if there are warnings and -Werror is set. // Avoids some analyses too. Set in two places as we're late to the party. diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index a2aadebb7de34..62e049dc67369 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2727,6 +2727,10 @@ LambdaScopeInfo *Sema::getCurGenericLambda() { } bool Sema::shouldRetainCommentsInAST(SourceLocation Loc) const { + if (!LangOpts.CommentOpts.RetainCommentsFromSystemHeaders && + SourceMgr.isInSystemHeader(Loc)) + return false; + if (LangOpts.CommentOpts.ParseAllComments) return true; @@ -2754,9 +2758,6 @@ bool Sema::shouldRetainCommentsInAST(SourceLocation Loc) const { } void Sema::ActOnComment(SourceRange Comment) { - if (!LangOpts.CommentOpts.RetainCommentsFromSystemHeaders && - SourceMgr.isInSystemHeader(Comment.getBegin())) - return; if (!shouldRetainCommentsInAST(Comment.getBegin())) return; RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
