kadircet updated this revision to Diff 197077.
kadircet added a comment.
- Rebase
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59302/new/
https://reviews.llvm.org/D59302
Files:
clangd/Diagnostics.cpp
clangd/Diagnostics.h
clangd/unittests/DiagnosticsTests.cpp
clangd/unittests/TestTU.cpp
clangd/unittests/TestTU.h
Index: clangd/unittests/TestTU.h
===================================================================
--- clangd/unittests/TestTU.h
+++ clangd/unittests/TestTU.h
@@ -18,8 +18,13 @@
#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_TESTTU_H
#include "ClangdUnit.h"
+#include "Path.h"
#include "index/Index.h"
+#include "llvm/ADT/StringMap.h"
#include "gtest/gtest.h"
+#include <string>
+#include <utility>
+#include <vector>
namespace clang {
namespace clangd {
@@ -45,6 +50,9 @@
std::string HeaderCode;
std::string HeaderFilename = "TestTU.h";
+ // Name and contents of each file.
+ llvm::StringMap<std::string> AdditionalFiles;
+
// Extra arguments for the compiler invocation.
std::vector<const char *> ExtraArgs;
Index: clangd/unittests/TestTU.cpp
===================================================================
--- clangd/unittests/TestTU.cpp
+++ clangd/unittests/TestTU.cpp
@@ -21,11 +21,17 @@
std::string FullFilename = testPath(Filename),
FullHeaderName = testPath(HeaderFilename),
ImportThunk = testPath("import_thunk.h");
- std::vector<const char *> Cmd = {"clang", FullFilename.c_str()};
// We want to implicitly include HeaderFilename without messing up offsets.
// -include achieves this, but sometimes we want #import (to simulate a header
// guard without messing up offsets). In this case, use an intermediate file.
std::string ThunkContents = "#import \"" + FullHeaderName + "\"\n";
+
+ llvm::StringMap<std::string> Files(AdditionalFiles);
+ Files[FullFilename] = Code;
+ Files[FullHeaderName] = HeaderCode;
+ Files[ImportThunk] = ThunkContents;
+
+ std::vector<const char *> Cmd = {"clang", FullFilename.c_str()};
// FIXME: this shouldn't need to be conditional, but it breaks a
// GoToDefinition test for some reason (getMacroArgExpandedLocation fails).
if (!HeaderCode.empty()) {
@@ -39,9 +45,7 @@
Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
Inputs.CompileCommand.Directory = testRoot();
Inputs.Contents = Code;
- Inputs.FS = buildTestFS({{FullFilename, Code},
- {FullHeaderName, HeaderCode},
- {ImportThunk, ThunkContents}});
+ Inputs.FS = buildTestFS(Files);
Inputs.Opts = ParseOptions();
Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
Inputs.Index = ExternalIndex;
Index: clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clangd/unittests/DiagnosticsTests.cpp
+++ clangd/unittests/DiagnosticsTests.cpp
@@ -9,10 +9,11 @@
#include "Annotations.h"
#include "ClangdUnit.h"
#include "Diagnostics.h"
+#include "Path.h"
#include "Protocol.h"
#include "SourceCode.h"
-#include "TestIndex.h"
#include "TestFS.h"
+#include "TestIndex.h"
#include "TestTU.h"
#include "index/MemIndex.h"
#include "clang/Basic/Diagnostic.h"
@@ -20,6 +21,7 @@
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
+#include <algorithm>
namespace clang {
namespace clangd {
@@ -61,7 +63,8 @@
"LSP diagnostic " + llvm::to_string(LSPDiag)) {
if (toJSON(arg) != toJSON(LSPDiag)) {
*result_listener << llvm::formatv("expected:\n{0:2}\ngot\n{1:2}",
- toJSON(LSPDiag), toJSON(arg)).str();
+ toJSON(LSPDiag), toJSON(arg))
+ .str();
return false;
}
return true;
@@ -83,7 +86,6 @@
return true;
}
-
// Helper function to make tests shorter.
Position pos(int line, int character) {
Position Res;
@@ -114,8 +116,7 @@
// This range spans lines.
AllOf(Diag(Test.range("typo"),
"use of undeclared identifier 'goo'; did you mean 'foo'?"),
- DiagSource(Diag::Clang),
- DiagName("undeclared_var_use_suggest"),
+ DiagSource(Diag::Clang), DiagName("undeclared_var_use_suggest"),
WithFix(
Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
// This is a pretty normal range.
@@ -301,7 +302,8 @@
MainLSP.severity = getSeverity(DiagnosticsEngine::Error);
MainLSP.code = "enum_class_reference";
MainLSP.source = "clang";
- MainLSP.message = R"(Something terrible happened (fix available)
+ MainLSP.message =
+ R"(Something terrible happened (fix available)
main.cpp:6:7: remark: declared somewhere in the main file
@@ -354,9 +356,8 @@
NoteInHeaderDRI.location.range = NoteInHeader.Range;
NoteInHeaderDRI.location.uri = HeaderFile;
MainLSP.relatedInformation = {NoteInMainDRI, NoteInHeaderDRI};
- EXPECT_THAT(
- LSPDiags,
- ElementsAre(Pair(EqualToLSPDiag(MainLSP), ElementsAre(EqualToFix(F)))));
+ EXPECT_THAT(LSPDiags, ElementsAre(Pair(EqualToLSPDiag(MainLSP),
+ ElementsAre(EqualToFix(F)))));
}
struct SymbolWithHeader {
@@ -646,7 +647,124 @@
"Add include \"x.h\" for symbol a::X")))));
}
+TEST(DiagsInHeaders, DiagInsideHeader) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ void foo() {})cpp");
+ Annotations Header("[[no_type_spec]];");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.AdditionalFiles = {{"a.h", Header.code()}};
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(AllOf(
+ Diag(Main.range(), "in included file: C++ requires a "
+ "type specifier for all declarations"),
+ WithNote(Diag(Header.range(), "error occurred here")))));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ void foo() {})cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.AdditionalFiles = {{"a.h", "#include \"b.h\""}, {"b.h", "no_type_spec;"}};
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(), "in included file: 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");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.AdditionalFiles = {{"a.h", "no_type_spec;"}, {"b.h", "no_type_spec;"}};
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range("a"), "in included file: C++ requires a type "
+ "specifier for all declarations"),
+ Diag(Main.range("b"), "in included file: C++ requires a type "
+ "specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, PreferExpansionLocation) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ #include "b.h"
+ void foo() {})cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.AdditionalFiles = {{"a.h", "#include \"b.h\"\n"},
+ {"b.h", "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(Diag(Main.range(),
+ "in included file: 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");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.AdditionalFiles = {{"a.h", "#include \"c.h\"\n"},
+ {"b.h", "#include \"c.h\"\n"},
+ {"c.h", "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(), "in included file: C++ requires a "
+ "type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, LimitDiagsOutsideMainFile) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ #include "b.h"
+ void foo() {})cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.AdditionalFiles = {{"a.h", "#include \"c.h\"\n"},
+ {"b.h", "#include \"c.h\"\n"},
+ {"c.h", R"cpp(
+ #ifndef X
+ #define X
+ no_type_spec_0;
+ no_type_spec_1;
+ no_type_spec_2;
+ no_type_spec_3;
+ no_type_spec_4;
+ no_type_spec_5;
+ no_type_spec_6;
+ no_type_spec_7;
+ no_type_spec_8;
+ no_type_spec_9;
+ no_type_spec_10;
+ #endif)cpp"}};
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(
+ Diag(Main.range(), "in included file: C++ requires a "
+ "type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, OnlyErrorOrFatal) {
+ Annotations Main(R"cpp(
+ #include [["a.h"]]
+ void foo() {})cpp");
+ Annotations Header(R"cpp(
+ [[no_type_spec]];
+ int x = 5/0;)cpp");
+ TestTU TU = TestTU::withCode(Main.code());
+ TU.AdditionalFiles = {{"a.h", Header.code()}};
+ auto diags = TU.build().getDiagnostics();
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ UnorderedElementsAre(AllOf(
+ Diag(Main.range(), "in included file: C++ requires "
+ "a type specifier for all declarations"),
+ WithNote(Diag(Header.range(), "error occurred here")))));
+}
} // namespace
+
} // namespace clangd
} // namespace clang
-
Index: clangd/Diagnostics.h
===================================================================
--- clangd/Diagnostics.h
+++ clangd/Diagnostics.h
@@ -14,6 +14,9 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LangOptions.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringSet.h"
#include <cassert>
@@ -135,6 +138,7 @@
std::vector<Diag> Output;
llvm::Optional<LangOptions> LangOpts;
llvm::Optional<Diag> LastDiag;
+ llvm::DenseSet<int> IncludeLinesWithErrors;
};
} // namespace clangd
Index: clangd/Diagnostics.cpp
===================================================================
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -13,11 +13,18 @@
#include "Protocol.h"
#include "SourceCode.h"
#include "clang/Basic/AllDiagnostics.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/Support/Capacity.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Support/Signals.h"
#include <algorithm>
namespace clang {
@@ -102,6 +109,39 @@
return halfOpenToRange(M, R);
}
+void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
+ const LangOptions &LangOpts) {
+ const SourceLocation &DiagLoc = Info.getLocation();
+ const SourceManager &SM = Info.getSourceManager();
+ SourceLocation IncludeInMainFile;
+ auto GetIncludeLoc = [&SM](SourceLocation SLoc) {
+ return SM.getIncludeLoc(SM.getFileID(SLoc));
+ };
+ for (auto IncludeLocation = GetIncludeLoc(DiagLoc); IncludeLocation.isValid();
+ IncludeLocation = GetIncludeLoc(IncludeLocation))
+ IncludeInMainFile = IncludeLocation;
+ if (IncludeInMainFile.isInvalid())
+ return;
+
+ // Update diag to point at include inside main file.
+ D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
+ D.Range.start = sourceLocToPosition(SM, IncludeInMainFile);
+ D.Range.end = sourceLocToPosition(
+ SM, Lexer::getLocForEndOfToken(IncludeInMainFile, 0, SM, LangOpts));
+
+ // Add a note that will point to real diagnostic.
+ const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
+ D.Notes.emplace_back();
+ Note &N = D.Notes.back();
+ N.AbsFile = FE->tryGetRealPathName();
+ N.File = FE->getName();
+ N.Message = "error occurred here";
+ N.Range = diagnosticRange(Info, LangOpts);
+
+ // Update message to mention original file.
+ D.Message = llvm::Twine("in included file: ", D.Message).str();
+}
+
bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
}
@@ -378,7 +418,7 @@
Msg.resize(Rest.size());
};
CleanMessage(Diag.Message);
- for (auto& Note : Diag.Notes)
+ for (auto &Note : Diag.Notes)
CleanMessage(Note.Message);
continue;
}
@@ -477,6 +517,7 @@
LastDiag = Diag();
LastDiag->ID = Info.getID();
FillDiagBase(*LastDiag);
+ adjustDiagFromHeader(*LastDiag, Info, *LangOpts);
if (!Info.getFixItHints().empty())
AddFix(true /* try to invent a message instead of repeating the diag */);
@@ -511,11 +552,15 @@
void StoreDiags::flushLastDiag() {
if (!LastDiag)
return;
- if (mentionsMainFile(*LastDiag))
+ // Only keeps diagnostics inside main file or the first one coming from a
+ // header.
+ if (mentionsMainFile(*LastDiag) ||
+ (LastDiag->Severity >= DiagnosticsEngine::Level::Error &&
+ IncludeLinesWithErrors.insert(LastDiag->Range.start.line).second)) {
Output.push_back(std::move(*LastDiag));
- else
- vlog("Dropped diagnostic outside main file: {0}: {1}", LastDiag->File,
- LastDiag->Message);
+ } else {
+ vlog("Dropped diagnostic: {0}: {1}", LastDiag->File, LastDiag->Message);
+ }
LastDiag.reset();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
