ilya-biryukov updated this revision to Diff 148413.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
- Invert flag, remove the default.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47274
Files:
clangd/CodeComplete.cpp
clangd/CodeCompletionStrings.cpp
clangd/CodeCompletionStrings.h
clangd/index/SymbolCollector.cpp
unittests/clangd/CodeCompleteTests.cpp
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -17,6 +17,7 @@
#include "SyncAPI.h"
#include "TestFS.h"
#include "index/MemIndex.h"
+#include "llvm/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -998,6 +999,46 @@
}
}
+TEST(CompletionTest, DocumentationFromChangedFileCrash) {
+ MockFSProvider FS;
+ auto FooH = testPath("foo.h");
+ auto FooCpp = testPath("foo.cpp");
+ FS.Files[FooH] = R"cpp(
+ // this is my documentation comment.
+ int func();
+ )cpp";
+ FS.Files[FooCpp] = "";
+
+ MockCompilationDatabase CDB;
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+ Annotations Source(R"cpp(
+ #include "foo.h"
+ int func() {
+ // This makes sure we have func from header in the AST.
+ }
+ int a = fun^
+ )cpp");
+ Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+ // We need to wait for preamble to build.
+ ASSERT_TRUE(Server.blockUntilIdleForTest());
+
+ // Change the header file. Completion will reuse the old preamble!
+ FS.Files[FooH] = R"cpp(
+ int func();
+ )cpp";
+
+ clangd::CodeCompleteOptions Opts;
+ Opts.IncludeComments = true;
+ CompletionList Completions =
+ cantFail(runCodeComplete(Server, FooCpp, Source.point(), Opts));
+ // We shouldn't crash. Unfortunately, current workaround is to not produce
+ // comments for symbols from headers.
+ EXPECT_THAT(Completions.items,
+ Contains(AllOf(Not(IsDocumented()), Named("func"))));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -381,7 +381,8 @@
/*EnableSnippets=*/false);
std::string FilterText = getFilterText(*CCS);
std::string Documentation =
- formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion));
+ formatDocumentation(*CCS, getDocComment(Ctx, SymbolCompletion,
+ /*CommentsFromHeaders=*/true));
std::string CompletionDetail = getDetail(*CCS);
std::string Include;
Index: clangd/CodeCompletionStrings.h
===================================================================
--- clangd/CodeCompletionStrings.h
+++ clangd/CodeCompletionStrings.h
@@ -25,18 +25,25 @@
/// markers stripped. See clang::RawComment::getFormattedText() for the detailed
/// explanation of how the comment text is transformed.
/// Returns empty string when no comment is available.
+/// If \p CommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
std::string getDocComment(const ASTContext &Ctx,
- const CodeCompletionResult &Result);
+ const CodeCompletionResult &Result,
+ bool CommentsFromHeaders);
/// Gets a minimally formatted documentation for parameter of \p Result,
/// corresponding to argument number \p ArgIndex.
/// This currently looks for comments attached to the parameter itself, and
/// doesn't extract them from function documentation.
/// Returns empty string when no comment is available.
+/// If \p CommentsFromHeaders parameter is set, only comments from the main
+/// file will be returned. It is used to workaround crashes when parsing
+/// comments in the stale headers, coming from completion preamble.
std::string
getParameterDocComment(const ASTContext &Ctx,
const CodeCompleteConsumer::OverloadCandidate &Result,
- unsigned ArgIndex);
+ unsigned ArgIndex, bool CommentsFromHeaders);
/// Gets label and insert text for a completion item. For example, for function
/// `Foo`, this returns <"Foo(int x, int y)", "Foo"> without snippts enabled.
Index: clangd/CodeCompletionStrings.cpp
===================================================================
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -10,6 +10,7 @@
#include "CodeCompletionStrings.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/RawCommentList.h"
+#include "clang/Basic/SourceManager.h"
#include <utility>
namespace clang {
@@ -122,10 +123,23 @@
}
}
+std::string getFormattedComment(const ASTContext &Ctx, const RawComment &RC,
+ bool CommentsFromHeaders) {
+ auto &SourceMgr = Ctx.getSourceManager();
+ // Parsing comments from invalid preamble can lead to crashes. So we only
+ // return comments from the main file when doing code completion. For
+ // indexing, we still read all the comments.
+ // FIXME: find a better fix, e.g. store file contents in the preamble or get
+ // doc comments from the index.
+ if (!CommentsFromHeaders && !SourceMgr.isWrittenInMainFile(RC.getLocStart()))
+ return "";
+ return RC.getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+}
} // namespace
std::string getDocComment(const ASTContext &Ctx,
- const CodeCompletionResult &Result) {
+ const CodeCompletionResult &Result,
+ bool CommentsFromHeaders) {
// FIXME: clang's completion also returns documentation for RK_Pattern if they
// contain a pattern for ObjC properties. Unfortunately, there is no API to
// get this declaration, so we don't show documentation in that case.
@@ -137,20 +151,20 @@
const RawComment *RC = getCompletionComment(Ctx, Decl);
if (!RC)
return "";
- return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+ return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
}
std::string
getParameterDocComment(const ASTContext &Ctx,
const CodeCompleteConsumer::OverloadCandidate &Result,
- unsigned ArgIndex) {
+ unsigned ArgIndex, bool CommentsFromHeaders) {
auto Func = Result.getFunction();
if (!Func)
return "";
const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
if (!RC)
return "";
- return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());
+ return getFormattedComment(Ctx, *RC, CommentsFromHeaders);
}
void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,
Index: clangd/CodeComplete.cpp
===================================================================
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -586,9 +586,11 @@
const auto *CCS = Candidate.CreateSignatureString(
CurrentArg, S, *Allocator, CCTUInfo, true);
assert(CCS && "Expected the CodeCompletionString to be non-null");
+ // FIXME: for headers, we need to get a comment from the index.
SigHelp.signatures.push_back(ProcessOverloadCandidate(
Candidate, *CCS,
- getParameterDocComment(S.getASTContext(), Candidate, CurrentArg)));
+ getParameterDocComment(S.getASTContext(), Candidate, CurrentArg,
+ /*CommentsFromHeader=*/false)));
}
}
@@ -1030,7 +1032,8 @@
SemaCCS = Recorder->codeCompletionString(*SR);
if (Opts.IncludeComments) {
assert(Recorder->CCSema);
- DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR);
+ DocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR,
+ /*CommentsFromHeader=*/false);
}
}
return Candidate.build(FileName, Scores, Opts, SemaCCS, Includes.get(), DocComment);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
