[clang-tools-extra] r334807 - [clangd] Do not report comments that only have special chars.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 27 05:03:17 PDT 2018
Hi Reid,
Thanks for landing the workaround.
The fact that this test didn't not work means our completion inside
templates is broken on Windows by default. I'll try searching for ways to
fix it.
On Mon, Jun 18, 2018 at 9:00 PM Reid Kleckner <rnk at google.com> wrote:
> And, as soon as I sent that, I realized what was up. Apparently
> auto-complete is driven by the parser, and delayed template parsing means
> we don't parse templates. That seems like a serious issue that should get
> fixed eventually.
>
> Anyway, I landed a crummy workaround in r334973 to green the bots.
>
> On Mon, Jun 18, 2018 at 11:53 AM Reid Kleckner <rnk at google.com> wrote:
>
>> This test has been failing on Windows since it has been added:
>> http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/12549
>>
>> I took a look at the unittest code, but I'm not familiar enough with this
>> code to debug it. Can you take a look at this?
>>
>> On Fri, Jun 15, 2018 at 1:35 AM Ilya Biryukov via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: ibiryukov
>>> Date: Fri Jun 15 01:31:17 2018
>>> New Revision: 334807
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=334807&view=rev
>>> Log:
>>> [clangd] Do not report comments that only have special chars.
>>>
>>> Summary:
>>> Like the following:
>>> // -------
>>> // =======
>>> // *******
>>>
>>> It does not cover all the cases, but those are definitely not very
>>> useful.
>>>
>>> Reviewers: sammccall, ioeric, hokein
>>>
>>> Reviewed By: sammccall
>>>
>>> Subscribers: MaskRay, jkorous, cfe-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D48171
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
>>> clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>>>
>>> Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=334807&r1=334806&r2=334807&view=diff
>>>
>>> ==============================================================================
>>> --- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)
>>> +++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Fri Jun 15
>>> 01:31:17 2018
>>> @@ -151,6 +151,16 @@ bool canRequestComment(const ASTContext
>>> const ObjCPropertyDecl *PDecl = M ? M->findPropertyDecl() : nullptr;
>>> return !PDecl || canRequestForDecl(*PDecl);
>>> }
>>> +
>>> +bool LooksLikeDocComment(llvm::StringRef CommentText) {
>>> + // We don't report comments that only contain "special" chars.
>>> + // This avoids reporting various delimiters, like:
>>> + // =================
>>> + // -----------------
>>> + // *****************
>>> + return CommentText.find_first_not_of("/*-= \t\r\n") !=
>>> llvm::StringRef::npos;
>>> +}
>>> +
>>> } // namespace
>>>
>>> std::string getDocComment(const ASTContext &Ctx,
>>> @@ -167,7 +177,10 @@ std::string getDocComment(const ASTConte
>>> const RawComment *RC = getCompletionComment(Ctx, Decl);
>>> if (!RC)
>>> return "";
>>> - return RC->getFormattedText(Ctx.getSourceManager(),
>>> Ctx.getDiagnostics());
>>> + std::string Doc = RC->getFormattedText(Ctx.getSourceManager(),
>>> Ctx.getDiagnostics());
>>> + if (!LooksLikeDocComment(Doc))
>>> + return "";
>>> + return Doc;
>>> }
>>>
>>> std::string
>>> @@ -180,7 +193,10 @@ getParameterDocComment(const ASTContext
>>> const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);
>>> if (!RC)
>>> return "";
>>> - return RC->getFormattedText(Ctx.getSourceManager(),
>>> Ctx.getDiagnostics());
>>> + std::string Doc = RC->getFormattedText(Ctx.getSourceManager(),
>>> Ctx.getDiagnostics());
>>> + if (!LooksLikeDocComment(Doc))
>>> + return "";
>>> + return Doc;
>>> }
>>>
>>> void getLabelAndInsertText(const CodeCompletionString &CCS, std::string
>>> *Label,
>>>
>>> Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=334807&r1=334806&r2=334807&view=diff
>>>
>>> ==============================================================================
>>> --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
>>> (original)
>>> +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri
>>> Jun 15 01:31:17 2018
>>> @@ -1100,6 +1100,65 @@ TEST(CompletionTest, DocumentationFromCh
>>> Contains(AllOf(Not(IsDocumented()), Named("func"))));
>>> }
>>>
>>> +TEST(CompletionTest, NonDocComments) {
>>> + MockFSProvider FS;
>>> + auto FooCpp = testPath("foo.cpp");
>>> + FS.Files[FooCpp] = "";
>>> +
>>> + MockCompilationDatabase CDB;
>>> + IgnoreDiagnostics DiagConsumer;
>>> + ClangdServer Server(CDB, FS, DiagConsumer,
>>> ClangdServer::optsForTest());
>>> +
>>> + Annotations Source(R"cpp(
>>> + // ------------------
>>> + int comments_foo();
>>> +
>>> + // A comment and a decl are separated by newlines.
>>> + // Therefore, the comment shouldn't show up as doc comment.
>>> +
>>> + int comments_bar();
>>> +
>>> + // this comment should be in the results.
>>> + int comments_baz();
>>> +
>>> +
>>> + template <class T>
>>> + struct Struct {
>>> + int comments_qux();
>>> + int comments_quux();
>>> + };
>>> +
>>> +
>>> + // This comment should not be there.
>>> +
>>> + template <class T>
>>> + int Struct<T>::comments_qux() {
>>> + }
>>> +
>>> + // This comment **should** be in results.
>>> + template <class T>
>>> + int Struct<T>::comments_quux() {
>>> + int a = comments^;
>>> + }
>>> + )cpp");
>>> + Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
>>> + CompletionList Completions = cantFail(runCodeComplete(
>>> + Server, FooCpp, Source.point(), clangd::CodeCompleteOptions()));
>>> +
>>> + // We should not get any of those comments in completion.
>>> + EXPECT_THAT(
>>> + Completions.items,
>>> + UnorderedElementsAre(AllOf(Not(IsDocumented()),
>>> Named("comments_foo")),
>>> + AllOf(IsDocumented(), Named("comments_baz")),
>>> + AllOf(IsDocumented(),
>>> Named("comments_quux")),
>>> + // FIXME(ibiryukov): the following items
>>> should have
>>> + // empty documentation, since they are
>>> separated from
>>> + // a comment with an empty line.
>>> Unfortunately, I
>>> + // couldn't make Sema tests pass if we
>>> ignore those.
>>> + AllOf(IsDocumented(), Named("comments_bar")),
>>> + AllOf(IsDocumented(),
>>> Named("comments_qux"))));
>>> +}
>>> +
>>> TEST(CompletionTest, CompleteOnInvalidLine) {
>>> auto FooCpp = testPath("foo.cpp");
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180627/32a4693e/attachment-0001.html>
More information about the cfe-commits
mailing list