<div dir="ltr">Hi Reid,<br><div><br></div><div>Thanks for landing the workaround.</div><div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 18, 2018 at 9:00 PM Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><br></div><div>Anyway, I landed a crummy workaround in r334973 to green the bots.</div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Jun 18, 2018 at 11:53 AM Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This test has been failing on Windows since it has been added:<div><a href="http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/12549" target="_blank">http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015/builds/12549</a><br></div><div><br></div><div>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?</div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Jun 15, 2018 at 1:35 AM Ilya Biryukov via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ibiryukov<br>
Date: Fri Jun 15 01:31:17 2018<br>
New Revision: 334807<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=334807&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=334807&view=rev</a><br>
Log:<br>
[clangd] Do not report comments that only have special chars.<br>
<br>
Summary:<br>
Like the following:<br>
 // -------<br>
 // =======<br>
 // *******<br>
<br>
It does not cover all the cases, but those are definitely not very<br>
useful.<br>
<br>
Reviewers: sammccall, ioeric, hokein<br>
<br>
Reviewed By: sammccall<br>
<br>
Subscribers: MaskRay, jkorous, cfe-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D48171" rel="noreferrer" target="_blank">https://reviews.llvm.org/D48171</a><br>
<br>
Modified:<br>
  clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp<br>
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp<br>
<br>
Modified: clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=334807&r1=334806&r2=334807&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp?rev=334807&r1=334806&r2=334807&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp (original)<br>
+++ clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp Fri Jun 15 01:31:17 2018<br>
@@ -151,6 +151,16 @@ bool canRequestComment(const ASTContext<br>
  const ObjCPropertyDecl *PDecl = M ? M->findPropertyDecl() : nullptr;<br>
  return !PDecl || canRequestForDecl(*PDecl);<br>
 }<br>
+<br>
+bool LooksLikeDocComment(llvm::StringRef CommentText) {<br>
+Â // We don't report comments that only contain "special" chars.<br>
+Â // This avoids reporting various delimiters, like:<br>
+Â //Â Â =================<br>
+Â //Â Â -----------------<br>
+Â //Â Â *****************<br>
+Â return CommentText.find_first_not_of("/*-= \t\r\n") != llvm::StringRef::npos;<br>
+}<br>
+<br>
 } // namespace<br>
<br>
 std::string getDocComment(const ASTContext &Ctx,<br>
@@ -167,7 +177,10 @@ std::string getDocComment(const ASTConte<br>
  const RawComment *RC = getCompletionComment(Ctx, Decl);<br>
  if (!RC)<br>
   return "";<br>
-Â return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());<br>
+Â std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());<br>
+Â if (!LooksLikeDocComment(Doc))<br>
+Â Â return "";<br>
+Â return Doc;<br>
 }<br>
<br>
 std::string<br>
@@ -180,7 +193,10 @@ getParameterDocComment(const ASTContext<br>
  const RawComment *RC = getParameterComment(Ctx, Result, ArgIndex);<br>
  if (!RC)<br>
   return "";<br>
-Â return RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());<br>
+Â std::string Doc = RC->getFormattedText(Ctx.getSourceManager(), Ctx.getDiagnostics());<br>
+Â if (!LooksLikeDocComment(Doc))<br>
+Â Â return "";<br>
+Â return Doc;<br>
 }<br>
<br>
 void getLabelAndInsertText(const CodeCompletionString &CCS, std::string *Label,<br>
<br>
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=334807&r1=334806&r2=334807&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=334807&r1=334806&r2=334807&view=diff</a><br>
==============================================================================<br>
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)<br>
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri Jun 15 01:31:17 2018<br>
@@ -1100,6 +1100,65 @@ TEST(CompletionTest, DocumentationFromCh<br>
        Contains(AllOf(Not(IsDocumented()), Named("func"))));<br>
 }<br>
<br>
+TEST(CompletionTest, NonDocComments) {<br>
+Â MockFSProvider FS;<br>
+Â auto FooCpp = testPath("foo.cpp");<br>
+Â FS.Files[FooCpp] = "";<br>
+<br>
+Â MockCompilationDatabase CDB;<br>
+Â IgnoreDiagnostics DiagConsumer;<br>
+Â ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());<br>
+<br>
+Â Annotations Source(R"cpp(<br>
+Â Â // ------------------<br>
+Â Â int comments_foo();<br>
+<br>
+Â Â // A comment and a decl are separated by newlines.<br>
+Â Â // Therefore, the comment shouldn't show up as doc comment.<br>
+<br>
+Â Â int comments_bar();<br>
+<br>
+Â Â // this comment should be in the results.<br>
+Â Â int comments_baz();<br>
+<br>
+<br>
+Â Â template <class T><br>
+Â Â struct Struct {<br>
+Â Â Â int comments_qux();<br>
+Â Â Â int comments_quux();<br>
+Â Â };<br>
+<br>
+<br>
+Â Â // This comment should not be there.<br>
+<br>
+Â Â template <class T><br>
+Â Â int Struct<T>::comments_qux() {<br>
+Â Â }<br>
+<br>
+Â Â // This comment **should** be in results.<br>
+Â Â template <class T><br>
+Â Â int Struct<T>::comments_quux() {<br>
+Â Â Â int a = comments^;<br>
+Â Â }<br>
+Â )cpp");<br>
+Â Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);<br>
+Â CompletionList Completions = cantFail(runCodeComplete(<br>
+Â Â Â Server, FooCpp, Source.point(), clangd::CodeCompleteOptions()));<br>
+<br>
+Â // We should not get any of those comments in completion.<br>
+Â EXPECT_THAT(<br>
+Â Â Â Completions.items,<br>
+Â Â Â UnorderedElementsAre(AllOf(Not(IsDocumented()), Named("comments_foo")),<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â AllOf(IsDocumented(), Named("comments_baz")),<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â AllOf(IsDocumented(), Named("comments_quux")),<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â // FIXME(ibiryukov): the following items should have<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â // empty documentation, since they are separated from<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â // a comment with an empty line. Unfortunately, I<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â // couldn't make Sema tests pass if we ignore those.<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â AllOf(IsDocumented(), Named("comments_bar")),<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â AllOf(IsDocumented(), Named("comments_qux"))));<br>
+}<br>
+<br>
 TEST(CompletionTest, CompleteOnInvalidLine) {<br>
  auto FooCpp = testPath("foo.cpp");<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>