kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Clangd ignores fixits if the diagnsotics is outside the main file (e.g.
a note on a declaration from a header), but the fix might still be inside the
main file (e.g. change the function call).
This patch changes the logic to retain fixes that touch main file, if the
diagnostic owning them is still inside main file, even if they are attached to a
note outside the main file.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122315
Files:
clang-tools-extra/clangd/Diagnostics.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
@@ -22,6 +22,7 @@
#include "support/Path.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticSema.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/TargetSelect.h"
#include "gmock/gmock.h"
@@ -1658,6 +1659,28 @@
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
}
+TEST(DiagnosticsTest, FixItFromHeader) {
+ llvm::StringLiteral Header(R"cpp(
+ void foo(int *);
+ void foo(int *, int);)cpp");
+ Annotations Source(R"cpp(
+ /*error-ok*/
+ void bar() {
+ int x;
+ $diag[[foo]]($fix[[]]x, 1);
+ })cpp");
+ TestTU TU;
+ TU.Code = Source.code().str();
+ TU.HeaderCode = Header.str();
+ EXPECT_THAT(
+ *TU.build().getDiagnostics(),
+ UnorderedElementsAre(AllOf(
+ Diag(Source.range("diag"), "no matching function for call to 'foo'"),
+ withFix(Fix(Source.range("fix"), "&",
+ "candidate function not viable: no known conversion from
"
+ "'int' to 'int *' for 1st argument; take the address of "
+ "the argument with &")))));
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -31,6 +31,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
+#include <cassert>
#include <cstddef>
#include <vector>
@@ -719,9 +720,9 @@
auto AddFix = [&](bool SyntheticMessage) -> bool {
assert(!Info.getFixItHints().empty() &&
"diagnostic does not have attached fix-its");
- if (!InsideMainFile)
+ // No point in generating fixes, if the diagnostic is for a different file.
+ if (!LastDiag->InsideMainFile)
return false;
-
// Copy as we may modify the ranges.
auto FixIts = Info.getFixItHints().vec();
llvm::SmallVector<TextEdit, 1> Edits;
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -22,6 +22,7 @@
#include "support/Path.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticSema.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/TargetSelect.h"
#include "gmock/gmock.h"
@@ -1658,6 +1659,28 @@
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
}
+TEST(DiagnosticsTest, FixItFromHeader) {
+ llvm::StringLiteral Header(R"cpp(
+ void foo(int *);
+ void foo(int *, int);)cpp");
+ Annotations Source(R"cpp(
+ /*error-ok*/
+ void bar() {
+ int x;
+ $diag[[foo]]($fix[[]]x, 1);
+ })cpp");
+ TestTU TU;
+ TU.Code = Source.code().str();
+ TU.HeaderCode = Header.str();
+ EXPECT_THAT(
+ *TU.build().getDiagnostics(),
+ UnorderedElementsAre(AllOf(
+ Diag(Source.range("diag"), "no matching function for call to 'foo'"),
+ withFix(Fix(Source.range("fix"), "&",
+ "candidate function not viable: no known conversion from "
+ "'int' to 'int *' for 1st argument; take the address of "
+ "the argument with &")))));
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -31,6 +31,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
+#include <cassert>
#include <cstddef>
#include <vector>
@@ -719,9 +720,9 @@
auto AddFix = [&](bool SyntheticMessage) -> bool {
assert(!Info.getFixItHints().empty() &&
"diagnostic does not have attached fix-its");
- if (!InsideMainFile)
+ // No point in generating fixes, if the diagnostic is for a different file.
+ if (!LastDiag->InsideMainFile)
return false;
-
// Copy as we may modify the ranges.
auto FixIts = Info.getFixItHints().vec();
llvm::SmallVector<TextEdit, 1> Edits;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
