[PATCH] D17820: Clang Code Completion Filtering

[Ben Langmuir via cfe-commits]

benlangmuir added a comment.

Overall approach seems good to me, but this needs tests.  Other feedback inline.


================
Comment at: include/clang/Sema/CodeCompleteConsumer.h:920
@@ +919,3 @@
+                                CodeCompletionResult Results) {
+    return true;
+  }
----------------
I would expect the default to be `false`, since if you don't implement filtering presumably you want every result.

Also: Please fix alignment of the results parameter.

================
Comment at: lib/Sema/CodeCompleteConsumer.cpp:435
@@ +434,3 @@
+  case CodeCompletionResult::RK_Declaration: {
+    return !(Result.Declaration &&
+            (*Result.Declaration).getIdentifier() &&
----------------
Can `Declaration` ever be legitimately null?  We're going to unconditionally filter it out if so, which seems odd.  Same applies to the other cases below.

================
Comment at: lib/Sema/CodeCompleteConsumer.cpp:450
@@ +449,3 @@
+  }
+  return false;
+}
----------------
Is this reachable?  If not, consider using `llvm_unreachable()` instead of `return`.


Repository:
  rL LLVM

http://reviews.llvm.org/D17820