This revision was automatically updated to reflect the committed changes.
Closed by commit rG6529b0c48aab: [clangd] Enable diagnostic fixes within macro
argument expansions. (authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D78338?vs=258202&id=258812#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78338/new/
https://reviews.llvm.org/D78338
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
@@ -104,6 +104,7 @@
// Check we report correct ranges, including various edge-cases.
Annotations Test(R"cpp(
// error-ok
+ #define ID(X) X
namespace test{};
void $decl[[foo]]();
class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@
struct Foo { int x; }; Foo a;
a.$nomember[[y]];
test::$nomembernamespace[[test]];
+ $macro[[ID($macroarg[[fod]])]]();
}
)cpp");
auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@
Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+ AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean
'foo'?"),
+ WithFix(Fix(Test.range("macroarg"), "foo",
+ "change 'fod' to 'foo'"))),
// We make sure here that the entire token is highlighted
AllOf(Diag(Test.range("constructor"),
"single-argument constructors must be marked explicit to "
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -556,10 +556,23 @@
if (!InsideMainFile)
return false;
+ // Copy as we may modify the ranges.
+ auto FixIts = Info.getFixItHints().vec();
llvm::SmallVector<TextEdit, 1> Edits;
- for (auto &FixIt : Info.getFixItHints()) {
- // Follow clang's behavior, don't apply FixIt to the code in macros,
- // we are less certain it is the right fix.
+ for (auto &FixIt : FixIts) {
+ // Allow fixits within a single macro-arg expansion to be applied.
+ // This can be incorrect if the argument is expanded multiple times in
+ // different contexts. Hopefully this is rare!
+ if (FixIt.RemoveRange.getBegin().isMacroID() &&
+ FixIt.RemoveRange.getEnd().isMacroID() &&
+ SM.getFileID(FixIt.RemoveRange.getBegin()) ==
+ SM.getFileID(FixIt.RemoveRange.getEnd())) {
+ FixIt.RemoveRange = CharSourceRange(
+ {SM.getTopMacroCallerLoc(FixIt.RemoveRange.getBegin()),
+ SM.getTopMacroCallerLoc(FixIt.RemoveRange.getEnd())},
+ FixIt.RemoveRange.isTokenRange());
+ }
+ // Otherwise, follow clang's behavior: no fixits in macros.
if (FixIt.RemoveRange.getBegin().isMacroID() ||
FixIt.RemoveRange.getEnd().isMacroID())
return false;
@@ -570,8 +583,8 @@
llvm::SmallString<64> Message;
// If requested and possible, create a message like "change 'foo' to
'bar'".
- if (SyntheticMessage && Info.getNumFixItHints() == 1) {
- const auto &FixIt = Info.getFixItHint(0);
+ if (SyntheticMessage && FixIts.size() == 1) {
+ const auto &FixIt = FixIts.front();
bool Invalid = false;
llvm::StringRef Remove =
Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, &Invalid);
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -104,6 +104,7 @@
// Check we report correct ranges, including various edge-cases.
Annotations Test(R"cpp(
// error-ok
+ #define ID(X) X
namespace test{};
void $decl[[foo]]();
class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@
struct Foo { int x; }; Foo a;
a.$nomember[[y]];
test::$nomembernamespace[[test]];
+ $macro[[ID($macroarg[[fod]])]]();
}
)cpp");
auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@
Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+ AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean 'foo'?"),
+ WithFix(Fix(Test.range("macroarg"), "foo",
+ "change 'fod' to 'foo'"))),
// We make sure here that the entire token is highlighted
AllOf(Diag(Test.range("constructor"),
"single-argument constructors must be marked explicit to "
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -556,10 +556,23 @@
if (!InsideMainFile)
return false;
+ // Copy as we may modify the ranges.
+ auto FixIts = Info.getFixItHints().vec();
llvm::SmallVector<TextEdit, 1> Edits;
- for (auto &FixIt : Info.getFixItHints()) {
- // Follow clang's behavior, don't apply FixIt to the code in macros,
- // we are less certain it is the right fix.
+ for (auto &FixIt : FixIts) {
+ // Allow fixits within a single macro-arg expansion to be applied.
+ // This can be incorrect if the argument is expanded multiple times in
+ // different contexts. Hopefully this is rare!
+ if (FixIt.RemoveRange.getBegin().isMacroID() &&
+ FixIt.RemoveRange.getEnd().isMacroID() &&
+ SM.getFileID(FixIt.RemoveRange.getBegin()) ==
+ SM.getFileID(FixIt.RemoveRange.getEnd())) {
+ FixIt.RemoveRange = CharSourceRange(
+ {SM.getTopMacroCallerLoc(FixIt.RemoveRange.getBegin()),
+ SM.getTopMacroCallerLoc(FixIt.RemoveRange.getEnd())},
+ FixIt.RemoveRange.isTokenRange());
+ }
+ // Otherwise, follow clang's behavior: no fixits in macros.
if (FixIt.RemoveRange.getBegin().isMacroID() ||
FixIt.RemoveRange.getEnd().isMacroID())
return false;
@@ -570,8 +583,8 @@
llvm::SmallString<64> Message;
// If requested and possible, create a message like "change 'foo' to 'bar'".
- if (SyntheticMessage && Info.getNumFixItHints() == 1) {
- const auto &FixIt = Info.getFixItHint(0);
+ if (SyntheticMessage && FixIts.size() == 1) {
+ const auto &FixIt = FixIts.front();
bool Invalid = false;
llvm::StringRef Remove =
Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, &Invalid);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
