Skip to content

Commit 9818b16

Browse files
lionel-lionel-
committed
Disable library() diagnostics to avoid r_lock! 
1 parent 5f6c50e commit 9818b16

File tree

1 file changed

+90
-88
lines changed

1 file changed

+90
-88
lines changed

‎crates/ark/src/lsp/diagnostics.rs

Lines changed: 90 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
use std::collections::HashMap;
99
use std::collections::HashSet;
10-
use std::os::raw::c_void;
1110
use std::time::Duration;
1211

1312
use anyhow::bail;
@@ -17,9 +16,7 @@ use harp::exec::RFunctionExt;
1716
use harp::external_ptr::ExternalPointer;
1817
use harp::object::RObject;
1918
use harp::protect::RProtect;
20-
use harp::r_lock;
2119
use harp::r_symbol;
22-
use harp::utils::r_is_null;
2320
use harp::utils::r_symbol_quote_invalid;
2421
use harp::utils::r_symbol_valid;
2522
use harp::vector::CharacterVector;
@@ -644,72 +641,72 @@ impl<'a> TreeSitterCall<'a> {
644641
// required for `&tree_sitter::Node<'_>` to implement `Send`
645642
// rustc [E0277]: `*const c_void` cannot be shared between threads safely
646643

647-
fn recurse_call_arguments_custom(
648-
node: Node,
649-
context: &mut DiagnosticContext,
650-
diagnostics: &mut Vec<Diagnostic>,
651-
function: &str,
652-
diagnostic_function: &str,
653-
) -> Result<()> {
654-
r_lock! {
655-
// Build a call that mixes treesitter nodes (as external pointers)
656-
// library(foo, pos = 2 + 2)
657-
// ->
658-
// library([0, <node0>], pos = [1, <node1>])
659-
// where:
660-
// - node0 is an external pointer to a treesitter Node for the identifier `foo`
661-
// - node1 is an external pointer to a treesitter Node for the call `2 + 2`
662-
//
663-
// The TreeSitterCall object holds on to the nodes, so that they can be
664-
// safely passed down to the R side as external pointers
665-
let call = TreeSitterCall::new(node, function, context)?;
666-
667-
let custom_diagnostics = RFunction::from(diagnostic_function)
668-
.add(&call)
669-
.add(ExternalPointer::new(&context.source))
670-
.call()?;
671-
672-
if !r_is_null(*custom_diagnostics) {
673-
let n = XLENGTH(*custom_diagnostics);
674-
for i in 0..n {
675-
// diag is a list with:
676-
// - The kind of diagnostic: skip, default, simple
677-
// - The node external pointer, i.e. the ones made in TreeSitterCall::new
678-
// - The message, when kind is "simple"
679-
let diag = VECTOR_ELT(*custom_diagnostics, i);
680-
681-
let kind: String = RObject::view(VECTOR_ELT(diag, 0)).try_into()?;
682-
683-
if kind == "skip" {
684-
// skip the diagnostic entirely, e.g.
685-
// library(foo)
686-
// ^^^
687-
continue;
688-
}
689-
690-
let ptr = VECTOR_ELT(diag, 1);
691-
let value = *(R_ExternalPtrAddr(ptr) as *const c_void as *const Node);
692-
693-
if kind == "default" {
694-
// the R side gives up, so proceed as normal, e.g.
695-
// library(foo, pos = ...)
696-
// ^^^
697-
recurse(value, context, diagnostics)?;
698-
} else if kind == "simple" {
699-
// Simple diagnostic from R, e.g.
700-
// library("ggplot3")
701-
// ^^^^^^^ Package 'ggplot3' is not installed
702-
let message: String = RObject::view(VECTOR_ELT(diag, 2)).try_into()?;
703-
let range: Range = value.range().into();
704-
let diagnostic = Diagnostic::new_simple(range.into(), message.into());
705-
diagnostics.push(diagnostic);
706-
}
707-
}
708-
}
709-
710-
().ok()
711-
}
712-
}
644+
// fn recurse_call_arguments_custom(
645+
// node: Node,
646+
// context: &mut DiagnosticContext,
647+
// diagnostics: &mut Vec<Diagnostic>,
648+
// function: &str,
649+
// diagnostic_function: &str,
650+
// ) -> Result<()> {
651+
// r_lock! {
652+
// // Build a call that mixes treesitter nodes (as external pointers)
653+
// // library(foo, pos = 2 + 2)
654+
// // ->
655+
// // library([0, <node0>], pos = [1, <node1>])
656+
// // where:
657+
// // - node0 is an external pointer to a treesitter Node for the identifier `foo`
658+
// // - node1 is an external pointer to a treesitter Node for the call `2 + 2`
659+
// //
660+
// // The TreeSitterCall object holds on to the nodes, so that they can be
661+
// // safely passed down to the R side as external pointers
662+
// let call = TreeSitterCall::new(node, function, context)?;
663+
664+
// let custom_diagnostics = RFunction::from(diagnostic_function)
665+
// .add(&call)
666+
// .add(ExternalPointer::new(&context.source))
667+
// .call()?;
668+
669+
// if !r_is_null(*custom_diagnostics) {
670+
// let n = XLENGTH(*custom_diagnostics);
671+
// for i in 0..n {
672+
// // diag is a list with:
673+
// // - The kind of diagnostic: skip, default, simple
674+
// // - The node external pointer, i.e. the ones made in TreeSitterCall::new
675+
// // - The message, when kind is "simple"
676+
// let diag = VECTOR_ELT(*custom_diagnostics, i);
677+
678+
// let kind: String = RObject::view(VECTOR_ELT(diag, 0)).try_into()?;
679+
680+
// if kind == "skip" {
681+
// // skip the diagnostic entirely, e.g.
682+
// // library(foo)
683+
// // ^^^
684+
// continue;
685+
// }
686+
687+
// let ptr = VECTOR_ELT(diag, 1);
688+
// let value = *(R_ExternalPtrAddr(ptr) as *const c_void as *const Node);
689+
690+
// if kind == "default" {
691+
// // the R side gives up, so proceed as normal, e.g.
692+
// // library(foo, pos = ...)
693+
// // ^^^
694+
// recurse(value, context, diagnostics)?;
695+
// } else if kind == "simple" {
696+
// // Simple diagnostic from R, e.g.
697+
// // library("ggplot3")
698+
// // ^^^^^^^ Package 'ggplot3' is not installed
699+
// let message: String = RObject::view(VECTOR_ELT(diag, 2)).try_into()?;
700+
// let range: Range = value.range().into();
701+
// let diagnostic = Diagnostic::new_simple(range.into(), message.into());
702+
// diagnostics.push(diagnostic);
703+
// }
704+
// }
705+
// }
706+
707+
// ().ok()
708+
// }
709+
// }
713710

714711
fn recurse_call(
715712
node: Node,
@@ -729,25 +726,30 @@ fn recurse_call(
729726
// things like 'local({ ... })'.
730727
let fun = callee.utf8_text(context.source.as_bytes())?;
731728
match fun {
732-
// TODO: there should be some sort of registration mechanism so
733-
// that functions can declare that they know how to generate
734-
// diagnostics, i.e. ggplot2::aes() would skip diagnostics
735-
736-
// special case to deal with library() and require() nse
737-
"library" => recurse_call_arguments_custom(
738-
node,
739-
context,
740-
diagnostics,
741-
"library",
742-
".ps.diagnostics.custom.library",
743-
)?,
744-
"require" => recurse_call_arguments_custom(
745-
node,
746-
context,
747-
diagnostics,
748-
"require",
749-
".ps.diagnostics.custom.require",
750-
)?,
729+
// FIXME: These are currently disabled because they use the R
730+
// runtime to match arguments with an approach that meshes R and
731+
// treesitter objects. This is not compatible with `r_task()`
732+
// because treesitter objects are not Send/Sync.
733+
734+
// // TODO: there should be some sort of registration mechanism so
735+
// // that functions can declare that they know how to generate
736+
// // diagnostics, i.e. ggplot2::aes() would skip diagnostics
737+
738+
// // special case to deal with library() and require() nse
739+
// "library" => recurse_call_arguments_custom(
740+
// node,
741+
// context,
742+
// diagnostics,
743+
// "library",
744+
// ".ps.diagnostics.custom.library",
745+
// )?,
746+
// "require" => recurse_call_arguments_custom(
747+
// node,
748+
// context,
749+
// diagnostics,
750+
// "require",
751+
// ".ps.diagnostics.custom.require",
752+
// )?,
751753

752754
// default case: recurse into each argument
753755
_ => recurse_call_arguments_default(node, context, diagnostics)?,

0 commit comments

Comments
 (0)