search

boostorg / statechart

Boost.org statechart module
http://boost.org/libs/statechart
22 stars 43 forks source link

Use Boost.TypeIndex CTTI to implement type info when RTTI is not enabled #12

Open Lastique opened 5 years ago

Lastique commented 5 years ago

This allows to solve the problem with comparing addresses of global id_provider objects, which fail if the objects reside in different modules. Boost.TypeIndex solves this problem by comparing specially crafted strings, which are equal if the respective types are the same.

Unfortunately, this will likely result in a performance drop as string comparison is likely more expensive than comparing pointers. Any performance optimizations are better placed in Boost.TypeIndex.

This is the next step after fixing the build in https://github.com/boostorg/statechart/pull/10. I'm posting it as a separate PR as this change is more controversial.

Relates to and fixes https://github.com/boostorg/statechart/issues/9.

jeking3 commented 5 years ago

If you think the combination of this and #10 solve the build issues, rebase this to test that.

Lastique commented 5 years ago

If you think the combination of this and #10 solve the build issues, rebase this to test that.

Done.

jeking3 commented 5 years ago

Looks like we have one build still failing, the OSX one. Is the failure related or environmental? It looks like it is a unit test failure. I'm going to rekick it once to see if it is transient.

Here's the job that failed: https://travis-ci.org/boostorg/statechart/jobs/447312868

Lastique commented 5 years ago

Is the failure related or environmental?

I don't know, I don't have OS X to investigate.

apolukhin commented 5 years ago

That's a very unusual way to use Boost.TypeIndex library. Initially it was designed to hide all the typeid related workarounds under the hood and deal with the symbol visibilities/rtti in different shared objects.

So in my head the PR should drop all the BOOST_STATECHART_USE_NATIVE_RTTI related lines, replace boost::typeindex::ctti_type_index::type_info_t with boost::typeindex::type_index, replace &boost::typeindex::ctti_type_index::type_id< MostDerived >().type_info() with &boost::typeindex::type_id< MostDerived >().

This will shorten the code and provide the most efficient way for typeid comparisons.

Lastique commented 5 years ago

The reason I did it the way I did is because BOOST_STATECHART_USE_NATIVE_RTTI is a documented user config macro. Removing it would be a breaking change, and I'm not totally sure how that would affect the library performance.