https://github.com/kadircet updated https://github.com/llvm/llvm-project/pull/112517
From c0d2e4590308ebaaee78d7fe130a72b8586f6a88 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya <kadir...@google.com> Date: Mon, 14 Oct 2024 11:20:55 +0200 Subject: [PATCH 1/5] [clang] Introduce diagnostics suppression mappings This implements https://discourse.llvm.org/t/rfc-add-support-for-controlling-diagnostics-severities-at-file-level-granularity-through-command-line/81292. Users now can suppress warnings for certain headers by providing a mapping with globs, a sample file looks like: ``` [unused] src:* src:*clang/*=emit ``` This will suppress warnings from `-Wunused` group in all files that aren't under `clang/` directory. This mapping file can be passed to clang via `--warning-suppression-mappings=foo.txt`. At a high level, mapping file is stored in DiagnosticOptions and then processed with rest of the warning flags when creating a DiagnosticsEngine. This is a functor that uses SpecialCaseLists underneath to match against globs coming from the mappings file. This implies processing warning options now performs IO, relevant interfaces are updated to take in a VFS, falling back to RealFileSystem when one is not available. --- .../ExpandModularHeadersPPCallbacks.cpp | 3 +- clang/include/clang/Basic/Diagnostic.h | 15 +- .../clang/Basic/DiagnosticDriverKinds.td | 3 + clang/include/clang/Basic/DiagnosticOptions.h | 3 + clang/include/clang/Driver/Options.td | 5 + .../include/clang/Frontend/CompilerInstance.h | 10 +- clang/lib/Basic/Diagnostic.cpp | 12 ++ clang/lib/Basic/DiagnosticIDs.cpp | 27 ++- clang/lib/Basic/Warnings.cpp | 107 ++++++++++++ clang/lib/Driver/ToolChains/Clang.cpp | 7 + clang/lib/Frontend/ASTUnit.cpp | 15 +- clang/lib/Frontend/CompilerInstance.cpp | 21 ++- clang/lib/Frontend/CompilerInvocation.cpp | 8 + clang/lib/Frontend/PrecompiledPreamble.cpp | 2 +- clang/lib/Interpreter/CodeCompletion.cpp | 4 +- clang/lib/Serialization/ASTReader.cpp | 4 +- .../test/Misc/Inputs/suppression-mapping.txt | 4 + .../Misc/warning-suppression-mappings.cpp | 16 ++ clang/tools/driver/cc1gen_reproducer_main.cpp | 7 +- clang/tools/driver/driver.cpp | 7 +- clang/unittests/Basic/DiagnosticTest.cpp | 164 ++++++++++++++++++ .../Frontend/CompilerInvocationTest.cpp | 10 ++ llvm/include/llvm/Support/SpecialCaseList.h | 1 - 23 files changed, 419 insertions(+), 36 deletions(-) create mode 100644 clang/test/Misc/Inputs/suppression-mapping.txt create mode 100644 clang/test/Misc/warning-suppression-mappings.cpp diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp index fef086c5a99d86..4c34f9ea122d9e 100644 --- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp +++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp @@ -81,7 +81,8 @@ ExpandModularHeadersPPCallbacks::ExpandModularHeadersPPCallbacks( Diags.setSourceManager(&Sources); // FIXME: Investigate whatever is there better way to initialize DiagEngine // or whatever DiagEngine can be shared by multiple preprocessors - ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts()); + ProcessWarningOptions(Diags, Compiler.getDiagnosticOpts(), + Compiler.getVirtualFileSystem()); LangOpts.Modules = false; diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 3b1efdb12824c7..79b7545e92d6da 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -20,11 +20,13 @@ #include "clang/Basic/Specifiers.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/FunctionExtras.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Support/Compiler.h" +#include "llvm/Support/VirtualFileSystem.h" #include <cassert> #include <cstdint> #include <limits> @@ -555,6 +557,10 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { void *ArgToStringCookie = nullptr; ArgToStringFnTy ArgToStringFn; + /// Whether the diagnostic should be suppressed in FilePath. + llvm::unique_function<bool(diag::kind, llvm::StringRef /*FilePath*/) const> + DiagSuppressionMapping; + public: explicit DiagnosticsEngine(IntrusiveRefCntPtr<DiagnosticIDs> Diags, IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts, @@ -946,6 +952,13 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { return (Level)Diags->getDiagnosticLevel(DiagID, Loc, *this); } + /// Diagnostic suppression mappings can be used to ignore diagnostics based on + /// the file they occur in. + /// These take presumed locations into account, and can still be overriden by + /// clang-diagnostics pragmas. + void setDiagSuppressionMapping(decltype(DiagSuppressionMapping) Mapping); + bool isSuppressedViaMapping(diag::kind D, llvm::StringRef FilePath) const; + /// Issue the message to the client. /// /// This actually returns an instance of DiagnosticBuilder which emits the @@ -1759,7 +1772,7 @@ const char ToggleHighlight = 127; /// warning options specified on the command line. void ProcessWarningOptions(DiagnosticsEngine &Diags, const DiagnosticOptions &Opts, - bool ReportDiags = true); + llvm::vfs::FileSystem &VFS, bool ReportDiags = true); void EscapeStringForDiagnostic(StringRef Str, SmallVectorImpl<char> &OutStr); } // namespace clang diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td index 65551bd7761a9d..b5bdfa538cc741 100644 --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td @@ -835,4 +835,7 @@ def err_drv_triple_version_invalid : Error< def warn_missing_include_dirs : Warning< "no such include directory: '%0'">, InGroup<MissingIncludeDirs>, DefaultIgnore; + +def err_drv_malformed_warning_suppression_mapping : Error< + "failed to process suppression mapping file '%0' : %1">; } diff --git a/clang/include/clang/Basic/DiagnosticOptions.h b/clang/include/clang/Basic/DiagnosticOptions.h index 30141c2b8f4475..0770805624cec8 100644 --- a/clang/include/clang/Basic/DiagnosticOptions.h +++ b/clang/include/clang/Basic/DiagnosticOptions.h @@ -108,6 +108,9 @@ class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{ /// The file to serialize diagnostics to (non-appending). std::string DiagnosticSerializationFile; + /// File that defines suppression mappings. + std::string SuppressionMappingsFile; + /// The list of -W... options used to alter the diagnostic mappings, with the /// prefixes removed. std::vector<std::string> Warnings; diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 5df6ddd5e6a0c5..381557f05dec4a 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -8998,3 +8998,8 @@ def wasm_opt : Flag<["--"], "wasm-opt">, Group<m_Group>, HelpText<"Enable the wasm-opt optimizer (default)">, MarshallingInfoNegativeFlag<LangOpts<"NoWasmOpt">>; + +def warning_suppression_mappings_EQ : Joined<["--"], + "warning-suppression-mappings=">, Group<Diag_Group>, + HelpText<"File containing list of sources to suppress diagnostics for. See XXX for format">, + Visibility<[ClangOption, CC1Option]>; diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 3464654284f199..338eb3c6bd028d 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -24,6 +24,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/BuryPointer.h" #include "llvm/Support/FileSystem.h" +#include "llvm/Support/VirtualFileSystem.h" #include <cassert> #include <list> #include <memory> @@ -701,11 +702,10 @@ class CompilerInstance : public ModuleLoader { /// used by some diagnostics printers (for logging purposes only). /// /// \return The new object on success, or null on failure. - static IntrusiveRefCntPtr<DiagnosticsEngine> - createDiagnostics(DiagnosticOptions *Opts, - DiagnosticConsumer *Client = nullptr, - bool ShouldOwnClient = true, - const CodeGenOptions *CodeGenOpts = nullptr); + static IntrusiveRefCntPtr<DiagnosticsEngine> createDiagnostics( + DiagnosticOptions *Opts, DiagnosticConsumer *Client = nullptr, + bool ShouldOwnClient = true, const CodeGenOptions *CodeGenOpts = nullptr, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS = nullptr); /// Create the file manager and replace any existing one with it. /// diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index e23362fc7af00d..e536effd25072f 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -21,6 +21,7 @@ #include "clang/Basic/SourceManager.h" #include "clang/Basic/Specifiers.h" #include "clang/Basic/TokenKinds.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" @@ -28,6 +29,7 @@ #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/CrashRecoveryContext.h" #include "llvm/Support/Unicode.h" +#include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" #include <algorithm> #include <cassert> @@ -477,6 +479,16 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor Flavor, setSeverity(Diag, Map, Loc); } +void DiagnosticsEngine::setDiagSuppressionMapping( + decltype(DiagSuppressionMapping) Mapping) { + DiagSuppressionMapping = std::move(Mapping); +} + +bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind D, + llvm::StringRef FilePath) const { + return DiagSuppressionMapping && DiagSuppressionMapping(D, FilePath); +} + void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) { DiagnosticStorage DiagStorage; DiagStorage.DiagRanges.append(storedDiag.range_begin(), diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index d45bb0f392d457..4f213e02fa9324 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -575,6 +575,12 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, 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; + + const auto &SM = Diag.getSourceManager(); // Custom diagnostics always are emitted in system headers. bool ShowInSystemHeader = !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemHeader; @@ -582,18 +588,29 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, // If we are in a system header, we ignore it. We look at the diagnostic class // because we also want to ignore extensions and warnings in -Werror and // -pedantic-errors modes, which *map* warnings/extensions to errors. - if (State->SuppressSystemWarnings && !ShowInSystemHeader && Loc.isValid() && - Diag.getSourceManager().isInSystemHeader( - Diag.getSourceManager().getExpansionLoc(Loc))) + if (State->SuppressSystemWarnings && !ShowInSystemHeader && + SM.isInSystemHeader(SM.getExpansionLoc(Loc))) return diag::Severity::Ignored; // We also ignore warnings due to system macros bool ShowInSystemMacro = !GetDiagInfo(DiagID) || GetDiagInfo(DiagID)->WarnShowInSystemMacro; - if (State->SuppressSystemWarnings && !ShowInSystemMacro && Loc.isValid() && - Diag.getSourceManager().isInSystemMacro(Loc)) + if (State->SuppressSystemWarnings && !ShowInSystemMacro && + SM.isInSystemMacro(Loc)) return diag::Severity::Ignored; + // Apply suppression mappings if severity wasn't explicitly mapped with a + // clang-diagnostics pragma to ensure pragmas always take precedence over + // mappings. + // We also use presumed locations here to improve reproducibility for + // preprocessed inputs. + if (!Mapping.isPragma()) { + if (auto PLoc = SM.getPresumedLoc(Loc); + PLoc.isValid() && + Diag.isSuppressedViaMapping(DiagID, PLoc.getFilename())) + return diag::Severity::Ignored; + } + return Result; } diff --git a/clang/lib/Basic/Warnings.cpp b/clang/lib/Basic/Warnings.cpp index 5a5ac555633886..1047980b84d7db 100644 --- a/clang/lib/Basic/Warnings.cpp +++ b/clang/lib/Basic/Warnings.cpp @@ -23,10 +23,21 @@ // simpler because a remark can't be promoted to an error. #include "clang/Basic/AllDiagnostics.h" #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/DiagnosticDriver.h" +#include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticOptions.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/StringMapEntry.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/SpecialCaseList.h" +#include "llvm/Support/VirtualFileSystem.h" #include <algorithm> #include <cstring> +#include <memory> #include <utility> +#include <vector> using namespace clang; // EmitUnknownDiagWarning - Emit a warning and typo hint for unknown warning @@ -41,8 +52,96 @@ static void EmitUnknownDiagWarning(DiagnosticsEngine &Diags, << (Prefix.str() += std::string(Suggestion)); } +namespace { +class WarningsSpecialCaseList : public llvm::SpecialCaseList { +public: + static std::unique_ptr<WarningsSpecialCaseList> + create(const llvm::MemoryBuffer &MB, std::string &Err) { + auto SCL = std::make_unique<WarningsSpecialCaseList>(); + if (SCL->createInternal(&MB, Err)) + return SCL; + return nullptr; + } + + // Section names refer to diagnostic groups, which cover multiple individual + // diagnostics. Expand diagnostic groups here to individual diagnostics. + // A diagnostic can have multiple diagnostic groups associated with it, we let + // the last section take precedence in such cases. + void processSections(DiagnosticsEngine &Diags) { + // Drop the default section introduced by special case list, we only support + // exact diagnostic group names. + Sections.erase("*"); + // Make sure we iterate sections by their line numbers. + std::vector<std::pair<unsigned, const llvm::StringMapEntry<Section> *>> + LineAndSection; + for (const auto &Entry : Sections) { + LineAndSection.emplace_back( + Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry); + } + llvm::sort(LineAndSection); + for (const auto &[_, Entry] : LineAndSection) { + SmallVector<diag::kind, 256> GroupDiags; + if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup( + clang::diag::Flavor::WarningOrError, Entry->first(), + GroupDiags)) { + EmitUnknownDiagWarning(Diags, clang::diag::Flavor::WarningOrError, "", + Entry->first()); + continue; + } + for (auto D : GroupDiags) + DiagToSection[D] = &Entry->getValue(); + } + } + + bool isDiagSuppressed(diag::kind D, llvm::StringRef FilePath) const { + auto Section = DiagToSection.find(D); + if (Section == DiagToSection.end()) + return false; + auto SrcEntries = Section->second->Entries.find("src"); + if(SrcEntries == Section->second->Entries.end()) + return false; + // Find the longest glob pattern that matches FilePath. A positive match + // implies D should be suppressed for FilePath. + llvm::StringRef LongestMatch; + bool LongestWasNegative; + for (const auto &CatIt : SrcEntries->second) { + bool IsNegative = CatIt.first() == "emit"; + for (const auto &GlobIt : CatIt.second.Globs) { + if (GlobIt.getKeyLength() < LongestMatch.size()) + continue; + if (!GlobIt.second.first.match(FilePath)) + continue; + LongestMatch = GlobIt.getKey(); + LongestWasNegative = IsNegative; + } + } + return !LongestMatch.empty() && !LongestWasNegative; + } + +private: + llvm::DenseMap<diag::kind, const Section*> DiagToSection; +}; + +void parseSuppressionMappings(const llvm::MemoryBuffer &MB, + DiagnosticsEngine &Diags) { + std::string Err; + auto SCL = WarningsSpecialCaseList::create(MB, Err); + if (!SCL) { + Diags.Report(diag::err_drv_malformed_warning_suppression_mapping) + << MB.getBufferIdentifier() << Err; + return; + } + SCL->processSections(Diags); + Diags.setDiagSuppressionMapping( + [SCL(std::move(SCL))](diag::kind K, llvm::StringRef Path) { + return SCL->isDiagSuppressed(K, Path); + }); +} +} // namespace + void clang::ProcessWarningOptions(DiagnosticsEngine &Diags, const DiagnosticOptions &Opts, + llvm::vfs::FileSystem& VFS, bool ReportDiags) { Diags.setSuppressSystemWarnings(true); // Default to -Wno-system-headers Diags.setIgnoreAllWarnings(Opts.IgnoreWarnings); @@ -70,6 +169,14 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags, else Diags.setExtensionHandlingBehavior(diag::Severity::Ignored); + if (!Opts.SuppressionMappingsFile.empty()) { + if (auto Buf = VFS.getBufferForFile(Opts.SuppressionMappingsFile)) { + parseSuppressionMappings(**Buf, Diags); + } else if (ReportDiags) { + Diags.Report(diag::err_drv_no_such_file) << Opts.SuppressionMappingsFile; + } + } + SmallVector<diag::kind, 10> _Diags; const IntrusiveRefCntPtr< DiagnosticIDs > DiagIDs = Diags.getDiagnosticIDs(); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 04b3832327a99c..27a841a0a7a8e9 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -4493,6 +4493,13 @@ static void RenderDiagnosticsOptions(const Driver &D, const ArgList &Args, Args.addOptOutFlag(CmdArgs, options::OPT_fspell_checking, options::OPT_fno_spell_checking); + + if (const Arg *A = + Args.getLastArg(options::OPT_warning_suppression_mappings_EQ)) { + if (!D.getVFS().exists(A->getValue())) + D.Diag(clang::diag::err_drv_no_such_file) << A->getValue(); + CmdArgs.push_back(Args.MakeArgString(A->getSpelling() + A->getValue())); + } } DwarfFissionKind tools::getDebugFissionKind(const Driver &D, diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index bffff0d27af3ab..464ed8090a0ec5 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1367,7 +1367,7 @@ ASTUnit::getMainBufferWithPrecompiledPreamble( // after parsing the preamble. getDiagnostics().Reset(); ProcessWarningOptions(getDiagnostics(), - PreambleInvocationIn.getDiagnosticOpts()); + PreambleInvocationIn.getDiagnosticOpts(), *VFS); getDiagnostics().setNumWarnings(NumWarningsInPreamble); PreambleRebuildCountdown = 1; @@ -1593,7 +1593,8 @@ ASTUnit *ASTUnit::LoadFromCompilerInvocationAction( // We'll manage file buffers ourselves. CI->getPreprocessorOpts().RetainRemappedFileBuffers = true; CI->getFrontendOpts().DisableFree = false; - ProcessWarningOptions(AST->getDiagnostics(), CI->getDiagnosticOpts()); + ProcessWarningOptions(AST->getDiagnostics(), CI->getDiagnosticOpts(), + AST->getFileManager().getVirtualFileSystem()); // Create the compiler instance to use for building the AST. std::unique_ptr<CompilerInstance> Clang( @@ -1701,7 +1702,7 @@ bool ASTUnit::LoadFromCompilerInvocation( Invocation->getPreprocessorOpts().RetainRemappedFileBuffers = true; Invocation->getFrontendOpts().DisableFree = false; getDiagnostics().Reset(); - ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts()); + ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), *VFS); std::unique_ptr<llvm::MemoryBuffer> OverrideMainBuffer; if (PrecompilePreambleAfterNParses > 0) { @@ -1709,7 +1710,7 @@ bool ASTUnit::LoadFromCompilerInvocation( OverrideMainBuffer = getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS); getDiagnostics().Reset(); - ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts()); + ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), *VFS); } SimpleTimer ParsingTimer(WantTiming); @@ -1902,7 +1903,8 @@ bool ASTUnit::Reparse(std::shared_ptr<PCHContainerOperations> PCHContainerOps, // Clear out the diagnostics state. FileMgr.reset(); getDiagnostics().Reset(); - ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts()); + ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), + *VFS); if (OverrideMainBuffer) getDiagnostics().setNumWarnings(NumWarningsInPreamble); @@ -2241,7 +2243,8 @@ void ASTUnit::CodeComplete( CaptureDroppedDiagnostics Capture(CaptureDiagsKind::All, Clang->getDiagnostics(), &StoredDiagnostics, nullptr); - ProcessWarningOptions(Diag, Inv.getDiagnosticOpts()); + ProcessWarningOptions(Diag, Inv.getDiagnosticOpts(), + FileMgr.getVirtualFileSystem()); // Create the target instance. if (!Clang->createTarget()) { diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 240305b33824b8..ecc6782c7cb4fb 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -39,6 +39,7 @@ #include "clang/Serialization/ASTReader.h" #include "clang/Serialization/GlobalModuleIndex.h" #include "clang/Serialization/InMemoryModuleCache.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/Statistic.h" @@ -54,6 +55,7 @@ #include "llvm/Support/Signals.h" #include "llvm/Support/TimeProfiler.h" #include "llvm/Support/Timer.h" +#include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/Host.h" #include <optional> @@ -332,19 +334,22 @@ static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts, void CompilerInstance::createDiagnostics(DiagnosticConsumer *Client, bool ShouldOwnClient) { - Diagnostics = createDiagnostics(&getDiagnosticOpts(), Client, - ShouldOwnClient, &getCodeGenOpts()); + Diagnostics = createDiagnostics( + &getDiagnosticOpts(), Client, ShouldOwnClient, &getCodeGenOpts(), + FileMgr ? FileMgr->getVirtualFileSystemPtr() : nullptr); } -IntrusiveRefCntPtr<DiagnosticsEngine> -CompilerInstance::createDiagnostics(DiagnosticOptions *Opts, - DiagnosticConsumer *Client, - bool ShouldOwnClient, - const CodeGenOptions *CodeGenOpts) { +IntrusiveRefCntPtr<DiagnosticsEngine> CompilerInstance::createDiagnostics( + DiagnosticOptions *Opts, DiagnosticConsumer *Client, bool ShouldOwnClient, + const CodeGenOptions *CodeGenOpts, + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) { IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs()); IntrusiveRefCntPtr<DiagnosticsEngine> Diags(new DiagnosticsEngine(DiagID, Opts)); + if (!VFS) + VFS = llvm::vfs::getRealFileSystem(); + // Create the diagnostic client for reporting errors or for // implementing -verify. if (Client) { @@ -367,7 +372,7 @@ CompilerInstance::createDiagnostics(DiagnosticOptions *Opts, Opts->DiagnosticSerializationFile); // Configure our handling of diagnostics. - ProcessWarningOptions(*Diags, *Opts); + ProcessWarningOptions(*Diags, *Opts, *VFS); return Diags; } diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index db7c791059a32e..5efb159bdea3bd 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -2521,6 +2521,11 @@ void CompilerInvocationBase::GenerateDiagnosticArgs( Consumer(StringRef("-R") + Remark); } + + if (!Opts.SuppressionMappingsFile.empty()) { + GenerateArg(Consumer, OPT_warning_suppression_mappings_EQ, + Opts.SuppressionMappingsFile); + } } std::unique_ptr<DiagnosticOptions> @@ -2597,6 +2602,9 @@ bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args, Opts.TabStop = DiagnosticOptions::DefaultTabStop; } + if (const Arg *A = Args.getLastArg(OPT_warning_suppression_mappings_EQ)) + Opts.SuppressionMappingsFile = A->getValue(); + addDiagnosticArgs(Args, OPT_W_Group, OPT_W_value_Group, Opts.Warnings); addDiagnosticArgs(Args, OPT_R_Group, OPT_R_value_Group, Opts.Remarks); diff --git a/clang/lib/Frontend/PrecompiledPreamble.cpp b/clang/lib/Frontend/PrecompiledPreamble.cpp index cab5838fceb24d..d1bac09830b04b 100644 --- a/clang/lib/Frontend/PrecompiledPreamble.cpp +++ b/clang/lib/Frontend/PrecompiledPreamble.cpp @@ -479,7 +479,7 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build( // Clear out old caches and data. Diagnostics.Reset(); - ProcessWarningOptions(Diagnostics, Clang->getDiagnosticOpts()); + ProcessWarningOptions(Diagnostics, Clang->getDiagnosticOpts(), *VFS); VFS = createVFSFromCompilerInvocation(Clang->getInvocation(), Diagnostics, VFS); diff --git a/clang/lib/Interpreter/CodeCompletion.cpp b/clang/lib/Interpreter/CodeCompletion.cpp index 791426807cb91d..bbc8830d76bc00 100644 --- a/clang/lib/Interpreter/CodeCompletion.cpp +++ b/clang/lib/Interpreter/CodeCompletion.cpp @@ -380,8 +380,8 @@ void ReplCodeCompleter::codeComplete(CompilerInstance *InterpCI, AU->CodeComplete(CodeCompletionFileName, 1, Col, RemappedFiles, false, false, false, consumer, std::make_shared<clang::PCHContainerOperations>(), *diag, - InterpCI->getLangOpts(), InterpCI->getSourceManager(), - InterpCI->getFileManager(), sd, tb, std::move(Act)); + InterpCI->getLangOpts(), AU->getSourceManager(), + AU->getFileManager(), sd, tb, std::move(Act)); } } // namespace clang diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 7d9170e7f0b479..56a5422ee62673 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -603,7 +603,9 @@ bool PCHValidator::ReadDiagnosticOptions( new DiagnosticsEngine(DiagIDs, DiagOpts.get())); // This should never fail, because we would have processed these options // before writing them to an ASTFile. - ProcessWarningOptions(*Diags, *DiagOpts, /*Report*/false); + ProcessWarningOptions(*Diags, *DiagOpts, + PP.getFileManager().getVirtualFileSystem(), + /*Report*/ false); ModuleManager &ModuleMgr = Reader.getModuleManager(); assert(ModuleMgr.size() >= 1 && "what ASTFile is this then"); diff --git a/clang/test/Misc/Inputs/suppression-mapping.txt b/clang/test/Misc/Inputs/suppression-mapping.txt new file mode 100644 index 00000000000000..d1fd0e89256d3b --- /dev/null +++ b/clang/test/Misc/Inputs/suppression-mapping.txt @@ -0,0 +1,4 @@ +# Suppress unused warnings in all files, apart from the ones under `foo/`. +[unused] +src:* +src:*foo/*=emit diff --git a/clang/test/Misc/warning-suppression-mappings.cpp b/clang/test/Misc/warning-suppression-mappings.cpp new file mode 100644 index 00000000000000..0626d7279e804a --- /dev/null +++ b/clang/test/Misc/warning-suppression-mappings.cpp @@ -0,0 +1,16 @@ +// Check for warnings +// RUN: not %clang --warning-suppression-mappings=foo.txt -fsyntax-only %s 2>&1 | FileCheck -check-prefix MISSING_MAPPING %s +// RUN: not %clang -cc1 --warning-suppression-mappings=foo.txt -fsyntax-only %s 2>&1 | FileCheck -check-prefix MISSING_MAPPING %s +// MISSING_MAPPING: error: no such file or directory: 'foo.txt' + +// Check that it's no-op when diagnostics aren't enabled. +// RUN: %clang -cc1 -Werror --warning-suppression-mappings=%S/Inputs/suppression-mapping.txt -fsyntax-only %s 2>&1 | FileCheck -check-prefix WARNINGS_DISABLED --allow-empty %s +// WARNINGS_DISABLED-NOT: warning: +// WARNINGS_DISABLED-NOT: error: + +// RUN: %clang -cc1 -verify -Wunused --warning-suppression-mappings=%S/Inputs/suppression-mapping.txt -fsyntax-only %s + +namespace { void foo(); } + +#line 42 "foo/bar.h" +namespace { void bar(); } // expected-warning{{unused function 'bar'}} diff --git a/clang/tools/driver/cc1gen_reproducer_main.cpp b/clang/tools/driver/cc1gen_reproducer_main.cpp index be081cac8c03b1..df59b53f9ef186 100644 --- a/clang/tools/driver/cc1gen_reproducer_main.cpp +++ b/clang/tools/driver/cc1gen_reproducer_main.cpp @@ -121,9 +121,10 @@ generateReproducerForInvocationArguments(ArrayRef<const char *> Argv, IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs()); DiagnosticsEngine Diags(DiagID, &*DiagOpts, new IgnoringDiagConsumer()); - ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false); - Driver TheDriver(ToolContext.Path, llvm::sys::getDefaultTargetTriple(), - Diags); + auto VFS = llvm::vfs::getRealFileSystem(); + ProcessWarningOptions(Diags, *DiagOpts, *VFS, /*ReportDiags=*/false); + Driver TheDriver(ToolContext.Path, llvm::sys::getDefaultTargetTriple(), Diags, + /*Title=*/"clang LLVM compiler", VFS); TheDriver.setTargetAndMode(TargetAndMode); if (ToolContext.NeedsPrependArg) TheDriver.setPrependArg(ToolContext.PrependArg); diff --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp index 686eaea0aa7c83..12038de476ace1 100644 --- a/clang/tools/driver/driver.cpp +++ b/clang/tools/driver/driver.cpp @@ -47,6 +47,7 @@ #include "llvm/Support/StringSaver.h" #include "llvm/Support/TargetSelect.h" #include "llvm/Support/Timer.h" +#include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" #include "llvm/TargetParser/Host.h" #include <memory> @@ -334,9 +335,11 @@ int clang_main(int Argc, char **Argv, const llvm::ToolContext &ToolContext) { Diags.takeClient(), std::move(SerializedConsumer))); } - ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false); + auto VFS = llvm::vfs::getRealFileSystem(); + ProcessWarningOptions(Diags, *DiagOpts, *VFS, /*ReportDiags=*/false); - Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags); + Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags, + /*Title=*/"clang LLVM compiler", VFS); auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(ProgName); TheDriver.setTargetAndMode(TargetAndMode); // If -canonical-prefixes is set, GetExecutablePath will have resolved Path diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp index d8d23e3b670097..141cec442dee92 100644 --- a/clang/unittests/Basic/DiagnosticTest.cpp +++ b/clang/unittests/Basic/DiagnosticTest.cpp @@ -10,8 +10,17 @@ #include "clang/Basic/DiagnosticError.h" #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticLex.h" +#include "clang/Basic/DiagnosticSema.h" +#include "clang/Basic/FileManager.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/VirtualFileSystem.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include <optional> +#include <vector> using namespace llvm; using namespace clang; @@ -167,4 +176,159 @@ TEST(DiagnosticTest, storedDiagEmptyWarning) { // Make sure an empty warning can round-trip with \c StoredDiagnostic. Diags.Report(CaptureConsumer.StoredDiags.front()); } + +class SuppressionMappingTest : public testing::Test { +public: + SuppressionMappingTest() { + Diags.setClient(&CaptureConsumer, /*ShouldOwnClient=*/false); + } + +protected: + llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FS = + llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>(); + DiagnosticsEngine Diags{new DiagnosticIDs(), new DiagnosticOptions}; + + std::vector<StoredDiagnostic> takeDiags() { + return std::move(CaptureConsumer.StoredDiags); + } + +private: + class CaptureDiagnosticConsumer : public DiagnosticConsumer { + public: + std::vector<StoredDiagnostic> StoredDiags; + + void HandleDiagnostic(DiagnosticsEngine::Level level, + const Diagnostic &Info) override { + StoredDiags.push_back(StoredDiagnostic(level, Info)); + } + }; + CaptureDiagnosticConsumer CaptureConsumer; +}; + +MATCHER_P(WithMessage, Msg, "has diagnostic message") { + return arg.getMessage() == Msg; +} + +TEST_F(SuppressionMappingTest, MissingMappingFile) { + Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); + EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage( + "no such file or directory: 'foo.txt'"))); +} + +TEST_F(SuppressionMappingTest, MalformedFile) { + Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + FS->addFile("foo.txt", /*ModificationTime=*/{}, + llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt")); + clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); + EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage( + "failed to process suppression mapping file " + "'foo.txt' : malformed line 1: 'asdf'"))); +} + +TEST_F(SuppressionMappingTest, UnknownDiagName) { + Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + FS->addFile("foo.txt", /*ModificationTime=*/{}, + llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]")); + clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); + EXPECT_THAT(takeDiags(), + testing::ElementsAre(WithMessage( + "unknown warning option 'non-existing-warning'"))); +} + +TEST_F(SuppressionMappingTest, SuppressesGroup) { + llvm::StringLiteral SuppressionMappingFile = R"txt( + [unused] + src:* + )txt"; + Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + FS->addFile("foo.txt", /*ModificationTime=*/{}, + llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); + clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); + EXPECT_THAT(takeDiags(), testing::IsEmpty()); + + EXPECT_TRUE( + Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp")); + EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_deprecated, "foo.cpp")); +} + +TEST_F(SuppressionMappingTest, ExclusionsWork) { + llvm::StringLiteral SuppressionMappingFile = R"txt( + [unused] + src:* + src:*foo.cpp=emit + )txt"; + Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + FS->addFile("foo.txt", /*ModificationTime=*/{}, + llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); + clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); + EXPECT_THAT(takeDiags(), testing::IsEmpty()); + + EXPECT_TRUE( + Diags.isSuppressedViaMapping(diag::warn_unused_function, "bar.cpp")); + EXPECT_FALSE( + Diags.isSuppressedViaMapping(diag::warn_unused_function, "foo.cpp")); +} + +TEST_F(SuppressionMappingTest, LongestMatchWins) { + llvm::StringLiteral SuppressionMappingFile = R"txt( + [unused] + src:*clang/* + src:*clang/lib/Sema/*=emit + src:*clang/lib/Sema/foo* + )txt"; + Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + FS->addFile("foo.txt", /*ModificationTime=*/{}, + llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); + clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); + EXPECT_THAT(takeDiags(), testing::IsEmpty()); + + EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function, + "clang/lib/Basic/foo.h")); + EXPECT_FALSE(Diags.isSuppressedViaMapping(diag::warn_unused_function, + "clang/lib/Sema/bar.h")); + EXPECT_TRUE(Diags.isSuppressedViaMapping(diag::warn_unused_function, + "clang/lib/Sema/foo.h")); +} + +TEST_F(SuppressionMappingTest, IsIgnored) { + llvm::StringLiteral SuppressionMappingFile = R"txt( + [unused] + src:*clang/* + )txt"; + Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().Warnings = {"unused"}; + FS->addFile("foo.txt", /*ModificationTime=*/{}, + llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); + clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); + EXPECT_THAT(takeDiags(), testing::IsEmpty()); + + FileManager FM({}, FS); + SourceManager SM(Diags, FM); + + auto ClangID = + SM.createFileID(llvm::MemoryBuffer::getMemBuffer("", "clang/foo.h")); + auto NonClangID = + SM.createFileID(llvm::MemoryBuffer::getMemBuffer("", "llvm/foo.h")); + auto PresumedClangID = + SM.createFileID(llvm::MemoryBuffer::getMemBuffer("", "llvm/foo2.h")); + // Add a line directive to point into clang/foo.h + SM.AddLineNote(SM.getLocForStartOfFile(PresumedClangID), 42, + SM.getLineTableFilenameID("clang/foo.h"), false, false, + clang::SrcMgr::C_User); + + EXPECT_TRUE(Diags.isIgnored(diag::warn_unused_function, + SM.getLocForStartOfFile(ClangID))); + EXPECT_FALSE(Diags.isIgnored(diag::warn_unused_function, + SM.getLocForStartOfFile(NonClangID))); + EXPECT_TRUE(Diags.isIgnored(diag::warn_unused_function, + SM.getLocForStartOfFile(PresumedClangID))); + + // Pretend we have a clang-diagnostic pragma to enforce the warning. Make sure + // suppressing mapping doesn't take over. + Diags.setSeverity(diag::warn_unused_function, diag::Severity::Error, + SM.getLocForStartOfFile(ClangID)); + EXPECT_FALSE(Diags.isIgnored(diag::warn_unused_function, + SM.getLocForStartOfFile(ClangID))); } +} // namespace diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp index 7912253b761e9b..1cf2bbb6576621 100644 --- a/clang/unittests/Frontend/CompilerInvocationTest.cpp +++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp @@ -1046,4 +1046,14 @@ TEST_F(CommandLineTest, PluginArgsRoundTripDeterminism) { ASSERT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags)); } + +TEST_F(CommandLineTest, WarningSuppressionMappings) { + const char *Args[] = {"--warning-suppression-mappings=foo.txt"}; + + EXPECT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags)); + EXPECT_EQ(Invocation.getDiagnosticOpts().SuppressionMappingsFile, "foo.txt"); + + Invocation.generateCC1CommandLine(GeneratedArgs, *this); + EXPECT_THAT(GeneratedArgs, Contains(StrEq(Args[0]))); +} } // anonymous namespace diff --git a/llvm/include/llvm/Support/SpecialCaseList.h b/llvm/include/llvm/Support/SpecialCaseList.h index 6dc1a29c5a281d..30e3fc644bbc33 100644 --- a/llvm/include/llvm/Support/SpecialCaseList.h +++ b/llvm/include/llvm/Support/SpecialCaseList.h @@ -122,7 +122,6 @@ class SpecialCaseList { // Returns zero if no match is found. unsigned match(StringRef Query) const; - private: StringMap<std::pair<GlobPattern, unsigned>> Globs; std::vector<std::pair<std::unique_ptr<Regex>, unsigned>> RegExes; }; From 11b7250a955b6f4b7411562ed208cea46d308fa1 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya <kadir...@google.com> Date: Thu, 17 Oct 2024 10:40:14 +0200 Subject: [PATCH 2/5] clang-format --- clang/lib/Basic/Warnings.cpp | 6 +++--- clang/lib/Frontend/ASTUnit.cpp | 6 ++++-- clang/unittests/Basic/DiagnosticTest.cpp | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/clang/lib/Basic/Warnings.cpp b/clang/lib/Basic/Warnings.cpp index 1047980b84d7db..c074d719a77234 100644 --- a/clang/lib/Basic/Warnings.cpp +++ b/clang/lib/Basic/Warnings.cpp @@ -98,7 +98,7 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList { if (Section == DiagToSection.end()) return false; auto SrcEntries = Section->second->Entries.find("src"); - if(SrcEntries == Section->second->Entries.end()) + if (SrcEntries == Section->second->Entries.end()) return false; // Find the longest glob pattern that matches FilePath. A positive match // implies D should be suppressed for FilePath. @@ -119,7 +119,7 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList { } private: - llvm::DenseMap<diag::kind, const Section*> DiagToSection; + llvm::DenseMap<diag::kind, const Section *> DiagToSection; }; void parseSuppressionMappings(const llvm::MemoryBuffer &MB, @@ -141,7 +141,7 @@ void parseSuppressionMappings(const llvm::MemoryBuffer &MB, void clang::ProcessWarningOptions(DiagnosticsEngine &Diags, const DiagnosticOptions &Opts, - llvm::vfs::FileSystem& VFS, + llvm::vfs::FileSystem &VFS, bool ReportDiags) { Diags.setSuppressSystemWarnings(true); // Default to -Wno-system-headers Diags.setIgnoreAllWarnings(Opts.IgnoreWarnings); diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 464ed8090a0ec5..5739eeca4d7826 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -1702,7 +1702,8 @@ bool ASTUnit::LoadFromCompilerInvocation( Invocation->getPreprocessorOpts().RetainRemappedFileBuffers = true; Invocation->getFrontendOpts().DisableFree = false; getDiagnostics().Reset(); - ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), *VFS); + ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), + *VFS); std::unique_ptr<llvm::MemoryBuffer> OverrideMainBuffer; if (PrecompilePreambleAfterNParses > 0) { @@ -1710,7 +1711,8 @@ bool ASTUnit::LoadFromCompilerInvocation( OverrideMainBuffer = getMainBufferWithPrecompiledPreamble(PCHContainerOps, *Invocation, VFS); getDiagnostics().Reset(); - ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), *VFS); + ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts(), + *VFS); } SimpleTimer ParsingTimer(WantTiming); diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp index 141cec442dee92..387ae5da14aff0 100644 --- a/clang/unittests/Basic/DiagnosticTest.cpp +++ b/clang/unittests/Basic/DiagnosticTest.cpp @@ -195,7 +195,7 @@ class SuppressionMappingTest : public testing::Test { private: class CaptureDiagnosticConsumer : public DiagnosticConsumer { public: - std::vector<StoredDiagnostic> StoredDiags; + std::vector<StoredDiagnostic> StoredDiags; void HandleDiagnostic(DiagnosticsEngine::Level level, const Diagnostic &Info) override { From 51439829be6e82ab0bc07a258a85bf42059d6de1 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya <kadir...@google.com> Date: Tue, 22 Oct 2024 10:31:43 +0200 Subject: [PATCH 3/5] Rename diag-option && move parsing to DiagnosticsEngine --- clang/include/clang/Basic/Diagnostic.h | 16 ++- clang/include/clang/Basic/DiagnosticOptions.h | 4 +- clang/lib/Basic/Diagnostic.cpp | 96 ++++++++++++++++- clang/lib/Basic/Warnings.cpp | 102 ++---------------- clang/lib/Frontend/CompilerInvocation.cpp | 6 +- clang/unittests/Basic/DiagnosticTest.cpp | 14 +-- .../Frontend/CompilerInvocationTest.cpp | 3 +- 7 files changed, 127 insertions(+), 114 deletions(-) diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index 79b7545e92d6da..d3706460a62a0f 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -26,6 +26,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/iterator_range.h" #include "llvm/Support/Compiler.h" +#include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/VirtualFileSystem.h" #include <cassert> #include <cstdint> @@ -953,10 +954,21 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { } /// Diagnostic suppression mappings can be used to ignore diagnostics based on - /// the file they occur in. + /// the file they occur in. Mapping file is expected to be a special case list + /// with sections denoting diagnostic groups and `src` entries for globs to + /// suppress. `emit` category can be used to disable suppression. Longest glob + /// that matches a filepath takes precendence. For example: + /// [unused] + /// src:*clang/* + /// src:*clang/foo/*=emit + /// src:*clang/foo/bar/* + /// + /// Such a mappings file suppress all diagnostics produced by -Wunused in all + /// sources under `clang/` directory apart from `clang/foo/`. Diagnostics + /// under `clang/foo/bar/` will also be suppressed. /// These take presumed locations into account, and can still be overriden by /// clang-diagnostics pragmas. - void setDiagSuppressionMapping(decltype(DiagSuppressionMapping) Mapping); + void setDiagSuppressionMapping(llvm::MemoryBuffer &MB); bool isSuppressedViaMapping(diag::kind D, llvm::StringRef FilePath) const; /// Issue the message to the client. diff --git a/clang/include/clang/Basic/DiagnosticOptions.h b/clang/include/clang/Basic/DiagnosticOptions.h index 0770805624cec8..53ffb265cdc870 100644 --- a/clang/include/clang/Basic/DiagnosticOptions.h +++ b/clang/include/clang/Basic/DiagnosticOptions.h @@ -108,8 +108,8 @@ class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{ /// The file to serialize diagnostics to (non-appending). std::string DiagnosticSerializationFile; - /// File that defines suppression mappings. - std::string SuppressionMappingsFile; + /// File that defines diagnostic suppression mappings. + std::string DiagnosticSuppressionMappingsFile; /// The list of -W... options used to alter the diagnostic mappings, with the /// prefixes removed. diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index e536effd25072f..beea68c003e2fb 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -12,7 +12,9 @@ #include "clang/Basic/Diagnostic.h" #include "clang/Basic/CharInfo.h" +#include "clang/Basic/DiagnosticDriver.h" #include "clang/Basic/DiagnosticError.h" +#include "clang/Basic/DiagnosticFrontend.h" #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/IdentifierTable.h" @@ -28,6 +30,9 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/CrashRecoveryContext.h" +#include "llvm/Support/Error.h" +#include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/SpecialCaseList.h" #include "llvm/Support/Unicode.h" #include "llvm/Support/VirtualFileSystem.h" #include "llvm/Support/raw_ostream.h" @@ -37,6 +42,7 @@ #include <cstdint> #include <cstring> #include <limits> +#include <memory> #include <string> #include <utility> #include <vector> @@ -479,9 +485,93 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor Flavor, setSeverity(Diag, Map, Loc); } -void DiagnosticsEngine::setDiagSuppressionMapping( - decltype(DiagSuppressionMapping) Mapping) { - DiagSuppressionMapping = std::move(Mapping); +namespace { +class WarningsSpecialCaseList : public llvm::SpecialCaseList { +public: + static std::unique_ptr<WarningsSpecialCaseList> + create(const llvm::MemoryBuffer &MB, std::string &Err) { + auto SCL = std::make_unique<WarningsSpecialCaseList>(); + if (SCL->createInternal(&MB, Err)) + return SCL; + return nullptr; + } + + // Section names refer to diagnostic groups, which cover multiple individual + // diagnostics. Expand diagnostic groups here to individual diagnostics. + // A diagnostic can have multiple diagnostic groups associated with it, we let + // the last section take precedence in such cases. + void processSections(DiagnosticsEngine &Diags) { + // Drop the default section introduced by special case list, we only support + // exact diagnostic group names. + Sections.erase("*"); + // Make sure we iterate sections by their line numbers. + std::vector<std::pair<unsigned, const llvm::StringMapEntry<Section> *>> + LineAndSection; + for (const auto &Entry : Sections) { + LineAndSection.emplace_back( + Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry); + } + llvm::sort(LineAndSection); + static constexpr auto kFlavor = clang::diag::Flavor::WarningOrError; + for (const auto &[_, Entry] : LineAndSection) { + SmallVector<diag::kind, 256> GroupDiags; + if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup( + kFlavor, Entry->first(), GroupDiags)) { + StringRef Suggestion = + DiagnosticIDs::getNearestOption(kFlavor, Entry->first()); + Diags.Report(diag::warn_unknown_diag_option) + << static_cast<unsigned>(kFlavor) << Entry->first() + << !Suggestion.empty() << Suggestion; + continue; + } + for (auto D : GroupDiags) + DiagToSection[D] = &Entry->getValue(); + } + } + + bool isDiagSuppressed(diag::kind D, llvm::StringRef FilePath) const { + auto Section = DiagToSection.find(D); + if (Section == DiagToSection.end()) + return false; + auto SrcEntries = Section->second->Entries.find("src"); + if (SrcEntries == Section->second->Entries.end()) + return false; + // Find the longest glob pattern that matches FilePath. A positive match + // implies D should be suppressed for FilePath. + llvm::StringRef LongestMatch; + bool LongestWasNegative; + for (const auto &CatIt : SrcEntries->second) { + bool IsNegative = CatIt.first() == "emit"; + for (const auto &GlobIt : CatIt.second.Globs) { + if (GlobIt.getKeyLength() < LongestMatch.size()) + continue; + if (!GlobIt.second.first.match(FilePath)) + continue; + LongestMatch = GlobIt.getKey(); + LongestWasNegative = IsNegative; + } + } + return !LongestMatch.empty() && !LongestWasNegative; + } + +private: + llvm::DenseMap<diag::kind, const Section *> DiagToSection; +}; +} // namespace + +void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &MB) { + std::string Err; + auto SCL = WarningsSpecialCaseList::create(MB, Err); + if (!SCL) { + Report(diag::err_drv_malformed_warning_suppression_mapping) + << MB.getBufferIdentifier() << Err; + return; + } + SCL->processSections(*this); + DiagSuppressionMapping = [SCL(std::move(SCL))](diag::kind K, + llvm::StringRef Path) { + return SCL->isDiagSuppressed(K, Path); + }; } bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind D, diff --git a/clang/lib/Basic/Warnings.cpp b/clang/lib/Basic/Warnings.cpp index c074d719a77234..bcad90abf1c21b 100644 --- a/clang/lib/Basic/Warnings.cpp +++ b/clang/lib/Basic/Warnings.cpp @@ -26,17 +26,11 @@ #include "clang/Basic/DiagnosticDriver.h" #include "clang/Basic/DiagnosticIDs.h" #include "clang/Basic/DiagnosticOptions.h" -#include "llvm/ADT/DenseMap.h" -#include "llvm/ADT/STLExtras.h" -#include "llvm/ADT/StringMapEntry.h" #include "llvm/ADT/StringRef.h" -#include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/SpecialCaseList.h" #include "llvm/Support/VirtualFileSystem.h" -#include <algorithm> #include <cstring> #include <memory> -#include <utility> #include <vector> using namespace clang; @@ -52,92 +46,6 @@ static void EmitUnknownDiagWarning(DiagnosticsEngine &Diags, << (Prefix.str() += std::string(Suggestion)); } -namespace { -class WarningsSpecialCaseList : public llvm::SpecialCaseList { -public: - static std::unique_ptr<WarningsSpecialCaseList> - create(const llvm::MemoryBuffer &MB, std::string &Err) { - auto SCL = std::make_unique<WarningsSpecialCaseList>(); - if (SCL->createInternal(&MB, Err)) - return SCL; - return nullptr; - } - - // Section names refer to diagnostic groups, which cover multiple individual - // diagnostics. Expand diagnostic groups here to individual diagnostics. - // A diagnostic can have multiple diagnostic groups associated with it, we let - // the last section take precedence in such cases. - void processSections(DiagnosticsEngine &Diags) { - // Drop the default section introduced by special case list, we only support - // exact diagnostic group names. - Sections.erase("*"); - // Make sure we iterate sections by their line numbers. - std::vector<std::pair<unsigned, const llvm::StringMapEntry<Section> *>> - LineAndSection; - for (const auto &Entry : Sections) { - LineAndSection.emplace_back( - Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry); - } - llvm::sort(LineAndSection); - for (const auto &[_, Entry] : LineAndSection) { - SmallVector<diag::kind, 256> GroupDiags; - if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup( - clang::diag::Flavor::WarningOrError, Entry->first(), - GroupDiags)) { - EmitUnknownDiagWarning(Diags, clang::diag::Flavor::WarningOrError, "", - Entry->first()); - continue; - } - for (auto D : GroupDiags) - DiagToSection[D] = &Entry->getValue(); - } - } - - bool isDiagSuppressed(diag::kind D, llvm::StringRef FilePath) const { - auto Section = DiagToSection.find(D); - if (Section == DiagToSection.end()) - return false; - auto SrcEntries = Section->second->Entries.find("src"); - if (SrcEntries == Section->second->Entries.end()) - return false; - // Find the longest glob pattern that matches FilePath. A positive match - // implies D should be suppressed for FilePath. - llvm::StringRef LongestMatch; - bool LongestWasNegative; - for (const auto &CatIt : SrcEntries->second) { - bool IsNegative = CatIt.first() == "emit"; - for (const auto &GlobIt : CatIt.second.Globs) { - if (GlobIt.getKeyLength() < LongestMatch.size()) - continue; - if (!GlobIt.second.first.match(FilePath)) - continue; - LongestMatch = GlobIt.getKey(); - LongestWasNegative = IsNegative; - } - } - return !LongestMatch.empty() && !LongestWasNegative; - } - -private: - llvm::DenseMap<diag::kind, const Section *> DiagToSection; -}; - -void parseSuppressionMappings(const llvm::MemoryBuffer &MB, - DiagnosticsEngine &Diags) { - std::string Err; - auto SCL = WarningsSpecialCaseList::create(MB, Err); - if (!SCL) { - Diags.Report(diag::err_drv_malformed_warning_suppression_mapping) - << MB.getBufferIdentifier() << Err; - return; - } - SCL->processSections(Diags); - Diags.setDiagSuppressionMapping( - [SCL(std::move(SCL))](diag::kind K, llvm::StringRef Path) { - return SCL->isDiagSuppressed(K, Path); - }); -} -} // namespace void clang::ProcessWarningOptions(DiagnosticsEngine &Diags, const DiagnosticOptions &Opts, @@ -169,11 +77,13 @@ void clang::ProcessWarningOptions(DiagnosticsEngine &Diags, else Diags.setExtensionHandlingBehavior(diag::Severity::Ignored); - if (!Opts.SuppressionMappingsFile.empty()) { - if (auto Buf = VFS.getBufferForFile(Opts.SuppressionMappingsFile)) { - parseSuppressionMappings(**Buf, Diags); + if (!Opts.DiagnosticSuppressionMappingsFile.empty()) { + if (auto Buf = + VFS.getBufferForFile(Opts.DiagnosticSuppressionMappingsFile)) { + Diags.setDiagSuppressionMapping(**Buf); } else if (ReportDiags) { - Diags.Report(diag::err_drv_no_such_file) << Opts.SuppressionMappingsFile; + Diags.Report(diag::err_drv_no_such_file) + << Opts.DiagnosticSuppressionMappingsFile; } } diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 5efb159bdea3bd..87d63440f599f4 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -2522,9 +2522,9 @@ void CompilerInvocationBase::GenerateDiagnosticArgs( Consumer(StringRef("-R") + Remark); } - if (!Opts.SuppressionMappingsFile.empty()) { + if (!Opts.DiagnosticSuppressionMappingsFile.empty()) { GenerateArg(Consumer, OPT_warning_suppression_mappings_EQ, - Opts.SuppressionMappingsFile); + Opts.DiagnosticSuppressionMappingsFile); } } @@ -2603,7 +2603,7 @@ bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args, } if (const Arg *A = Args.getLastArg(OPT_warning_suppression_mappings_EQ)) - Opts.SuppressionMappingsFile = A->getValue(); + Opts.DiagnosticSuppressionMappingsFile = A->getValue(); addDiagnosticArgs(Args, OPT_W_Group, OPT_W_value_Group, Opts.Warnings); addDiagnosticArgs(Args, OPT_R_Group, OPT_R_value_Group, Opts.Remarks); diff --git a/clang/unittests/Basic/DiagnosticTest.cpp b/clang/unittests/Basic/DiagnosticTest.cpp index 387ae5da14aff0..62996fee4bb778 100644 --- a/clang/unittests/Basic/DiagnosticTest.cpp +++ b/clang/unittests/Basic/DiagnosticTest.cpp @@ -210,14 +210,14 @@ MATCHER_P(WithMessage, Msg, "has diagnostic message") { } TEST_F(SuppressionMappingTest, MissingMappingFile) { - Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt"; clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); EXPECT_THAT(takeDiags(), testing::ElementsAre(WithMessage( "no such file or directory: 'foo.txt'"))); } TEST_F(SuppressionMappingTest, MalformedFile) { - Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt"; FS->addFile("foo.txt", /*ModificationTime=*/{}, llvm::MemoryBuffer::getMemBuffer("asdf", "foo.txt")); clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); @@ -227,7 +227,7 @@ TEST_F(SuppressionMappingTest, MalformedFile) { } TEST_F(SuppressionMappingTest, UnknownDiagName) { - Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt"; FS->addFile("foo.txt", /*ModificationTime=*/{}, llvm::MemoryBuffer::getMemBuffer("[non-existing-warning]")); clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); @@ -241,7 +241,7 @@ TEST_F(SuppressionMappingTest, SuppressesGroup) { [unused] src:* )txt"; - Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt"; FS->addFile("foo.txt", /*ModificationTime=*/{}, llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); @@ -258,7 +258,7 @@ TEST_F(SuppressionMappingTest, ExclusionsWork) { src:* src:*foo.cpp=emit )txt"; - Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt"; FS->addFile("foo.txt", /*ModificationTime=*/{}, llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); @@ -277,7 +277,7 @@ TEST_F(SuppressionMappingTest, LongestMatchWins) { src:*clang/lib/Sema/*=emit src:*clang/lib/Sema/foo* )txt"; - Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt"; FS->addFile("foo.txt", /*ModificationTime=*/{}, llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); clang::ProcessWarningOptions(Diags, Diags.getDiagnosticOptions(), *FS); @@ -296,7 +296,7 @@ TEST_F(SuppressionMappingTest, IsIgnored) { [unused] src:*clang/* )txt"; - Diags.getDiagnosticOptions().SuppressionMappingsFile = "foo.txt"; + Diags.getDiagnosticOptions().DiagnosticSuppressionMappingsFile = "foo.txt"; Diags.getDiagnosticOptions().Warnings = {"unused"}; FS->addFile("foo.txt", /*ModificationTime=*/{}, llvm::MemoryBuffer::getMemBuffer(SuppressionMappingFile)); diff --git a/clang/unittests/Frontend/CompilerInvocationTest.cpp b/clang/unittests/Frontend/CompilerInvocationTest.cpp index 1cf2bbb6576621..45478de5e6f7c8 100644 --- a/clang/unittests/Frontend/CompilerInvocationTest.cpp +++ b/clang/unittests/Frontend/CompilerInvocationTest.cpp @@ -1051,7 +1051,8 @@ TEST_F(CommandLineTest, WarningSuppressionMappings) { const char *Args[] = {"--warning-suppression-mappings=foo.txt"}; EXPECT_TRUE(CompilerInvocation::CreateFromArgs(Invocation, Args, *Diags)); - EXPECT_EQ(Invocation.getDiagnosticOpts().SuppressionMappingsFile, "foo.txt"); + EXPECT_EQ(Invocation.getDiagnosticOpts().DiagnosticSuppressionMappingsFile, + "foo.txt"); Invocation.generateCC1CommandLine(GeneratedArgs, *this); EXPECT_THAT(GeneratedArgs, Contains(StrEq(Args[0]))); From 6c5e0d1414598ac787e000bc47b63081c6d7b7ae Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya <kadir...@google.com> Date: Mon, 28 Oct 2024 12:18:52 +0100 Subject: [PATCH 4/5] Add docs and address comments --- clang/docs/ReleaseNotes.rst | 3 + clang/docs/WarningSuppressionMappings.rst | 92 +++++++++++++++++++ clang/include/clang/Basic/Diagnostic.h | 11 ++- clang/include/clang/Basic/DiagnosticOptions.h | 2 +- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Basic/Diagnostic.cpp | 64 +++++++------ 6 files changed, 140 insertions(+), 34 deletions(-) create mode 100644 clang/docs/WarningSuppressionMappings.rst diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 920a2369f96435..bb872d5c0d270e 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -768,6 +768,9 @@ New features attribute, the compiler can generate warnings about the use of any language features, or calls to other functions, which may block. +- Introduced ``-warning-suppression-mappings`` flag to control diagnostic + suppressions per file. See `documentation <https://clang.llvm.org/docs/WarningSuppressionMappings.html>_` for details. + Crash and bug fixes ^^^^^^^^^^^^^^^^^^^ diff --git a/clang/docs/WarningSuppressionMappings.rst b/clang/docs/WarningSuppressionMappings.rst new file mode 100644 index 00000000000000..0cab48b8972538 --- /dev/null +++ b/clang/docs/WarningSuppressionMappings.rst @@ -0,0 +1,92 @@ +============================ +Warning suppression mappings +============================ + +.. contents:: + :local: + +Introduction +============ + +Warning suppression mappings enables users to suppress clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in dependencies or other parts of +the codebase. + +Goal and usage +============== + +Clang allows diagnostics to be configured at a translation-unit granularity. +If a foo.cpp is compiled with -Wfoo, all transitively included headers also need +to be clean. Hence turning on new warnings at large codebases is quite difficult +today: +- It requires cleaning up all the existing warnings, which might not be possible + when some dependencies aren't in project owner's control. +- Preventing backsliding in the meanwhile as the diagnostic can't be enforced at + all until codebase is cleaned up. + +Warning suppression mappings aims to alleviate some of these concerns by making +diagnostic configuration granularity finer, at a source file level. + +To achieve this, user may create a file listing which diagnostic groups to +suppress in which files, and pass it as a command line argument to clang with +``--warning-suppression-mappings`` flag. + +Note that this mechanism won't enable any diagnostics on its own. Users should +still turn on warnings in their compilations with explicit ``-Wfoo`` flags. + +Example +======= + +.. code-block:: bash + + $ cat my/user/code.cpp + #include <foo/bar.h> + namespace { void unused_func1(); } + + $ cat foo/bar.h + namespace { void unused_func2(); } + + $ cat suppression_mappings.txt + # Suppress -Wunused warnings in all files, apart from the ones under `foo/`. + [unused] + src:* + src:*foo/*=emit + $ clang -Wunused --warning-suppression-mappings=suppression_mappings.txt my/user/code.cpp + # prints warning: unused function 'unused_func2', but no warnings for `unused_func1`. + +Format +====== + +Warning suppression mappings uses a format similar to +:doc:`SanitizerSpecialCaseList`. + +Users can mention sections to describe which diagnostic group behaviours to +change. Sections are denoted as ``[unused]`` in this format. Each section name +must match a diagnostic group. +When a diagnostic is matched by multiple groups, the latest one takes +precendence. + +Afterwards in each section, users can have multiple entities that match source +files based on the globs. These entities look like ``src:*/my/dir/*``. +Users can also use ``emit`` category to exclude a subdirectory from suppression. +Source files are matched against these globs either as paths relative to current +working directory, or as absolute paths. +When a source file matches multiple globs, the longest one takes precendence. + +.. code-block:: bash + + # Lines starting with # are ignored. + # Configure suppression globs for `-Wunused` warnings + [unused] + # Suppress on all files by default. + src:* + # But enforce for all the sources under foo/. + src:*foo/*=emit + + # unused-function warnings are a subgroup of `-Wunused`. So this section + # takes precedence over the previous one for unused-function warnings, but + # not for unused-variable warnings. + [unused-function] + # Only suppress for sources under bar/. + src:*bar/* diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index d3706460a62a0f..ed3a652be97f15 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -953,11 +953,12 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { return (Level)Diags->getDiagnosticLevel(DiagID, Loc, *this); } - /// Diagnostic suppression mappings can be used to ignore diagnostics based on - /// the file they occur in. Mapping file is expected to be a special case list - /// with sections denoting diagnostic groups and `src` entries for globs to - /// suppress. `emit` category can be used to disable suppression. Longest glob - /// that matches a filepath takes precendence. For example: + /// Diagnostic suppression mappings can be used to suppress specific + /// diagnostics in specific files. + /// Mapping file is expected to be a special case list with sections denoting + /// diagnostic groups and `src` entries for globs to suppress. `emit` category + /// can be used to disable suppression. Longest glob that matches a filepath + /// takes precendence. For example: /// [unused] /// src:*clang/* /// src:*clang/foo/*=emit diff --git a/clang/include/clang/Basic/DiagnosticOptions.h b/clang/include/clang/Basic/DiagnosticOptions.h index 53ffb265cdc870..e2faf3d0df95cb 100644 --- a/clang/include/clang/Basic/DiagnosticOptions.h +++ b/clang/include/clang/Basic/DiagnosticOptions.h @@ -108,7 +108,7 @@ class DiagnosticOptions : public RefCountedBase<DiagnosticOptions>{ /// The file to serialize diagnostics to (non-appending). std::string DiagnosticSerializationFile; - /// File that defines diagnostic suppression mappings. + /// Path for the file that defines diagnostic suppression mappings. std::string DiagnosticSuppressionMappingsFile; /// The list of -W... options used to alter the diagnostic mappings, with the diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 381557f05dec4a..28f0c5446a9559 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -9001,5 +9001,5 @@ def wasm_opt : Flag<["--"], "wasm-opt">, def warning_suppression_mappings_EQ : Joined<["--"], "warning-suppression-mappings=">, Group<Diag_Group>, - HelpText<"File containing list of sources to suppress diagnostics for. See XXX for format">, + HelpText<"File containing list of sources to suppress diagnostics for.">, Visibility<[ClangOption, CC1Option]>; diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index beea68c003e2fb..677f2bcf69b43a 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -27,6 +27,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/ConvertUTF.h" #include "llvm/Support/CrashRecoveryContext.h" @@ -491,9 +492,9 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList { static std::unique_ptr<WarningsSpecialCaseList> create(const llvm::MemoryBuffer &MB, std::string &Err) { auto SCL = std::make_unique<WarningsSpecialCaseList>(); - if (SCL->createInternal(&MB, Err)) - return SCL; - return nullptr; + if (!SCL->createInternal(&MB, Err)) + return nullptr; + return SCL; } // Section names refer to diagnostic groups, which cover multiple individual @@ -506,26 +507,28 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList { Sections.erase("*"); // Make sure we iterate sections by their line numbers. std::vector<std::pair<unsigned, const llvm::StringMapEntry<Section> *>> - LineAndSection; + LineAndSectionEntry; + LineAndSectionEntry.reserve(Sections.size()); for (const auto &Entry : Sections) { - LineAndSection.emplace_back( + LineAndSectionEntry.emplace_back( Entry.second.SectionMatcher->Globs.at(Entry.first()).second, &Entry); } - llvm::sort(LineAndSection); + llvm::sort(LineAndSectionEntry); static constexpr auto kFlavor = clang::diag::Flavor::WarningOrError; - for (const auto &[_, Entry] : LineAndSection) { + for (const auto &[_, SectionEntry] : LineAndSectionEntry) { SmallVector<diag::kind, 256> GroupDiags; - if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup( - kFlavor, Entry->first(), GroupDiags)) { + llvm::StringRef DiagGroup = SectionEntry->getKey(); + if (Diags.getDiagnosticIDs()->getDiagnosticsInGroup(kFlavor, DiagGroup, + GroupDiags)) { StringRef Suggestion = - DiagnosticIDs::getNearestOption(kFlavor, Entry->first()); + DiagnosticIDs::getNearestOption(kFlavor, DiagGroup); Diags.Report(diag::warn_unknown_diag_option) - << static_cast<unsigned>(kFlavor) << Entry->first() + << static_cast<unsigned>(kFlavor) << DiagGroup << !Suggestion.empty() << Suggestion; continue; } - for (auto D : GroupDiags) - DiagToSection[D] = &Entry->getValue(); + for (diag::kind D : GroupDiags) + DiagToSection[D] = &SectionEntry->getValue(); } } @@ -533,28 +536,35 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList { auto Section = DiagToSection.find(D); if (Section == DiagToSection.end()) return false; - auto SrcEntries = Section->second->Entries.find("src"); - if (SrcEntries == Section->second->Entries.end()) + auto &DiagEntries = Section->second->Entries; + auto SrcEntries = DiagEntries.find("src"); + if (SrcEntries == DiagEntries.end()) return false; - // Find the longest glob pattern that matches FilePath. A positive match - // implies D should be suppressed for FilePath. + return globsMatches(SrcEntries->second, FilePath); + } + +private: + // Find the longest glob pattern that matches FilePath amongst + // CategoriesToMatchers, return true iff the match exists and belongs to a + // positive category. + bool globsMatches(llvm::StringMap<Matcher> CategoriesToMatchers, + llvm::StringRef FilePath) const { llvm::StringRef LongestMatch; - bool LongestWasNegative; - for (const auto &CatIt : SrcEntries->second) { - bool IsNegative = CatIt.first() == "emit"; - for (const auto &GlobIt : CatIt.second.Globs) { - if (GlobIt.getKeyLength() < LongestMatch.size()) + bool LongestIsPositive = false; + for (const auto &[Category, Matcher] : CategoriesToMatchers) { + bool IsPositive = Category != "emit"; + for (const auto &[Pattern, Glob] : Matcher.Globs) { + if (Pattern.size() < LongestMatch.size()) continue; - if (!GlobIt.second.first.match(FilePath)) + if (!Glob.first.match(FilePath)) continue; - LongestMatch = GlobIt.getKey(); - LongestWasNegative = IsNegative; + LongestMatch = Pattern; + LongestIsPositive = IsPositive; } } - return !LongestMatch.empty() && !LongestWasNegative; + return LongestIsPositive; } -private: llvm::DenseMap<diag::kind, const Section *> DiagToSection; }; } // namespace From 739d178e36a7200a3dd07e12052f6380478dac76 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya <kadir...@google.com> Date: Tue, 29 Oct 2024 15:08:37 +0100 Subject: [PATCH 5/5] update usersmanual && some renaming --- clang/docs/UsersManual.rst | 27 ++++++++++++++++++++++++++ clang/include/clang/Basic/Diagnostic.h | 12 +++++++----- clang/include/clang/Driver/Options.td | 4 ++-- clang/lib/Basic/Diagnostic.cpp | 12 ++++++------ clang/lib/Basic/DiagnosticIDs.cpp | 6 ++++-- clang/lib/Basic/Warnings.cpp | 1 - 6 files changed, 46 insertions(+), 16 deletions(-) diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index 4f03388bc87bd0..d3a79191a96ca5 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -151,6 +151,10 @@ Options to Control Error and Warning Messages instantiation backtrace for a single warning or error. The default is 10, and the limit can be disabled with `-ftemplate-backtrace-limit=0`. +.. option:: --warning-suppression-mappings=foo.txt + + :ref:`Suppress certain diagnostics for certain files. <warning_suppression_mappings>` + .. _cl_diag_formatting: Formatting of Diagnostics @@ -1315,6 +1319,29 @@ with its corresponding `Wno-` option. Note that when combined with :option:`-w` (which disables all warnings), disabling all warnings wins. +.. _warning_suppression_mappings: + +Controlling Diagnostics via Suppression Mappings +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Warning suppression mappings enables users to suppress clang's diagnostics in a +per-file granular manner. Enabling enforcement of diagnostics in specific parts +of the project, even if there are violations in dependencies or other parts of +the codebase. + +.. code-block:: console + + $ cat mappings.txt + [unused] + src:foo/* + + $ clang --warning-suppression-mappings=mapping.txt -Wunused foo/bar.cc + # This compilation won't emit any unused findings for sources under foo/ + # directory. But it'll still complain for all the other sources. + +See :doc:`WarningSuppressionMappings` for details about the file format and +functionality. + Controlling Static Analyzer Diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h index ed3a652be97f15..9584ca8d795d2f 100644 --- a/clang/include/clang/Basic/Diagnostic.h +++ b/clang/include/clang/Basic/Diagnostic.h @@ -960,17 +960,19 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> { /// can be used to disable suppression. Longest glob that matches a filepath /// takes precendence. For example: /// [unused] - /// src:*clang/* - /// src:*clang/foo/*=emit - /// src:*clang/foo/bar/* + /// src:clang/* + /// src:clang/foo/*=emit + /// src:clang/foo/bar/* /// /// Such a mappings file suppress all diagnostics produced by -Wunused in all /// sources under `clang/` directory apart from `clang/foo/`. Diagnostics - /// under `clang/foo/bar/` will also be suppressed. + /// under `clang/foo/bar/` will also be suppressed. Note that the FilePath is + /// matched against the globs as-is. /// These take presumed locations into account, and can still be overriden by /// clang-diagnostics pragmas. void setDiagSuppressionMapping(llvm::MemoryBuffer &MB); - bool isSuppressedViaMapping(diag::kind D, llvm::StringRef FilePath) const; + bool isSuppressedViaMapping(diag::kind DiagId, + llvm::StringRef FilePath) const; /// Issue the message to the client. /// diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 28f0c5446a9559..4c29cc9a4f642b 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -9001,5 +9001,5 @@ def wasm_opt : Flag<["--"], "wasm-opt">, def warning_suppression_mappings_EQ : Joined<["--"], "warning-suppression-mappings=">, Group<Diag_Group>, - HelpText<"File containing list of sources to suppress diagnostics for.">, - Visibility<[ClangOption, CC1Option]>; + HelpText<"File containing diagnostic suppresion mappings. See user manual for" + " file format.">, Visibility<[ClangOption, CC1Option]>; diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp index 677f2bcf69b43a..4a020b006f0aae 100644 --- a/clang/lib/Basic/Diagnostic.cpp +++ b/clang/lib/Basic/Diagnostic.cpp @@ -532,8 +532,8 @@ class WarningsSpecialCaseList : public llvm::SpecialCaseList { } } - bool isDiagSuppressed(diag::kind D, llvm::StringRef FilePath) const { - auto Section = DiagToSection.find(D); + bool isDiagSuppressed(diag::kind DiagId, llvm::StringRef FilePath) const { + auto Section = DiagToSection.find(DiagId); if (Section == DiagToSection.end()) return false; auto &DiagEntries = Section->second->Entries; @@ -578,15 +578,15 @@ void DiagnosticsEngine::setDiagSuppressionMapping(llvm::MemoryBuffer &MB) { return; } SCL->processSections(*this); - DiagSuppressionMapping = [SCL(std::move(SCL))](diag::kind K, + DiagSuppressionMapping = [SCL(std::move(SCL))](diag::kind DiagId, llvm::StringRef Path) { - return SCL->isDiagSuppressed(K, Path); + return SCL->isDiagSuppressed(DiagId, Path); }; } -bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind D, +bool DiagnosticsEngine::isSuppressedViaMapping(diag::kind DiagId, llvm::StringRef FilePath) const { - return DiagSuppressionMapping && DiagSuppressionMapping(D, FilePath); + return DiagSuppressionMapping && DiagSuppressionMapping(DiagId, FilePath); } void DiagnosticsEngine::Report(const StoredDiagnostic &storedDiag) { diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp index 4f213e02fa9324..c27bbbe3257dde 100644 --- a/clang/lib/Basic/DiagnosticIDs.cpp +++ b/clang/lib/Basic/DiagnosticIDs.cpp @@ -17,6 +17,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Support/ErrorHandling.h" +#include "llvm/Support/Path.h" #include <map> #include <optional> using namespace clang; @@ -606,8 +607,9 @@ DiagnosticIDs::getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc, // preprocessed inputs. if (!Mapping.isPragma()) { if (auto PLoc = SM.getPresumedLoc(Loc); - PLoc.isValid() && - Diag.isSuppressedViaMapping(DiagID, PLoc.getFilename())) + PLoc.isValid() && Diag.isSuppressedViaMapping( + DiagID, llvm::sys::path::remove_leading_dotslash( + PLoc.getFilename()))) return diag::Severity::Ignored; } diff --git a/clang/lib/Basic/Warnings.cpp b/clang/lib/Basic/Warnings.cpp index bcad90abf1c21b..7b5d68b9f0eb96 100644 --- a/clang/lib/Basic/Warnings.cpp +++ b/clang/lib/Basic/Warnings.cpp @@ -46,7 +46,6 @@ static void EmitUnknownDiagWarning(DiagnosticsEngine &Diags, << (Prefix.str() += std::string(Suggestion)); } - void clang::ProcessWarningOptions(DiagnosticsEngine &Diags, const DiagnosticOptions &Opts, llvm::vfs::FileSystem &VFS, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
