Looping over a temporary vector leads to error. #22
Description
Problem
The generated style of the foreach loop is not a good design choice. It breaks when looping over temporaries.
Insights
Let's say we are in this situation:
struct Object {
std::vector<T> allTs();
};
struct message {
Object o;
};
And the view is trying something like:
<% foreach t in o.allTs() %>
<ul>
<% item %>
<li><%= t.name() %></li>
<% end %>
</ul>
<% end %>
Consider now that the function signature may look like std::string_view T::name() const; and that Object::allTs() is constructing a new std::vector every time it is invoked.
When we look at the C++ Code for the view it looks somewhat like:
for(
auto t_ptr = (content.o.allTs()).begin(),
t_ptr_end = (content.o.allTs()).end();
t_ptr != t_ptr_end; ++t_ptr) {
auto& t = *t_ptr;
With this setup t_ptr is a iterator value of a temporary object (which might be already gone) and t_ptr_end is a iterator value of a second temporary object (which might also be gone).
So any call to t is basically broken. So if t.name() now returns a string_view then we can be pretty sure (and that's precisely the problem I currently have) that the viewed memory section is not valid.
Solution
Change the generator to produce code that looks like:
{
decltype(content.o.allTs()) t_iterable = content.o.allTs();
// I dropped the if condition for simplicity
for (auto t_ptr = t_iterable.begin(), t_ptr_end = t_iterable.end();
t_ptr != t_ptr_end; ++t_ptr) {
auto& t = *t_ptr;
// Stuff
}
}
BUT PAY ATTENTION to the use of auto vs decltype. Every auto keyword in this code is the expansion of the CPPCMS_TYPEOF macro. But when you use auto in the case above you may introduce unnecessary copies. That is because auto won't deduce to a reference-type. So if allTs() returnes a std::vector<T>& auto will deduce to std::vector<T> and you will copy the whole vector. decltype on the other hand will deduce to std::vector<T>&.