[PATCH] D114667: [clangd] Add fixes for clang "include <foo.h>" diagnostics

Nathan Ridge via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 29 00:50:21 PST 2021

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?

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list