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
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
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -81,7 +81,7 @@
// 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;
+ ParsedAST build(ParseOptions Opts = {}) const;
std::shared_ptr<const PreambleData>
preamble(PreambleParsedCallback PreambleCallback = nullptr) const;
ParseInputs inputs(MockFS &FS) const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -104,9 +104,10 @@
/*StoreInMemory=*/true, PreambleCallback);
}
-ParsedAST TestTU::build() const {
+ParsedAST TestTU::build(ParseOptions Opts) const {
MockFS FS;
auto Inputs = inputs(FS);
+ Inputs.Opts = Opts;
StoreDiags Diags;
auto CI = buildCompilerInvocation(Inputs, Diags);
assert(CI && "Failed to build compilation invocation.");
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -402,6 +402,31 @@
"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) { return static_cast<T&&>(t); }
+ template <typename T, typename... A> T* make_shared(A&&... args) {
+ return new T(std::forward<A>(args)...);
+ }
+ }
+ )cpp";
+ EXPECT_THAT(
+ *TU.build({/*PreambleParseForwardingFunctions=*/true}).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 {
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -498,6 +498,14 @@
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",
@@ -935,6 +943,7 @@
Opts.ClangTidyProvider = ClangTidyOptProvider;
}
Opts.UseDirtyHeaders = UseDirtyHeaders;
+ Opts.PreambleParseForwardingFunctions = PreambleParseForwardingFunctions;
Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
Opts.TweakFilter = [&](const Tweak &T) {
if (T.hidden() && !HiddenFeatures)
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -126,6 +126,8 @@
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);
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -66,9 +66,10 @@
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 @@
return IWYUHandler.get();
}
+ 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 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 @@
const clang::LangOptions *LangOpts = nullptr;
const SourceManager *SourceMgr = nullptr;
PreambleBuildStats *Stats;
+ bool ParseForwardingFunctions;
std::function<void(CompilerInstance &)> BeforeExecuteCallback;
};
@@ -483,11 +518,13 @@
// 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);
Index: clang-tools-extra/clangd/Compiler.h
===================================================================
--- clang-tools-extra/clangd/Compiler.h
+++ clang-tools-extra/clangd/Compiler.h
@@ -39,7 +39,7 @@
// 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.
Index: clang-tools-extra/clangd/ClangdServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -164,6 +164,9 @@
/// 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 @@
bool UseDirtyHeaders = false;
+ bool PreambleParseForwardingFunctions = false;
+
// GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
llvm::StringMap<llvm::Optional<FuzzyFindRequest>>
CachedCompletionFuzzyFindRequestByFile;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -148,7 +148,9 @@
: 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)) {
@@ -216,7 +218,7 @@
llvm::StringRef Version,
WantDiagnostics WantDiags, bool ForceRebuild) {
std::string ActualVersion = DraftMgr.addDraft(File, Version, Contents);
- ParseOptions Opts;
+ ParseOptions Opts{PreambleParseForwardingFunctions};
// Compile command is set asynchronously during update, as it can be slow.
ParseInputs Inputs;
@@ -910,7 +912,7 @@
// 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()));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
