[PATCH] D46002: [clangd] Parse all comments in Sema and completion.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdUnit.cpp:362
     CI->getFrontendOpts().DisableFree = false;
+    CI->getLangOpts()->CommentOpts.ParseAllComments = true;
   }
----------------
sammccall wrote:
> Any idea about whether this will affect performance significantly?
> Less for this patch, and more for whether this should be an option in the future that we might e.g. only do during indexing.
Hopefully, there should be no significant difference, but I'll do some benchmarks with both real code and artificial comment-heave code to make sure that's the case.
It would certainly eat a bit more memory, but it shouldn't be significant as we only store an extra list of sourceranges for comments.
Even for this patch, maybe we would want to only enable it in the AST but not in the preamble. I'll get some numbers and come back to this.



================
Comment at: clangd/CodeComplete.cpp:707
   CI->getFrontendOpts().DisableFree = false;
+  CI->getLangOpts()->CommentOpts.ParseAllComments = true;
 
----------------
sammccall wrote:
> Are we sure we want to do this in code complete? I would have thought the more natural approach would be to implement resolve in terms of lookup in the index, and only provide it there.
> The lack of good support for resolve in YCM LSP shouldn't be a problem as YCM doesn't actually use doc comments (I think?).
It seems to give a better user-experience:
  - Some items come only from Sema, so getting documentation for local variables and class members is currently only possible through Sema. 
    We could probably omit the docs for the local vars and put class members into the index, though.
  - If the comment for the completion is in the current file, we should prefer the latest version, not one from the index that might be stale.

However, it does mean sending docs along with the results, I don't think LSP will allow us to properly handle the lifetime of docs from the completion AST, so we won't be able to delay their result until resolve time.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46002