hokein added inline comments.
================ Comment at: clang-tools-extra/clangd/ClangdServer.cpp:671 + // FIMXE: this is tricky + llvm::StringRef(LSPDiag.Message) + .starts_with_insensitive(Diag.Message)) ---------------- hokein wrote: > kadircet wrote: > > this is tricky indeed and conceptually really hard to achieve inside > > ClangdServer layer. > > > > what about keeping the cache in ClangdLSPServer, but changing the format? > > Similar to `TweakRef`, we now have a `DiagRef`, which is > > ClangdServer-native. So we can keep a cache from `(FilePath, > > clangd::Diagnostic)` to `clangd::DiagRef`, and pass those `DiagRef`s to > > `ClangdServer` and make sure we're doing the search for sure on the right > > domain here? > > > > this also gives us the flexibility to change the definition of a `DiagRef` > > in the future. > I'm not a fan of keeping a cache in `ClangdLSPServer`, I'd like to remove it > entirely. > > What do you think about this alternative? > > - pull out and exposed the `mainMessage` API from the `toLSPDiags` > - we add the `ClangdDiagnosticOptions` to the `CodeActionInputs`. > - when searching a diagnostic in `ClangdServer.cpp`, we check the equality by > checking `mainMessage(ClangdServerDiagnostic.message, Inputs.DiagOpts) == > DiagRef.Message` > > The downside here is that we have to pay `O(N * cost of mainMessage)` to find > a matched diagnostic. > I updated the patch with the `mainMessage` approach, please take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155173/new/ https://reviews.llvm.org/D155173 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
