<div dir="ltr">Hi Frank,<div><br></div><div>Your patch indeed prints out a nice compiler error when I try to copy construct a FinalState from a VetoedFinalState. I have two issues with this solution, though (apologies if they have been mentioned earlier):</div><div><br></div><div>1) We can no longer copy FinalStates at all. Do we really want to prevent things like</div><div><br></div><div><font face="monospace, monospace">FinalState fs;</font></div><div><font face="monospace, monospace">FinalState fs1 = fs;</font></div><div><br></div><div>?? Maybe there's a way around this, but I am not clever enough to see it immediately.</div><div><br></div><div>2) If we decide to hide the copy constructor of FinalState to prevent confusion, then shouldn't we in principal do the same for every base class in Rivet? I don't like where this is going...</div><div><br></div><div>I am starting to think that we should go ahead and finalize the release and save this issue for the next one. I am not convinced we will come to a conclusion in the next few days, but please pipe in if you feel otherwise.</div><div><br></div><div>Chris</div>
<div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 9, 2015 at 9:38 AM, Chris Pollard <span dir="ltr"><<a href="mailto:cpollard@cern.ch" target="_blank">cpollard@cern.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi Frank,<div><br></div><div>I tried out your test analysis and hepmc file last night after making a local change to remainingFinalState(), and I found the same issues as you have. Unfortunately.</div><span><br>"Error in MY_RFSTEST2c::init method: No projections registered for<br>parent 0x7ffe07be8840"<div><br></div></span><div>This only seems to crash when "remainder" is a VetoedFinalState. When it is declared a "const VetoedFinalState&", "FinalState", or "const FinalState&", the bookkeeping works (although the physics fails in analyze() for FinalState).</div><div><br></div><div>I am booked for the next few hours but will hopefully be able to take another look in the afternoon. Curse c++.</div><span><font color="#888888"><div><br></div><div>Chris</div></font></span><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 9, 2015 at 8:49 AM, Frank Siegert <span dir="ltr"><<a href="mailto:frank.siegert@cern.ch" target="_blank">frank.siegert@cern.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>>> We both agree that the best thing to do is to have remainingFinalState()<br>
>> return "const VetoedFinalState&", which is the actual type of the underlying<br>
>> returned object. Currently it returns "const FinalState&".<br>
><br>
> I agree, and this is part of the patch I posted in this thread.<br>
><br>
>> This change<br>
>> *should* generate compile-time warnings when a user tries something like<br>
>><br>
>> FinalState fs = zfinder.remainingFinalState();<br>
>><br>
>> because fs is being copy-constructed from a more complex class. @Frank, I<br>
>> think this will also make your attached broken example work as expected.<br>
><br>
> Is this a compile-time warning or error, and are you sure it will<br>
> really complain? I can test it myself later, but maybe you have<br>
> already tested it.<br>
<br>
</span>I just tested compiling my example containing a<br>
FinalState remainder = zfinder.remainingFinalState();<br>
with the current tip, to which David has pushed those changes already.<br>
There is no warning, let alone a fatal error, when compiling with<br>
this.<br>
<br>
So, do we want to go ahead with my proposed changes to protect the<br>
user from the slicing mistake? Consider that even experienced Riveters<br>
like Holger and Christian have tripped over this!<br>
<br>
In that case we would need to understand why<br>
VetoedFinalState remainder = zfinder.remainingFinalState();<br>
addProjection(remainder, "RFS");<br>
(i.e. making a full copy, not just by reference)<br>
makes Rivet's projection book-keeping fall over:<br>
<span><br>
Error in MY_RFSTEST2c::init method: No projections registered for<br>
parent 0x7ffe07be8840<br>
<br>
</span>I am again attaching the patch (updated to remove the changes David<br>
has already committed, needs tip), and the problematic use case<br>
(MY_RFSTEST2c.cc) and a HepMC file to test this with:<br>
<br>
$ cd rivet; patch -p1 < hidden-copy.patch; make install<br>
$ rivet-buildplugin MY_RFSTEST2c.cc<br>
<span>$ rivet --pwd -a MY_RFSTEST2c file.hepmc<br>
<br>
</span>Cheers,<br>
Frank<br>
</blockquote></div><br></div></div></div></div>
</blockquote></div><br></div></div>