Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/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); Index: clang-tools-extra/trunk/clangd/CodeCompletionStrings.h =================================================================== --- clang-tools-extra/trunk/clangd/CodeCompletionStrings.h +++ clang-tools-extra/trunk/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: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp +++ clang-tools-extra/trunk/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 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: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp @@ -389,7 +389,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: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/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