nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.
Thanks, this is nice low-hanging fruit.
================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:830
+ if (!ReplacementFixes.empty()) {
+ LastDiag->Fixes.insert(LastDiag->Fixes.end(), ReplacementFixes.begin(),
+ ReplacementFixes.end());
----------------
Do we maybe want to assert `Info.getFixItHints().empty()` here, so that if
clang later introduces its own fix-it it gets on our radar and we can decide
which one to prefer?
================
Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:52
+llvm::Optional<llvm::StringRef> getArgStr(const clang::Diagnostic &Info,
+ unsigned I) {
+ switch (Info.getArgKind(I)) {
----------------
nit: Index or Idx?
================
Comment at: clang-tools-extra/clangd/IncludeFixer.cpp:495
+ if (auto Edit = Inserter->insert(Name))
+ return {Fix{llvm::formatv("Include {0}", Name).str(), {std::move(*Edit)}}};
+ return {};
----------------
In the `note_include_header_or_declare` case, we could pull the second arg from
the diagnostic (the name of the symbol) and use it to provide the slightly more
detailed hint description that [fixUnresolvedName()
does](https://searchfox.org/llvm/rev/168bc7ce7e2ebe6527bf3fdd9262ef5c0deab4fc/clang-tools-extra/clangd/IncludeFixer.cpp#168).
Up to you if you think that's worth doing.
================
Comment at: clang-tools-extra/clangd/IncludeFixer.h:43
/// 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,
----------------
The added comment describes the behaviour of calling code, not the behaviour of
this method. Perhaps it would make more sense in the calling code?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114667/new/
https://reviews.llvm.org/D114667
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
