UndefinedBehaviorSanitizer is complaining in testPVManager

[killenb]

test 1
      Start  1: testPVManager

1: Test command: /scratch/build-ChimeraTK-ControlSystemAdapter/testPVManager
1: Test timeout computed to be: 9.99988e+06
...
1: /scratch/source/include/ChimeraTK/ControlSystemAdapter/DevicePVManager.h:184:12: runtime error: load of value 4294967295, which is not a valid value for type 'ChimeraTK::SynchronizationDirection'
1: SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /scratch/source/include/ChimeraTK/ControlSystemAdapter/DevicePVManager.h:184:12 in 

The message is repeated 10 times after the first one.

[mhier]

This is caused by this check:

  BOOST_CHECK_THROW(
      devManager->createProcessArray<T>(static_cast<SynchronizationDirection>(-1), name + "ShouldFail", 1),
      ChimeraTK::logic_error);

It intentionally casts an invalid value into a SynchronizationDirection enum, hence the sanitizer complains.

I will remove this check and replace the throw with an assert. This type of error IMO is beyond a logic_error... (You will also not get a logic_error if you cast a memory block filled with -1 into a PVManager object or so and start using it.)

[mhier]

Now I see that SynchronizationDirection is a weak enum. Any number could be put in without using a static_cast even. IMO we should change this, but it is an interface change. @killenb How shall we proceed?

[killenb]

Yes. Please make it a class-type enum. We should do this wherever possible. I guess this weak enum is still from the "no C++11" times in the very beginning.

This also makes the test or an assert obsolete. The compiler already does the job for us.

The only place where it is really used probably is ApplciationCore any way. We already have some API breaking changes in that direction, and I don't expect any problems. It there are any it is good that we find then.

[mhier]

Ok. I will keep the assert in, just in case the enum gets extended in future for some reason and the new case is forgotten to be implemented...