[PATCH] D155173: [clangd] Refine the workflow for diagnostic Fixits.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 18 04:46:02 PDT 2023


kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1014-1016
+  llvm::ArrayRef<clangd::Diagnostic> LSPDiags = Params.context.diagnostics;
+  std::map<ClangdServer::DiagRef, /*index of LSPDiags*/ unsigned>
+      ToLSPDiagsIndex;
----------------
we're already making a copy of `Params.context.diagnostics` when creating the `CB`.

so you might as well have a `std::map<ClangdServer::DiagRef, clangd::Diagnostic> RefsToDiags;` and get rid of the extra index conversions.


================
Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:240
+  /// A map from LSP diagnostic to clangd-naive diagnostic.
+  typedef std::map<clangd::Diagnostic, ClangdServer::DiagRef,
                    LSPDiagnosticCompare>
----------------
instead of storing `clangd::Diagnostic` as the key, can we have a `DiagnosticKey` struct here (we can have a helper `diagKey(const clangd::Diagnostic&) -> DiagnosticKey`)? to make sure we don't get into trouble if there are too many "big" diagnostics :)

that way we can also get rid of the custom comparator.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155173/new/

https://reviews.llvm.org/D155173



More information about the cfe-commits mailing list