Page MenuHomePhabricator

[clangd] Don't crash on LSP calls for non-added files
ClosedPublic

Authored by ilya-biryukov on Jan 17 2018, 3:53 AM.

Details

Summary

We will return errors for non-added files for now.
Another alternative for clangd would be to read non-added files from
disk and provide useful features anyway.

There are still some cases that fail with assertion (e.g., code
complete). We should address those too, but they require more subtle
changes to the code and therefore out of scope of this patch.

Diff Detail

Event Timeline

ilya-biryukov created this revision.Jan 17 2018, 3:53 AM
sammccall accepted this revision.Jan 17 2018, 4:19 AM
sammccall added inline comments.
clangd/ClangdLSPServer.cpp
158

don't you want to do this before computing replacements?

160

invalidparams

This revision is now accepted and ready to land.Jan 17 2018, 4:19 AM
  • Renamed crash.test to crash-non-added-files.test
  • Do the check in rename before calling into ClangdServer
Harbormaster completed remote builds in B13907: Diff 130127.
ilya-biryukov added inline comments.Jan 17 2018, 4:31 AM
clangd/ClangdLSPServer.cpp
158

Makes sense.

160

I used InternalError, because rename should've returned an error for non-added file. If we do the check before calling rename, InvalidParams make more sense.

This revision was automatically updated to reflect the committed changes.