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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
