nridge created this revision.
nridge added a reviewer: hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric.
Herald added a project: clang.
This partially fixes https://bugs.llvm.org/show_bug.cgi?id=41218.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D60953
Files:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clangd/ClangdUnit.cpp
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -237,6 +237,39 @@
const LangOptions &LangOpts;
};
+// A wrapper around StoreDiags to handle suppression comments for
+// clang-tidy diagnostics (and possibly other clang-tidy customizations in the
+// future).
+class ClangdDiagnosticConsumer : public StoreDiags {
+public:
+ void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+ const clang::Diagnostic &Info) override {
+ if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
+ return;
+
+ if (CTContext) {
+ bool isClangTidyDiag = !CTContext->getCheckName(Info.getID()).empty();
+ if (isClangTidyDiag &&
+ tidy::ShouldSuppressDiagnostic(DiagLevel, Info, *CTContext)) {
+ // Ignored a warning, should ignore related notes as well
+ LastErrorWasIgnored = true;
+ return;
+ }
+ }
+
+ LastErrorWasIgnored = false;
+ StoreDiags::HandleDiagnostic(DiagLevel, Info);
+ }
+
+ void setClangTidyContext(tidy::ClangTidyContext *CTContext) {
+ this->CTContext = CTContext;
+ }
+
+private:
+ bool LastErrorWasIgnored = false;
+ tidy::ClangTidyContext *CTContext = nullptr;
+};
+
} // namespace
void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
@@ -256,7 +289,7 @@
const PrecompiledPreamble *PreamblePCH =
Preamble ? &Preamble->Preamble : nullptr;
- StoreDiags ASTDiags;
+ ClangdDiagnosticConsumer ASTDiags;
std::string Content = Buffer->getBuffer();
auto Clang = prepareCompilerInstance(std::move(CI), PreamblePCH,
@@ -294,6 +327,7 @@
CTContext->setASTContext(&Clang->getASTContext());
CTContext->setCurrentFile(MainInput.getFile());
CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+ ASTDiags.setClangTidyContext(CTContext.getPointer());
Preprocessor *PP = &Clang->getPreprocessor();
for (const auto &Check : CTChecks) {
// FIXME: the PP callbacks skip the entire preamble.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -217,6 +217,17 @@
bool AllowEnablingAnalyzerAlphaCheckers;
};
+/// Check whether a given diagnostic should be suppressed due to the presence
+/// of a "NOLINT" suppression comment.
+/// This is exposed so that other tools that present clang-tidy diagnostics
+/// (such as clangd) can respect the same suppression rules as clang-tidy.
+/// This does not handle suppression of notes following a suppressed diagnostic;
+/// that is left to the caller is it requires maintaining state in between calls
+/// to this function.
+bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
+ const Diagnostic &Info,
+ ClangTidyContext &Context);
+
/// \brief A diagnostic consumer that turns each \c Diagnostic into a
/// \c SourceManager-independent \c ClangTidyError.
//
@@ -246,7 +257,7 @@
/// \brief Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter
/// according to the diagnostic \p Location.
- void checkFilters(SourceLocation Location, const SourceManager& Sources);
+ void checkFilters(SourceLocation Location, const SourceManager &Sources);
bool passesLineFilter(StringRef FileName, unsigned LineNumber) const;
ClangTidyContext &Context;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -160,11 +160,13 @@
bool contains(StringRef S) {
switch (auto &Result = Cache[S]) {
- case Yes: return true;
- case No: return false;
- case None:
- Result = Globs.contains(S) ? Yes : No;
- return Result == Yes;
+ case Yes:
+ return true;
+ case No:
+ return false;
+ case None:
+ Result = Globs.contains(S) ? Yes : No;
+ return Result == Yes;
}
llvm_unreachable("invalid enum");
}
@@ -381,16 +383,29 @@
return false;
}
+namespace clang {
+namespace tidy {
+
+bool ShouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
+ const Diagnostic &Info,
+ ClangTidyContext &Context) {
+ return Info.getLocation().isValid() &&
+ DiagLevel != DiagnosticsEngine::Error &&
+ DiagLevel != DiagnosticsEngine::Fatal &&
+ LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(),
+ Info.getLocation(), Info.getID(),
+ Context);
+}
+
+} // namespace tidy
+} // namespace clang
+
void ClangTidyDiagnosticConsumer::HandleDiagnostic(
DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) {
if (LastErrorWasIgnored && DiagLevel == DiagnosticsEngine::Note)
return;
- if (Info.getLocation().isValid() && DiagLevel != DiagnosticsEngine::Error &&
- DiagLevel != DiagnosticsEngine::Fatal &&
- LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(),
- Info.getLocation(), Info.getID(),
- Context)) {
+ if (ShouldSuppressDiagnostic(DiagLevel, Info, Context)) {
++Context.Stats.ErrorsIgnoredNOLINT;
// Ignored a warning, should ignore related notes as well
LastErrorWasIgnored = true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
