Skip to content

Update AsgxAODNTupleMakerAlg.cxx to avoid use of reinterpet_cast in getElement

Will Buttinger requested to merge will-main-patch-31216 into main

This MR fixes an issue I was encountering with the TopCPToolkit framework. At the moment the NtupleMaker uses a reinterpret_cast<const SG::AuxElement*> to cast a bare pointer to the specific instance to the base class. As stated in the comments, this assumes that the inheritance is simple.

However, in TopCPToolkit's case, they had a custom class where, crucially, they had added a virtual destructor:

namespace xAOD {
   class PartonHistory: public SG::AuxElement { 
    ....
     virtual ~PartonHistory() { }

This breaks the reinterpretability of the class, which could be seen from the ROOT prompt:

root [0] xAOD::PartonHistory* ph = new xAOD::PartonHistory
(xAOD::PartonHistory *) 0x1df46c50
root [1] static_cast<const SG::AuxElement*>(ph)
(const SG::AuxElement *) 0x1df46c58

Note the change of pointer.

I was able to fix this issue just by getting rid of the virtual destructor:

root [0] xAOD::PartonHistory* ph = new xAOD::PartonHistory
(xAOD::PartonHistory *) 0x1169eb90
root [1] static_cast<SG::AuxElement*>(ph)
(SG::AuxElement *) 0x1169eb90

But in the process of debugging this, I found that getElement method in the NutpleMaker looks like it could be improved, with simpler use of the storegate interface, and the retrieve method will correctly cast things as well!

So I'm submitting this MR as a suggested improvement. It might be worth doing something similar in the getVector method to avoid the reinterpret_cast going on there, but we could start with this change and see how it goes?

Edited by Will Buttinger

Merge request reports