CertainLach/jrsonnet

How to register custom builtin functions?

iwinux opened this issue · 3 comments

iwinux commented

I've defined a function like this (crate version 0.5.0-pre94):

use jrsonnet_evaluator::function::builtin;

#[builtin]
fn timestamp() -> String {
    use std::time::{SystemTime, UNIX_EPOCH};
    let ts = SystemTime::now().duration_since(UNIX_EPOCH).unwrap();
    ts.as_secs_f64().to_string()
}

However, when I try to register it into jrsonnet_stdlib::ContextInitializer, I get all kinds of errors:

  1. ctx.add_native("name".into(), timestamp) -> "jrsonnet_evaluator::function::builtin::Builtin is not implemented for fn item..."
  2. ctx.add_native("name".into(), timestamp::INST) -> the same
  3. ctx.add_native("name".into(), Into::<impl Builtin>::into(timestamp::INST)) -> "impl Trait only allowed in function and inherent method return types"

From the source code of the builtin macro, I can be sure that the Builtin trait has already been implemented. Yet add_native keeps complaining about type mismatch.

Please enlighten me how should this be done 😿

CertainLach commented

ctx.add_native("name".into(), timestamp {}) is the correct code here, let me explain:

#[builtin] macros generates struct, with the same name as the function, so in case of ctx.add_native("name".into(), timestamp) it complains because you have given it a function, not a struct.

The generated code is roughly this:

struct timestamp {}
impl timestamp {
    pub const INST: &'static dyn StaticBuiltin = &#name {};
}

So ctx.add_native("name".into(), timestamp::INST) will fail with another issue, because there is no impl Builtin for &'static dyn StaticBuiltin due to rust limitations.

The only possible way here is to have a dynamic builtin, using struct initialization syntax.

This is an ergonomic problem, which I did not considered before, because I mostly use custom globals instead of registering std.native functions:

jrsonnet/crates/jrsonnet-stdlib/src/lib.rs

Line 59 in 0d47e9f

for (name, builtin) in [

https://github.com/UniqueNetwork/chainql/blob/649bf8a7885e713c14638834b051978e5a7d2275/crates/chainql-core/src/lib.rs#L1463

iwinux commented

Wow! That's so obvious in hindsight. How could I miss it 🤣 . Thank you for explanation.

CertainLach commented

I have improved ergonomics of many methods in the master branch, now

ctx.add_native("timestamp", timestamp::INST)
//                        ^ .into() is no longer needed too

Will just work.

  • 👍1