Add Sync + Send to FunctionRelaxed

[CaptainSpof]

34435db introduced cannot be sent between threads safely error.

Disclaimer I have no idea what I am doing, feel free to disregard this PR.

I've noticed a regression when upgrading to tera 1.13.0. The error was:

`(dyn FunctionRelaxed + 'static)` cannot be sent between threads safely

After some digging I identified 34435db responsible for the regression. Following the compiler advises, I've managed to "fix" the issue (as in, the code build and the tests are green)

Again, I'm still new to Rust and I haven't looked at what the code I modified do. So handle with care, I guess.

Might be related to #458

[Keats]

Mmmh you could not have been usingFunctionRelaxed as it's new so that's odd it's breaking since Function is still Send + Sync.
What does your code that broke look like?

[CaptainSpof]

Yes, I didn't use FunctionRelaxed by myself. I'm pretty sure the problem was due to the fact that I used rayon in my code. I've managed to reproduced the behavior with a minimal code, roughly doing what I intended:

use tera::{Tera, Context};

use rayon::prelude::*;

fn do_stuff(tera: &Tera, context: &Context) -> String {
    tera.render("test", context).unwrap()
}

fn main() {
    debug!("tera_test");
    let mut tera = Tera::default();
    tera.add_raw_template("test", "{{ name }}");
    let mut context = Context::new();
    context.insert("name", "toto");

    let r: Vec<String> = (0..10).into_par_iter().map(|_| do_stuff(&tera, &context)).collect();
    println!("{:?}", r);
}

Discarding rayon with a into_iter() does not cause problems.

[Keats]

Oh then it looks like the change made Context not sync/send anymore, that's unfortunate.
cc @moschroe any idea on how to solve that?

[moschroe]

Unfortunately, FunctionRelaxed cannot be Send + Sync. Otherwise, it could not be used for the purpose I initially needed this whole ordeal for (passing a closure which itself captures a !Sync + !Send value). I don't see any good way to make the Context sometimes Sync, sometimes not.

At the moment the only iron-clad measure would be to discriminate between the "classic" Context and a new RelaxedContext Not sure if 'relaxed' was the best term, but it is less stringent in its requirements, namedly not requiring Send + Sync.

The 'big' solution coming to mind would be to abstract Context away into a trait which then Tera::render() could consume, then in theory the provided impl ContextTrait could decide 'on its own' whether or not to be Send, Sync, PolkaDotted or whatever :) This should then also be compatible with existing code.

[moschroe]

@CaptainSpof
I implemented a POC for a Context with variable guarantees: https://github.com/moschroe/tera/tree/feature_variably-constrained-context, could you check whether this would work for you?
For an example use case that will not work without relaxing the Send requirement: https://github.com/moschroe/test-scoped-tera

@Keats Should this get a new issue? New PR?

[Keats]

Inheritance would be nice here :/
Your solution work but adds a lot of type annotation across the codebase for a minor feature. I'm wondering if adding those local functions was a mistake

[moschroe]

Of course it has to touch everything Context touches. But it does not change the API, I tried to keep that stable. However, I think it is not a minor feature. My original use case would otherwise almost rule out the use of tera altogether, if not requiring hideous workarounds. Also, other people did ask for having non-global functions, or functionality that could often be solved by having this available, e.g. #668, #605 or #543.

All the 'overhead' stays internal (unless a consumer opts in with Context::new_relaxed(), TODO: docs) and I don't think it is too much in the first place. Rust is a very precise language, so we need to be precise in describing type bounds 🤷. Maybe nice, more expressive type aliases could be defined?

At the moment I don't see any less complicated way to express all necessary constraints, keep the functionality of having non-Send things in a context (allowing whole classes of use cases) and keep a unified API. And I also could not come up with something less invasive, sorry 😬

[CaptainSpof]

Thanks @moschroe , your version works for my use case (I needed to add the from_serialize for the relaxed variant).

The added verbosity is unfortunate, but at least I got it working.

Feel free to close this PR and continue elsewhere.

[Keats]

@moschroe please send a PR so I can review it better

[Keats]

ping @moschroe , I'm going to yank the last version in the meantime.

[moschroe]

new PR in #688

@CaptainSpof using simply Context::new() should create a Context that is just as Send+Sync as ever and your rayon example works for me when using that, no need for any 'corrections'.