Skip to content

[cas] Fix caching of diagnostics using getCustomDiagID #9905

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
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(new DiagnosticIDs, 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 @@ -3496,18 +3496,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
20 changes: 18 additions & 2 deletions clang/include/clang/Basic/Diagnostic.h
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,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 @@ -871,11 +873,25 @@ 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 Clang.
// [[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));
}

unsigned getCustomDiagID(DiagnosticIDs::CustomDiagDesc Desc) {
return Diags->getCustomDiagID(std::move(Desc));
}

std::optional<unsigned> getMaxCustomDiagID() const {
return Diags->getMaxCustomDiagID();
}
const DiagnosticIDs::CustomDiagDesc &
getCustomDiagDesc(unsigned DiagID) const {
return Diags->getCustomDiagDesc(DiagID);
}

/// Converts a diagnostic argument (as an intptr_t) into the string
/// that represents it.
void ConvertArgToString(ArgumentKind Kind, intptr_t Val,
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