[clangd] parse all make_unique-like functions in preamble

[upsj]

Summary

I am working on support for forwarding parameter names in make_unique-like functions, first for inlay hints, later maybe for signature help.
For that to work generically, I'd like to parse all of these functions in the preamble. Not sure how this impacts performance on large codebases though.

[nridge]

I'll have a proper look at this when I get a chance but meanwhile adding Sam and Kadir who may have thoughts

[sammccall]

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)

clang-tools-extra/clangd/Preamble.cpp

177

this is a linear search, and so is the next line, let's just do it once :-)

177

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

[sammccall]

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  | 56763768
SelectionDAGISel.cpp | 37546764  | 37668564
internal_s.cc        | 59582216  | 60024876
internal_m.cc        | 192084984 | 193850560
internal_l.cc        | 365811816 | 368841388

I can't see any reason to think that RAM/CPU usage would be out of proportion to this.
A 1% regression here isn't trivial but seems worthwhile for significant functional improvements.
I suppose we can add:

• diagnostics (already with this patch)
• inlay hints
• signature help

WDYT about keeping this behind a flag until these are working?
For diagnostics alone, I'm not sure this is a good tradeoff (otherwise why would we restrict it to variadics? FWIW allowing all function templates to be parsed is +4% to preamble size for internal_l.cc.

I'm less concerned about the effects on the main file, partly because preambles are a bigger performance cliff, and partly because we're probably only regressing in cases where the user really is seeing benefits too.

[upsj]

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 have some pointers on what needs to be done to add a new flag? I am still kind of new to LLVM development :)

[sammccall]

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 command-line flag seems fine.

This needs to be passed from the main binary => ClangdServer => Preamble::build, as something like bool AlwaysParseForwardingFunctions.

I think the most appropriate place to pass this to Preamble::build is in ParseOptions defined in Compiler.h (currently empty) which is embedded in ParseInputs. Something like bool AlwaysParseForwardingFunctions.
And the usual way to pass command line flags into ClangdServer is via ClangdServer::Options.

See https://github.com/llvm/llvm-project/commit/e6be5c7cd6d227144f874623e2764890f80cad32 where we removed a couple of ParseOptions, I think you'd basically want the opposite of that.
(Sorry about the plumbing, configuration is always a pain).

For testing, you want the ability to turn this option on in TestTU. I'd just add ParseOptions TestTU::ParseOpts as a public member that your test can set, and use it from TestTU::inputs() instead of the current Inputs.Opts = ParseOptions();.

[nridge]

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 metric that's more noticeable for users.

[sammccall]

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 preamble sizes a good proxy for preamble build times as well? I suspect that may be the metric that's more noticeable for users.

Agree, yes I think they're a pretty good proxy for this sort of change which parses more/less code. (Preamble size more or less counting AST nodes).

[upsj]

Rewrote detection of forwarding functions in preamble, added --preamble-parse-forwarding flag

I ran clangd over bits/stdc++.h, and it seems like the condition works very well:

It picked up

• container::emplace and friends
• allocator::construct and friends
• make_unique, make_shared and friends
• some parts of functional
• some parts of tuple
• some parts of thread

most of which seem like good candidates for forwarding parameters.

In terms of runtime, I can't see any noticeable difference between parsing with or without --preamble-parse-forwarding, but of course, all of libstdc++ is not the most realistic scenario

[upsj]

clang-tools-extra/clangd/Preamble.cpp

197

Can PackTypePtr ever be nullptr?

[upsj]

Work around the currently broken isExpandedParameter, also fix the broken diff

[upsj]

accidentally pushed to the wrong revision, this is the previous version

[sammccall]

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 TEST(DiagnosticTest, MakeUnique) which checks the existing make_unique support in this way. We should have a similar function with a different name and verify that it does/doesn't produce diags depending on the flag. We should probably also have a check that two levels of forwarding works.

clang-tools-extra/clangd/Preamble.cpp

180

nit: reverse the order of the && here, the isStr should be cheaper to check

185

rather than inlining all this here, can you pull out a function isLikelyForwardingFunction(FunctionTemplateDecl*)?
That documents the high-level goal and moves that logic away from the flow control of the different types of decls/options that shouldSkipFunctionBody deals with. It also avoids the confusing "return false" when you found what you were looking for!

185

somewhere we need to explain *why* we're parsing these functions. Something like:

Usually we don't need to look inside the bodies of header functions to understand the program. However when forwarding function like emplace() forward their arguments to some other function, the interesting overload resolution happens inside the forwarding function's body. To provide more meaningful meaningful diagnostics, code completion, and parameter hints we should parse (and later instantiate) the bodies.

This could either be here, or possibly on the option inside ParseOptions.

186

nit: getTemplatedDecl().

(getAsFunction() is a loose do-what-I-mean helper, and you know the precise case you're dealing with here)

There's no need for a null check here, you can't have a FunctionTemplateDecl with no function.

190

nit: if (const auto* PET = dyn_cast<PackExpansionType>(LastParam->getType()))
and then use it below

193

again, this is a loose function where you can use a precise one.
PET->getPattern()

197

sure: template <class...T> int foo(T*...).
or even some other more exotic derived type.

I don't think we want to include that case, so a null check is fine, with another comment: // ... of the form T&&...

clang-tools-extra/clangd/Preamble.h

103 ↗

(On Diff #427854)

instead of passing this as a separate parameter, if you make it part of the ParseOptions then it's available in ParseInputs.
This reduces the amount of plumbing/noise needed.

clang-tools-extra/clangd/TUScheduler.h

208 ↗

(On Diff #427854)

can you change "make_unique" to "emplace"?
It's confusing if our example is the one function that gets parsed either way.

209 ↗

(On Diff #427854)

TUScheduler should not need to know about this option at all, it can be passed from ClangdServer through to buildPreamble by placing it in the ParseInputs, specifically in the ParseOptions struct.

clang-tools-extra/clangd/tool/ClangdMain.cpp

501

Preamble is a bit jargony for a command-line flag. And "forwarding" is dangling.
Maybe "parse-forwarding-functions"?
(Internally the name is fine though)

502

again, make_unique -> emplace

503

nit: use a trailing comma to get consistent formatting with the others (UseDirtyHeaders is an anomaly)

[upsj]

review updates:

• Pass ParseForwardingFunctions via ParseOptions
• Simplify check for forwarding function
• Add test checking diagnostics for make_shared

[upsj]

update test docs

[upsj]

@sammccall @nridge can you give this another look? :)

[nridge]

I looked through this patch, and it looks good to me! I will leave the final approval to Sam though.

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

416

I'm confused about this line: you say "show its body is not needed" but forward has a body here

[upsj]

remove std::forward bodies from diagnostic tests

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

416

Good catch, that was copy-pasted from std::make_unique above, but it doesn't seem to be necessary there either. I'll remove the bodies both.

[sammccall]

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 if you want it.

clang-tools-extra/clangd/ClangdServer.cpp

221–222

nit: rather assign
Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions

booleans in these opts structs are particularly prone to accidentally reordering and mis-assigning

One day we'll have designated initializers for this...

clang-tools-extra/clangd/Preamble.cpp

140

nit: this can be static, and I think it'd be slightly clearer (e.g. that this doesn't interact with the policy flag)

166

nit: only one "meaningful" :-)

clang-tools-extra/clangd/unittests/TestTU.h

87

Instead of adding a parameter here, we should add it to the TestTU struct and use it in inputs().

*many* things affect the behavior of build(), and tests would quickly become unreadable if we passed many of them as parameters. So we hold the line at zero :-)

[upsj]

review updates

[upsj]

@sammccall Feel free to merge with Author: Tobias Ribizel <[e-mail]>
Thanks for the vote of confidence, I'll apply once the forwarding stuff is done :)

clang-tools-extra/clangd/ClangdServer.h

struct Options {
 
     /// Enable preview of FoldingRanges feature.
     bool FoldingRanges = false;
 
     FeatureModuleSet *FeatureModules = nullptr;
     /// If true, use the dirty buffer contents when building Preambles.
     bool UseDirtyHeaders = false;
 
+    // If true, parse emplace-like functions in the preamble.
+    bool PreambleParseForwardingFunctions = false;
+
     explicit operator TUScheduler::Options() const;
   };
   // Sensible default options for use in tests.
   // Features like indexing must be enabled if desired.
   static Options optsForTest();
 
   /// Creates a new ClangdServer instance.
   ///
private:
   // Storage for merged views of the various indexes.
   std::vector<std::unique_ptr<SymbolIndex>> MergedIdx;
 
   // When set, provides clang-tidy options for a specific file.
   TidyProviderRef ClangTidyProvider;
 
   bool UseDirtyHeaders = false;
 
+  bool PreambleParseForwardingFunctions = false;
+
   // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
   llvm::StringMap<llvm::Optional<FuzzyFindRequest>>
       CachedCompletionFuzzyFindRequestByFile;
   mutable std::mutex CachedCompletionFuzzyFindRequestMutex;
 
   llvm::Optional<std::string> WorkspaceRoot;
   llvm::Optional<TUScheduler> WorkScheduler;
   // Invalidation policy used for actions that we assume are "transient".

clang-tools-extra/clangd/ClangdServer.cpp

 }
 
 ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
                            const ThreadsafeFS &TFS, const Options &Opts,
                            Callbacks *Callbacks)
     : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
       DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
       ClangTidyProvider(Opts.ClangTidyProvider),
-      UseDirtyHeaders(Opts.UseDirtyHeaders), WorkspaceRoot(Opts.WorkspaceRoot),
+      UseDirtyHeaders(Opts.UseDirtyHeaders),
+      PreambleParseForwardingFunctions(Opts.PreambleParseForwardingFunctions),
+      WorkspaceRoot(Opts.WorkspaceRoot),
       Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
                                           : TUScheduler::NoInvalidation),
       DirtyFS(std::make_unique<DraftStoreFS>(TFS, DraftMgr)) {
   // Pass a callback into `WorkScheduler` to extract symbols from a newly
   // parsed file and rebuild the file index synchronously each time an AST
   // is parsed.
   WorkScheduler.emplace(
       CDB, TUScheduler::Options(Opts),
for (auto &Mod : *FeatureModules)
       Mod.blockUntilIdle(Deadline::infinity());
   }
 }
 
 void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
                                llvm::StringRef Version,
                                WantDiagnostics WantDiags, bool ForceRebuild) {
   std::string ActualVersion = DraftMgr.addDraft(File, Version, Contents);
   ParseOptions Opts;
+  Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions;
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
   Inputs.TFS = &getHeaderFS();
   Inputs.Contents = std::string(Contents);
   Inputs.Version = std::move(ActualVersion);
   Inputs.ForceRebuild = ForceRebuild;
   Inputs.Opts = std::move(Opts);
void ClangdServer::getAST(PathRef File, llvm::Optional<Range> R,
   auto Action =
       [R, CB(std::move(CB))](llvm::Expected<InputsAndAST> Inputs) mutable {
         if (!Inputs)
           return CB(Inputs.takeError());
         if (!R) {
           // It's safe to pass in the TU, as dumpAST() does not
           // deserialize the preamble.
           auto Node = DynTypedNode::create(
-                *Inputs->AST.getASTContext().getTranslationUnitDecl());
           return CB(dumpAST(Node, Inputs->AST.getTokens(),
                             Inputs->AST.getASTContext()));
         }
         unsigned Start, End;
         if (auto Offset = positionToOffset(Inputs->Inputs.Contents, R->start))
           Start = *Offset;
         else
           return CB(Offset.takeError());

clang-tools-extra/clangd/Compiler.h

static void log(DiagnosticsEngine::Level DiagLevel,
                   const clang::Diagnostic &Info);
 
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const clang::Diagnostic &Info) override;
 };
 
 // Options to run clang e.g. when parsing AST.
 struct ParseOptions {
-  // (empty at present, formerly controlled recovery AST, include-fixer etc)
+  bool PreambleParseForwardingFunctions = false;
 };
 
 /// Information required to run clang, e.g. to parse AST or do code completion.
 struct ParseInputs {
   tooling::CompileCommand CompileCommand;
   const ThreadsafeFS *TFS;
   std::string Contents;
   // Version identifier for Contents, provided by the client and opaque to us.

clang-tools-extra/clangd/Preamble.cpp

bool compileCommandsAreEqual(const tooling::CompileCommand &LHS,
   return LHS.Directory == RHS.Directory && LHS.Filename == RHS.Filename &&
          llvm::makeArrayRef(LHS.CommandLine).equals(RHS.CommandLine);
 }
 
 class CppFilePreambleCallbacks : public PreambleCallbacks {
 public:
   CppFilePreambleCallbacks(
       PathRef File, PreambleParsedCallback ParsedCallback,
-      PreambleBuildStats *Stats,
+      PreambleBuildStats *Stats, bool ParseForwardingFunctions,
       std::function<void(CompilerInstance &)> BeforeExecuteCallback)
       : File(File), ParsedCallback(ParsedCallback), Stats(Stats),
+        ParseForwardingFunctions(ParseForwardingFunctions),
         BeforeExecuteCallback(std::move(BeforeExecuteCallback)) {}
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
   MainFileMacros takeMacros() { return std::move(Macros); }
 
   std::vector<PragmaMark> takeMarks() { return std::move(Marks); }
 
return std::make_unique<PPChainedCallbacks>(
         collectPragmaMarksCallback(*SourceMgr, Marks));
   }
 
   CommentHandler *getCommentHandler() override {
     IWYUHandler = collectIWYUHeaderMaps(&CanonIncludes);
     return IWYUHandler.get();
   }
 
+  static bool isLikelyForwardingFunction(FunctionTemplateDecl *FT) {
+    const auto *FD = FT->getTemplatedDecl();
+    const auto NumParams = FD->getNumParams();
+    // Check whether its last parameter is a parameter pack...
+    if (NumParams > 0) {
+      const auto *LastParam = FD->getParamDecl(NumParams - 1);
+      if (const auto *PET = dyn_cast<PackExpansionType>(LastParam->getType())) {
+        // ... of the type T&&... or T...
+        const auto BaseType = PET->getPattern().getNonReferenceType();
+        if (const auto *TTPT =
+                dyn_cast<TemplateTypeParmType>(BaseType.getTypePtr())) {
+          // ... whose template parameter comes from the function directly
+          if (FT->getTemplateParameters()->getDepth() == TTPT->getDepth()) {
+            return true;
+          }
+        }
+      }
+    }
+    return false;
+  }
+
   bool shouldSkipFunctionBody(Decl *D) override {
-    // Generally we skip function bodies in preambles for speed.
-    // We can make exceptions for functions that are cheap to parse and
-    // instantiate, widely used, and valuable (e.g. commonly produce errors).
-    if (const auto *FT = llvm::dyn_cast<clang::FunctionTemplateDecl>(D)) {
-      if (const auto *II = FT->getDeclName().getAsIdentifierInfo())
+    // Usually we don't need to look inside the bodies of header functions
+    // to understand the program. However when forwarding function like
+    // emplace() forward their arguments to some other function, the
+    // interesting overload resolution happens inside the forwarding
+    // function's body. To provide more meaningful diagnostics,
+    // code completion, and parameter hints we should parse (and later
+    // instantiate) the bodies.
+    if (auto *FT = llvm::dyn_cast<clang::FunctionTemplateDecl>(D)) {
+      if (ParseForwardingFunctions) {
+        // Don't skip parsing the body if it looks like a forwarding function
+        if (isLikelyForwardingFunction(FT))
+          return false;
+      } else {
+        // By default, only take care of make_unique
         // std::make_unique is trivial, and we diagnose bad constructor calls.
+        if (const auto *II = FT->getDeclName().getAsIdentifierInfo()) {
           if (II->isStr("make_unique") && FT->isInStdNamespace())
             return false;
         }
+      }
+    }
     return true;
   }
 
 private:
   PathRef File;
   PreambleParsedCallback ParsedCallback;
   IncludeStructure Includes;
   CanonicalIncludes CanonIncludes;
   MainFileMacros Macros;
   std::vector<PragmaMark> Marks;
   bool IsMainFileIncludeGuarded = false;
   std::unique_ptr<CommentHandler> IWYUHandler = nullptr;
   const clang::LangOptions *LangOpts = nullptr;
   const SourceManager *SourceMgr = nullptr;
   PreambleBuildStats *Stats;
+  bool ParseForwardingFunctions;
   std::function<void(CompilerInstance &)> BeforeExecuteCallback;
 };
 
 // Represents directives other than includes, where basic textual information is
 // enough.
 struct TextualPPDirective {
   unsigned DirectiveLine;
   // Full text that's representing the directive, including the `#`.
buildPreamble(PathRef FileName, CompilerInvocation CI,
   // Skip function bodies when building the preamble to speed up building
   // the preamble and make it smaller.
   assert(!CI.getFrontendOpts().SkipFunctionBodies);
   CI.getFrontendOpts().SkipFunctionBodies = true;
   // We don't want to write comment locations into PCH. They are racy and slow
   // to read back. We rely on dynamic index for the comments instead.
   CI.getPreprocessorOpts().WriteCommentListToPCH = false;
 
-  CppFilePreambleCallbacks CapturedInfo(FileName, PreambleCallback, Stats,
-                                        [&ASTListeners](CompilerInstance &CI) {
-                                          for (const auto &L : ASTListeners)
-                                            L->beforeExecute(CI);
-                                        });
+  CppFilePreambleCallbacks CapturedInfo(
+      FileName, PreambleCallback, Stats,
+      Inputs.Opts.PreambleParseForwardingFunctions,
   auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
   llvm::SmallString<32> AbsFileName(FileName);
   VFS->makeAbsolute(AbsFileName);
   auto StatCache = std::make_unique<PreambleFileStatusCache>(AbsFileName);
   auto StatCacheFS = StatCache->getProducingFS(VFS);
   llvm::IntrusiveRefCntPtr<TimerFS> TimedFS(new TimerFS(StatCacheFS));
 
   WallTimer PreambleTimer;

clang-tools-extra/clangd/tool/Check.cpp

public:
   // Prepare inputs and build CompilerInvocation (parsed compile command).
   bool buildInvocation(const ThreadsafeFS &TFS,
                        llvm::Optional<std::string> Contents) {
     StoreDiags CaptureInvocationDiags;
     std::vector<std::string> CC1Args;
     Inputs.CompileCommand = Cmd;
     Inputs.TFS = &TFS;
     Inputs.ClangTidyProvider = Opts.ClangTidyProvider;
+    Inputs.Opts.PreambleParseForwardingFunctions =
+        Opts.PreambleParseForwardingFunctions;
     if (Contents.hasValue()) {
       Inputs.Contents = *Contents;
       log("Imaginary source file contents:\n{0}", Inputs.Contents);
     } else {
       if (auto Contents = TFS.view(llvm::None)->getBufferForFile(File)) {
         Inputs.Contents = Contents->get()->getBuffer().str();
       } else {
         elog("Couldn't read {0}: {1}", File, Contents.getError().message());

clang-tools-extra/clangd/tool/ClangdMain.cpp

 };
 
 opt<bool> UseDirtyHeaders{"use-dirty-headers", cat(Misc),
                           desc("Use files open in the editor when parsing "
                                "headers instead of reading from the disk"),
                           Hidden,
                           init(ClangdServer::Options().UseDirtyHeaders)};
 
+opt<bool> PreambleParseForwardingFunctions{
+    "parse-forwarding-functions",
+    cat(Misc),
+    desc("Parse all emplace-like functions in included headers"),
+    Hidden,
+    init(ParseOptions().PreambleParseForwardingFunctions),
+};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt<bool> EnableMallocTrim{
     "malloc-trim",
     cat(Misc),
     desc("Release memory periodically via malloc_trim(3)."),
     init(true),
 };
 
if (EnableClangTidy) {
     if (EnableConfig)
       Providers.push_back(provideClangdConfig());
     Providers.push_back(provideDefaultChecks());
     Providers.push_back(disableUnusableChecks());
     ClangTidyOptProvider = combine(std::move(Providers));
     Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
   Opts.UseDirtyHeaders = UseDirtyHeaders;
+  Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak &T) {
     if (T.hidden() && !HiddenFeatures)
       return false;
     if (TweakList.getNumOccurrences())
       return llvm::is_contained(TweakList, T.id());
     return true;
   };

clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Annotations Main(R"cpp(
     struct S { S(char*); };
     auto x = std::[[make_unique]]<S>(42); // error-ok
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
   TU.HeaderCode = R"cpp(
     namespace std {
     // These mocks aren't quite right - we omit unique_ptr for simplicity.
     // forward is included to show its body is not needed to get the diagnostic.
-    template <typename T> T&& forward(T& t) { return static_cast<T&&>(t); }
+    template <typename T> T&& forward(T& t);
     template <typename T, typename... A> T* make_unique(A&&... args) {
        return new T(std::forward<A>(args)...);
     }
     }
   )cpp";
   EXPECT_THAT(*TU.build().getDiagnostics(),
               UnorderedElementsAre(
                   Diag(Main.range(),
                        "in template: "
                        "no matching constructor for initialization of 'S'")));
 }
 
+TEST(DiagnosticTest, MakeShared) {
+  // We usually miss diagnostics from header functions as we don't parse them.
+  // std::make_shared is only parsed when --parse-forwarding-functions is set
+  Annotations Main(R"cpp(
+    struct S { S(char*); };
+    auto x = std::[[make_shared]]<S>(42); // error-ok
+  )cpp");
+  TestTU TU = TestTU::withCode(Main.code());
+  TU.HeaderCode = R"cpp(
+    namespace std {
+    // 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 <typename T> T&& forward(T& t);
+    template <typename T, typename... A> T* make_shared(A&&... args) {
+       return new T(std::forward<A>(args)...);
+    }
+    }
+  )cpp";
+  TU.ParseOpts.PreambleParseForwardingFunctions = true;
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+              UnorderedElementsAre(
+                  Diag(Main.range(),
+                       "in template: "
+                       "no matching constructor for initialization of 'S'")));
+}
+
 TEST(DiagnosticTest, NoMultipleDiagnosticInFlight) {
   Annotations Main(R"cpp(
     template <typename T> struct Foo {
       T *begin();
       T *end();
     };
     struct LabelInfo {
       int a;

clang-tools-extra/clangd/unittests/TestTU.h

struct TestTU {
 
   TidyProvider ClangTidyProvider = {};
   // Index to use when building AST.
   const SymbolIndex *ExternalIndex = nullptr;
 
   // Simulate a header guard of the header (using an #import directive).
   bool ImplicitHeaderGuard = true;
 
+  // Parse options pass on to the ParseInputs
+  ParseOptions ParseOpts = {};
+
   // Whether to use overlay the TestFS over the real filesystem. This is
   // required for use of implicit modules.where the module file is written to
   // disk and later read back.
   // FIXME: Change the way reading/writing modules work to allow us to keep them
   // in memory across multiple clang invocations, at least in tests, to
   // eliminate the need for real file system here.
   // Please avoid using this for things other than implicit modules. The plan is
   // to eliminate this option some day.
   bool OverlayRealFileSystemForModules = false;
 
   FeatureModuleSet *FeatureModules = nullptr;
 
   // By default, build() will report Error diagnostics as GTest errors.
   // Suppress this behavior by adding an 'error-ok' comment to the code.
   // The result will always have getDiagnostics() populated.
   ParsedAST build() const;
   std::shared_ptr<const PreambleData>
   preamble(PreambleParsedCallback PreambleCallback = nullptr) const;
   ParseInputs inputs(MockFS &FS) const;
   SymbolSlab headerSymbols() const;
   RefSlab headerRefs() const;
   std::unique_ptr<SymbolIndex> index() const;
 };
 

clang-tools-extra/clangd/unittests/TestTU.cpp

auto ModuleCacheDeleter = llvm::make_scope_exit(
       std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath));
   return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
                                       /*StoreInMemory=*/true, PreambleCallback);
 }
 
 ParsedAST TestTU::build() const {
   MockFS FS;
   auto Inputs = inputs(FS);
+  Inputs.Opts = ParseOpts;
   StoreDiags Diags;
   auto CI = buildCompilerInvocation(Inputs, Diags);
   assert(CI && "Failed to build compilation invocation.");
   if (OverlayRealFileSystemForModules)
     initializeModuleCache(*CI);
   auto ModuleCacheDeleter = llvm::make_scope_exit(
       std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath));