Minimize use of argument references for functions in templates

[softdevca]

Functions called in templates shouldn't require references for argument types that are Copy.

For example: {{{ my_function(my_integer) }}} will require my_function to accept a &u32 if my_integer is a template field. Passing a u32 by reference is odd. Also, calling my_function(42) later in the same template would fail unless it was written my_function(&42).

This might not have an easy solution. This could be getting into compile-time type detection.

A work-around is to call a function on the argument to prevent Rinja from automatically adding the reference. {{ my_function(my_integer.clone()) }} works and allows the function to take a u32 instead of &u32.

An example can be found in testing/tests/calls.rs.

#[derive(Template)]
#[template(source = "{{ b(value) }}", ext = "txt")]
struct OneFunction {
    value: u32,
}

impl OneFunction {
    fn b(&self, x: &u32) -> u32 {
        self.value + x
    }
}

[GuillaumeGomez]

You can accept 42 and &42 with AsRef<u32>.

Would be nice to mention it in the book.

[softdevca]

Good suggestion to make the templates clearer. I do feel that AsRef<u32> in the code base will make for some WTF moments by readers until they figure out why.

In my case, they'd be wondering why an enum variant takes a AsRef<u32>.

[GuillaumeGomez]

In any case, proc-macros are very limited amount of information. We cannot assume the type most of the time. Calling clone() is an absolute no-go. What we could do is always pass literals behind a reference.

Anyway, providing this information in the book in the filters chapter and explain why with examples would make the current situation better for everyone.

[softdevca]

I agree that proc-macros doesn't provide enough information.

Could we get {{ my_function(my_field | deref) }} to mark the argument as not needing to be referenced? Or something similar.

Or maybe {{ my_function(*my_field) }} could cause the argument to be treated the same as a literal or function call and not take a reference.

I think users will be okay without Rinja magically doing the right thing as long as it's clear and obvious. Especially if it doesn't require them to change existing code to work with templates.

[GuillaumeGomez]

I don't think using filters in arguments is a good idea. And I'd prefer to avoid doing too much magic with operators too. The current behaviour is coherent across all operations: we add a reference and filters should all expect a reference.

[softdevca]

I think the behavior of filters is fine. It's arguments to functions, including functions on template impl's, that are the trouble mainly because of these things I have experienced:

1. Existing code must change to accept or return references, or a compatibility layer added
2. Functions are not consistently called with a reference, like when it is passed a literal

I'm not sure there's a great solution but what happens now feels awkward and clunky.

For #1 I've worked around it by adding a .to_owned() on arguments when calling existing functions that do not take references or where references make no sense. Nothing special about .to_owned(), any other function that returns the item would work to cause Rinja to not take a reference.

For #2 it might be worth removing the special case handling of literals to avoid confusion.

For the most part Rinja has been really nice to use and I see function arguments as the only real pain point I've encountered.

[GuillaumeGomez]

Overall, whether we decide to remove reference in some cases or not, what's most important is coherency. If n some cases, there are no references and some others there are, it makes using rinja more tricky. I really think the best way forward here is to use references all the time and to recommend to our users to use AsRef<> as much as possible.

For the most part Rinja has been really nice to use and I see function arguments as the only real pain point I've encountered.

Glad you like it. :)

[softdevca]

Improving consistency in this area is probably be the easiest solution with with the most positive impact. I did have to look at the code to understand why sometimes I needed to accept a reference and sometimes not, removing the need for that would be helpful.

I have found that adding helper methods on the Template implementations to be the easiest, rather than trying to integrate directly with existing code.