kaj / ructe

Rust Compiled Templates with static-file handling
https://crates.io/crates/ructe
453 stars 33 forks source link

Generated out argument should use dyn or impl Write #47

Closed bitfusion-miha closed 5 years ago

bitfusion-miha commented 5 years ago

With the Rust 2018 edition, using Ructe produces lots of warning: trait objects without an explicit 'dyn' are deprecated, one for every generated template function, because of the &mut Write argument. According to the Edition Guide, you should use &mut dyn Write for dynamic dispatch and &mut impl Write for static dispatch.

Further, I don't think dynamic dispatch is necessary here, so &mut impl Write should be preferred to avoid a vtable lookup on every template function call. But I'm not completely sure that this doesn't break some existing code. If it does break some compatibility guarantees, it would still be beneficial to provide a configuration option at generate-time.

In either case, I am willing to prepare a PR if such a change is welcome.

kaj commented 5 years ago

Yes, I've seen those warnings. So far they only seem to be enabled in nightly rust, but as the edition guide is clear enough it should probably be fixed.

Then for the question of if we should use &mut dyn Write or &mut impl Write. I think pros and cons of switching to impl is as follows:

Pros:

Cons:

Is there more pros and cons that I haven't thought of? Otherwise, I think the pro outweighs the cons, as most templates will only be called from one place, or at least only on one kind of writer, and the risk of actual breakage in this case should be slim.

So unless there's any important arguments I haven't thought of, I'd be happy to accept a PR chaning the out argument to &mut impl Write.

Noughmad commented 5 years ago

(I'm the OP, but using a personal account)

I agree with you, the main advantage is runtime speed, which I would assume is important since it's an important reason to choose this over other template engines (and other languages). I'm not afraid of monomorphization explosion since most of the time you'll output to a single type (such as web response or file), but even if you use different times, you can still prevent it by passing a &mut Write as the argument to essentially get the previous behavior.

I opened the PR and pasted the cargo bench improvements.

kaj commented 5 years ago

Fixed by PR #48 and released in 0.6.4. Thank you!