Skip to content

Commit 0865ecc

Browse files
authored
[clang] Extend diagnose_if to accept more detailed warning information, take 2 (llvm#119712)
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.
1 parent aab25f2 commit 0865ecc

30 files changed

+562
-224
lines changed

clang-tools-extra/clangd/Diagnostics.cpp

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,17 @@ std::vector<Diag> StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) {
577577
for (auto &Diag : Output) {
578578
if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
579579
// Warnings controlled by -Wfoo are better recognized by that name.
580-
StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID);
580+
StringRef Warning = [&] {
581+
if (OrigSrcMgr) {
582+
return OrigSrcMgr->getDiagnostics()
583+
.getDiagnosticIDs()
584+
->getWarningOptionForDiag(Diag.ID);
585+
}
586+
if (!DiagnosticIDs::IsCustomDiag(Diag.ID))
587+
return DiagnosticIDs{}.getWarningOptionForDiag(Diag.ID);
588+
return StringRef{};
589+
}();
590+
581591
if (!Warning.empty()) {
582592
Diag.Name = ("-W" + Warning).str();
583593
} else {
@@ -894,20 +904,23 @@ void StoreDiags::flushLastDiag() {
894904
Output.push_back(std::move(*LastDiag));
895905
}
896906

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

906-
if (const char *CodePtr = getDiagnosticCode(ID)) {
917+
if (const char *CodePtr = getDiagnosticCode(Diag.getID())) {
907918
if (Suppress.contains(normalizeSuppressedCode(CodePtr)))
908919
return true;
909920
}
910-
StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(ID);
921+
StringRef Warning =
922+
Diag.getDiags()->getDiagnosticIDs()->getWarningOptionForDiag(
923+
Diag.getID());
911924
if (!Warning.empty() && Suppress.contains(Warning))
912925
return true;
913926
return false;

clang-tools-extra/clangd/Diagnostics.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,11 @@ class StoreDiags : public DiagnosticConsumer {
181181
};
182182

183183
/// Determine whether a (non-clang-tidy) diagnostic is suppressed by config.
184-
bool isBuiltinDiagnosticSuppressed(unsigned ID,
185-
const llvm::StringSet<> &Suppressed,
186-
const LangOptions &);
184+
bool isDiagnosticSuppressed(const clang::Diagnostic &Diag,
185+
const llvm::StringSet<> &Suppressed,
186+
const LangOptions &);
187187
/// Take a user-specified diagnostic code, and convert it to a normalized form
188-
/// stored in the config and consumed by isBuiltinDiagnosticsSuppressed.
188+
/// stored in the config and consumed by isDiagnosticsSuppressed.
189189
///
190190
/// (This strips err_ and -W prefix so we can match with or without them.)
191191
llvm::StringRef normalizeSuppressedCode(llvm::StringRef);

clang-tools-extra/clangd/ParsedAST.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ void applyWarningOptions(llvm::ArrayRef<std::string> ExtraArgs,
342342
if (Enable) {
343343
if (Diags.getDiagnosticLevel(ID, SourceLocation()) <
344344
DiagnosticsEngine::Warning) {
345-
auto Group = DiagnosticIDs::getGroupForDiag(ID);
345+
auto Group = Diags.getDiagnosticIDs()->getGroupForDiag(ID);
346346
if (!Group || !EnabledGroups(*Group))
347347
continue;
348348
Diags.setSeverity(ID, diag::Severity::Warning, SourceLocation());
@@ -585,8 +585,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
585585
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
586586
const clang::Diagnostic &Info) {
587587
if (Cfg.Diagnostics.SuppressAll ||
588-
isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
589-
Clang->getLangOpts()))
588+
isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress,
589+
Clang->getLangOpts()))
590590
return DiagnosticsEngine::Ignored;
591591

592592
auto It = OverriddenSeverity.find(Info.getID());

clang-tools-extra/clangd/Preamble.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -622,8 +622,8 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
622622
PreambleDiagnostics.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
623623
const clang::Diagnostic &Info) {
624624
if (Cfg.Diagnostics.SuppressAll ||
625-
isBuiltinDiagnosticSuppressed(Info.getID(), Cfg.Diagnostics.Suppress,
626-
CI.getLangOpts()))
625+
isDiagnosticSuppressed(Info, Cfg.Diagnostics.Suppress,
626+
CI.getLangOpts()))
627627
return DiagnosticsEngine::Ignored;
628628
switch (Info.getID()) {
629629
case diag::warn_no_newline_eof:

clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -298,20 +298,41 @@ TEST_F(ConfigCompileTests, DiagnosticSuppression) {
298298
"unreachable-code", "unused-variable",
299299
"typecheck_bool_condition",
300300
"unexpected_friend", "warn_alloca"));
301-
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
302-
diag::warn_unreachable, Conf.Diagnostics.Suppress, LangOptions()));
301+
clang::DiagnosticsEngine DiagEngine(new DiagnosticIDs, nullptr,
302+
new clang::IgnoringDiagConsumer);
303+
304+
using Diag = clang::Diagnostic;
305+
{
306+
auto D = DiagEngine.Report(diag::warn_unreachable);
307+
EXPECT_TRUE(isDiagnosticSuppressed(
308+
Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
309+
}
303310
// Subcategory not respected/suppressed.
304-
EXPECT_FALSE(isBuiltinDiagnosticSuppressed(
305-
diag::warn_unreachable_break, Conf.Diagnostics.Suppress, LangOptions()));
306-
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
307-
diag::warn_unused_variable, Conf.Diagnostics.Suppress, LangOptions()));
308-
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(diag::err_typecheck_bool_condition,
309-
Conf.Diagnostics.Suppress,
310-
LangOptions()));
311-
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
312-
diag::err_unexpected_friend, Conf.Diagnostics.Suppress, LangOptions()));
313-
EXPECT_TRUE(isBuiltinDiagnosticSuppressed(
314-
diag::warn_alloca, Conf.Diagnostics.Suppress, LangOptions()));
311+
{
312+
auto D = DiagEngine.Report(diag::warn_unreachable_break);
313+
EXPECT_FALSE(isDiagnosticSuppressed(
314+
Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
315+
}
316+
{
317+
auto D = DiagEngine.Report(diag::warn_unused_variable);
318+
EXPECT_TRUE(isDiagnosticSuppressed(
319+
Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
320+
}
321+
{
322+
auto D = DiagEngine.Report(diag::err_typecheck_bool_condition);
323+
EXPECT_TRUE(isDiagnosticSuppressed(
324+
Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
325+
}
326+
{
327+
auto D = DiagEngine.Report(diag::err_unexpected_friend);
328+
EXPECT_TRUE(isDiagnosticSuppressed(
329+
Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
330+
}
331+
{
332+
auto D = DiagEngine.Report(diag::warn_alloca);
333+
EXPECT_TRUE(isDiagnosticSuppressed(
334+
Diag{&DiagEngine, D}, Conf.Diagnostics.Suppress, LangOptions()));
335+
}
315336

316337
Frag.Diagnostics.Suppress.emplace_back("*");
317338
EXPECT_TRUE(compileAndApply());

clang/include/clang/Basic/Attr.td

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3458,18 +3458,16 @@ def DiagnoseIf : InheritableAttr {
34583458
let Spellings = [GNU<"diagnose_if">];
34593459
let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>;
34603460
let Args = [ExprArgument<"Cond">, StringArgument<"Message">,
3461-
EnumArgument<"DiagnosticType", "DiagnosticType",
3461+
EnumArgument<"DefaultSeverity",
3462+
"DefaultSeverity",
34623463
/*is_string=*/true,
3463-
["error", "warning"],
3464-
["DT_Error", "DT_Warning"]>,
3464+
["error", "warning"],
3465+
["DS_error", "DS_warning"]>,
3466+
StringArgument<"WarningGroup", /*optional*/ 1>,
34653467
BoolArgument<"ArgDependent", 0, /*fake*/ 1>,
34663468
DeclArgument<Named, "Parent", 0, /*fake*/ 1>];
34673469
let InheritEvenIfAlreadyPresent = 1;
34683470
let LateParsed = LateAttrParseStandard;
3469-
let AdditionalMembers = [{
3470-
bool isError() const { return diagnosticType == DT_Error; }
3471-
bool isWarning() const { return diagnosticType == DT_Warning; }
3472-
}];
34733471
let TemplateDependent = 1;
34743472
let Documentation = [DiagnoseIfDocs];
34753473
}

clang/include/clang/Basic/Diagnostic.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,12 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
375375
// Map extensions to warnings or errors?
376376
diag::Severity ExtBehavior = diag::Severity::Ignored;
377377

378-
DiagState()
378+
DiagnosticIDs &DiagIDs;
379+
380+
DiagState(DiagnosticIDs &DiagIDs)
379381
: IgnoreAllWarnings(false), EnableAllWarnings(false),
380382
WarningsAsErrors(false), ErrorsAsFatal(false),
381-
SuppressSystemWarnings(false) {}
383+
SuppressSystemWarnings(false), DiagIDs(DiagIDs) {}
382384

383385
using iterator = llvm::DenseMap<unsigned, DiagnosticMapping>::iterator;
384386
using const_iterator =
@@ -893,6 +895,8 @@ class DiagnosticsEngine : public RefCountedBase<DiagnosticsEngine> {
893895
/// \param FormatString A fixed diagnostic format string that will be hashed
894896
/// and mapped to a unique DiagID.
895897
template <unsigned N>
898+
// TODO: Deprecate this once all uses are removed from Clang.
899+
// [[deprecated("Use a CustomDiagDesc instead of a Level")]]
896900
unsigned getCustomDiagID(Level L, const char (&FormatString)[N]) {
897901
return Diags->getCustomDiagID((DiagnosticIDs::Level)L,
898902
StringRef(FormatString, N - 1));

clang/include/clang/Basic/DiagnosticCategories.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,12 @@ namespace clang {
2121
};
2222

2323
enum class Group {
24-
#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs) \
25-
GroupName,
24+
#define DIAG_ENTRY(GroupName, FlagNameOffset, Members, SubGroups, Docs) \
25+
GroupName,
2626
#include "clang/Basic/DiagnosticGroups.inc"
2727
#undef CATEGORY
2828
#undef DIAG_ENTRY
29+
NUM_GROUPS
2930
};
3031
} // end namespace diag
3132
} // end namespace clang

0 commit comments

Comments
 (0)