[Rivet] Dangerous casting to FinalState

[Frank Siegert]

Hi David,

any ideas? Do you need more information from me?

Cheers,
Frank



On 7 May 2015 at 14:18, Frank Siegert <frank.siegert at cern.ch> wrote:
> 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
>>>