Make the comparison operators == / != of ito::RetVal const if the AddInInterface is changed the next time

[M. Gronle]

The operators ito::RetVal::operator== and ito::RetVal::operator!= can be const, to allow a comparison for const ito::RetVal, too.

Change this the next time, the addInInterface is increased.

[Oliver Schwanke]

is it possible to also make the constructor argument explicit(so it takes int or ito::retVal enums)?

enums only would be awesome but i think lots of plugins will have to be adapted then…

I have seen code that just returns false on error, which implicitly casts to an ito::RetVal(ito::retOk), which is most likely not as intended by the programmer.

check Qitom/organitzer/widgetWrapper.cpp::932ff

[M. Gronle]

I like the idea with the explicit constructors. If this explicit keyword does not break the binary compatibility, we can also do it right now. Else I prefer to wait for a major version change of itom and the underlying AddInInterface. However, I would not accept explicit integer values instead of the enumeration again. I don’t know why we have that kind of constructor right now; I would delete this one in the future . I don’t think that many plugins have used an integer instead of retOk, retWarning, retError and if so, this should be changed (I will check this at least for the open source plugins).

Your mentioned bug in widgetWrapper is already fixed.

[M. Gronle]

I just tried it out and I guess that it is not that easy:

If we would write:

‌

class RetVal
{
RetVal() {...}
explicit RetVal(ito::tRetValue retValue) ...
}

an initialization like

‌

ito::RetVal r = ito::retError

does not work anymore, since this requires a copy initilization, which is not covered by the explicit keyword. It must be implicit.

See also: https://en.cppreference.com/w/cpp/language/copy_initialization and the examples in https://en.cppreference.com/w/cpp/language/explicit

‌

However, removing the RetVal(int retValue) constructor is still a good idea.

[M. Gronle]

Ok, you were right: There are still some parts in the code, where a value 0 was used instead of retOk. This is no nice code. I started changed this: https://bitbucket.org/itom/itom/commits/dd176c529b19c74f896d8513c2dbd8a8dfac5b7f

[Oliver Schwanke]

Hi Marc,

i’m not sure whether it breaks binary compatibility.

But some plugins(at least of ours) will not compile anymore.

This is because “no nice code” is pretty usual.

As a first approach, maybe it would be just possible to use

class RetVal {

...

RetVal(bool) = delete;

...}

to find the most probable bugs.

The = delete syntax is cpp11 and should work on Tobi’s PC(should be switched on in the CMakeLsits explicitly for some veryvery old gcc...)

So, i’m not actually sure on how to introduce this sort of changes to itom.

I would suggest some sort of compiler switch, that switches compilation mode to something more strict,

so the compiler complains and developers are guided to produce some nicer code.

But you can still compile old code if you really wanted to.

[M. Gronle]

fixes issue 137: comparison operators of ito::RetVal are const now.

→ <<cset 77277b7d2bde>>