amallia / better_assert

A better C++ assertion library which also prints a debug message.
Apache License 2.0
6 stars 1 forks source link

Why create the temporary std::string's? #1

Closed niXman closed 6 years ago

niXman commented 6 years ago

Hi,

There is no need to create the two temporary file and condition objects that, moreover, allocates memory in the heap: https://github.com/amallia/better_assert/blob/c5e3033a9b17e27401094e13ee93037f94f9d639/include/better_assert/better_assert.hpp#L19

amallia commented 6 years ago

I believe this is needed. If you use __FILE__ and __LINE__ directly in the function it will print better_assert.hpp as fine and the line of the cerr call. Try it yourself if you dont believe it :)

Anyway it is passed as reference and it is referring to a compiler defined value which is probably const static. So it should not allocate anything :)

niXman commented 6 years ago

I think this also applies to the message object.

amallia commented 6 years ago

Can you provide an example?

niXman commented 6 years ago

Anyway it is passed as reference and it is referring to a compiler defined value which is probably const static. So it should not allocate anything :)

You're wrong. The two temporary objects will be created here that allocate memory on the heap and copy the static const char [N] arrays.

amallia commented 6 years ago

Okay, that is true. Can you provide an example to avoid it?

niXman commented 6 years ago

https://wandbox.org/permlink/t9aCUbPmfZ8FWqNK

amallia commented 6 years ago

Yes, I should have said in C++style :)

amallia commented 6 years ago

I can change it for file and condition, will not change it for message as it might be the case that someone passes an actual string.

niXman commented 6 years ago

Moreover, the file and line you can be put in one static const char[] array: https://wandbox.org/permlink/OfHVWipSg1sSKMgV

niXman commented 6 years ago

I can change it for file and condition, will not change it for message as it might be the case that someone passes an actual string.

Or maybe the user of your code will always be hardcoding messages? ;) But your code in this case will always create a temporary object ;)

niXman commented 6 years ago

In C++ it is considered a bad form to "pay" for what you do not use. I mean the creation of a temporary object and the memory allocation in the heap.

amallia commented 6 years ago

In C++ it is considered a bad form to "pay" for what you do not use. I mean the creation of a temporary object and the memory allocation in the heap. You might want to reuse the same message among multiple asserts. So it makes sense to allocate a single string and pass it around.

I can have two overloads...

https://wandbox.org/permlink/OfHVWipSg1sSKMgV

I like this one, a single #define _STRINGIZE(x) #x should be enough... EDIT: no is not

amallia commented 6 years ago

Added your suggestions. Thanks a lot! Also I have done the overload for char*/string&.

Appreciated your help!

niXman commented 6 years ago

But, what's this?: std::cerr << "oooooooo" << std::endl; ;) In line 33.

amallia commented 6 years ago

Oh sh*t :)