[clang-tools-extra] 71cb8c8 - [clangd] parse all make_unique-like functions in preamble
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon May 16 02:23:46 PDT 2022
Author: Tobias Ribizel
Date: 2022-05-16T11:17:25+02:00
New Revision: 71cb8c8cb9c162448159f2bffd2c77a8ee82d71f
URL: https://github.com/llvm/llvm-project/commit/71cb8c8cb9c162448159f2bffd2c77a8ee82d71f
DIFF: https://github.com/llvm/llvm-project/commit/71cb8c8cb9c162448159f2bffd2c77a8ee82d71f.diff
LOG: [clangd] parse all make_unique-like functions in preamble
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.
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D124688
Added:
Modified:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/Compiler.h
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index f9ac911aaf5a..80d7d5c5ece1 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -148,7 +148,9 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
: 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)) {
@@ -217,6 +219,7 @@ void ClangdServer::addDocument(PathRef File, llvm::StringRef Contents,
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;
@@ -910,7 +913,7 @@ void ClangdServer::getAST(PathRef File, llvm::Optional<Range> R,
// It's safe to pass in the TU, as dumpAST() does not
// deserialize the preamble.
auto Node = DynTypedNode::create(
- *Inputs->AST.getASTContext().getTranslationUnitDecl());
+ *Inputs->AST.getASTContext().getTranslationUnitDecl());
return CB(dumpAST(Node, Inputs->AST.getTokens(),
Inputs->AST.getASTContext()));
}
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index a37b3c97d911..d1854c5f903e 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -164,6 +164,9 @@ class ClangdServer {
/// 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.
@@ -416,6 +419,8 @@ class ClangdServer {
bool UseDirtyHeaders = false;
+ bool PreambleParseForwardingFunctions = false;
+
// GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
llvm::StringMap<llvm::Optional<FuzzyFindRequest>>
CachedCompletionFuzzyFindRequestByFile;
diff --git a/clang-tools-extra/clangd/Compiler.h b/clang-tools-extra/clangd/Compiler.h
index ca856a8bde9a..4ddb737c543e 100644
--- a/clang-tools-extra/clangd/Compiler.h
+++ b/clang-tools-extra/clangd/Compiler.h
@@ -39,7 +39,7 @@ class IgnoreDiagnostics : public DiagnosticConsumer {
// 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.
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index 81d7c463ed7a..9fc249afcc22 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -66,9 +66,10 @@ 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); }
@@ -136,15 +137,48 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
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())
- // std::make_unique is trivial, and we diagnose bad constructor calls.
- if (II->isStr("make_unique") && FT->isInStdNamespace())
+ // 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;
}
@@ -161,6 +195,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
const clang::LangOptions *LangOpts = nullptr;
const SourceManager *SourceMgr = nullptr;
PreambleBuildStats *Stats;
+ bool ParseForwardingFunctions;
std::function<void(CompilerInstance &)> BeforeExecuteCallback;
};
@@ -484,11 +519,13 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
// 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,
+ [&ASTListeners](CompilerInstance &CI) {
+ for (const auto &L : ASTListeners)
+ L->beforeExecute(CI);
+ });
auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
llvm::SmallString<32> AbsFileName(FileName);
VFS->makeAbsolute(AbsFileName);
diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index 64df1390175b..7b73215364a7 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -128,6 +128,8 @@ class Checker {
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);
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 4e3005c15480..d86b5dab51e9 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -497,6 +497,14 @@ opt<bool> UseDirtyHeaders{"use-dirty-headers", cat(Misc),
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",
@@ -934,6 +942,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
Opts.ClangTidyProvider = ClangTidyOptProvider;
}
Opts.UseDirtyHeaders = UseDirtyHeaders;
+ Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions;
Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
Opts.TweakFilter = [&](const Tweak &T) {
if (T.hidden() && !HiddenFeatures)
diff --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index f6c347a919ed..50794f1a584e 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -389,7 +389,7 @@ TEST(DiagnosticTest, MakeUnique) {
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)...);
}
@@ -402,6 +402,32 @@ TEST(DiagnosticTest, MakeUnique) {
"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 {
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 78a8406c7911..c47ed2910baa 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -109,6 +109,7 @@ TestTU::preamble(PreambleParsedCallback PreambleCallback) const {
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.");
diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h
index 2140017af297..b045efb727c7 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.h
+++ b/clang-tools-extra/clangd/unittests/TestTU.h
@@ -66,6 +66,9 @@ struct TestTU {
// 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.
More information about the cfe-commits
mailing list