nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous,
MaskRay, ilya-biryukov.
Herald added a project: clang.
Not handling this was a side-effect of being overly cautious when trying
to avoid reading files for which clangd doesn't have the source mapped.
Fixes https://github.com/clangd/clangd/issues/266
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75286
Files:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -274,7 +274,9 @@
double d = 8 / i; // NOLINT
// NOLINTNEXTLINE
double e = 8 / i;
- double f = [[8]] / i;
+ #define BAD 8 / i
+ double f = BAD; // NOLINT
+ double g = [[8]] / i;
}
)cpp");
TestTU TU = TestTU::withCode(Main.code());
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -290,9 +290,10 @@
bool IsInsideMainFile =
Info.hasSourceManager() &&
isInsideMainFile(Info.getLocation(), Info.getSourceManager());
- if (IsInsideMainFile && tidy::shouldSuppressDiagnostic(
- DiagLevel, Info, *CTContext,
- /* CheckMacroExpansion = */ false)) {
+ if (IsInsideMainFile &&
+ tidy::shouldSuppressDiagnostic(
+ DiagLevel, Info, *CTContext,
+ Info.getSourceManager().getMainFileID())) {
return DiagnosticsEngine::Ignored;
}
}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -212,15 +212,14 @@
/// 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.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the SourceMapper.
+/// If a valid FileID is passed as `FileFilter`, the function does not attempt
+/// to read source files other than the one identified by the filter. A degree
+/// of control over this is needed because in some use cases we cannot rely on
+/// files other than the one containing the diagnostic being mapped in the
+/// SourceManager.
bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
const Diagnostic &Info, ClangTidyContext &Context,
- bool CheckMacroExpansion = true);
+ FileID FileFilter = FileID{});
/// A diagnostic consumer that turns each \c Diagnostic into a
/// \c SourceManager-independent \c ClangTidyError.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -120,7 +120,6 @@
: tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
IsWarningAsError(IsWarningAsError) {}
-
class ClangTidyContext::CachedGlobList {
public:
CachedGlobList(StringRef Globs) : Globs(Globs) {}
@@ -341,13 +340,18 @@
static bool LineIsMarkedWithNOLINTinMacro(const SourceManager &SM,
SourceLocation Loc, unsigned DiagID,
- const ClangTidyContext &Context) {
+ const ClangTidyContext &Context,
+ FileID FileFilter) {
while (true) {
if (LineIsMarkedWithNOLINT(SM, Loc, DiagID, Context))
return true;
if (!Loc.isMacroID())
return false;
Loc = SM.getImmediateExpansionRange(Loc).getBegin();
+ if (FileFilter.isValid() && Loc.isFileID() &&
+ SM.getFileID(Loc) != FileFilter) {
+ return false;
+ }
}
return false;
}
@@ -357,14 +361,13 @@
bool shouldSuppressDiagnostic(DiagnosticsEngine::Level DiagLevel,
const Diagnostic &Info, ClangTidyContext &Context,
- bool CheckMacroExpansion) {
+ FileID FileFilter) {
return Info.getLocation().isValid() &&
DiagLevel != DiagnosticsEngine::Error &&
DiagLevel != DiagnosticsEngine::Fatal &&
- (CheckMacroExpansion ? LineIsMarkedWithNOLINTinMacro
- : LineIsMarkedWithNOLINT)(Info.getSourceManager(),
- Info.getLocation(),
- Info.getID(), Context);
+ LineIsMarkedWithNOLINTinMacro(Info.getSourceManager(),
+ Info.getLocation(), Info.getID(),
+ Context, FileFilter);
}
} // namespace tidy
@@ -706,9 +709,9 @@
const tooling::DiagnosticMessage &M1 = LHS.Message;
const tooling::DiagnosticMessage &M2 = RHS.Message;
- return
- std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName, M1.Message) <
- std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, M2.Message);
+ return std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
+ M1.Message) <
+ std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName, M2.Message);
}
};
struct EqualClangTidyError {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
