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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG71cb8c8cb9c1: [clangd] parse all make_unique-like functions in preamble (authored by upsj, committed by sammccall). Repository: rG LLVM Github

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

upsj added a comment. @sammccall Feel free to merge with Author: Tobias Ribizel Thanks for the vote of confidence, I'll apply once the forwarding stuff is done :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/

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

upsj updated this revision to Diff 429653. upsj marked 4 inline comments as done. upsj added a comment. review updates Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/ https://reviews.llvm.org/D124688 Files:

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

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Thanks, this looks good now! A last few nits here. Would you like me to commit for you? From your patches so far, I think it's appropriate to request commit access

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

upsj added inline comments. Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:416 +// These mocks aren't quite right - we omit shared_ptr for simplicity. +// forward is included to show its body is not needed to get the diagnostic. +template T&&

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

upsj updated this revision to Diff 429630. upsj marked an inline comment as done. upsj added a comment. remove std::forward bodies from diagnostic tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/ https://reviews.llvm.org/D124688

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

nridge added a comment. I looked through this patch, and it looks good to me! I will leave the final approval to Sam though. Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:416 +// These mocks aren't quite right - we omit shared_ptr for simplicity. +

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

upsj added a comment. @sammccall @nridge can you give this another look? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/ https://reviews.llvm.org/D124688 ___ cfe-commits mailing list

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

upsj updated this revision to Diff 427911. upsj added a comment. update test docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/ https://reviews.llvm.org/D124688 Files: clang-tools-extra/clangd/ClangdServer.cpp

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

upsj updated this revision to Diff 427910. upsj marked 13 inline comments as done. upsj added a comment. review updates: - Pass ParseForwardingFunctions via ParseOptions - Simplify check for forwarding function - Add test checking diagnostics for make_shared Repository: rG LLVM Github

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

sammccall added a comment. Thanks, this looks good! - the plumbing can be simplified by moving the option into ParseOptions, sorry about the churn! - we need to add some tests, probably easiest by checking for the diagnostics that are now produced. In `unittests/DiagnosticsTests.cpp` there's

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

upsj updated this revision to Diff 427854. upsj added a comment. accidentally pushed to the wrong revision, this is the previous version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/ https://reviews.llvm.org/D124688 Files:

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

upsj updated this revision to Diff 427852. upsj added a comment. Work around the currently broken isExpandedParameter, also fix the broken diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/ https://reviews.llvm.org/D124688 Files:

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

upsj added inline comments. Comment at: clang-tools-extra/clangd/Preamble.cpp:167 +// ... whose template parameter comes from the function directly +if (FT->getTemplateParameters()->getDepth() == +PackTypePtr->getDepth()) {

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

upsj updated this revision to Diff 426387. upsj marked 2 inline comments as done. upsj added a comment. Herald added a subscriber: javed.absar. Rewrote detection of forwarding functions in preamble, added `--preamble-parse-forwarding` flag I ran clangd over `bits/stdc++.h`, and it seems like

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

sammccall added a comment. In D124688#3485042 , @nridge wrote: > In D124688#3483895 , @sammccall > wrote: > >> Trying this on a few files, this seems like it increases preamble sizes up >> to 1% or so > > Are

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

nridge added a comment. In D124688#3483895 , @sammccall wrote: > Trying this on a few files, this seems like it increases preamble sizes up to > 1% or so Are preamble sizes a good proxy for preamble build times as well? I suspect that may be the

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

sammccall added a comment. Great! If this were to be a public, user-controlled feature we'd make it part of the config file which is a bit more involved, but I think this is rather just a developer toggle until it's time to turn it on by default, so a

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

upsj added a comment. Thanks for checking! Putting it behind a flag was my intention from the start anyways, since ideally I would like to traverse the AST through something like emplace_back ->construct/realloc_insert -> allocator::construct until I reach a non-forwarding function. Do you

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

sammccall added a comment. Trying this on a few files, this seems like it increases preamble sizes up to 1% or so: | Before| After -+---+-- AST.cpp | 42249648 | 42419732 XRefsTests.cpp | 56525116 |

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

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

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

nridge added reviewers: sammccall, kadircet. nridge added a comment. I'll have a proper look at this when I get a chance but meanwhile adding Sam and Kadir who may have thoughts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124688/new/

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

upsj created this revision. upsj added a reviewer: nridge. Herald added subscribers: usaxena95, kadircet, arphaman. Herald added a project: All. upsj requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. I am