kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555
+  case Config::ExternalIndexSpec::None:
+    break;
   case Config::ExternalIndexSpec::Server:
----------------
sammccall wrote:
> I think you hit llvm_unreachable here - is this an invariant enforced by the 
> caller, or did you mean to return nullptr?
it is enforced by the caller (i.e. `ProjectAwareIndex::getIndex`) hence 
fallback to unreachable was deliberate.

It was more explicit before as `Generator` takes a reference, also semantically 
it feels wrong for generator to be called with a `None` spec.

Even though we are lucky and can indicate the error by returning null in this 
case, I'd rather keep it as an assertion failure, similar to before. Gonna 
update the documentation on the `IndexFactory` to mention that spec is never 
None though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100308

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to