Skip to content

Commit

Permalink
[clang] Extend diagnose_if to accept more detailed warning information (
Browse files Browse the repository at this point in the history
llvm#70976)

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.
  • Loading branch information
philnik777 committed Sep 12, 2024
1 parent 2670565 commit 030c6da
Show file tree
Hide file tree
Showing 28 changed files with 544 additions and 223 deletions.
27 changes: 20 additions & 7 deletions clang-tools-extra/clangd/Diagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,17 @@ 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);
const StringRef Warning = [&] {
if (OrigSrcMgr) {
return OrigSrcMgr->getDiagnostics()
.getDiagnosticIDs()
->getWarningOptionForDiag(Diag.ID);
}
if (!DiagnosticIDs::IsCustomDiag(Diag.ID))
return DiagnosticIDs{}.getWarningOptionForDiag(Diag.ID);
return StringRef{};
}();

if (!Warning.empty()) {
Diag.Name = ("-W" + Warning).str();
} else {
Expand Down Expand Up @@ -896,20 +906,23 @@ void StoreDiags::flushLastDiag() {
Output.push_back(std::move(*LastDiag));
}

bool isBuiltinDiagnosticSuppressed(unsigned ID,
const llvm::StringSet<> &Suppress,
const LangOptions &LangOpts) {
bool isDiagnosticSuppressed(const clang::Diagnostic &Diag,
const llvm::StringSet<> &Suppress,
const LangOptions &LangOpts) {
// Don't complain about header-only stuff in mainfiles if it's a header.
// FIXME: would be cleaner to suppress in clang, once we decide whether the
// behavior should be to silently-ignore or respect the pragma.
if (ID == diag::pp_pragma_sysheader_in_main_file && LangOpts.IsHeaderFile)
if (Diag.getID() == diag::pp_pragma_sysheader_in_main_file &&
LangOpts.IsHeaderFile)
return true;

if (const char *CodePtr = getDiagnosticCode(ID)) {
if (const char *CodePtr = getDiagnosticCode(Diag.getID())) {
if (Suppress.contains(normalizeSuppressedCode(CodePtr)))
return true;
}
StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(ID);
StringRef Warning =
Diag.getDiags()->getDiagnosticIDs()->getWarningOptionForDiag(
Diag.getID());
if (!Warning.empty() && Suppress.contains(Warning))
return true;
return false;
Expand Down
8 changes: 4 additions & 4 deletions clang-tools-extra/clangd/Diagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,11 @@ class StoreDiags : public DiagnosticConsumer {
};

/// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
bool isBuiltinDiagnosticSuppressed(unsigned ID,
const llvm::StringSet<> &Suppressed,
const LangOptions &);
bool isDiagnosticSuppressed(const clang::Diagnostic &Diag,
const llvm::StringSet<> &Suppressed,
const LangOptions &);
/// Take a user-specified diagnostic code, and convert it to a normalized form
/// stored in the config and consumed by isBuiltinDiagnosticsSuppressed.
/// stored in the config and consumed by isDiagnosticsSuppressed.
///
/// (This strips err_ and -W prefix so we can match with or without them.)
llvm::StringRef normalizeSuppressedCode(llvm::StringRef);
Expand Down
6 changes: 3 additions & 3 deletions clang-tools-extra/clangd/ParsedAST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -583,8 +583,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
if (Cfg.Diagnostics.SuppressAll ||
isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
Clang->getLangOpts()))
isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress,
Clang->getLangOpts()))
return DiagnosticsEngine::Ignored;

auto It = OverriddenSeverity.find(Info.getID());
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/Preamble.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
PreambleDiagnostics.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
if (Cfg.Diagnostics.SuppressAll ||
isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
CI.getLangOpts()))
isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress,
CI.getLangOpts()))
return DiagnosticsEngine::Ignored;
switch (Info.getID()) {
case diag::warn_no_newline_eof:
Expand Down
47 changes: 34 additions & 13 deletions clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,20 +298,41 @@ TEST_F(ConfigCompileTests, DiagnosticSuppression) {
"unreachable-code", "unused-variable",
"typecheck_bool_condition",
"unexpected_friend", "warn_alloca"));
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
diag::warn_unreachable, Conf.Diagnostics.Suppress, LangOptions()));
clang::DiagnosticsEngine DiagEngine(nullptr, nullptr,
new clang::IgnoringDiagConsumer);

using Diag = clang::Diagnostic;
{
auto D = DiagEngine.Report(diag::warn_unreachable);
EXPECT_TRUE(isDiagnosticSuppressed(
Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
}
// Subcategory not respected/suppressed.
EXPECT_FALSE(isBuiltinDiagnosticSuppressed(
diag::warn_unreachable_break, Conf.Diagnostics.Suppress, LangOptions()));
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
diag::warn_unused_variable, Conf.Diagnostics.Suppress, LangOptions()));
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::err_typecheck_bool_condition,
Conf.Diagnostics.Suppress,
LangOptions()));
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
diag::err_unexpected_friend, Conf.Diagnostics.Suppress, LangOptions()));
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
diag::warn_alloca, Conf.Diagnostics.Suppress, LangOptions()));
{
auto D = DiagEngine.Report(diag::warn_unreachable_break);
EXPECT_FALSE(isDiagnosticSuppressed(
Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::warn_unused_variable);
EXPECT_TRUE(isDiagnosticSuppressed(
Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::err_typecheck_bool_condition);
EXPECT_TRUE(isDiagnosticSuppressed(
Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::err_unexpected_friend);
EXPECT_TRUE(isDiagnosticSuppressed(
Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
}
{
auto D = DiagEngine.Report(diag::warn_alloca);
EXPECT_TRUE(isDiagnosticSuppressed(
Diag{&DiagEngine}, Conf.Diagnostics.Suppress, LangOptions()));
}

Frag.Diagnostics.Suppress.emplace_back("*");
EXPECT_TRUE(compileAndApply());
Expand Down
12 changes: 5 additions & 7 deletions clang/include/clang/Basic/Attr.td
Original file line number Diff line number Diff line change
Expand Up @@ -3358,18 +3358,16 @@ def DiagnoseIf : InheritableAttr {
let Spellings = [GNU<"diagnose_if">];
let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
let Args = [ExprArgument<"Cond">, StringArgument<"Message">,
EnumArgument<"DiagnosticType", "DiagnosticType",
EnumArgument<"DefaultSeverity",
"DefaultSeverity",
/*is_string=*/true,
["error", "warning"],
["DT_Error", "DT_Warning"]>,
["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 = LateAttrParseStandard;
let AdditionalMembers = [{
bool isError() const { return diagnosticType == DT_Error; }
bool isWarning() const { return diagnosticType == DT_Warning; }
}];
let TemplateDependent = 1;
let Documentation = [DiagnoseIfDocs];
}
Expand Down
8 changes: 6 additions & 2 deletions clang/include/clang/Basic/Diagnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -336,10 +336,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 =
Expand Down Expand Up @@ -870,6 +872,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>
// TODO: Deprecate this once all uses are removed from LLVM
// [[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));
Expand Down
5 changes: 3 additions & 2 deletions clang/include/clang/Basic/DiagnosticCategories.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ namespace clang {
};

enum class Group {
#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs) \
GroupName,
#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs) \
GroupName,
#include "clang/Basic/DiagnosticGroups.inc"
#undef CATEGORY
#undef DIAG_ENTRY
NUM_GROUPS
};
} // end namespace diag
} // end namespace clang
Expand Down
Loading

0 comments on commit 030c6da

Please sign in to comment.