[PATCH] D98505: [clangd] Propagate data in diagnostics

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang-tools-extra/clangd/Diagnostics.h:77
+  // list.
+  std::vector<llvm::json::Object> OpaqueData;
 };
----------------
Hmm, you've replaced the json::Array with an array of objects :-)
Any reason we can't just use llvm::json::Object here?

We don't really handle conflicts anyway, and I don't think having one tweak read another one's data out of this field is a real concern.


================
Comment at: clang-tools-extra/clangd/Protocol.cpp:616
+  if (auto *Data = Params.getAsObject()->getObject("data"))
+    R.data = std::move(*Data);
+  return O.map("range", R.range) && O.map("message", R.message) &&
----------------
std::move is a fancy way to spell copy here, since Params is const.

json::Value(*Data) (or can you just use mapOptOrNull?)


================
Comment at: clang-tools-extra/clangd/Protocol.h:413
+  /// textDocument.publishDiagnostics.dataSupport
+  bool DiagnosticDataSupport = false;
+
----------------
this is now unused


================
Comment at: clang-tools-extra/clangd/refactor/Tweak.h:71
+    /// Diagnostics related to this selection.
+    llvm::ArrayRef<clangd::Diagnostic> Diags;
     // FIXME: provide a way to get sources and ASTs for other files.
----------------
unrelated?
(well not quite, but neither populated or used in this patch)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98505