hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1746 + + if (DiagOpts.EmbedFixesInDiagnostics && !CodeActions.empty()) { + Diag.codeActions.emplace(CodeActions); ---------------- kadircet wrote: > we didn't have the empty check before, let's not introduce it now (i.e. we'll > still reply with an empty set of code actions rather than "none" when there > are no fixes known to clangd) yeah, I added this empty check to keep `fixits-embed-in-diagnostic.test` unchanged (we don't emit code-action for diagnostic notes), but I think it is fair enough to change it. ================ Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1751-1753 auto &FixItsForDiagnostic = LocalFixIts[Diag]; - llvm::copy(Fixes, std::back_inserter(FixItsForDiagnostic)); + llvm::move(std::move(CodeActions), + std::back_inserter(FixItsForDiagnostic)); ---------------- kadircet wrote: > i am not sure why this logic was appending to the vector rather than just > overwriting. but we shouldn't receive the same diagnostics from the callback > here, am i missing something? so what about just: > ``` > LocalFixIts[Diag] = std::move(CodeActions); > ``` Agreed, changed to overwriting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148783/new/ https://reviews.llvm.org/D148783 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits