[Rivet] Dangerous casting to FinalState

[Frank Siegert]

Hi David,

We want to apply the attached patch, which hides the dangerous slicing
constructors for classes derived from FinalState. I then tested
various options of initialising/copying projections, all of which do
the correct thing now (errors during compilation for slicing
constructors, no errors for faithful ones) except for one:

ZFinder zfinder(fs, cut, PID::ELECTRON, 65*GeV, 115*GeV, 0.2,
ZFinder::CLUSTERNODECAY, ZFinder::TRACK);
VetoedFinalState remainder = zfinder.remainingFinalState();
addProjection(remainder, "RFS");

I would expect that this should work fine, but at runtime I get:
  Error in MY_RFSTEST2c::init method: No projections registered for
parent 0x7ffe07be8840

Attached is the analysis code and a HepMC file with one event to
reproduce it quickly after having applied the patch to Rivet:
  $ rivet-buildplugin MY_RFSTEST2c
  $ rivet --pwd -a MY_RFSTEST2c file.hepmc2g

It would be great if you would have an idea what is going wrong with
the projection cloning/registration.

Cheers,
Frank


On 7 May 2015 at 10:50, David Grellscheid
<david.grellscheid at durham.ac.uk> wrote:
> Hi all,
>
> I'm sorry I haven't followed this from the beginning and I'm confused
> about if we're talking about registered projections or locally created
> ones, as the required copying behaviour is more tricky in the first
> case. Can someone summarise where I can find the current status?
>
>   - what you see as the problem
>   - what you would expect instead
>
> Thanks,
>
>   David
>
> On 04/05/15 17:28, Andy Buckley wrote:
>> On 04/05/15 17:04, Frank Siegert wrote:
>>> Hi Andy,
>>>
>>>> Does that problem disappear if you define remainder as a reference,
>>>> either as VetoedFinalState& or FinalState&?
>>>
>>> Yes, even for FinalState& the problem disappears. That's case 2a in my
>>> original examples.
>>>
>>>> It would also be useful if you could use the -l TRACE flag (if I
>>>> remember the syntax correctly) when running, and send the resulting log
>>>> so we can see if that parent address corresponds to something. Offhand
>>>> it looks like a bit of an odd address value: is this the sort of pointer
>>>> value that corresponds to a non-heap-allocated local?
>>>
>>> I have no idea on the pointer value here, but I have put the full log into
>>> http://cern.ch/fsiegert/tmp/trace.log
>>
>> Thanks!
>>
>>>> When assigning a Projection like this, a copy will be made. Does
>>>> VetoedFinalState have a copy assignment operator, copy constructor, etc.
>>>> defined as per the Rule of Three
>>>> (http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29)
>>>> to make this safe? I genuinely can't remember if we do that, since
>>>> copying of projections isn't really the idea. If we don't perhaps we
>>>> could make it a rule in Projection implementation to always hide (i.e.
>>>> make private) the copy constructor and copy assignment operators to
>>>> simply ban copy by value and hence force use of references. I forget if
>>>> this can be done on the Projection base class or if each new derived
>>>> class would need to explicitly re-hide them. Anyway, food for thought
>>>> albeit not quite at black belt level ;-)
>>>
>>> We don't seem to explicitly define any of the three (copy assignment
>>> operator, copy constructor, destructor). I guess the copying that you
>>> mean stems from the clone() function, which probably is using the
>>> constructor from a FinalState instead of a copy constructor? If our
>>> projection registration/cloning can't cope with normal C++ copying
>>> then we should indeed either fix it or hide the copy
>>> constructors/operators. I have to admit that I didn't even realise
>>> that we are internally cloning projections upon registration, and
>>> whatever magic is connected to that. It's probably nothing
>>> super-urgent to fix, since at least it won't give wrong results. But
>>> I'd expect the black-belts to understand this much better than I
>>> (challenge!).
>>
>> Hmm, good point about the cloning. I was just thinking of the local copy
>> that occurs if the lvalue isn't a reference type in your example. I'll
>> have to have a look at the current clone implementations to know how
>> safe they are -- in general if we're not defining copy constructor,
>> assignment, and destructor then any projection which holds pointer
>> members could be dodgy in copying. I think we've covered the destructor
>> angle, but not necessarily the copying ones... so we might want to ban
>> those, depending on how clone() is currently written. Old rusty memories
>> for me...
>>
>> Andy
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: MY_RFSTEST2c.cc
Type: text/x-c++src
Size: 1430 bytes
Desc: not available
URL: <https://www.hepforge.org/lists-archive/rivet/attachments/20150507/781bb2df/attachment.cc>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: file.hepmc2g
Type: application/octet-stream
Size: 3305 bytes
Desc: not available
URL: <https://www.hepforge.org/lists-archive/rivet/attachments/20150507/781bb2df/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hidden-copy.patch
Type: text/x-patch
Size: 3064 bytes
Desc: not available
URL: <https://www.hepforge.org/lists-archive/rivet/attachments/20150507/781bb2df/attachment.bin>