Teuchos: Resolves issues with Teuchos Debug Unit Tests and Clang (#449)
Created by: MicheldeMessieres
Summary is this fixes the tests except for one which seems to be a clang error. I submitted that to Apple. (attached code demonstrates the issue). I'll post if I get new information on that. It happens for both Debug and Release builds if optimization level is O0.
Details:
TeuchosParameterList_StringIndexedOrderedValueObjectContainer:
This appears to be a clang error, likely related to std::deque and templates, which happens for optimization level O0 only. So it turns out this is not a debug issue and happens for release builds as well if optimization was O0. The attached code demonstrates the problem and is a simplified version of the failing unit test.
So unfortunately I don't have a fix yet or a work around but this does seem to be a serious problem which may cause significant trouble.
TeuchosParameterList_ObjectBuilder:
The unit test setParameterList calls the following line: TEST_THROW( ob = null, std::logic_error );
On Clang debug, this will throw in the destructor of ObjectBuilder and clang will terminate. To make ObjectBuilder behave like gcc, we add noexcept false to ParameterListAcceptor (which ObjectBuilder derives from).
Then VerboseObjectBase should also be declared noexcept false because the AlgorithmA class has multiple inheritance from ParameterListAcceptor and VerboseObjectBase. Once we change ParameterListAcceptor, this second change is necessary to compile.
Now the unit test can proceed to the end - however it will seg fault when the RCP for the ObjectBuilder tries to clean up. This is because that line ‘ob = null’ does not actually set ob since it threw. When ob tries to clean itself up as an RCP at the end, clang detects a double delete and terminates.
To avoid this situation we must get the parameter list back into a valid state. We call ob->setParamList(parameterList()) which puts ob into a valid state.
The next test unsetParameterList was modified in same way, for the same reasons.
Note that these modifications will be for debug builds only and do not impact release.
We could skip out on the TEUCHOS_NOEXCEPT_FALSE since we are avoiding the destructor delete anyways but this might make future work less confusing if we keep it.
TeuchosCore_MemoryManagement
In the circularReference_a_then_c test, we have some similar issues where destructors throw and then try to clean themselves up. I’ve disabled that portion of the test for clang - this will also be debug only - it's not great to disable part of the test and we could do something fancier in RCP to handle these situations but upcoming changes to make RCP thread safe will likely eliminate throws from destructors or change this behavior significantly - so it may not be productive to do anything complicated for this one.