[Rivet] Dangerous casting to FinalState

[Andy Buckley]

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

-- 
Dr Andy Buckley, Lecturer / Royal Society University Research Fellow
Particle Physics Expt Group, University of Glasgow