ilya-biryukov marked 8 inline comments as done.
ilya-biryukov added inline comments.
================
Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:41
+ if (!SuffixHeaderMapping)
+ return Header;
----------------
sammccall wrote:
> nit: can we write `if (SuffixHeaderMapping) { ... }` for consistency with
> above?
I'd prefer less nesting to consistency, but happy let me know if you feel
that's very important.
================
Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:84
+ llvm::StringMap<std::string> SuffixHeaderMapping;
+ int MaxSuffixComponents = 0;
+};
----------------
sammccall wrote:
> so this is a constant, and it's 3.
>
> Can we avoid the calculation/propagation and defining a struct, and just add
> an assert(llvm::all_of(...)) after the initialization?
The assertion is inside the loop that's adding the mappings instead of
`llvm::all_of`.
================
Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:758
+void CanonicalIncludes::addSystemHeadersMapping(const LangOptions &Language) {
+ static constexpr std::pair<const char *, const char *> SymbolMap[] = {
+#define SYMBOL(Name, NameSpace, Header) {#NameSpace #Name, #Header},
----------------
sammccall wrote:
> if you want to save CPU, move to the scope where it's used? Lit tests and
> many interesting subsets will never use C.
Done. Also initialized everything directly as StringMap
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67172/new/
https://reviews.llvm.org/D67172
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
