Iterating over elements of an iterator inside a macro moves the inner collection

[msrd0]

For some reason, askama won't use references of the inner collection when using macros, resulting in a compiler error

error[E0507]: cannot move out of `foo.list` which is behind a shared reference
   --> src/main.rs:
    |
280 |     #[derive(Template)]
    |              ^^^^^^^^
    |              |
    |              `foo.list` moved due to this method call
    |              move occurs because `foo.list` has type `BTreeSet<std::string::String>`, which does not implement the `Copy` trait
    |
note: this function takes ownership of the receiver `self`, which moves `foo.list`
   --> /usr/lib/rustlib/src/rust/library/core/src/iter/traits/collect.rs:261:18
    |
261 |     fn into_iter(self) -> Self::IntoIter;
    |                  ^^^^
    = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0507]: cannot move out of `foo.list` which is behind a shared reference
   --> src/main.rs:
    |
280 |     #[derive(Template)]
    |              ^^^^^^^^
    |              |
    |              `foo.list` moved due to this method call
    |              move occurs because `foo.list` has type `BTreeSet<std::string::String>`, which does not implement the `Copy` trait
    |
    = note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)

When taking the code from the macro body and placing it at the call side, it works as expected.

This is a minimal template to reproduce the problem:

{%- macro list(list) -%}
<ul>
	{%- for item in list %}
	<li>{{item}}</li>
	{%- endfor %}
</ul>
<ol>
	{%- for item in list %}
	<li>{{item}}</li>
	{%- endfor %}
</ol>
{%- endmacro -%}

{%- for foo in foos %}
{% call list(foo.list) %}
{%- endfor %}

#[derive(Template)]
#[template(path = "list.j2", print = "code")]
struct List {
	foos: Vec<Foo>
}

struct Foo {
	list: BTreeSet<String>
}

Generated code by askama

impl ::askama::Template for List {
    fn render_into(&self, writer: &mut (impl ::std::fmt::Write + ?Sized)) -> ::askama::Result<()> {
        include_bytes! ("/path/to/list.j2") ;
        {
            let mut _did_loop = false;
            let _iter = (&self.foos).into_iter();
            for (foo, _loop_item) in ::askama::helpers::TemplateLoop::new(_iter) {
                _did_loop = true;
                writer.write_str("\n")?;
                {
                    writer.write_str("<ul>")?;
                    {
                        let mut _did_loop = false;
                        let _iter = (foo.list).into_iter();
                        for (item, _loop_item) in ::askama::helpers::TemplateLoop::new(_iter) {
                            _did_loop = true;
                            ::std::write!(
                                writer,
                                "\n\t<li>{expr0}</li>",
                                expr0 = &::askama::MarkupDisplay::new_unsafe(&(item), ::askama::Html),
                            )?;
                        }
                        if !_did_loop {
                        }
                    }
                    writer.write_str("\n</ul>\n<ol>")?;
                    {
                        let mut _did_loop = false;
                        let _iter = (foo.list).into_iter();
                        for (item, _loop_item) in ::askama::helpers::TemplateLoop::new(_iter) {
                            _did_loop = true;
                            ::std::write!(
                                writer,
                                "\n\t<li>{expr1}</li>",
                                expr1 = &::askama::MarkupDisplay::new_unsafe(&(item), ::askama::Html),
                            )?;
                        }
                        if !_did_loop {
                        }
                    }
                    writer.write_str("\n</ol>")?;
                }
            }
            if !_did_loop {
            }
        }
        ::askama::Result::Ok(())
    }
    const EXTENSION: ::std::option::Option<&'static ::std::primitive::str> =
    Some("j2")
    ;
    const SIZE_HINT: ::std::primitive::usize =
    0
    ;
    const MIME_TYPE: &'static ::std::primitive::str =
    "application/octet-stream"
    ;
}
impl ::std::fmt::Display for List {
    #[inline]
    fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        ::askama::Template::render_into(self, f).map_err(|_| ::std::fmt::Error {})
    }
}

I'm using askama version 0.11.1 from crates.io.

[djc]

Yeah, I guess we should maybe take a reference of whatever we're passing to the macro? Would you be able to submit a PR?

[msrd0]

I can try to do a PR but I need some more details on how you think this should be addressed. It sounds like the problem is with the macro somehow having a different logic to decide if the iterator needs to be taken over a reference or not than the template outside the macro? Can you tell me how this works in askama and point me to the relevant code?

[msrd0]

By looking briefly at your loop code, it looks like askama "forgets" that self is involved and therefore doesn't take a reference. The behaviour can be replicated like this:

{%- for foo in foos %}
{%- set list = foo.list %}
<ul>
	{%- for item in list %}
	<li>{{item}}</li>
	{%- endfor %}
</ul>
<ol>
	{%- for item in list %}
	<li>{{item}}</li>
	{%- endfor %}
</ol>
{%- endfor %}

This produces the exact same error message. Maybe looping over a variable should always loop over that variable's reference?

[djc]

I think this is the relevant PR: https://github.com/djc/askama/commit/230263711e7edb0110e1679f6d31353ba7cdc919. We may want to tweak that logic? Navigating whether we want to pass references or ownership is often tricky. I think it could make sense that, when referencing a self field, we want to pass a reference only to the macro.

[djc]

https://github.com/djc/askama/commit/c7697cbd406dce0962618e99a6b78116b14fdb1c contains similar changes for the macro calling code, I think.

[msrd0]

I tried to implement your solution, but either I didn't get it quite right or just always taking a reference of macro parameters doesn't work.