[PATCH] D51598: [clangd] Avoid crashes in override completions

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 3 08:21:46 PDT 2018


ilya-biryukov added inline comments.


================
Comment at: unittests/clangd/CodeCompleteTests.cpp:1770
+  // Check the completions call does not crash.
+  completions(R"cpp(
+    struct Base {
----------------
ioeric wrote:
> ilya-biryukov wrote:
> > Was wondering if testing for crashes LG this way, or adding more assertions might make sense too
> Hmm, I think this is okay, but I'd probably do some sanity check on the results, just to make it look less odd ;)
Exactly my feelings: this looks odd.
However, couldn't come up with a decent sanity check at this point.
The reason is: we don't store enough information to tell override completion from non-override ones. 

It means I can assert something like `Not(Contains(Labelled("~Base() override")))`, but lots of broken outputs can still make the test pass, e.g.:
- `void ~Base() override`
- `~Derived() override`
- ...

Will probably keep as this and think how to factor out overriden completions from the results better...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51598





More information about the cfe-commits mailing list