[PATCH] D124688: [clangd] parse all make_unique-like functions in preamble

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Apr 30 03:00:48 PDT 2022


sammccall added a subscriber: adamcz.
sammccall added a comment.

It was intentional to only (initially) support std::make_unique as its implementation is trivial. Whereas I looked at std::make_shared and it appears to result in instantiating a lot of templates.
This was more "I'm sure this is useful and cheap" than "I'm sure covering more functions is too expensive" though.

I suspect that parsing all forwarding function bodies is quite expensive (well, parsing them => instantiating them => instantiating everything from their bodies).
We can measure this in a couple of ways:

- some work will happen while parsing headers, this will show up as increased preamble size which is easy to measure reliably
- some will only happen when parsing main files, this will cause increased RAM and latency in main-file parses, which is less reproducible

I'll try patching this and at least see what happens to preamble sizes on some big files

@adamcz FYI (Adam was looking at forwarding functions, for completion/signature help, but isn't anymore I believe)



================
Comment at: clang-tools-extra/clangd/Preamble.cpp:144
+      // ... with a template parameter pack...
+      if (FT->getTemplateParameters()->hasParameterPack()) {
+        auto PackIt = std::find_if(
----------------
this is a linear search, and so is the next line, let's just do it once :-)


================
Comment at: clang-tools-extra/clangd/Preamble.cpp:144
+      // ... with a template parameter pack...
+      if (FT->getTemplateParameters()->hasParameterPack()) {
+        auto PackIt = std::find_if(
----------------
sammccall wrote:
> this is a linear search, and so is the next line, let's just do it once :-)
I feel that checking template params -> injected template args -> params is a bit roundabout. (And it involves some searching and construction of the injected template args).

Possibly more direct, and I think equivalent:
- get the last function param, check that it's a pack and unwrap
- check that its (non-ref) type is a TemplateTypeParmType
- verify that the param is from the function template rather than elsewhere, by comparing type->getDepth() == funcTemplate->getTemplateParameters()->getDepth()
I think all these are constant-time, too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124688



More information about the cfe-commits mailing list