https://github.com/aengelke created https://github.com/llvm/llvm-project/pull/219807
Reverts llvm/llvm-project#206363 due to huge compile-time regression. >From 3963672d7afd7bf73c254cfc68efcb00952f1e2f Mon Sep 17 00:00:00 2001 From: Anonmiraj <[email protected]> Date: Sun, 30 Aug 2026 17:00:41 +0300 Subject: [PATCH] =?UTF-8?q?Revert=20"[clang]=20Don't=20add=20documentation?= =?UTF-8?q?=20comments=20to=20the=20AST=20if=20not=20requested=20=E2=80=A6?= =?UTF-8?q?"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit e046dce4a4c80610b49d67bc02c85f86b1a6353d. --- .../clang-doc/tool/ClangDocMain.cpp | 3 +- clang-tools-extra/clangd/Compiler.cpp | 3 +- .../clangd/index/IndexAction.cpp | 3 +- clang/docs/ReleaseNotes.md | 6 - clang/include/clang/Basic/CommentOptions.h | 8 - clang/include/clang/Basic/Diagnostic.h | 10 - clang/include/clang/Basic/DiagnosticIDs.h | 6 - clang/include/clang/Basic/LangOptions.def | 2 + clang/include/clang/Options/Options.td | 8 +- clang/include/clang/Sema/Sema.h | 5 - clang/lib/AST/ASTContext.cpp | 2 +- clang/lib/Basic/DiagnosticIDs.cpp | 196 ++++++++---------- clang/lib/Driver/ToolChains/Clang.cpp | 2 - clang/lib/ExtractAPI/ExtractAPIConsumer.cpp | 3 - clang/lib/Frontend/ASTUnit.cpp | 6 - clang/lib/Frontend/CompilerInvocation.cpp | 1 - clang/lib/Frontend/FrontendActions.cpp | 6 - clang/lib/Sema/Sema.cpp | 33 +-- clang/lib/Sema/SemaDecl.cpp | 7 +- clang/test/AST/ast-dump-comment-retention.cpp | 28 --- .../warn-documentation-comment-retention.cpp | 38 ---- 21 files changed, 98 insertions(+), 278 deletions(-) delete mode 100644 clang/test/AST/ast-dump-comment-retention.cpp delete mode 100644 clang/test/Sema/warn-documentation-comment-retention.cpp diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp index 0b81a3cb20351..00290d7cdc74b 100644 --- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp +++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp @@ -285,8 +285,7 @@ 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 = getInsertArgumentAdjuster( - "-fretain-comments", tooling::ArgumentInsertPosition::END); + ArgumentsAdjuster ArgAdjuster; if (!DoxygenOnly) ArgAdjuster = combineAdjusters( getInsertArgumentAdjuster("-fparse-all-comments", diff --git a/clang-tools-extra/clangd/Compiler.cpp b/clang-tools-extra/clangd/Compiler.cpp index aaeaab30b96db..4644cd75c0833 100644 --- a/clang-tools-extra/clangd/Compiler.cpp +++ b/clang-tools-extra/clangd/Compiler.cpp @@ -121,8 +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; + CI->getLangOpts().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 21e055b82d722..489c61f1ff424 100644 --- a/clang-tools-extra/clangd/index/IndexAction.cpp +++ b/clang-tools-extra/clangd/index/IndexAction.cpp @@ -167,8 +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; + CI.getLangOpts().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/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 30afc3706b3c4..bdbabf2cd98d0 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -405,12 +405,6 @@ features cannot lower the translation-unit ABI level; - 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. diff --git a/clang/include/clang/Basic/CommentOptions.h b/clang/include/clang/Basic/CommentOptions.h index 73e7cba91cca8..7d142fc32f511 100644 --- a/clang/include/clang/Basic/CommentOptions.h +++ b/clang/include/clang/Basic/CommentOptions.h @@ -30,14 +30,6 @@ 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/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 699cd89791619..834f026aff62d 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -974,16 +974,6 @@ 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 1bb0529c3ff26..148d772a9e593 100644 --- a/clang/include/clang/Basic/DiagnosticIDs.h +++ b/clang/include/clang/Basic/DiagnosticIDs.h @@ -516,12 +516,6 @@ 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/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index 1422125b77741..ad993ce7e5d95 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -405,6 +405,8 @@ 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/Options/Options.td b/clang/include/clang/Options/Options.td index 20fcab50feeac..3b88dce9c822b 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -2221,12 +2221,6 @@ 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<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 @@ -3948,7 +3942,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<"CommentOpts.RetainCommentsFromSystemHeaders">>; + MarshallingInfoFlag<LangOpts<"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/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 9b0e1b5044df2..4650bd53775f7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1126,11 +1126,6 @@ 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 shouldRetainCommentsInAST(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/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index ba2c289502999..8b3315eca1cdc 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.CommentOpts.RetainCommentsFromSystemHeaders || + assert(LangOpts.RetainCommentsFromSystemHeaders || !SourceMgr.isInSystemHeader(RC.getSourceRange().getBegin())); Comments.addComment(RC, LangOpts.CommentOpts, BumpAlloc); } diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index ef9db935ca752..3709528e497d2 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -541,131 +541,103 @@ DiagnosticIDs::getDiagnosticLevel(unsigned DiagID, SourceLocation Loc, diag::Severity DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, const DiagnosticsEngine &Diag) const { - return getDiagnosticListHighestSeverity({DiagID}, Loc, Diag); -} - -diag::Severity DiagnosticIDs::getDiagnosticListHighestSeverity( - llvm::ArrayRef<diag::kind> DiagIDs, SourceLocation Loc, - const DiagnosticsEngine &Diag) const { - DiagnosticsEngine::DiagState *State = Diag.GetDiagStateForLoc(Loc); + bool IsCustomDiag = DiagnosticIDs::IsCustomDiag(DiagID); + assert(getDiagClass(DiagID) != CLASS_NOTE); - 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; + // 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; - // 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; - } + // Get the mapping information, or compute it lazily. + 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); + } - // 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; - } + // At this point, ignored errors can no longer be upgraded. + if (Result == diag::Severity::Ignored) + return Result; - // 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; - } + // 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 explicitly requested, map fatal errors to errors. - if (Result == diag::Severity::Fatal && - DiagID != diag::fatal_too_many_errors && Diag.FatalsAsError) + // 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; + } - // 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 -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; + } - // Clang-diagnostics pragmas always take precedence over suppression - // mapping. - if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc)) - 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; + // Rest of the mappings are only applicable for diagnostics associated with a + // SourceLocation, bail out early for others. + if (!Diag.hasSourceManager()) return Result; - }; - 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; + // 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; } - return CompositeResult; + // Clang-diagnostics pragmas always take precedence over suppression mapping. + if (!Mapping.isPragma() && Diag.isSuppressedViaMapping(DiagID, Loc)) + return diag::Severity::Ignored; + + return Result; } bool DiagnosticIDs::shouldSuppressAsSystemWarning( diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 85beb4e7a2983..072664e6040f3 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -8163,8 +8163,6 @@ 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)) { diff --git a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp index 4e3c5466e577f..c81d76764643b 100644 --- a/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp +++ b/clang/lib/ExtractAPI/ExtractAPIConsumer.cpp @@ -455,9 +455,6 @@ ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { } bool ExtractAPIAction::PrepareToExecuteAction(CompilerInstance &CI) { - // ExtractAPI reads documentation comments off the AST. - CI.getLangOpts().CommentOpts.RetainComments = true; - // Public API can never be inside function bodies, so skip parsing them. CI.getFrontendOpts().SkipFunctionBodies = true; diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 738fe99cef46b..75e4f7772f47c 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1533,9 +1533,6 @@ 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->getLangOpts().CommentOpts.RetainComments = true; CI->getFrontendOpts().DisableFree = false; ProcessWarningOptions(AST->getDiagnostics(), CI->getDiagnosticOpts(), AST->getFileManager().getVirtualFileSystem()); @@ -1644,9 +1641,6 @@ 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->getLangOpts().CommentOpts.RetainComments = true; Invocation->getFrontendOpts().DisableFree = false; getDiagnostics().Reset(); ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index e89a4ed125934..2f5e882d59084 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -5296,7 +5296,6 @@ 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, diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index aaaf0b6401502..e7b05740b8376 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -86,8 +86,6 @@ ASTPrintAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { std::unique_ptr<ASTConsumer> ASTDumpAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) { - // 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,10 +250,6 @@ GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance &CI, bool GenerateModuleInterfaceAction::PrepareToExecuteAction( CompilerInstance &CI) { - // Documentation comments must still be serialized into the BMI - // so importers can query them. - CI.getLangOpts().CommentOpts.RetainComments = true; - for (const auto &FIF : CI.getFrontendOpts().Inputs) { if (const auto InputFormat = FIF.getKind().getFormat(); InputFormat != InputKind::Format::Source) { diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index f933444c22adf..21f71d7f8b40e 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2749,39 +2749,10 @@ LambdaScopeInfo *Sema::getCurGenericLambda() { return nullptr; } -bool Sema::shouldRetainCommentsInAST(SourceLocation Loc) const { - if (!LangOpts.CommentOpts.RetainCommentsFromSystemHeaders && - SourceMgr.isInSystemHeader(Loc)) - return false; - - if (LangOpts.CommentOpts.ParseAllComments) - return true; - - if (LangOpts.CommentOpts.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 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; -} void Sema::ActOnComment(SourceRange Comment) { - if (!shouldRetainCommentsInAST(Comment.getBegin())) + if (!LangOpts.RetainCommentsFromSystemHeaders && + SourceMgr.isInSystemHeader(Comment.getBegin())) return; RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false); if (RC.isAlmostTrailingComment() || RC.hasUnsupportedSplice(SourceMgr)) { diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 064d1ebf5a642..07c6157ab8f31 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -30,6 +30,7 @@ #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" @@ -15739,8 +15740,10 @@ void Sema::ActOnDocumentableDecls(ArrayRef<Decl *> Group) { if (Group.empty() || !Group[0]) return; - if (Diags.areAllIgnored("documentation", Group[0]->getLocation()) && - Diags.areAllIgnored("documentation-pedantic", Group[0]->getLocation())) + if (Diags.isIgnored(diag::warn_doc_param_not_found, + Group[0]->getLocation()) && + Diags.isIgnored(diag::warn_unknown_comment_command_name, + Group[0]->getLocation())) return; if (Group.size() >= 2) { diff --git a/clang/test/AST/ast-dump-comment-retention.cpp b/clang/test/AST/ast-dump-comment-retention.cpp deleted file mode 100644 index e2a68e2f3221d..0000000000000 --- a/clang/test/AST/ast-dump-comment-retention.cpp +++ /dev/null @@ -1,28 +0,0 @@ -// 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 deleted file mode 100644 index 5d7a50b0bb970..0000000000000 --- a/clang/test/Sema/warn-documentation-comment-retention.cpp +++ /dev/null @@ -1,38 +0,0 @@ -// 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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
