sammccall added inline comments.

================
Comment at: clangd/ClangdUnit.cpp:941
           createInvocationFromCommandLine(ArgStrs, CommandLineDiagsEngine, 
VFS);
+      CI->LangOpts->CommentOpts.ParseAllComments = true;
       // createInvocationFromCommandLine sets DisableFree.
----------------
Doesn't this slow down parsing by a lot? I thought @ilya-biryukov had seen that 
it was responsible for big performance losses in code complete.

ISTM we might prefer to look up the comment from the index by USR, and just 
live without it if we can't find it.

(Apologies if this has been discussed)



================
Comment at: clangd/ClangdUnit.h:1
 //===--- ClangdUnit.h -------------------------------------------*- C++-*-===//
 //
----------------
Please don't cram more stuff into clangdunit that's not directly related to 
building ASTs and managing their lifetimes. Instead, can we pull as much as 
possible out into `Documentation.h` or similar?

Note not `Hover.h` as there are other use cases, like we should use this logic 
for the code completion item documentation too.


================
Comment at: test/clangd/hover.test:1
+# RUN: clangd -run-synchronously < %s | FileCheck %s
+# It is absolutely vital that this file has CRLF line endings.
----------------
Could I ask you to do primary/exhaustive testing of this with unit tests, and 
just a basic smoke test with lit?
Then it's possible to read the tests, understand whether they're correct, and 
understand the failure messages.

I'm trying to pull more of the features in this direction - see 
`CodeCompleteTests.cpp` for an example of how this can work.

We've had quite a few cases of bugs that had tests, but nobody noticed that the 
tests were wrong because it's just not possible to read them.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D35894



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to