sammccall created this revision.
sammccall added a reviewer: nridge.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay, ilya-biryukov.
Herald added a reviewer: jdoerfert.
Herald added a project: clang-tools-extra.
Clang doesn't offer these fixes I guess for a couple of reasons:
- where to insert includes is a formatting concern, and clang shouldn't depend
on clang-format
- the way clang prints diagnostics, we'd show a bunch of basically irrelevant
context of "this is where we'd want to insert the include"
Maybe it's possible to hack around 1, but 2 is still a concern.
Meanwhile, bolting this onto include-fixer gets the job done.
Fixes https://github.com/clangd/clangd/issues/355
Fixes https://github.com/clangd/clangd/issues/937
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114667
Files:
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/IncludeFixer.h
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
@@ -1034,7 +1034,7 @@
"Add include \"b.h\" for symbol na::nb::X")))));
}
-TEST(IncludeFixerTest, NoCrashMemebrAccess) {
+TEST(IncludeFixerTest, NoCrashMemberAccess) {
Annotations Test(R"cpp(// error-ok
struct X { int xyz; };
void g() { X x; x.$[[xy]]; }
@@ -1206,6 +1206,26 @@
ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
}
+TEST(IncludeFixerTest, HeaderNamedInDiag) {
+ Annotations Test(R"cpp(
+ $insert[[]]int main() {
+ [[printf]]("");
+ }
+ )cpp");
+ auto TU = TestTU::withCode(Test.code());
+ TU.ExtraArgs = {"-xc"};
+ auto Index = buildIndexWithSymbol({});
+ TU.ExternalIndex = Index.get();
+
+ EXPECT_THAT(
+ *TU.build().getDiagnostics(),
+ ElementsAre(AllOf(
+ Diag(Test.range(), "implicitly declaring library function 'printf' "
+ "with type 'int (const char *, ...)'"),
+ WithFix(Fix(Test.range("insert"), "#include <stdio.h>\n",
+ "Include <stdio.h>")))));
+}
+
TEST(DiagsInHeaders, DiagInsideHeader) {
Annotations Main(R"cpp(
#include [["a.h"]]
Index: clang-tools-extra/clangd/IncludeFixer.h
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.h
+++ clang-tools-extra/clangd/IncludeFixer.h
@@ -40,6 +40,7 @@
IndexRequestLimit(IndexRequestLimit) {}
/// Returns include insertions that can potentially recover the diagnostic.
+ /// If Info describes a note, it will be replaced by any returned fixes.
std::vector<Fix> fix(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) const;
@@ -55,6 +56,8 @@
/// Generates header insertion fixes for all symbols. Fixes are deduplicated.
std::vector<Fix> fixesForSymbols(const SymbolSlab &Syms) const;
+ std::vector<Fix> fixMissingHeader(llvm::StringRef Name) const;
+
struct UnresolvedName {
std::string Name; // E.g. "X" in foo::X.
SourceLocation Loc; // Start location of the unresolved name.
Index: clang-tools-extra/clangd/IncludeFixer.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeFixer.cpp
+++ clang-tools-extra/clangd/IncludeFixer.cpp
@@ -47,6 +47,19 @@
namespace clang {
namespace clangd {
+namespace {
+llvm::Optional<llvm::StringRef> getArgStr(const clang::Diagnostic &Info,
+ unsigned I) {
+ switch (Info.getArgKind(I)) {
+ case DiagnosticsEngine::ak_c_string:
+ return llvm::StringRef(Info.getArgCStr(I));
+ case DiagnosticsEngine::ak_std_string:
+ return llvm::StringRef(Info.getArgStdStr(I));
+ default:
+ return llvm::None;
+ }
+}
+} // namespace
std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) const {
@@ -102,7 +115,32 @@
LastUnresolvedName->Loc == Info.getLocation())
return fixUnresolvedName();
}
+ break;
+ // Cases where clang explicitly knows which header to include.
+ // (There's no fix provided for boring formatting reasons).
+ case diag::err_implied_std_initializer_list_not_found:
+ return fixMissingHeader("<initializer_list>");
+ case diag::err_need_header_before_typeid:
+ return fixMissingHeader("<typeid>");
+ case diag::err_need_header_before_ms_uuidof:
+ return fixMissingHeader("<guiddef.h>");
+ case diag::err_need_header_before_placement_new:
+ case diag::err_implicit_coroutine_std_nothrow_type_not_found:
+ return fixMissingHeader("<new>");
+ case diag::err_omp_implied_type_not_found:
+ case diag::err_omp_interop_type_not_found:
+ return fixMissingHeader("<omp.h>");
+ case diag::err_implied_coroutine_type_not_found:
+ return fixMissingHeader("<coroutine>");
+ case diag::err_implied_comparison_category_type_not_found:
+ return fixMissingHeader("<compare>");
+ case diag::note_include_header_or_declare:
+ if (Info.getNumArgs() > 0)
+ if (auto Header = getArgStr(Info, 0))
+ return fixMissingHeader(("<" + *Header + ">").str());
+ break;
}
+
return {};
}
@@ -452,5 +490,11 @@
return &E.first->second;
}
+std::vector<Fix> IncludeFixer::fixMissingHeader(llvm::StringRef Name) const {
+ if (auto Edit = Inserter->insert(Name))
+ return {Fix{llvm::formatv("Include {0}", Name).str(), {std::move(*Edit)}}};
+ return {};
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -823,6 +823,16 @@
if (LastDiag->Severity == DiagnosticsEngine::Ignored)
return;
+ // Give include-fixer a chance to replace a note with a fix.
+ if (Fixer) {
+ auto ReplacementFixes = Fixer(LastDiag->Severity, Info);
+ if (!ReplacementFixes.empty()) {
+ LastDiag->Fixes.insert(LastDiag->Fixes.end(), ReplacementFixes.begin(),
+ ReplacementFixes.end());
+ return;
+ }
+ }
+
if (!Info.getFixItHints().empty()) {
// A clang note with fix-it is not a separate diagnostic in clangd. We
// attach it as a Fix to the main diagnostic instead.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
