[clang] Extend diagnose_if to accept more detailed warning information #70976
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
philnik777
added
enhancement
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang-tidy Author: None (philnik777) ChangesThis implements parts of the extension proposed in https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092/7. Specifically, this makes it possible to specify a diagnostic group in an optional third argument. Patch is 51.10 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/70976.diff 25 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index b19a84f5dc21577..a2f78d8f060a1a3 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -177,8 +177,8 @@ DiagnosticBuilder ClangTidyContext::diag(
StringRef CheckName, SourceLocation Loc, StringRef Description,
DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) {
assert(Loc.isValid());
- unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(
- Level, (Description + " [" + CheckName + "]").str());
+ unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(Level,
+ (Description + " [" + CheckName + "]").str());
CheckNamesByDiagnosticID.try_emplace(ID, CheckName);
return DiagEngine->Report(Loc, ID);
}
@@ -186,8 +186,8 @@ DiagnosticBuilder ClangTidyContext::diag(
DiagnosticBuilder ClangTidyContext::diag(
StringRef CheckName, StringRef Description,
DiagnosticIDs::Level Level /* = DiagnosticIDs::Warning*/) {
- unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(
- Level, (Description + " [" + CheckName + "]").str());
+ unsigned ID = DiagEngine->getDiagnosticIDs()->getCustomDiagID(Level,
+ (Description + " [" + CheckName + "]").str());
CheckNamesByDiagnosticID.try_emplace(ID, CheckName);
return DiagEngine->Report(ID);
}
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 9280eb1e1f218df..c7694ad05f03e5c 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -79,6 +79,10 @@ class ClangTidyContext {
this->DiagEngine = DiagEngine;
}
+ const DiagnosticsEngine* getDiagnosticsEngine() const {
+ return DiagEngine;
+ }
+
~ClangTidyContext();
/// Report any errors detected using this method.
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp
index 704e61b1e4dd792..0962fd971342fdb 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -579,7 +579,9 @@ std::vector<Diag> StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) {
for (auto &Diag : Output) {
if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
// Warnings controlled by -Wfoo are better recognized by that name.
- StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID);
+ StringRef Warning = Tidy->getDiagnosticsEngine()
+ ->getDiagnosticIDs()
+ ->getWarningOptionForDiag(Diag.ID);
if (!Warning.empty()) {
Diag.Name = ("-W" + Warning).str();
} else {
@@ -909,7 +911,7 @@ bool isBuiltinDiagnosticSuppressed(unsigned ID,
if (Suppress.contains(normalizeSuppressedCode(CodePtr)))
return true;
}
- StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(ID);
+ StringRef Warning = DiagnosticIDs{}.getWarningOptionForDiag(ID);
if (!Warning.empty() && Suppress.contains(Warning))
return true;
return false;
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index edd0f77b1031ef0..57d21fa27117933 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -340,7 +340,7 @@ void applyWarningOptions(llvm::ArrayRef<std::string> ExtraArgs,
if (Enable) {
if (Diags.getDiagnosticLevel(ID, SourceLocation()) <
DiagnosticsEngine::Warning) {
- auto Group = DiagnosticIDs::getGroupForDiag(ID);
+ auto Group = Diags.getDiagnosticIDs()->getGroupForDiag(ID);
if (!Group || !EnabledGroups(*Group))
continue;
Diags.setSeverity(ID, diag::Severity::Warning, SourceLocation());
diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td
index 25231c5b82b907c..e08b7720508d40e 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2959,18 +2959,15 @@ def DiagnoseIf : InheritableAttr {
let Spellings = [GNU<"diagnose_if">];
let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
let Args = [ExprArgument<"Cond">, StringArgument<"Message">,
- EnumArgument<"DiagnosticType",
- "DiagnosticType",
- ["error", "warning"],
- ["DT_Error", "DT_Warning"]>,
+ EnumArgument<"DefaultSeverity",
+ "DefaultSeverity",
+ ["error", "warning"],
+ ["DS_error", "DS_warning"]>,
+ StringArgument<"WarningGroup", /*optional*/ 1>,
BoolArgument<"ArgDependent", 0, /*fake*/ 1>,
DeclArgument<Named, "Parent", 0, /*fake*/ 1>];
let InheritEvenIfAlreadyPresent = 1;
let LateParsed = 1;
- let AdditionalMembers = [{
- bool isError() const { return diagnosticType == DT_Error; }
- bool isWarning() const { return diagnosticType == DT_Warning; }
- }];
let TemplateDependent = 1;
let Documentation = [DiagnoseIfDocs];
}
diff --git a/clang/include/clang/Basic/Diagnostic.h b/clang/include/clang/Basic/Diagnostic.h
index 3df037b793b3946..168e403a798a9e7 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -331,10 +331,12 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
// Map extensions to warnings or errors?
diag::Severity ExtBehavior = diag::Severity::Ignored;
- DiagState()
+ DiagnosticIDs& DiagIDs;
+
+ DiagState(DiagnosticIDs &DiagIDs)
: IgnoreAllWarnings(false), EnableAllWarnings(false),
WarningsAsErrors(false), ErrorsAsFatal(false),
- SuppressSystemWarnings(false) {}
+ SuppressSystemWarnings(false), DiagIDs(DiagIDs) {}
using iterator = llvm::DenseMap<unsigned, DiagnosticMapping>::iterator;
using const_iterator =
@@ -865,7 +867,8 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
/// \param FormatString A fixed diagnostic format string that will be hashed
/// and mapped to a unique DiagID.
template <unsigned N>
- unsigned getCustomDiagID(Level L, const char (&FormatString)[N]) {
+ [[deprecated("Use a CustomDiagDesc instead of a Level")]] unsigned
+ getCustomDiagID(Level L, const char (&FormatString)[N]) {
return Diags->getCustomDiagID((DiagnosticIDs::Level)L,
StringRef(FormatString, N - 1));
}
diff --git a/clang/include/clang/Basic/DiagnosticCategories.h b/clang/include/clang/Basic/DiagnosticCategories.h
index 14be326f7515f8f..e9a1fe87202d828 100644
--- a/clang/include/clang/Basic/DiagnosticCategories.h
+++ b/clang/include/clang/Basic/DiagnosticCategories.h
@@ -26,6 +26,7 @@ namespace clang {
#include "clang/Basic/DiagnosticGroups.inc"
#undef CATEGORY
#undef DIAG_ENTRY
+ NUM_GROUPS
};
} // end namespace diag
} // end namespace clang
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h
index 06ef1c6904c31d1..b8aedc732fc3cd9 100644
--- a/clang/include/clang/Basic/DiagnosticIDs.h
+++ b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -14,6 +14,7 @@
#ifndef LLVM_CLANG_BASIC_DIAGNOSTICIDS_H
#define LLVM_CLANG_BASIC_DIAGNOSTICIDS_H
+#include "clang/Basic/DiagnosticCategories.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/ADT/StringRef.h"
@@ -80,7 +81,7 @@ namespace clang {
/// to either Ignore (nothing), Remark (emit a remark), Warning
/// (emit a warning) or Error (emit as an error). It allows clients to
/// map ERRORs to Error or Fatal (stop emitting diagnostics after this one).
- enum class Severity {
+ enum class Severity : uint8_t {
// NOTE: 0 means "uncomputed".
Ignored = 1, ///< Do not present this diagnostic, ignore it.
Remark = 2, ///< Present this diagnostic as a remark.
@@ -171,13 +172,61 @@ class DiagnosticMapping {
class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
public:
/// The level of the diagnostic, after it has been through mapping.
- enum Level {
+ enum Level : uint8_t {
Ignored, Note, Remark, Warning, Error, Fatal
};
+ // Diagnostic classes.
+ enum Class {
+ CLASS_NOTE = 0x01,
+ CLASS_REMARK = 0x02,
+ CLASS_WARNING = 0x03,
+ CLASS_EXTENSION = 0x04,
+ CLASS_ERROR = 0x05
+ };
+
+ struct CustomDiagDesc {
+ diag::Severity DefaultSeverity : 3 = diag::Severity::Warning;
+ unsigned Class : 3 = CLASS_WARNING;
+ unsigned ShowInSystemHeader : 1 = false;
+ unsigned ShowInSystemMacro : 1 = false;
+ unsigned HasGroup : 1 = false;
+ diag::Group Group = {};
+ std::string Description;
+
+ friend bool operator==(const CustomDiagDesc &lhs, const CustomDiagDesc &rhs) {
+ return lhs.DefaultSeverity == rhs.DefaultSeverity &&
+ lhs.Class == rhs.Class &&
+ lhs.ShowInSystemHeader == rhs.ShowInSystemHeader &&
+ lhs.ShowInSystemMacro == rhs.ShowInSystemMacro &&
+ lhs.HasGroup == rhs.HasGroup &&
+ (!lhs.HasGroup || lhs.Group == rhs.Group) &&
+ lhs.Description == rhs.Description;
+ }
+
+ friend bool operator<(const CustomDiagDesc& lhs, const CustomDiagDesc& rhs) {
+ if (lhs.DefaultSeverity != rhs.DefaultSeverity)
+ return lhs.DefaultSeverity < rhs.DefaultSeverity;
+ if (lhs.Class != rhs.Class)
+ return lhs.Class < rhs.Class;
+ if (lhs.ShowInSystemHeader != rhs.ShowInSystemHeader)
+ return lhs.ShowInSystemHeader < rhs.ShowInSystemHeader;
+ if (lhs.ShowInSystemMacro != rhs.ShowInSystemMacro)
+ return lhs.ShowInSystemMacro < rhs.ShowInSystemMacro;
+ if (lhs.HasGroup != rhs.HasGroup)
+ return lhs.HasGroup < rhs.HasGroup;
+ if (lhs.HasGroup && lhs.Group != rhs.Group)
+ return lhs.Group < rhs.Group;
+ return lhs.Description < rhs.Description;
+ }
+ };
+
private:
/// Information for uniquing and looking up custom diags.
std::unique_ptr<diag::CustomDiagInfo> CustomDiagInfo;
+ std::unique_ptr<diag::Severity[]> GroupSeverity =
+ std::make_unique<diag::Severity[]>(
+ static_cast<size_t>(diag::Group::NUM_GROUPS));
public:
DiagnosticIDs();
@@ -192,7 +241,37 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
// FIXME: Replace this function with a create-only facilty like
// createCustomDiagIDFromFormatString() to enforce safe usage. At the time of
// writing, nearly all callers of this function were invalid.
- unsigned getCustomDiagID(Level L, StringRef FormatString);
+ unsigned getCustomDiagID(CustomDiagDesc Diag);
+
+ [[deprecated("Use a CustomDiagDesc instead of a Level")]] unsigned
+ getCustomDiagID(Level Level, StringRef Message) {
+ return getCustomDiagID([&] -> CustomDiagDesc {
+ switch (Level) {
+ case DiagnosticIDs::Level::Ignored:
+ return {.DefaultSeverity = diag::Severity::Ignored,
+ .Description = std::string(Message)};
+ case DiagnosticIDs::Level::Note:
+ return {.DefaultSeverity = diag::Severity::Fatal,
+ .Class = CLASS_NOTE,
+ .Description = std::string(Message)};
+ case DiagnosticIDs::Level::Remark:
+ return {.DefaultSeverity = diag::Severity::Remark,
+ .Description = std::string(Message)};
+ case DiagnosticIDs::Level::Warning:
+ return {.DefaultSeverity = diag::Severity::Warning,
+ .Class = CLASS_WARNING,
+ .Description = std::string(Message)};
+ case DiagnosticIDs::Level::Error:
+ return {.DefaultSeverity = diag::Severity::Error,
+ .Class = CLASS_ERROR,
+ .Description = std::string(Message)};
+ case DiagnosticIDs::Level::Fatal:
+ return {.DefaultSeverity=diag::Severity::Fatal,
+ .Class = CLASS_ERROR,
+ .Description = std::string(Message)};
+ }
+ }());
+ }
//===--------------------------------------------------------------------===//
// Diagnostic classification and reporting interfaces.
@@ -204,35 +283,34 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
/// Return true if the unmapped diagnostic levelof the specified
/// diagnostic ID is a Warning or Extension.
///
- /// This only works on builtin diagnostics, not custom ones, and is not
- /// legal to call on NOTEs.
- static bool isBuiltinWarningOrExtension(unsigned DiagID);
+ /// This is not legal to call on NOTEs.
+ bool isWarningOrExtension(unsigned DiagID) const;
/// Return true if the specified diagnostic is mapped to errors by
/// default.
- static bool isDefaultMappingAsError(unsigned DiagID);
+ bool isDefaultMappingAsError(unsigned DiagID) const;
/// Get the default mapping for this diagnostic.
- static DiagnosticMapping getDefaultMapping(unsigned DiagID);
+ DiagnosticMapping getDefaultMapping(unsigned DiagID) const;
- /// Determine whether the given built-in diagnostic ID is a Note.
- static bool isBuiltinNote(unsigned DiagID);
+ /// Determine whether the given diagnostic ID is a Note.
+ bool isNote(unsigned DiagID) const;
- /// Determine whether the given built-in diagnostic ID is for an
+ /// Determine whether the given diagnostic ID is for an
/// extension of some sort.
- static bool isBuiltinExtensionDiag(unsigned DiagID) {
+ bool isExtensionDiag(unsigned DiagID) const {
bool ignored;
- return isBuiltinExtensionDiag(DiagID, ignored);
+ return isExtensionDiag(DiagID, ignored);
}
- /// Determine whether the given built-in diagnostic ID is for an
+ /// Determine whether the given diagnostic ID is for an
/// extension of some sort, and whether it is enabled by default.
///
/// This also returns EnabledByDefault, which is set to indicate whether the
/// diagnostic is ignored by default (in which case -pedantic enables it) or
/// treated as a warning/error by default.
///
- static bool isBuiltinExtensionDiag(unsigned DiagID, bool &EnabledByDefault);
+ bool isExtensionDiag(unsigned DiagID, bool &EnabledByDefault) const;
/// Given a group ID, returns the flag that toggles the group.
/// For example, for Group::DeprecatedDeclarations, returns
@@ -242,19 +320,21 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
/// Given a diagnostic group ID, return its documentation.
static StringRef getWarningOptionDocumentation(diag::Group GroupID);
+ void setGroupSeverity(StringRef Group, diag::Severity);
+
/// Given a group ID, returns the flag that toggles the group.
/// For example, for "deprecated-declarations", returns
/// Group::DeprecatedDeclarations.
static std::optional<diag::Group> getGroupForWarningOption(StringRef);
/// Return the lowest-level group that contains the specified diagnostic.
- static std::optional<diag::Group> getGroupForDiag(unsigned DiagID);
+ std::optional<diag::Group> getGroupForDiag(unsigned DiagID) const;
/// Return the lowest-level warning option that enables the specified
/// diagnostic.
///
/// If there is no -Wfoo flag that controls the diagnostic, this returns null.
- static StringRef getWarningOptionForDiag(unsigned DiagID);
+ StringRef getWarningOptionForDiag(unsigned DiagID);
/// Return the category number that a specified \p DiagID belongs to,
/// or 0 if no category.
@@ -352,6 +432,8 @@ class DiagnosticIDs : public RefCountedBase<DiagnosticIDs> {
getDiagnosticSeverity(unsigned DiagID, SourceLocation Loc,
const DiagnosticsEngine &Diag) const LLVM_READONLY;
+ Class getDiagClass(unsigned DiagID) const;
+
/// Used to report a diagnostic that is finally fully formed.
///
/// \returns \c true if the diagnostic was emitted, \c false if it was
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 453bd8a9a340425..f8d00ff802f1ecd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2865,9 +2865,15 @@ def ext_constexpr_function_never_constant_expr : ExtWarn<
"constant expression">, InGroup<DiagGroup<"invalid-constexpr">>, DefaultError;
def err_attr_cond_never_constant_expr : Error<
"%0 attribute expression never produces a constant expression">;
+def err_diagnose_if_unknown_warning : Error<"unknown warning group">;
def err_diagnose_if_invalid_diagnostic_type : Error<
"invalid diagnostic type for 'diagnose_if'; use \"error\" or \"warning\" "
"instead">;
+def err_diagnose_if_unknown_option : Error<"unknown diagnostic option">;
+def err_diagnose_if_expected_equals : Error<
+ "expected '=' after diagnostic option">;
+def err_diagnose_if_unexpected_value : Error<
+ "unexpected value; use 'true' or 'false'">;
def err_constexpr_body_no_return : Error<
"no return statement in %select{constexpr|consteval}0 function">;
def err_constexpr_return_missing_expr : Error<
diff --git a/clang/lib/Basic/Diagnostic.cpp b/clang/lib/Basic/Diagnostic.cpp
index 0208ccc31bd7fc0..f67cf9c507fdc8d 100644
--- a/clang/lib/Basic/Diagnostic.cpp
+++ b/clang/lib/Basic/Diagnostic.cpp
@@ -138,7 +138,7 @@ void DiagnosticsEngine::Reset(bool soft /*=false*/) {
// Create a DiagState and DiagStatePoint representing diagnostic changes
// through command-line.
- DiagStates.emplace_back();
+ DiagStates.emplace_back(*Diags);
DiagStatesByLoc.appendFirst(&DiagStates.back());
}
}
@@ -167,7 +167,7 @@ DiagnosticsEngine::DiagState::getOrAddMapping(diag::kind Diag) {
// Initialize the entry if we added it.
if (Result.second)
- Result.first->second = DiagnosticIDs::getDefaultMapping(Diag);
+ Result.first->second = DiagIDs.getDefaultMapping(Diag);
return Result.first->second;
}
@@ -309,7 +309,8 @@ void DiagnosticsEngine::DiagStateMap::dump(SourceManager &SrcMgr,
for (auto &Mapping : *Transition.State) {
StringRef Option =
- DiagnosticIDs::getWarningOptionForDiag(Mapping.first);
+ SrcMgr.getDiagnostics().Diags->getWarningOptionForDiag(
+ Mapping.first);
if (!DiagName.empty() && DiagName != Option)
continue;
@@ -353,9 +354,7 @@ void DiagnosticsEngine::PushDiagStatePoint(DiagState *State,
void DiagnosticsEngine::setSeverity(diag::kind Diag, diag::Severity Map,
SourceLocation L) {
- assert(Diag < diag::DIAG_UPPER_LIMIT &&
- "Can only map builtin diagnostics");
- assert((Diags->isBuiltinWarningOrExtension(Diag) ||
+ assert((Diags->isWarningOrExtension(Diag) ||
(Map == diag::Severity::Fatal || Map == diag::Severity::Error)) &&
"Cannot map errors into warnings!");
assert((L.isInvalid() || SourceMgr) && "No SourceMgr for valid location");
@@ -406,6 +405,8 @@ bool DiagnosticsEngine::setSeverityForGroup(diag::Flavor Flavor,
if (Diags->getDiagnosticsInGroup(Flavor, Group, GroupDiags))
return true;
+ Diags->setGroupSeverity(Group, Map);
+
// Set the mapping.
for (diag::kind Diag : GroupDiags)
setSeverity(Diag, Map, Loc);
@@ -491,7 +492,7 @@ void DiagnosticsEngine::setSeverityForAll(diag::Flavor Flavor,
// Set the mapping.
for (diag::kind Diag : AllDiags)
- if (Diags->isBuiltinWarningOrExtension(Diag))
+ if (Diags->isWarningOrExtension(Diag))
setSeverity(Diag, Map, Loc);
}
diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index e5667d57f8cff11..6b2ba485533f904 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -99,13 +99,12 @@ const uint32_t StaticDiagInfoDescriptionOffsets[] = {
#undef DIAG
};
-// Diagnostic classes.
enum {
- CLASS_NOTE = 0x01,
- CLASS_REMARK = 0x02,
- CLASS_WARNING = 0x03,
- CLASS_EXTENSION = 0x04,
- CLASS_ERROR = 0x05
+ CLASS_NOTE = DiagnosticIDs::CLASS_NOTE,
+ CLASS_REMARK = DiagnosticIDs::CLASS_REMARK,
+ CLASS_WARNING = DiagnosticIDs::CLASS_WARNING,
+ CLASS_EXTENSION = DiagnosticIDs::CLASS_EXTENSION,
+ CLASS_ERROR = DiagnosticIDs::CLASS_ERROR,
};
struct S...
[truncated]
|
|
This is missing a few tests and probably has some bugs, but I'd like to know whether this is a good approach before putting a lot of work into it. |
You can test this locally with the following command:git-clang-format --diff b43302372f592fd48a22d32b2603f8efee40a88e 22509b9398a02f1f530232e6e7c1c09b3fd5111b --extensions cpp,c,h -- clang/test/SemaCXX/diagnose_if-warning-group.cpp clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clangd/Diagnostics.h clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/Preamble.cpp clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp clang/include/clang/Basic/Diagnostic.h clang/include/clang/Basic/DiagnosticCategories.h clang/include/clang/Basic/DiagnosticIDs.h clang/lib/Basic/Diagnostic.cpp clang/lib/Basic/DiagnosticIDs.cpp clang/lib/Frontend/LogDiagnosticPrinter.cpp clang/lib/Frontend/SerializedDiagnosticPrinter.cpp clang/lib/Frontend/TextDiagnosticPrinter.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaCUDA.cpp clang/lib/Sema/SemaDeclAttr.cpp clang/lib/Sema/SemaOverload.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTWriter.cpp clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp clang/test/Sema/diagnose_if.c clang/tools/diagtool/ListWarnings.cpp clang/tools/diagtool/ShowEnabledWarnings.cpp clang/tools/libclang/CXStoredDiagnostic.cppView the diff from clang-format here.diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
index ad59afb1f4..cae6642bd4 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -683,7 +683,7 @@ std::vector<std::string> DiagnosticIDs::getDiagnosticFlags() {
static bool getDiagnosticsInGroup(diag::Flavor Flavor,
const WarningOption *Group,
SmallVectorImpl<diag::kind> &Diags,
- diag::CustomDiagInfo* CustomDiagInfo) {
+ diag::CustomDiagInfo *CustomDiagInfo) {
// An empty group is considered to be a warning group: we have empty groups
// for GCC compatibility, and GCC does not have remarks.
if (!Group->Members && !Group->SubGroups)
diff --git a/clang/lib/Frontend/SerializedDiagnosticPrinter.cpp b/clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
index c741fd7ecf..d1db31763e 100644
--- a/clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
+++ b/clang/lib/Frontend/SerializedDiagnosticPrinter.cpp
@@ -536,7 +536,7 @@ unsigned SDiagsWriter::getEmitCategory(unsigned int category) {
}
unsigned SDiagsWriter::getEmitDiagnosticFlag(DiagnosticsEngine::Level DiagLevel,
- const Diagnostic* Diag) {
+ const Diagnostic *Diag) {
if (!Diag || DiagLevel == DiagnosticsEngine::Note)
return 0; // No flag for notes.
|
|
gentle ping~ |
erichkeane
reviewed
Nov 28, 2023
philnik777
force-pushed
the
extend_diagnose_if
branch
from
e0389b1 to
a91f499
Compare
|
Please don't force push... it makes these 100x more difficult to review. ALso, it seems that the latest push has broken quite a lot, according to the build bots. |
…e with ineligebile defaulted overloads" (llvm#97002) This reverts commit 567b2c6.
There was a problem hiding this comment.
Sorry for the delay in review!
This seems like the right direction to go, in general. As @erichkeane, it's a bit hard to review due to the whitespace-only changes and changes to __is_trivially_equality_comparable, you should remove those changes or split them into a separate PR.
philnik777
commented
Jul 10, 2024
There was a problem hiding this comment.
Sorry for the delay in review!
No worries! I dropped this for a few months myself. I can't expect to get a review within days when doing that :D
This seems like the right direction to go, in general. As @erichkeane, it's a bit hard to review due to the whitespace-only changes and changes to
__is_trivially_equality_comparable, you should remove those changes or split them into a separate PR.
Oops, looks like a commit got in here that was supposed to be part of another PR.
…ning true with ineligebile defaulted overloads" (llvm#97002)" This reverts commit 43b1972.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/89/builds/6211 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/6429 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/7578 Here is the relevant piece of the build log for the reference |
kazutakahirata
added a commit
that referenced
this pull request
Sep 12, 2024
…formation (#70976)" This reverts commit 030c6da. Several build bots are failing: https://lab.llvm.org/buildbot/#/builders/89/builds/6211 https://lab.llvm.org/buildbot/#/builders/157/builds/7578 https://lab.llvm.org/buildbot/#/builders/140/builds/6429
|
The build bots seem to be complaining about the same spot: Maybe you need to update the call to |
philnik777
added a commit
to philnik777/llvm-project
that referenced
this pull request
Sep 12, 2024
…nformation (llvm#70976)" This reverts commit e0cd11e. Fix flang
fmayer
added a commit
that referenced
this pull request
Sep 13, 2024
…arning information (#70976)" (#108453)" This reverts commit e7f782e. This had UBSan failures: [----------] 1 test from ConfigCompileTests [ RUN ] ConfigCompileTests.DiagnosticSuppression Config fragment: compiling <unknown>:0 -> 0x00007B8366E2F7D8 (trusted=false) /usr/local/google/home/fmayer/large/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:203:33: runtime error: reference binding to null pointer of type 'clang::DiagnosticIDs' UndefinedBehaviorSanitizer: undefined-behavior /usr/local/google/home/fmayer/large/llvm-project/llvm/include/llvm/ADT/IntrusiveRefCntPtr.h:203:33 Pull Request: #108645
philnik777
added a commit
to philnik777/llvm-project
that referenced
this pull request
Sep 14, 2024
…warning information (llvm#70976)" (llvm#108453)" This reverts commit e1bd974. Fixes incorrect use of the `DiagnosticsEngine` in the clangd tests.
|
Hello, I noticed that before this patch Is this on purpose? (I originally asked this in |
|
I think there are multiple bugs here, at least one seems to be preexisting. It seems that perhaps diagnostics issued early enough in the driver do not seem to honor diagnostic groups: https://godbolt.org/z/sh4e9noe1 Note how it's missing However, note how we're missing that diagnostic entirely now: https://godbolt.org/z/znr63dj6o |
We are seeing similar changes in MC/DC-related warnings. Steps to reproduce: cat > test.c << 'EOF'
int foo(int x) { return x; }
int main(void) {
int a, b, c, d, e, f, g, h;
a && b && c && d && e && f && g;
a && foo( b && c );
return 0;
}
EOF
clang -Xclang -fmcdc-max-conditions=6 -Werror -Wno-unused-value -fprofile-instr-generate -fcoverage-mapping -fcoverage-mcdc test.c -o testBefore After |
kadircet
reviewed
Sep 25, 2024
| @@ -489,13 +485,7 @@ static DiagnosticIDs::Level toLevel(diag::Severity SV) { | |||
| DiagnosticIDs::Level | |||
| DiagnosticIDs::getDiagnosticLevel(unsigned DiagID, SourceLocation Loc, | |||
| const DiagnosticsEngine &Diag) const { | |||
| // Handle custom diagnostics, which cannot be mapped. | |||
There was a problem hiding this comment.
so despite this change making sense at a high level, this is a big behavior change that isn't really obvious in context of this PR.
before this change, clang didn't propagate -Werror like mappings into custom diagnostics. Hence some compilations that succeed now with custom warning (clang-plugins etc.), will start failing after this change as those will now turn into errors.
We're already seeing this internally and both #70976 (comment) and #70976 (comment) are mentioning this as well.
Since this is triggering a backward incompatible behavior, can we have this bit reverted and decide how/whether we're planning to move forward with this?
There was a problem hiding this comment.
Since this is triggering a backward incompatible behavior, can we have this bit reverted and decide how/whether we're planning to move forward with this?
Afaict the author ( @philnik777 ) has neither responded to the post-commit comments/question here, neither in the PR related to the reapply at #108453 (comment) , and I don't know much about this to understand if there is an easy fix. But if we are suspecting "multiple bugs here" then maybe someone should just go ahead and revert this again.
Btw, it seems like the reapply already has been reverted once (e1bd974) and then reapplied a second time (e392056). And there are comments here e392056#commitcomment-146720911 about further fixups that might cause conflicts if reverting again.
PS. The commit message in e392056 is not very informative as it says that the commit msg body says that it is a revert, while it actually is a re-apply. Maybe that is another reason to revert this mess and reapply it using a new PR with an appropriate commit message describing the feature.
There was a problem hiding this comment.
I agree, let's revert if this is causing problems.
There was a problem hiding this comment.
ok, putting together a revert for e392056
|
Thanks @kadircet |
augusto2112
pushed a commit
to augusto2112/llvm-project
that referenced
this pull request
Sep 26, 2024
…formation (llvm#70976)" This reverts commit e392056. There are further discussions in llvm#70976, happening for past two weeks. Since there were no responses for couple weeks now, reverting until author is back.
Sterling-Augustine
pushed a commit
to Sterling-Augustine/llvm-project
that referenced
this pull request
Sep 27, 2024
…formation (llvm#70976)" This reverts commit e392056. There are further discussions in llvm#70976, happening for past two weeks. Since there were no responses for couple weeks now, reverting until author is back.
xgupta
pushed a commit
to xgupta/llvm-project
that referenced
this pull request
Oct 4, 2024
…formation (llvm#70976)" This reverts commit e392056. There are further discussions in llvm#70976, happening for past two weeks. Since there were no responses for couple weeks now, reverting until author is back.
philnik777
added a commit
to philnik777/llvm-project
that referenced
this pull request
Dec 12, 2024
…n, take 2 This is take two of llvm#70976. This iteration of the patch makes sure that custom diagnostics without any warning group don't get promoted by `-Werror` or `-Wfatal-errors`. This implements parts of the extension proposed in https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092/7. Specifically, this makes it possible to specify a diagnostic group in an optional third argument.
philnik777
added a commit
to philnik777/llvm-project
that referenced
this pull request
Jan 24, 2025
…n, take 2 This is take two of llvm#70976. This iteration of the patch makes sure that custom diagnostics without any warning group don't get promoted by `-Werror` or `-Wfatal-errors`. This implements parts of the extension proposed in https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092/7. Specifically, this makes it possible to specify a diagnostic group in an optional third argument.
philnik777
added a commit
to philnik777/llvm-project
that referenced
this pull request
Jan 28, 2025
…n, take 2 This is take two of llvm#70976. This iteration of the patch makes sure that custom diagnostics without any warning group don't get promoted by `-Werror` or `-Wfatal-errors`. This implements parts of the extension proposed in https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092/7. Specifically, this makes it possible to specify a diagnostic group in an optional third argument.
philnik777
added a commit
that referenced
this pull request
Jan 28, 2025
…n, take 2 (#119712) This is take two of #70976. This iteration of the patch makes sure that custom diagnostics without any warning group don't get promoted by `-Werror` or `-Wfatal-errors`. This implements parts of the extension proposed in https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092/7. Specifically, this makes it possible to specify a diagnostic group in an optional third argument.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This implements parts of the extension proposed in https://discourse.llvm.org/t/exposing-the-diagnostic-engine-to-c/73092/7.
Specifically, this makes it possible to specify a diagnostic group in an optional third argument.