sammccall added inline comments.
================
Comment at: clangd/CodeComplete.cpp:259
+ : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments) {
+ if (C.SemaResult) {
+ Completion.Name = llvm::StringRef(SemaCCS->getTypedText());
----------------
ioeric wrote:
> nit: shall we keep `assert(bool(SemaResult) == bool(SemaCCS));`?
Done (in `add()`, and hoisted the call to `add()` to the top here)
================
Comment at: clangd/CodeComplete.cpp:322
+ }
+ if (ExtractDocumentation && Completion.Documentation.empty()) {
+ if (C.IndexResult && C.IndexResult->Detail)
----------------
ioeric wrote:
> nit: the documentation for the bundling says `Documentation may contain a
> combined docstring from all comments`. The implementation seems to take the
> documentation from the last comment (which seems fine to me), but they should
> be consistent.
This was meant to be ambiguous ("may") to allow a better implementation later.
But rewrote it to mention the current behavior as a possibility :-)
================
Comment at: clangd/CodeComplete.cpp:1145
+ Scores.ExcludingName = Relevance.NameMatch
+ ? Scores.Total / Relevance.NameMatch
+ : Scores.Quality;
----------------
ioeric wrote:
> nit: this seems to assume that `NameMatch` is a factor of the final score,
> which seems to be changeable given the abstraction of
> `evaluateSymbolAndRelevance`. Not sure if there is an alternative to the
> current approach, but this probably deserves a comment.
Yeah, client-side filtering is the great abstraction-breaker :-(
Added the comment, not sure there's much more to be done.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D48762
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
