[PATCH] D56492: [clangd] Add Documentations for member completions.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 9 07:53:38 PST 2019


ilya-biryukov added inline comments.


================
Comment at: clangd/CodeComplete.cpp:1373
     // Convert the results to final form, assembling the expensive strings.
-    for (auto &C : Top) {
-      Output.Completions.push_back(toCodeCompletion(C.first));
-      Output.Completions.back().Score = C.second;
+    for (size_t i = 0; i < Top.size(); ++i) {
+      const ScoredBundle &BundleAndScope = Top[i];
----------------
naming NIT: the loop var should be `I`


================
Comment at: clangd/CodeComplete.cpp:1375
+      const ScoredBundle &BundleAndScope = Top[i];
+      const CompletionCandidate::Bundle &CandidateBundle = BundleAndScope.first;
+      Output.Completions.push_back(toCodeCompletion(CandidateBundle));
----------------
Since we already have two variables, maybe deconstruct the pair into one of them instead of keeping a variable for a pair too? I.e.
```
auto &Bundle = Top[I].first;
auto &Score = Top[I].second;
```

should make the code a little simpler



================
Comment at: clangd/CodeComplete.cpp:1382
+          Output.Completions.back().Documentation.empty()) {
+        if (CandidateBundle.size() == 1) {
+          if (const CodeCompletionResult *SemaR =
----------------
NIT: use early exits (see the [[ https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code | LLVM Style guide  ]]) to reduce nesting here.

```
if (size() != 1)
  continue;
auto *SemaR = ;
if (!SemaR)
  continue;
// ...


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56492/new/

https://reviews.llvm.org/D56492





More information about the cfe-commits mailing list