kadircet updated this revision to Diff 191323.
kadircet marked 2 inline comments as done.
kadircet added a comment.
- Show include stack in diagnostic message
- Point to exact include location
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59302/new/
https://reviews.llvm.org/D59302
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/Compiler.h
clangd/Diagnostics.cpp
clangd/Diagnostics.h
clangd/tool/ClangdMain.cpp
unittests/clangd/DiagnosticsTests.cpp
Index: unittests/clangd/DiagnosticsTests.cpp
===================================================================
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
#include "Annotations.h"
#include "ClangdUnit.h"
#include "SourceCode.h"
+#include "TestFS.h"
#include "TestIndex.h"
#include "TestTU.h"
#include "index/MemIndex.h"
@@ -73,7 +74,6 @@
return true;
}
-
// Helper function to make tests shorter.
Position pos(int line, int character) {
Position Res;
@@ -603,7 +603,174 @@
"Add include \"x.h\" for symbol a::X")))));
}
+ParsedAST
+build(const std::string &MainFile, const llvm::StringMap<std::string> &Files,
+ llvm::Optional<int64_t> LimitDiagsOutsideMainFile = llvm::None) {
+ std::vector<const char *> Cmd = {"clang", MainFile.c_str()};
+ ParseInputs Inputs;
+ Inputs.CompileCommand.Filename = MainFile;
+ Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+ Inputs.CompileCommand.Directory = testRoot();
+ Inputs.Contents = Files.lookup(MainFile);
+ Inputs.FS = buildTestFS(Files);
+ Inputs.Opts = ParseOptions();
+ Inputs.Opts.LimitDiagsOutsideMainFile = LimitDiagsOutsideMainFile;
+ auto PCHs = std::make_shared<PCHContainerOperations>();
+ auto CI = buildCompilerInvocation(Inputs);
+ assert(CI && "Failed to build compilation invocation.");
+ auto Preamble =
+ buildPreamble(MainFile, *CI,
+ /*OldPreamble=*/nullptr,
+ /*OldCompileCommand=*/Inputs.CompileCommand, Inputs, PCHs,
+ /*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+ auto AST = buildAST(MainFile, createInvocationFromCommandLine(Cmd), Inputs,
+ Preamble, PCHs);
+ if (!AST.hasValue()) {
+ ADD_FAILURE() << "Failed to build code:\n" << Files.lookup(MainFile);
+ llvm_unreachable("Failed to build TestTU!");
+ }
+ return std::move(*AST);
+}
+
+TEST(DiagsInHeaders, DiagInsideHeader) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ void foo() {})cpp");
+ std::string MainFile = testPath("main.cc");
+ llvm::StringMap<std::string> Files = {{MainFile, Main.code()},
+ {testPath("a.h"), "no_type_spec;"}};
+ auto FS = buildTestFS(Files);
+
+ EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(), "/clangd-test/a.h:1:1: C++ requires a "
+ "type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ void foo() {})cpp");
+ std::string MainFile = testPath("main.cc");
+ llvm::StringMap<std::string> Files = {{MainFile, Main.code()},
+ {testPath("a.h"), "#include \"b.h\""},
+ {testPath("b.h"), "no_type_spec;"}};
+ auto FS = buildTestFS(Files);
+
+ EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(),
+ "In file included from: "
+ "/clangd-test/a.h:1:10:\n/clangd-test/b.h:1:1: C++ "
+ "requires a type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInMultipleHeaders) {
+ Annotations Main(R"cpp(
+ #include $a[["a.h"]]
+ #include $b[["b.h"]]
+ void foo() {})cpp");
+ std::string MainFile = testPath("main.cc");
+ llvm::StringMap<std::string> Files = {{MainFile, Main.code()},
+ {testPath("a.h"), "no_type_spec;"},
+ {testPath("b.h"), "no_type_spec;"}};
+ auto FS = buildTestFS(Files);
+
+ EXPECT_THAT(
+ build(MainFile, Files).getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range("a"), "/clangd-test/a.h:1:1: C++ requires a type "
+ "specifier for all declarations"),
+ Diag(Main.range("b"), "/clangd-test/b.h:1:1: C++ requires a type "
+ "specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, PreferExpansionLocation) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ #include "b.h"
+ void foo() {})cpp");
+ std::string MainFile = testPath("main.cc");
+ llvm::StringMap<std::string> Files = {
+ {MainFile, Main.code()},
+ {testPath("a.h"), "#include \"b.h\"\n"},
+ {testPath("b.h"), "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+ auto FS = buildTestFS(Files);
+
+ EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(),
+ "In file included from: "
+ "/clangd-test/a.h:1:10:\n/clangd-test/b.h:3:1: C++ "
+ "requires a type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, PreferExpansionLocationMacros) {
+ Annotations Main(R"cpp(
+ #define X
+ #include "a.h"
+ #undef X
+ #include [["b.h"]]
+ void foo() {})cpp");
+ std::string MainFile = testPath("main.cc");
+ llvm::StringMap<std::string> Files = {
+ {MainFile, Main.code()},
+ {testPath("a.h"), "#include \"c.h\"\n"},
+ {testPath("b.h"), "#include \"c.h\"\n"},
+ {testPath("c.h"), "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+ auto FS = buildTestFS(Files);
+
+ EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(),
+ "In file included from: "
+ "/clangd-test/b.h:1:10:\n/clangd-test/c.h:3:1: C++ "
+ "requires a type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ #include "b.h"
+ void foo() {})cpp");
+ std::string MainFile = testPath("main.cc");
+ llvm::StringMap<std::string> Files = {
+ {MainFile, Main.code()},
+ {testPath("a.h"), "#include \"c.h\"\n"},
+ {testPath("b.h"), "#include \"c.h\"\n"},
+ {testPath("c.h"), "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+ auto FS = buildTestFS(Files);
+
+ EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(),
+ "In file included from: "
+ "/clangd-test/a.h:1:10:\n/clangd-test/c.h:3:1: C++ "
+ "requires a type specifier for all declarations")));
+
+ EXPECT_THAT(build(MainFile, Files, 0).getDiagnostics(),
+ UnorderedElementsAre());
+}
+
+TEST(DiagsInHeaders, OnlyErrorOrFatal) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ void foo() {})cpp");
+ std::string MainFile = testPath("main.cc");
+ llvm::StringMap<std::string> Files = {{MainFile, Main.code()},
+ {testPath("a.h"),
+ R"cpp(
+ no_type_spec;
+ int x = 5/0;
+ )cpp"}};
+ auto FS = buildTestFS(Files);
+
+ EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(), "/clangd-test/a.h:2:44: C++ requires a "
+ "type specifier for all declarations")));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
-
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -219,6 +219,12 @@
"includes using index."),
llvm::cl::init(true));
+static llvm::cl::opt<int> LimitDiagsOutsideMainFile(
+ "limit-diags-outside-mainfile",
+ llvm::cl::desc("Attempts to fix diagnostic errors caused by missing "
+ "includes using index."),
+ llvm::cl::init(100), llvm::cl::Hidden);
+
namespace {
/// \brief Supports a test URI scheme with relaxed constraints for lit tests.
@@ -458,6 +464,7 @@
}
Opts.ClangTidyOptProvider = ClangTidyOptProvider.get();
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
+ Opts.LimitDiagsOutsideMainFile = LimitDiagsOutsideMainFile;
ClangdLSPServer LSPServer(
*TransportLayer, FSProvider, CCOpts, CompileCommandsDirPath,
/*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs, Opts);
Index: clangd/Diagnostics.h
===================================================================
--- clangd/Diagnostics.h
+++ clangd/Diagnostics.h
@@ -14,6 +14,8 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringSet.h"
#include <cassert>
@@ -80,6 +82,9 @@
std::vector<Note> Notes;
/// *Alternative* fixes for this diagnostic, one should be chosen.
std::vector<Fix> Fixes;
+ /// If diagnostics is coming outside main file, points to relevant include
+ /// directive, otherwise None.
+ llvm::Optional<Position> IncludeDirective = llvm::None;
};
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D);
Index: clangd/Diagnostics.cpp
===================================================================
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -10,30 +10,19 @@
#include "Compiler.h"
#include "Logger.h"
#include "SourceCode.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
#include "llvm/Support/Capacity.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Signals.h"
#include <algorithm>
namespace clang {
namespace clangd {
namespace {
-
-bool mentionsMainFile(const Diag &D) {
- if (D.InsideMainFile)
- return true;
- // Fixes are always in the main file.
- if (!D.Fixes.empty())
- return true;
- for (auto &N : D.Notes) {
- if (N.InsideMainFile)
- return true;
- }
- return false;
-}
-
// Checks whether a location is within a half-open range.
// Note that clang also uses closed source ranges, which this can't handle!
bool locationInRange(SourceLocation L, CharSourceRange R,
@@ -379,6 +368,38 @@
LastDiag = Diag();
LastDiag->ID = Info.getID();
FillDiagBase(*LastDiag);
+ auto IncludeLocation = Info.getSourceManager()
+ .getPresumedLoc(Info.getLocation(),
+ /*UseLineDirectives=*/false)
+ .getIncludeLoc();
+ std::vector<std::string> IncludeStack;
+ while (IncludeLocation.isValid()) {
+ LastDiag->IncludeDirective =
+ sourceLocToPosition(Info.getSourceManager(), IncludeLocation);
+ IncludeStack.push_back(
+ IncludeLocation.printToString(Info.getSourceManager()));
+ IncludeLocation = Info.getSourceManager()
+ .getPresumedLoc(IncludeLocation,
+ /*UseLineDirectives=*/false)
+ .getIncludeLoc();
+ }
+ if (LastDiag->IncludeDirective) {
+ std::string Message;
+ {
+ llvm::raw_string_ostream OS(Message);
+ // We don't show the main file as part of include stack, which is the
+ // last element.
+ if (IncludeStack.size() > 1) {
+ IncludeStack.pop_back();
+ std::reverse(IncludeStack.begin(), IncludeStack.end());
+ for (llvm::StringRef Inc : IncludeStack)
+ OS << "In file included from: " << Inc << ":\n";
+ }
+ OS << Info.getLocation().printToString(Info.getSourceManager()) << ": "
+ << LastDiag->Message;
+ }
+ LastDiag->Message = std::move(Message);
+ }
if (!Info.getFixItHints().empty())
AddFix(true /* try to invent a message instead of repeating the diag */);
@@ -413,11 +434,7 @@
void StoreDiags::flushLastDiag() {
if (!LastDiag)
return;
- if (mentionsMainFile(*LastDiag))
- Output.push_back(std::move(*LastDiag));
- else
- vlog("Dropped diagnostic outside main file: {0}: {1}", LastDiag->File,
- LastDiag->Message);
+ Output.push_back(std::move(*LastDiag));
LastDiag.reset();
}
Index: clangd/Compiler.h
===================================================================
--- clangd/Compiler.h
+++ clangd/Compiler.h
@@ -21,6 +21,7 @@
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PrecompiledPreamble.h"
#include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/Optional.h"
namespace clang {
namespace clangd {
@@ -38,6 +39,7 @@
struct ParseOptions {
tidy::ClangTidyOptions ClangTidyOpts;
bool SuggestMissingIncludes = false;
+ llvm::Optional<int64_t> LimitDiagsOutsideMainFile;
};
/// Information required to run clang, e.g. to parse AST or do code completion.
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -14,11 +14,13 @@
#include "Headers.h"
#include "IncludeFixer.h"
#include "Logger.h"
+#include "Protocol.h"
#include "SourceCode.h"
#include "Trace.h"
#include "index/CanonicalIncludes.h"
#include "index/Index.h"
#include "clang/AST/ASTContext.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
@@ -34,9 +36,11 @@
#include "clang/Serialization/ASTWriter.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <memory>
@@ -45,6 +49,19 @@
namespace clangd {
namespace {
+bool mentionsMainFile(const Diag &D) {
+ if (D.InsideMainFile)
+ return true;
+ // Fixes are always in the main file.
+ if (!D.Fixes.empty())
+ return true;
+ for (auto &N : D.Notes) {
+ if (N.InsideMainFile)
+ return true;
+ }
+ return false;
+}
+
bool compileCommandsAreEqual(const tooling::CompileCommand &LHS,
const tooling::CompileCommand &RHS) {
// We don't check for Output, it should not matter to clangd.
@@ -236,6 +253,15 @@
const LangOptions &LangOpts;
};
+// Generates a mapping from start of an include directive to its range.
+std::map<Position, Range>
+positionToRangeMap(const std::vector<Inclusion> &MainFileIncludes) {
+ std::map<Position, Range> RangeMap;
+ for (const Inclusion &Inc : MainFileIncludes)
+ RangeMap[Inc.R.start] = Inc.R;
+ return RangeMap;
+}
+
} // namespace
void dumpAST(ParsedAST &AST, llvm::raw_ostream &OS) {
@@ -371,14 +397,45 @@
// So just inform the preprocessor of EOF, while keeping everything alive.
Clang->getPreprocessor().EndSourceFile();
- std::vector<Diag> Diags = ASTDiags.take();
+ std::vector<Diag> Diags;
+ int64_t DiagsOutsideMainFile = 0;
+ for (const Diag &D : ASTDiags.take()) {
+ if (!mentionsMainFile(D) && Opts.LimitDiagsOutsideMainFile &&
+ ++DiagsOutsideMainFile > *Opts.LimitDiagsOutsideMainFile)
+ continue;
+ Diags.push_back(D);
+ }
// Populate diagnostic source.
for (auto &D : Diags)
D.S =
!CTContext->getCheckName(D.ID).empty() ? Diag::ClangTidy : Diag::Clang;
// Add diagnostics from the preamble, if any.
- if (Preamble)
- Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
+ if (Preamble) {
+ for (const Diag &D : Preamble->Diags) {
+ if (!mentionsMainFile(D)) {
+ if (D.Severity != DiagnosticsEngine::Level::Error &&
+ D.Severity != DiagnosticsEngine::Level::Fatal)
+ continue;
+ if (Opts.LimitDiagsOutsideMainFile &&
+ DiagsOutsideMainFile >= *Opts.LimitDiagsOutsideMainFile)
+ continue;
+ DiagsOutsideMainFile++;
+ }
+ Diags.push_back(D);
+ }
+ }
+
+ // Remap diagnostics from headers into their include location inside main
+ // file.
+ const auto RangeMap = positionToRangeMap(Includes.MainFileIncludes);
+ for (auto &D : Diags) {
+ if (!mentionsMainFile(D)) {
+ assert(D.IncludeDirective &&
+ "Diagnostic is not mentioned in mainfile but also not imported?");
+ D.Range = RangeMap.at(*D.IncludeDirective);
+ D.File = MainInput.getFile();
+ }
+ }
return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
std::move(ParsedDecls), std::move(Diags),
std::move(Includes), std::move(CanonIncludes));
Index: clangd/ClangdServer.h
===================================================================
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -119,6 +119,8 @@
std::chrono::milliseconds(500);
bool SuggestMissingIncludes = false;
+
+ llvm::Optional<int64_t> LimitDiagsOutsideMainFile = llvm::None;
};
// Sensible default options for use in tests.
// Features like indexing must be enabled if desired.
@@ -294,6 +296,10 @@
// can be caused by missing includes (e.g. member access in incomplete type).
bool SuggestMissingIncludes = false;
+ // When set only that many number of diagnostics from headers will be surfaced
+ // in main file.
+ llvm::Optional<int64_t> LimitDiagsOutsideMainFile = llvm::None;
+
// GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
llvm::StringMap<llvm::Optional<FuzzyFindRequest>>
CachedCompletionFuzzyFindRequestByFile;
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -113,6 +113,7 @@
: nullptr),
ClangTidyOptProvider(Opts.ClangTidyOptProvider),
SuggestMissingIncludes(Opts.SuggestMissingIncludes),
+ LimitDiagsOutsideMainFile(Opts.LimitDiagsOutsideMainFile),
WorkspaceRoot(Opts.WorkspaceRoot),
PCHs(std::make_shared<PCHContainerOperations>()),
// Pass a callback into `WorkScheduler` to extract symbols from a newly
@@ -153,6 +154,7 @@
if (ClangTidyOptProvider)
Opts.ClangTidyOpts = ClangTidyOptProvider->getOptions(File);
Opts.SuggestMissingIncludes = SuggestMissingIncludes;
+ Opts.LimitDiagsOutsideMainFile = LimitDiagsOutsideMainFile;
// FIXME: some build systems like Bazel will take time to preparing
// environment to build the file, it would be nice if we could emit a
// "PreparingBuild" status to inform users, it is non-trivial given the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
