Author: Sam McCall Date: 2020-04-20T21:18:31+02:00 New Revision: 6529b0c48aab83bdada1d21a79da13b0971d1aec
URL: https://github.com/llvm/llvm-project/commit/6529b0c48aab83bdada1d21a79da13b0971d1aec DIFF: https://github.com/llvm/llvm-project/commit/6529b0c48aab83bdada1d21a79da13b0971d1aec.diff LOG: [clangd] Enable diagnostic fixes within macro argument expansions. Summary: This seems like a pretty safe case, and common enough to be useful. Reviewers: hokein Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D78338 Added: Modified: clang-tools-extra/clangd/Diagnostics.cpp clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index 32f6a9bf776c..d72c2bd68ce8 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -556,10 +556,23 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, 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 + // diff erent 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 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, 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); diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp index f64f8e9901a9..026e145ed65d 100644 --- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp +++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp @@ -104,6 +104,7 @@ TEST(DiagnosticsTest, DiagnosticRanges) { // 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 @@ o]](); 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 @@ o]](); 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 " _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits