Skip to content
This repository was archived by the owner on Aug 19, 2022. It is now read-only.

Upgrade to tower-lsp 0.13.0 #61

Merged
merged 2 commits into from
Aug 20, 2020
Merged

Upgrade to tower-lsp 0.13.0 #61

merged 2 commits into from
Aug 20, 2020

Conversation

ebkalderon
Copy link
Contributor

See changes in release v0.13.0 for details!

Note: since all the Client methods have been changed to async fn, as described in ebkalderon/tower-lsp#180 (comment), I identified a compile error caused by tree-sitter interacting poorly with async/await and Send futures:

error: future cannot be sent between threads safely
  --> crates/server/src/lsp/api.rs:25:65
   |
25 |       async fn did_open(&self, params: DidOpenTextDocumentParams) {
   |  _________________________________________________________________^
26 | |         crate::service::synchronizer::document::open(self.session.clone(), params)
27 | |             .await
28 | |             .unwrap()
29 | |     }
   | |_____^ future returned by `__did_open` is not `Send`
   |
   = help: within `impl core::future::future::Future`, the trait `std::marker::Send` is not implemented for `*const std::ffi::c_void`
note: future is not `Send` as this value is used across an await
  --> crates/server/src/service/auditor.rs:95:13
   |
17 |             let node = tree.root_node();
   |                 ---- has type `tree_sitter::Node<'_>` which is not `Send`
...
95 |             session.client.publish_diagnostics(uri.clone(), diagnostics, version).await;
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ await occurs here, with `node` maybe used later
96 |         }
   |         - `node` is later dropped here
   = note: required for the cast to the object type `dyn core::future::future::Future<Output = ()> + std::marker::Send`

This is fixed by wrapping the node variable in a lexical scope so that the variable gets dropped early before the session.client.publish_diagnostics(...).await; call occurs. This solution was inspired by rust-lang/rust#64856.

Let me know what you think, @darinmorrison! Does this look good to you? Any suggestions?

@ebkalderon
Copy link
Contributor Author

Force-pushed to fix formatting errors caught by CI.

It appears that certain `tree-sitter` values are not considered `Send`
(thread-safe) when retained across an `await` point. To fix this, we
wrap `node` in a scope so that the variable is destroyed before reaching
the `client.publish_diagnostics()` call, which must be `Send`.
@silvanshade silvanshade merged commit 245646b into wasm-lsp:main Aug 20, 2020
@silvanshade
Copy link
Collaborator

Looks good to me. Thanks for the PR!

@ebkalderon
Copy link
Contributor Author

No problem! Feel free to ping me if there are any further issues.

@ebkalderon ebkalderon deleted the upgrade-tower-lsp branch August 20, 2020 17:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants