In template class, temporaries are substituted for const ref arguments -- trying to diagnose why this causes a crash.

[Paul]

The below code compiles but crashes runtime with an unexpected termination.
I'm not sure why but it has something to do with the scope of temporaries
or something.
Please could someone explain exactly what the problem is?
Thank you.

Paul

#include <string>
#include <vector>
#include <iostream>
using namespace std;

enum class Color { red, green, blue };
enum class Size { small, medium, large };

struct Product
{
string name;
Color color;
Size size;
};

template <typename T> struct AndSpecification;

template <typename T> struct Specification
{
virtual ~Specification() = default;
virtual bool is_satisfied(T* item) const = 0;
};

// new:
template <typename T> AndSpecification<T> operator&&
(const Specification<T>& first, const Specification<T>& second)
{
return { first, second };
}

template <typename T> struct Filter
{
virtual vector<T*> filter(vector<T*> items,
Specification<T>& spec) = 0;
};

struct BetterFilter : Filter<Product>
{
vector<Product*> filter(vector<Product*> items,
Specification<Product> &spec) override
{
vector<Product*> result;
for (auto& p : items)
if (spec.is_satisfied(p))
result.push_back(p);
return result;
}
};

struct ColorSpecification : Specification<Product>
{
Color color;

ColorSpecification(Color color) : color(color) {}

bool is_satisfied(Product *item) const override {
return item->color == color;
}
};

struct SizeSpecification : Specification<Product>
{
Size size;

explicit SizeSpecification(const Size size)
: size{ size }
{
}
bool is_satisfied(Product* item) const override {
return item->size == size;
}
};



template <typename T> struct AndSpecification : Specification<T>
{
const Specification<T>& first;
const Specification<T>& second;

AndSpecification(const Specification<T>& first, const Specification<T>& second)
: first(first), second(second) {}

bool is_satisfied(T *item) const override {
return first.is_satisfied(item) && second.is_satisfied(item);
}

};

int main()
{
Product apple{"Apple", Color::green, Size::small};
Product tree{"Tree", Color::green, Size::large};
Product house{"House", Color::blue, Size::large};
const vector<Product*> all { &apple, &tree, &house };
BetterFilter bf;
// warning: the following will compile but will NOT work
auto spec2 = SizeSpecification(Size::large)&&
ColorSpecification(Color::green);
for (auto& x : bf.filter(all, spec2)){}

}

[Sam]

Paul writes:

> The below code compiles but crashes runtime with an unexpected termination.
> I'm not sure why but it has something to do with the scope of temporaries
> or something.
> Please could someone explain exactly what the problem is?
> Thank you.

> template <typename T> struct AndSpecification : Specification<T>
> {
> const Specification<T>& first;
> const Specification<T>& second;
>
> AndSpecification(const Specification<T>& first, const Specification<T>&
> second)
> : first(first), second(second) {}

You're grabbing references to these parameters and storing them in this
(template) class.

> auto spec2 = SizeSpecification(Size::large)&&
> ColorSpecification(Color::green);

You are constructing an instance of the AndSpecification template, using the
&& operator overload.

Both "SizeSpecification(Size::large)" and "ColorSpecification(Color::green)"
are temporary objects here, that get created when evaluating this expression.

And immediately destroyed after the expression gets evaluated.

Your instance of the AndSpecification template, thusly, ends up having
references to destroyed objects. They certainly exist when AndSpecification
gets constructured, and its constructor gets valid references to them. But
because the referenced objects are temporary objects, they get destroyed
soon thereafter.

Explicitly instantiating everything should work:

auto large=SizeSpecification(Size::large);
auto green=SizeSpecification(Size::green);

auto spec2 = large && green;

Now, both large and green objects will exist as long as the "spec2" object
exists, so the references to them, in spec2, remain valid.

Of course, this is not as convenient. But this is how your templates are
designed. Basically, if you store a reference somewhere, it's your
responsibility to make sure that the referenced objects still exist every
time you use that referenced object. C++ will not do it for you.

The only way to make sure that the object exist is store the object itself,
and not a reference to it. This is impossible to do, in your situation,
because the object themselves are abstract base classes.

The most common way this kind of concept gets implemented is with smart
pointers. And plenty of overloading.

[Jorgen Grahn]

On Sun, 2018-10-21, Sam wrote:
> Paul writes:
>
>> The below code compiles but crashes runtime with an unexpected termination.
>> I'm not sure why but it has something to do with the scope of temporaries
>> or something.
>> Please could someone explain exactly what the problem is?
>> Thank you.

(snip detailed explanation)

> The only way to make sure that the object exist is store the object itself,
> and not a reference to it. This is impossible to do, in your situation,
> because the object themselves are abstract base classes.
>
> The most common way this kind of concept gets implemented is with smart
> pointers. And plenty of overloading.

For perspective, I note that it's all massive overkill in this
specific case, which is about finding the large, green objects in a
sequence.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .

[Jorgen Grahn]

On Sat, 2018-10-20, Paul wrote:
> The below code compiles but crashes runtime with an unexpected termination.
> I'm not sure why but it has something to do with the scope of temporaries
> or something.
> Please could someone explain exactly what the problem is?
> Thank you.

...

> int main()
> {
> Product apple{"Apple", Color::green, Size::small};
> Product tree{"Tree", Color::green, Size::large};
> Product house{"House", Color::blue, Size::large};
> const vector<Product*> all { &apple, &tree, &house };

Unrelated to your problem, I suppose, but avoid storing pointer in
containers, when you can store the objects themselves:

const vector<Product> all {
{"Apple", Color::green, Size::small},
{"Tree", Color::green, Size::large},
{"House", Color::blue, Size::large},
};

[Paul]

These are dummy examples for the purposes of learning good technique.
I thought that, assuming a real-world example where the Product struct
would be much more complicated, then storing pointers saves memory.
Am I wrong?

Thanks a lot for your help,

Paul

[Jorgen Grahn]

On Sun, 2018-10-21, Paul wrote:
> On Sunday, October 21, 2018 at 3:41:38 PM UTC+1, Jorgen Grahn wrote:
>> On Sat, 2018-10-20, Paul wrote:
>> > The below code compiles but crashes runtime with an unexpected termination.
>> > I'm not sure why but it has something to do with the scope of temporaries
>> > or something.
>> > Please could someone explain exactly what the problem is?
>> > Thank you.
>> ...
>>
>> > int main()
>> > {
>> > Product apple{"Apple", Color::green, Size::small};
>> > Product tree{"Tree", Color::green, Size::large};
>> > Product house{"House", Color::blue, Size::large};
>> > const vector<Product*> all { &apple, &tree, &house };
>>
>> Unrelated to your problem, I suppose, but avoid storing pointer in
>> containers, when you can store the objects themselves:
>>
>> const vector<Product> all {
>> {"Apple", Color::green, Size::small},
>> {"Tree", Color::green, Size::large},
>> {"House", Color::blue, Size::large},
>> };
>
> These are dummy examples for the purposes of learning good technique.

As you may have sensed already ;-) I have issues with that strategy.

I understand that learning techniques is necessary, eventually. But
picking simple (yet synthetic) examples and applying advanced
solutions may teach you to overdesign things which should be simple.

A technique is only good in a context where it's fitting.

> I thought that, assuming a real-world example where the Product struct
> would be much more complicated, then storing pointers saves memory.
> Am I wrong?

Yes. Why would it save memory? Nowadays it doesn't even avoid a
copy, since you can construct the Product straight into the
std::vector<Product>.

The generic name "Product" made me think you eventually meant to have
it as a base class and derive Apple, Tree and House from it. In that
case you couldn't have a std::vector<Product>, and storing pointers
would be one solution. Even then, I think people would recommend a
vector of smart pointers instead.

(On the other hand, I don't think that would be a very realistic class
hierarchy. If you see everything as a product, you don't care if it's
an apple, and it won't have any apple-specific behavior. You may care
if a products can be shipped to Australia, can be frozen or be stored
for a year instead.)

[Öö Tiib]

On Sunday, 21 October 2018 21:58:24 UTC+3, Jorgen Grahn wrote:
> On Sun, 2018-10-21, Paul wrote:
> >
> > These are dummy examples for the purposes of learning good technique.
>
> As you may have sensed already ;-) I have issues with that strategy.
>
> I understand that learning techniques is necessary, eventually. But
> picking simple (yet synthetic) examples and applying advanced
> solutions may teach you to overdesign things which should be simple.
>
> A technique is only good in a context where it's fitting.

It is all correct but what we can suggest to Paul as good strategy for
learning programming?

My strategy was to make little useful tools to automate something
tedious about my computer usage and other hobbies. However that
was eighties when even most simple tools were often missing.

Other idea can be to solve the problems on programming
problem sites. There are countless such sites like Codeforces,
GeeksforGeeks, CodeChef, LeetCode, HackerRank, TopCoder,
CodinGame, CodeFights, Codewars, Coderbyte etc.
That might help to see how bloat YAGNI "techniques" can
make the result both complicated and inefficient.

Are there some other ideas? May be reading a book or series
like "Numerical Recipes"? I think there must always be some
actual practical problem to solve. Without it any engineering
turns into art of goalless patterns.

> > I thought that, assuming a real-world example where the Product struct
> > would be much more complicated, then storing pointers saves memory.
> > Am I wrong?
>
> Yes. Why would it save memory? Nowadays it doesn't even avoid a
> copy, since you can construct the Product straight into the
> std::vector<Product>.

He was apparently creating filtered vectors of pointers instead
of doing range based for or for_each on original and filtering OTW.

[Jorgen Grahn]

On Tue, 2018-10-23, Öö Tiib wrote:
> On Sunday, 21 October 2018 21:58:24 UTC+3, Jorgen Grahn wrote:
>> On Sun, 2018-10-21, Paul wrote:
>> >
>> > These are dummy examples for the purposes of learning good technique.
>>
>> As you may have sensed already ;-) I have issues with that strategy.
>>
>> I understand that learning techniques is necessary, eventually. But
>> picking simple (yet synthetic) examples and applying advanced
>> solutions may teach you to overdesign things which should be simple.
>>
>> A technique is only good in a context where it's fitting.
>
> It is all correct but what we can suggest to Paul as good strategy for
> learning programming?
>
> My strategy was to make little useful tools to automate something
> tedious about my computer usage and other hobbies. However that
> was eighties when even most simple tools were often missing.
>
> Other idea can be to solve the problems on programming
> problem sites. There are countless such sites like Codeforces,
> GeeksforGeeks, CodeChef, LeetCode, HackerRank, TopCoder,
> CodinGame, CodeFights, Codewars, Coderbyte etc.

I think that is a good idea.

There are old threads in comp.lang.c++ on this subject (how to learn?)
which are worth reading. I can't point to them though; last one I
remember was a year or more ago.

> That might help to see how bloat YAGNI "techniques" can
> make the result both complicated and inefficient.
>
> Are there some other ideas? May be reading a book or series
> like "Numerical Recipes"? I think there must always be some
> actual practical problem to solve. Without it any engineering
> turns into art of goalless patterns.

+1

I have to add though that people are different. My programming
improved a lot when I stopped trying to solve general problems, and
began to focus on implementing one, concrete, useful thing. But I
cannot say for sure it's like that for everyone.

There's also the problem: if you haven't tried the less commonly
useful and more advanced techniques, how do you know when to use them?
Let others review the code, maybe.

[Öö Tiib]

On Tuesday, 23 October 2018 23:09:42 UTC+3, Jorgen Grahn wrote:
>
> There's also the problem: if you haven't tried the less commonly
> useful and more advanced techniques, how do you know when to use them?
> Let others review the code, maybe.

There is plenty to learn about the basic techniques like usage of
standard library containers starting from std::string, usage of
standard library algorithms, RAII, const correctness, rule of five
and exception safety. Standard library itself is large enough and
also there are lot of mature third party libraries for any topic.
Accomplishing something using some of those would make one
familiarized with lot of techniques. It is pointless to start from
building "generic" square wheels instead.