-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[clang] Introduce diagnostics suppression mappings #112517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-clang Author: kadir çetinkaya (kadircet) ChangesThis implements Users now can suppress warnings for certain headers by providing a
This will suppress warnings from At a high level, mapping file is stored in DiagnosticOptions and then This implies processing warning options now performs IO, relevant Patch is 36.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112517.diff 23 Files Affected:
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 68722ad9633120..cbad93904a899f 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -831,4 +831,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 379e75b197cf96..827126f5af30ca 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8979,3 +8979,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 3fc39296f44281..55ec543c5de514 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4456,6 +4456,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(), Cl...
[truncated]
|
@llvm/pr-subscribers-clang-tools-extra Author: kadir çetinkaya (kadircet) ChangesThis implements Users now can suppress warnings for certain headers by providing a
This will suppress warnings from At a high level, mapping file is stored in DiagnosticOptions and then This implies processing warning options now performs IO, relevant Patch is 36.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112517.diff 23 Files Affected:
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 68722ad9633120..cbad93904a899f 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -831,4 +831,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 379e75b197cf96..827126f5af30ca 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8979,3 +8979,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 3fc39296f44281..55ec543c5de514 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4456,6 +4456,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(), Cl...
[truncated]
|
@llvm/pr-subscribers-clang-modules Author: kadir çetinkaya (kadircet) ChangesThis implements Users now can suppress warnings for certain headers by providing a
This will suppress warnings from At a high level, mapping file is stored in DiagnosticOptions and then This implies processing warning options now performs IO, relevant Patch is 36.96 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112517.diff 23 Files Affected:
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 68722ad9633120..cbad93904a899f 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -831,4 +831,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 379e75b197cf96..827126f5af30ca 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8979,3 +8979,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 3fc39296f44281..55ec543c5de514 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4456,6 +4456,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(), Cl...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have only done a quick pass, but still wanted to leave a few NITs.
gentle ping @AaronBallman, I believe you were supportive of this approach in final version of RFC but I'd like to make sure this agreement was implemented in an accurate manner. So it'd be great if you can check the overall interaction in https://github.com/llvm/llvm-project/blob/61b963131975ca3f6536b38427e25024f8759c57/clang/test/Misc/warning-suppression-mappings.cpp and https://github.com/llvm/llvm-project/blob/61b963131975ca3f6536b38427e25024f8759c57/clang/test/Misc/Inputs/suppression-mapping.txt. If those look good, I am happy to settle on the details of the implementation with other reviewers. |
Add that change to clang/docs/ReleaseNotes.rst? |
thanks for the review! @AaronBallman i'd like to land this early next week and start testing/polishing. It'd be great if you can give some explicit LGTM. |
@AaronBallman merging this to main now. PLMK if you have any concerns, I am happy to address them post-submit (or revert if need be) |
FYI, looks like this is causing following LLDB tests to fail: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/15106/execution/node/97/log/
Example assertion:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AaronBallman merging this to main now. PLMK if you have any concerns, I am happy to address them post-submit (or revert if need be)
Sorry, there was a US holiday on Monday and I was mostly out on Friday, so that's why there was a delay in review. In the future, I would recommend waiting before landing significant changes like these unless the review is accepted by a maintainer or someone known to be familiar with that area of the code. That reduces pressure on reviewers and allows folks to be sick/take vacations/etc without things landing out from under them. As it turns out, I can no longer leave suggested changes on the PR because it was already merged (thanks, GitHub....), so my review comments will be somewhat awkward.
|
||
Warning suppression mappings enable 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 some headers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comma: project even if there
Reword: some headers -> some headers which cannot be modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reword: some headers -> some headers which cannot be modified.
But this covers headers even inside the project (i.e. the incremental cleanup of a new warning). So I feel like which cannot be modified.
adds a non-existing limitation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, but I think we want to guide users towards fixing diagnostics rather than suppressing them. Users will use any suppression mechanisms we provide regardless of how we document it, so I don't think it's harmful to imply this is more about cases where you can't modify the code. But I also don't feel strongly. WDYT?
@@ -9017,3 +9017,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<["--"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should move elsewhere in Options.td, it's currently listed under clang-dxc
options.
Also, should this be exposed in clang-cl as well? What about flang? CC @zmodem @rnk @MaskRay @jansvoboda11 for opinions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should move elsewhere in Options.td, it's currently listed under clang-dxc options.
Oh didn't notice we actually had "lexical" sections in the doc (AFAICT it doesn't have any semantic affect afterwards, flag is still only visible for cc1 and clang-driver). Moved it near other warning options.
Also, should this be exposed in clang-cl as well? What about flang? CC @zmodem @rnk @MaskRay @jansvoboda11 for opinions
Going to carry this discussion into RFC to make sure it doesn't get lost. https://discourse.llvm.org/t/rfc-add-support-for-controlling-diagnostics-severities-at-file-level-granularity-through-command-line/81292/21?u=kadircet
FWIW, I am still trying to reproduce these failures. It seems to be hard on a linux machine as tests are all disabled with a link to https://discourse.llvm.org/t/lldb-test-failures-on-linux/80095. |
It looks like this increases time to build clang by 0.5% (see affected files here: https://llvm-compile-time-tracker.com/compare_clang.php?from=e385e0d3e71e17da0b2023f480259c95923707bd&to=41e3919ded78d8870f7c95e9181c7f7e29aa3cc4&stat=instructions%3Au&sortBy=interestingness) This usually indicates that you are including something expensive in a core header -- it would be good to check whether it can be avoided. |
Though looking at that file list in more details, this also affects files outside clang, so I think it might be adding some compile-time overhead that is only visible on specific workloads. E.g. |
AFAIK only a few of these are skipped on Linux. E.g., |
Hmm this one's a bit strange. Looks like the issue is in:
The assert triggers inside |
so 12e3ed8 should resolve both lldb buildbot breakage and compile time regression in clang. that being said, I don't think 12e3ed8 Basically I think LLDB is doing something similar, it's probably loading/building modules with one SourceManager and then it's trying to consume them using another. @Michael137, If you can provide more detailed instructions about how to set up a similar testing environment in linux (I couldn't get the std-modules bit to work), or if you can provide a detailed stack trace from assertion failure, I can try to dig deeper. |
Awesome thanks for the quick workaround! I agree, LLDB is doing something funky here. Example backtrace for the assertion:
Not sure if that's enough to go on though. Let me know what else could be helpful here.
Yea that sounds entirely plausible. |
Unfortunately it isn't, can you include all the frames? at least some call that originates from LLDB ? |
Thank you, I can confirm that this fixes the compile-time regression (or at least most of it). |
@Michael137 looks like there's a new failure after 12e3ed8. Unfortunately I can't repro this one either, despite being a non-mac machine, the tests are run this time but they all pass :/ Also I do not see how that can be triggered after these patches. I am out of time on my investigation for today, I'll revert the changes to not break head until tomorrow. But it'd be great if you can share any advise around reproducing these buildbots environment on other machines. |
This reverts commit 12e3ed8. This reverts commit 41e3919. There are some buildbot breakages in https://lab.llvm.org/buildbot/#/builders/18/builds/6832.
alright NVM, apparently that breakage was due to #107849. I'll still reland this tomorrow after addressing @AaronBallman's latest comments. |
Thank you for your quick attention to the issue! |
relands: eea8b44 [GVN] Handle empty attrs in Expression == (llvm#115761) 984bca9 [GVN][NewGVN] Take call attributes into account in expressions (llvm#114545) Reverts: breaks comgr build 41e3919 [clang] Introduce diagnostics suppression mappings (llvm#112517) Change-Id: I3189bf1b5d66b980d2d711c3fe1d5fe217699bb1
Includes updates to reflect these LLVM changes: * llvm/llvm-project#112517 * llvm/llvm-project#113331 --------- Co-authored-by: Josh L <[email protected]>
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.
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:
This will suppress warnings from
-Wunused
group in all files thataren't under
clang/
directory. This mapping file can be passed toclang 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.