hokein marked an inline comment as done.
hokein added inline comments.

================
Comment at: clang-tools-extra/clangd/Diagnostics.cpp:427
   }
+  // Deduplicate clang-tidy diagnostics -- some clang-tidy checks may emit
+  // duplicated messages due to various reasons (e.g. the check doesn't handle
----------------
sammccall wrote:
> hokein wrote:
> > sammccall wrote:
> > > This sorting/partitioning seems a bit heavyweight... what about
> > > 
> > > ```
> > > DenseSet<pair<Range, Message>> SeenDiagnostics;
> > > llvm::erase_if([&](const Diag &D) {
> > >   return !SeenDiagnostics.try_emplace(D.Range, D.Message).second;
> > > });
> > > ```
> > This is neat. DenseSet requires a hashvalue function of the value, which we 
> > don't have, switched to use `set`
> Can we add one instead? It'd be nice to get rid of the `operator<` on range, 
> use of `set` etc.
> I can do this is a followup if you prefer.
I'd leave it to a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64127



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to