kadircet marked an inline comment as done. kadircet added inline comments.
================ Comment at: clang-tools-extra/clangd/Preamble.cpp:281 + // We are only interested in newly added includes. + llvm::StringSet<> ExistingIncludes; + for (const auto &Inc : Preamble.LexedIncludes) ---------------- sammccall wrote: > kadircet wrote: > > sammccall wrote: > > > Why not a DenseSet<pair<PPKeywordKind, StringRef>>? > > > (The copies probably don't matter, but I think it'd be a bit clearer and > > > more typesafe) > > PPKeywordKind didn't have a DenseMapInfo, adding one. It already has two > > invalid enum values. > Ugh, I thought enums had those implicitly. The need for two invalid values is > really annoying, it's probably why we don't have more implicit ones. > > I wonder whether it's feasible (technically and legally) to replace > DenseHashMap with a fork of absl::flat_hash_map. I'm pretty sure it's faster, > and it doesn't have these weird API requirements. IANAL, but they both seemed to have apache v2 :D as for technicalities, how hard can it be to replace one of the most basic data structures in a code base :P Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77392/new/ https://reviews.llvm.org/D77392 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
