-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Really make reco::TrackBase destructor virtual #187
Really make reco::TrackBase destructor virtual #187
Conversation
Hi, I took CMSSW_7_0_X_2013-07-25-1400, pulled these changes, and when I was building I got an error for the package Fireworks/Core: >> Compiling /build/dmendezl/CMSSW_7_0_X_2013-07-25-1400/src/Fireworks/Calo/src/thetaBins.cc >> Building shared library tmp/slc5_amd64_gcc472/src/Fireworks/Core/src/FireworksCore/libFireworksCore.so >> Compiling /build/dmendezl/CMSSW_7_0_X_2013-07-25-1400/src/Fireworks/Muons/src/FWMuonBuilder.cc tmp/slc5_amd64_gcc472/src/Fireworks/Core/src/FireworksCore/FWInvMassDialog.o:FWInvMassDialog.cc:function FWInvMassDialog::Calculate(): error: undefined reference to 'typeinfo for reco::TrackBase' collect2: error: ld returned 1 exit status The error comes from the file FWInvMassDialog.cc, you can look at line 133 or 161: |
I've seen similar problems where SCRAM has failed to recompile some piece of dependent code. When I've done 'scram b clean' then 'scram b' the problem goes away. How clean of a build area did you start from? |
The build error is real. I did a full build before doing the pull request, but apparently I did not look carefully enough at the log file. The error is there. I will fix it. |
The pull request now links properly. There was a missing link dependency in the build file in Fireworks/Core. I added it, and the build now works. I updated the pull request. |
Hi, I pulled these changes on top of CMSSW_7_0_X_2013-07-29-1400, I built it and then ran the unit tests and the RelVals. All the tests passed. |
I would request that unless this request is signed promptly, that the signature be bypassed. It is a purely technical C++ fix that has nothing really to do with reconstruction. |
Really make reco::TrackBase destructor virtual
csc trackfinder: allow data and MC syncronization
Update watchers.yaml for echapon
TimeReport functionality, and heppy bugfixes.
Fix pass all true
Training with some kinematic variables.
Back in January 2008, an attempt was made to add a virtual destructor to reco::TrackBase and its derived classes, such as reco::Track. Unfortunately, the virtual keyword was not added in TrackBase, just a comment stating (incorrectly) that the destructor was virtual. This is a bug, as the class needs a virtual destructor.
This request merely adds the virtual keyword to the destructor to fix the bug. It was tested with checkdeps -a, and the unit tests pass.
This is a trivial C++ bug fix that has nothing really to do with RECO, so if it is not signed in a timely manner, I suggest that the signature be bypassed.
The bug causes a unit test failure in CommonTools/Utils when the Reflex functionality for invoking functions is replaced by ROOT. So, this fix is needed.