diff --git a/.github/workflows/root-ci.yml b/.github/workflows/root-ci.yml index 712c5d45f282a..ec9e736ee08e2 100644 --- a/.github/workflows/root-ci.yml +++ b/.github/workflows/root-ci.yml @@ -79,6 +79,7 @@ env: GLOBAL_OVERRIDES: 'asserts=ON LLVM_ENABLE_ASSERTIONS=ON' ROOT_TEST_S3_ACCESS_KEY: ${{ secrets.ROOT_TEST_S3_ACCESS_KEY }} ROOT_TEST_S3_SECRET_KEY: ${{ secrets.ROOT_TEST_S3_SECRET_KEY }} + ROOT_OBJECT_AUTO_REGISTRATION: 0 concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }} diff --git a/core/base/src/TROOT.cxx b/core/base/src/TROOT.cxx index 9ee7e332ca2b3..397d04d376295 100644 --- a/core/base/src/TROOT.cxx +++ b/core/base/src/TROOT.cxx @@ -295,24 +295,20 @@ namespace { return moduleHeaderInfoBuffer; } + /// State helper for object auto registration. enum class AutoReg : unsigned char { kNotInitialised = 0, kOn, kOff, }; - //////////////////////////////////////////////////////////////////////////////// - /// \brief Test if various objects (such as TH1-derived classes) should automatically register - /// themselves (ROOT 6 mode) or not (ROOT 7 mode). - /// A default can be set in a .rootrc using e.g. "Root.ObjectAutoRegistration: 1" or setting - /// the environment variable "ROOT_OBJECT_AUTO_REGISTRATION=0". - AutoReg &ObjectAutoRegistrationEnabledImpl() + /// Set up the default state for object auto registration by inspecting the environment. + /// This state is used to initialise the auto-registration state for each thread that starts. + AutoReg ObjectAutoRegistrationDefault() { static constexpr auto rcName = "Root.ObjectAutoRegistration"; // Update the docs if this is changed static constexpr auto envName = "ROOT_OBJECT_AUTO_REGISTRATION"; // Update the docs if this is changed - thread_local static AutoReg tlsState = AutoReg::kNotInitialised; - - static const AutoReg defaultState = []() { + static const AutoReg defaultFromEnvironment = []() { AutoReg autoReg = AutoReg::kOn; // ROOT 6 default std::stringstream infoMessage; @@ -356,10 +352,18 @@ namespace { return autoReg; }(); - if (tlsState == AutoReg::kNotInitialised) { - assert(defaultState != AutoReg::kNotInitialised); - tlsState = defaultState; - } + return defaultFromEnvironment; + } + + //////////////////////////////////////////////////////////////////////////////// + /// \brief Test if various objects (such as TH1-derived classes) should automatically register + /// themselves (ROOT 6 mode) or not (ROOT 7 mode). + /// A default can be set in a .rootrc using e.g. "Root.ObjectAutoRegistration: 1" or setting + /// the environment variable "ROOT_OBJECT_AUTO_REGISTRATION=0". + AutoReg &ObjectAutoRegistrationEnabledImpl() + { + thread_local static AutoReg tlsState = ObjectAutoRegistrationDefault(); + assert(tlsState != AutoReg::kNotInitialised); return tlsState; } @@ -727,16 +731,26 @@ namespace Internal { /// | RooPlot | Yes | RooPlot::addDirectoryStatus() | /// | TEfficiency | Yes | No | /// | TProfile2D | Yes | TH1::AddDirectoryStatus() | - /// | TEntryList | No, but planned for 6.42 | No | - /// | TEventList | No, but planned for 6.42 | No | + /// | TEntryList (+ derived)| Yes | No | + /// | TEventList | Yes | No | /// | TFunction | No, but work in progress | No | /// /// ## Setting defaults /// - /// A default can be set in a .rootrc using e.g. `Root.ObjectAutoRegistration: 1` or setting - /// the environment variable `ROOT_OBJECT_AUTO_REGISTRATION=0`. Note that this default affects - /// all the threads that get started. - /// When a default is set using one of these methods, ROOT will notify with an Info message. + /// A default can be set (in order of precedence): + /// 1. Setting the environment variable `ROOT_OBJECT_AUTO_REGISTRATION=[01]` + /// 2. Setting `Root.ObjectAutoRegistration: [01]` in a .rootrc file. + /// + /// To do this programmatically, one can use + /// \code{.cpp} + /// setenv("ROOT_OBJECT_AUTO_REGISTRATION", 1); + /// // or using ROOT's TEnv: + /// gEnv->SetValue("Root.ObjectAutoRegistration", 1); + /// \endcode + /// This has to be done *before* the first object with auto-registration is created. Once this is done, + /// every thread starts with the same default. A running thread's behaviour can only be changed using + /// Enable/DisableObjectAutoRegistration(). + /// When the default state is changed using the environment or .rootrc, ROOT issues a reminder. /// /// ## Difference to TH1::AddDirectoryStatus() /// @@ -765,6 +779,7 @@ namespace Internal { assert(state != AutoReg::kNotInitialised); return state == AutoReg::kOn; } + } // namespace Experimental } // end of ROOT namespace diff --git a/math/mlp/inc/TMultiLayerPerceptron.h b/math/mlp/inc/TMultiLayerPerceptron.h index 36d5d01d34aab..1c423321091da 100644 --- a/math/mlp/inc/TMultiLayerPerceptron.h +++ b/math/mlp/inc/TMultiLayerPerceptron.h @@ -15,8 +15,8 @@ #include "TObject.h" #include "TString.h" #include "TObjArray.h" -#include "TMatrixD.h" #include "TNeuron.h" +#include "TMatrixDfwd.h" class TTree; class TEventList; diff --git a/math/mlp/src/TMLPAnalyzer.cxx b/math/mlp/src/TMLPAnalyzer.cxx index 39271c1e00c7f..afe7851f69062 100644 --- a/math/mlp/src/TMLPAnalyzer.cxx +++ b/math/mlp/src/TMLPAnalyzer.cxx @@ -350,6 +350,8 @@ void TMLPAnalyzer::DrawNetwork(Int_t neuron, const char* signal, const char* bg) // build event lists for signal and background TEventList* signal_list = new TEventList("__tmpSig_MLPA"); TEventList* bg_list = new TEventList("__tmpBkg_MLPA"); + signal_list->SetDirectory(gDirectory); + bg_list->SetDirectory(gDirectory); data->Draw(">>__tmpSig_MLPA",signal,"goff"); data->Draw(">>__tmpBkg_MLPA",bg,"goff"); diff --git a/math/mlp/src/TMultiLayerPerceptron.cxx b/math/mlp/src/TMultiLayerPerceptron.cxx index 87a6068a1c348..f82aebbb1ea8a 100644 --- a/math/mlp/src/TMultiLayerPerceptron.cxx +++ b/math/mlp/src/TMultiLayerPerceptron.cxx @@ -242,6 +242,7 @@ neurons, hidden layers and inputs/outputs that does not apply to TMultiLayerPerc #include "TH2.h" #include "TGraph.h" #include "TLegend.h" +#include "TMatrixD.h" #include "TMultiGraph.h" #include "TDirectory.h" #include "TSystem.h" @@ -458,12 +459,12 @@ TMultiLayerPerceptron::TMultiLayerPerceptron(const char * layout, TTree * data, fCurrentTree = -1; fCurrentTreeWeight = 1; { - TDirectory::TContext ctxt; + TDirectory::TContext ctxt{nullptr}; fTraining = new TEventList(Form("fTrainingList_%zu",(size_t)this)); } fTrainingOwner = true; { - TDirectory::TContext ctxt; + TDirectory::TContext ctxt{nullptr}; fTest = new TEventList(Form("fTestList_%zu",(size_t)this)); } fTestOwner = true; @@ -478,8 +479,12 @@ TMultiLayerPerceptron::TMultiLayerPerceptron(const char * layout, TTree * data, fManager = nullptr; if (data) { BuildNetwork(); + fTraining->SetDirectory(gDirectory); + fTest->SetDirectory(gDirectory); data->Draw(Form(">>fTrainingList_%zu",(size_t)this),training,"goff"); data->Draw(Form(">>fTestList_%zu",(size_t)this),(const char *)testcut,"goff"); + fTraining->SetDirectory(nullptr); + fTest->SetDirectory(nullptr); AttachData(); } else { @@ -537,12 +542,12 @@ TMultiLayerPerceptron::TMultiLayerPerceptron(const char * layout, fCurrentTree = -1; fCurrentTreeWeight = 1; { - TDirectory::TContext ctxt; + TDirectory::TContext ctxt{nullptr}; fTraining = new TEventList(Form("fTrainingList_%zu",(size_t)this)); } fTrainingOwner = true; { - TDirectory::TContext ctxt; + TDirectory::TContext ctxt{nullptr}; fTest = new TEventList(Form("fTestList_%zu",(size_t)this)); } fTestOwner = true; @@ -557,8 +562,12 @@ TMultiLayerPerceptron::TMultiLayerPerceptron(const char * layout, fManager = nullptr; if (data) { BuildNetwork(); + fTraining->SetDirectory(gDirectory); + fTest->SetDirectory(gDirectory); data->Draw(Form(">>fTrainingList_%zu",(size_t)this),training,"goff"); data->Draw(Form(">>fTestList_%zu",(size_t)this),(const char *)testcut,"goff"); + fTraining->SetDirectory(nullptr); + fTest->SetDirectory(nullptr); AttachData(); } else { @@ -645,12 +654,14 @@ void TMultiLayerPerceptron::SetTrainingDataSet(const char * train) { if(fTraining && fTrainingOwner) delete fTraining; { - TDirectory::TContext ctxt; + TDirectory::TContext ctxt{nullptr}; fTraining = new TEventList(Form("fTrainingList_%zu",(size_t)this)); } fTrainingOwner = true; if (fData) { + fTraining->SetDirectory(gDirectory); fData->Draw(Form(">>fTrainingList_%zu",(size_t)this),train,"goff"); + fTraining->SetDirectory(nullptr); } else { Warning("TMultiLayerPerceptron::TMultiLayerPerceptron","Data not set. Cannot define datasets"); @@ -672,7 +683,9 @@ void TMultiLayerPerceptron::SetTestDataSet(const char * test) } fTestOwner = true; if (fData) { + fTraining->SetDirectory(gDirectory); fData->Draw(Form(">>fTestList_%zu",(size_t)this),test,"goff"); + fTraining->SetDirectory(nullptr); } else { Warning("TMultiLayerPerceptron::TMultiLayerPerceptron","Data not set. Cannot define datasets"); diff --git a/roottest/cling/specialobj/CMakeLists.txt b/roottest/cling/specialobj/CMakeLists.txt index 1b810989e50d1..0e5f06a232ed5 100644 --- a/roottest/cling/specialobj/CMakeLists.txt +++ b/roottest/cling/specialobj/CMakeLists.txt @@ -5,6 +5,7 @@ ROOTTEST_ADD_TEST(assertGPad ROOTTEST_ADD_TEST(stlwrite MACRO write.C + FIXTURES_SETUP cling-specialobj-stlwrite LABELS roottest regression cling) ROOTTEST_ADD_TEST(argtwice @@ -14,7 +15,7 @@ ROOTTEST_ADD_TEST(argtwice ROOTTEST_ADD_TEST(stlProxies MACRO runstlProxies.C OUTREF stlProxies.ref - DEPENDS stlwrite + FIXTURES_REQUIRED cling-specialobj-stlwrite LABELS roottest regression cling) if(${CMAKE_SYSTEM_PROCESSOR} MATCHES "x86_64.*|x86.*|amd64.*|AMD64.*|i686.*|i386.*") diff --git a/roottest/root/tree/cache/variableCluster.C b/roottest/root/tree/cache/variableCluster.C index a712392e5feb6..de107e8a40324 100644 --- a/roottest/root/tree/cache/variableCluster.C +++ b/roottest/root/tree/cache/variableCluster.C @@ -220,26 +220,13 @@ Long64_t checkBoundary(TTree *tree, Long64_t entry) TTree::TClusterIterator clusterIter = tree->GetClusterIterator(entry); fEntryCurrent = clusterIter(); fEntryNext = clusterIter.GetNextEntry(); - + // fprintf(stdout,"finds = %lld %lld\n",fEntryCurrent,fEntryNext); - + if (fEntryCurrent < fEntryMin) fEntryCurrent = fEntryMin; if (fEntryMax <= 0) fEntryMax = tree->GetEntries(); if (fEntryNext > fEntryMax) fEntryNext = fEntryMax; - - // Check if owner has a TEventList set. If yes we optimize for this - // Special case reading only the baskets containing entries in the - // list. -// TEventList *elist = fOwner->GetEventList(); -// Long64_t chainOffset = 0; -// if (elist) { -// if (fOwner->IsA() ==TChain::Class()) { -// TChain *chain = (TChain*)fOwner; -// Int_t t = chain->GetTreeNumber(); -// chainOffset = chain->GetTreeOffset()[t]; -// } -// } - + Int_t flushIntervals = 0; Long64_t minEntry = fEntryCurrent; Long64_t prevNtot; diff --git a/test/stress.cxx b/test/stress.cxx index 3eab8c15439de..24732779c5669 100644 --- a/test/stress.cxx +++ b/test/stress.cxx @@ -711,6 +711,7 @@ void stress7() char cutname[20]; TEventList *el[10]; TEventList *elistall = new TEventList("elistall","Sum of all cuts"); + elistall->SetDirectory(gDirectory); for (i=0;i<10;i++) { snprintf(elistname,20,">>elist%d",i); snprintf(cutname,20,"i 10 == %d",i); cutname[1] ='%'; diff --git a/test/stressEntryList.cxx b/test/stressEntryList.cxx index 8fd7fb38285d9..c9039d2ed650c 100644 --- a/test/stressEntryList.cxx +++ b/test/stressEntryList.cxx @@ -35,7 +35,6 @@ // *******************Deleting the data files**************************** // ********************************************************************** -#include #include #include #include @@ -52,7 +51,6 @@ #include "TFile.h" #include "TSystem.h" #include "TError.h" -#include "snprintf.h" Int_t stressEntryList(Int_t nentries = 10000, Int_t nfiles = 10); void MakeTrees(Int_t nentries, Int_t nfiles); @@ -219,8 +217,10 @@ Bool_t Test2() TCut cut1("cut1", "x>0"); TCut cut2("cut2", "y<0.1 && y>-0.1"); TEntryList *elist1 = new TEntryList("elist1", "elist1"); - chain->Draw(">>elist1", cut1, "entrylist"); TEntryList *elist2 = new TEntryList("elist2", "elist2"); + elist1->SetDirectory(gDirectory); + elist2->SetDirectory(gDirectory); + chain->Draw(">>elist1", cut1, "entrylist"); chain->Draw(">>elist2", cut2, "entrylist"); //add those 2 lists (1+2) @@ -229,6 +229,7 @@ Bool_t Test2() elistsum->Add(elist2); TEntryList *elistcheck = new TEntryList("elistcheck", "elistcheck"); + elistcheck->SetDirectory(gDirectory); chain->Draw(">>elistcheck", cut1 || cut2, "entrylist"); Int_t n=elistcheck->GetN(); @@ -264,6 +265,7 @@ Bool_t Test2() //add by using "+" in TTree::Draw TEntryList *elistsum3 = new TEntryList("elistsum3", "elistsum3"); + elistsum3->SetDirectory(gDirectory); chain->Draw(">>elistsum3", cut1, "entrylist"); chain->Draw(">>+elistsum3", cut2, "entrylist"); wrongentries3 = 0; @@ -282,6 +284,7 @@ Bool_t Test2() elistsum->Subtract(elist2); n = elistsum->GetN(); TEntryList *elistcheck2 = new TEntryList("elistcheck2","elistcheck2"); + elistcheck2->SetDirectory(gDirectory); chain->Draw(">>elistcheck2", cut1 && !cut2, "entrylist"); wrongentries4 = 0; diff --git a/tmva/tmva/src/Classification.cxx b/tmva/tmva/src/Classification.cxx index 58b21238dc63a..8a8ec50f03280 100644 --- a/tmva/tmva/src/Classification.cxx +++ b/tmva/tmva/src/Classification.cxx @@ -34,7 +34,6 @@ #include #include #include -#include #include #include #include diff --git a/tmva/tmva/src/DataInputHandler.cxx b/tmva/tmva/src/DataInputHandler.cxx index bde79ed57e965..2c725b85aa03f 100644 --- a/tmva/tmva/src/DataInputHandler.cxx +++ b/tmva/tmva/src/DataInputHandler.cxx @@ -36,12 +36,9 @@ Class that contains all the data information. #include "TMVA/DataLoader.h" #include "TMVA/MsgLogger.h" #include "TMVA/Types.h" -#include "TEventList.h" #include "TCut.h" #include "TTree.h" -#include "TMVA/Configurable.h" - #include #include diff --git a/tmva/tmva/src/DataSetFactory.cxx b/tmva/tmva/src/DataSetFactory.cxx index 8b09d1bb9eb14..96d9b2e85d334 100644 --- a/tmva/tmva/src/DataSetFactory.cxx +++ b/tmva/tmva/src/DataSetFactory.cxx @@ -41,26 +41,17 @@ Class that contains all the data information #include #include -#include -#include -#include #include "TMVA/DataSetFactory.h" -#include "TEventList.h" #include "TFile.h" #include "TRandom3.h" -#include "TMatrixF.h" -#include "TVectorF.h" #include "TMath.h" #include "TTree.h" #include "TBranch.h" #include "TMVA/MsgLogger.h" #include "TMVA/Configurable.h" -#include "TMVA/VariableIdentityTransform.h" -#include "TMVA/VariableDecorrTransform.h" -#include "TMVA/VariablePCATransform.h" #include "TMVA/DataSet.h" #include "TMVA/DataSetInfo.h" #include "TMVA/DataInputHandler.h" diff --git a/tmva/tmva/src/DataSetInfo.cxx b/tmva/tmva/src/DataSetInfo.cxx index 651f1e6b9b99d..6edf91cde5076 100644 --- a/tmva/tmva/src/DataSetInfo.cxx +++ b/tmva/tmva/src/DataSetInfo.cxx @@ -31,14 +31,8 @@ Class that contains all the data information. */ -#include - -#include "TEventList.h" #include "TH2.h" -#include "TRandom3.h" #include "TMatrixF.h" -#include "TVectorF.h" -#include "TROOT.h" #include "TMVA/MsgLogger.h" #include "TMVA/Tools.h" @@ -50,6 +44,8 @@ Class that contains all the data information. #include "TMVA/Types.h" #include "TMVA/VariableInfo.h" +#include + //////////////////////////////////////////////////////////////////////////////// /// constructor diff --git a/tmva/tmva/src/Factory.cxx b/tmva/tmva/src/Factory.cxx index 278730bbef214..42f3081621be6 100644 --- a/tmva/tmva/src/Factory.cxx +++ b/tmva/tmva/src/Factory.cxx @@ -71,26 +71,23 @@ evaluation phases. #include "TMVA/ResultsClassification.h" #include "TMVA/ResultsRegression.h" #include "TMVA/ResultsMulticlass.h" -#include -#include -#include #include "TMVA/Types.h" #include "TROOT.h" #include "TFile.h" -#include "TLeaf.h" -#include "TEventList.h" #include "TH2.h" #include "TGraph.h" #include "TStyle.h" -#include "TMatrixF.h" -#include "TMatrixDSym.h" -#include "TMultiGraph.h" #include "TPrincipal.h" #include "TMath.h" #include "TSystem.h" #include "TCanvas.h" +#include "TMultiGraph.h" + +#include +#include +#include const Int_t MinNoTrainingEvents = 10; // const Int_t MinNoTestEvents = 1; diff --git a/tmva/tmva/src/MethodTMlpANN.cxx b/tmva/tmva/src/MethodTMlpANN.cxx index f67da9507ba78..39b6b8abf2dad 100644 --- a/tmva/tmva/src/MethodTMlpANN.cxx +++ b/tmva/tmva/src/MethodTMlpANN.cxx @@ -46,7 +46,6 @@ for details on this ANN. #include "TMVA/MethodTMlpANN.h" #include "TMVA/Config.h" -#include "TMVA/Configurable.h" #include "TMVA/DataSet.h" #include "TMVA/DataSetInfo.h" #include "TMVA/IMethod.h" @@ -58,8 +57,6 @@ for details on this ANN. #include "TMVA/ClassifierFactory.h" #include "TMVA/Tools.h" -#include "TLeaf.h" -#include "TEventList.h" #include "TROOT.h" #include "TMultiLayerPerceptron.h" #include "ThreadLocalStorage.h" diff --git a/tree/tree/inc/TEntryList.h b/tree/tree/inc/TEntryList.h index 7152e9f7225ed..b6f1312bd5740 100644 --- a/tree/tree/inc/TEntryList.h +++ b/tree/tree/inc/TEntryList.h @@ -28,27 +28,27 @@ class TEntryList: public TNamed TEntryList& operator=(const TEntryList&); // Not implemented protected: - TList *fLists; ///< a list of underlying entry lists for each tree of a chain - TEntryList *fCurrent; ///Append(this); - - fLastIndexQueried = -1; - fLastIndexReturned = 0; - fShift = false; + if (ROOT::Experimental::ObjectAutoRegistrationEnabled()) { + fDirectory = gDirectory; + if (fDirectory) + fDirectory->Append(this); + } } //////////////////////////////////////////////////////////////////////////////// /// constructor with name and title, which also sets the tree -TEntryList::TEntryList(const char *name, const char *title, const TTree *tree):TNamed(name, title) +TEntryList::TEntryList(const char *name, const char *title, const TTree *tree) : TNamed(name, title) { - fLists = nullptr; - fCurrent = nullptr; - fBlocks = nullptr; - fN = 0; - fNBlocks = 0; - fTreeNumber = -1; TEntryList::SetTree(tree); - fReapply = false; - fDirectory = gDirectory; - if (fDirectory) fDirectory->Append(this); - - fLastIndexQueried = -1; - fLastIndexReturned = 0; - fShift = false; + if (ROOT::Experimental::ObjectAutoRegistrationEnabled()) { + fDirectory = gDirectory; + if (fDirectory) + fDirectory->Append(this); + } } //////////////////////////////////////////////////////////////////////////////// /// c-tor with name and title, which also sets the treename and the filename -TEntryList::TEntryList(const char *name, const char *title, const char *treename, const char *filename) : TNamed(name, title),fEntriesToProcess(0) +TEntryList::TEntryList(const char *name, const char *title, const char *treename, const char *filename) + : TNamed(name, title) { - fLists = nullptr; - fCurrent = nullptr; - fBlocks = nullptr; - fNBlocks = 0; - fN = 0; SetTree(treename, filename); - fTreeNumber = -1; - fReapply = false; - fDirectory = gDirectory; - if (fDirectory) fDirectory->Append(this); - - fLastIndexQueried = -1; - fLastIndexReturned = 0; - fShift = false; + if (ROOT::Experimental::ObjectAutoRegistrationEnabled()) { + fDirectory = gDirectory; + if (fDirectory) + fDirectory->Append(this); + } } //////////////////////////////////////////////////////////////////////////////// /// c-tor, which sets the tree -TEntryList::TEntryList(const TTree *tree) : fEntriesToProcess(0) +TEntryList::TEntryList(const TTree *tree) { - fLists = nullptr; - fCurrent = nullptr; - fBlocks = nullptr; - fNBlocks = 0; - fN = 0; - SetTree(tree); - fTreeNumber = -1; - fReapply = false; - fDirectory = gDirectory; - if (fDirectory) fDirectory->Append(this); - - fLastIndexQueried = -1; - fLastIndexReturned = 0; - fShift = false; + if (ROOT::Experimental::ObjectAutoRegistrationEnabled()) { + fDirectory = gDirectory; + if (fDirectory) + fDirectory->Append(this); + } } //////////////////////////////////////////////////////////////////////////////// /// copy c-tor -TEntryList::TEntryList(const TEntryList &elist) : TNamed(elist) +TEntryList::TEntryList(const TEntryList &elist) + : TNamed(elist), + fNBlocks(elist.fNBlocks), + fN(elist.fN), + fEntriesToProcess(elist.fEntriesToProcess), + fTreeName(elist.fTreeName), + fFileName(elist.fFileName), + fStringHash(elist.fStringHash), + fTreeNumber(elist.fTreeNumber), + fShift(elist.fShift), + fReapply(elist.fReapply) { - fNBlocks = elist.fNBlocks; - fTreeName = elist.fTreeName; - fFileName = elist.fFileName; - fStringHash = elist.fStringHash; - fTreeNumber = elist.fTreeNumber; - fLastIndexQueried = -1; - fLastIndexReturned = 0; - fN = elist.fN; - fShift = elist.fShift; - fLists = nullptr; - fBlocks = nullptr; - fReapply = elist.fReapply; - fCurrent = nullptr; - fEntriesToProcess = elist.fEntriesToProcess; if (elist.fLists){ fLists = new TList(); TEntryList *el1 = nullptr; @@ -320,8 +262,6 @@ TEntryList::TEntryList(const TEntryList &elist) : TNamed(elist) } fCurrent = this; } - fDirectory = nullptr; - } //////////////////////////////////////////////////////////////////////////////// diff --git a/tree/tree/src/TEventList.cxx b/tree/tree/src/TEventList.cxx index 735db20b22930..972c0779dcedb 100644 --- a/tree/tree/src/TEventList.cxx +++ b/tree/tree/src/TEventList.cxx @@ -50,52 +50,36 @@ the TEventList object created in the above commands: #include "TBuffer.h" #include "TCut.h" #include "TDirectory.h" -#include "TList.h" -#include "TMath.h" -#include "strlcpy.h" -#include "snprintf.h" - - -//////////////////////////////////////////////////////////////////////////////// -/// Default constructor for a EventList. - -TEventList::TEventList(): TNamed() -{ - fN = 0; - fSize = 100; - fDelta = 100; - fList = nullptr; - fDirectory = nullptr; - fReapply = false; -} +#include "TCollection.h" +#include "TMathBase.h" +#include "TROOT.h" //////////////////////////////////////////////////////////////////////////////// /// Create a EventList. /// /// This Eventlist is added to the list of objects in current directory. -TEventList::TEventList(const char *name, const char *title, Int_t initsize, Int_t delta) - :TNamed(name,title), fReapply(false) +TEventList::TEventList(const char *name, const char *title, Int_t initsize, Int_t delta) : TNamed(name, title) { - fN = 0; if (initsize > 100) fSize = initsize; else fSize = 100; if (delta > 100) fDelta = delta; else fDelta = 100; fList = nullptr; - fDirectory = gDirectory; - if (fDirectory) fDirectory->Append(this); + if (ROOT::Experimental::ObjectAutoRegistrationEnabled()) { + fDirectory = gDirectory; + if (fDirectory) + fDirectory->Append(this); + } } //////////////////////////////////////////////////////////////////////////////// /// Copy constructor. -TEventList::TEventList(const TEventList &list) : TNamed(list) +TEventList::TEventList(const TEventList &list) + : TNamed(list), fN(list.fN), fSize(list.fSize), fDelta(list.fDelta), fList(new Long64_t[fSize]) { - fN = list.fN; - fSize = list.fSize; - fDelta = list.fDelta; - fList = new Long64_t[fSize]; + for (Int_t i=0; iRemove(this); - fDirectory = nullptr; + delete[] fList; + if (fDirectory) + fDirectory->Remove(this); } //////////////////////////////////////////////////////////////////////////////// diff --git a/tree/tree/src/TTree.cxx b/tree/tree/src/TTree.cxx index 8023b9b939bf3..2b67ed2f0d7c4 100644 --- a/tree/tree/src/TTree.cxx +++ b/tree/tree/src/TTree.cxx @@ -4172,9 +4172,9 @@ Long64_t TTree::Draw(const char* varexp, const TCut& selection, Option_t* option /// // 100 bins in x-direction; lower limit on x-axis is 10; upper limit is 60 /// // 50 bins in y-direction; lower limit on y-axis is .1; upper limit is .5 /// ~~~ -/// By default, the specified histogram is reset. -/// To continue to append data to an existing histogram, use "+" in front -/// of the histogram name. +/// By default, if a histogram with the same name is already registered to the current +/// ROOT directory, the specified histogram is reset. To continue to append data to an +/// existing histogram, use "+" in front of the histogram name. /// /// A '+' in front of the histogram name is ignored, when the name is followed by /// binning information as described in the previous paragraph. @@ -4184,6 +4184,15 @@ Long64_t TTree::Draw(const char* varexp, const TCut& selection, Option_t* option /// will not reset `hsqrt`, but will continue filling. This works for 1-D, 2-D /// and 3-D histograms. /// +/// Note that when the automatic registration of histograms is off (see \ref DisableObjectAutoRegistration() ), +/// external histogram are not visible to TTree::Draw unless they are registered to the current directory explicitly. +/// ~~~ {.cpp} +/// auto histo = new TH1D("histo", ...); +/// histo->SetDirectory(gDirectory); +/// tree.Draw("sqrt(x)>>histo","y>0") +/// ~~~ +/// When auto-registration is off, histograms created by TTree::Draw will still be registered to the current directory. +/// /// ### Accessing collection objects /// /// TTree::Draw default's handling of collections is to assume that any @@ -4434,6 +4443,14 @@ Long64_t TTree::Draw(const char* varexp, const TCut& selection, Option_t* option /// will not reset yplus, but will enter the selected entries at the end /// of the existing list. /// +/// Note that when the automatic registration of event lists is off (see \ref DisableObjectAutoRegistration() ), +/// they are not visible to TTree::Draw unless they are registered to the current directory explicitly. +/// ~~~ {.cpp} +/// auto elist = new TEventList("elist", ...); +/// elist->SetDirectory(gDirectory); +/// tree.Draw(">>+elist","y>0") +/// ~~~ +/// /// ### Using a TEventList, TEntryList or TEntryListArray as Input /// /// Once a TEventList or a TEntryList object has been generated, it can be used as input diff --git a/tree/treeplayer/src/TSelectorDraw.cxx b/tree/treeplayer/src/TSelectorDraw.cxx index 8aa71aec11512..6f04bc04843f7 100644 --- a/tree/treeplayer/src/TSelectorDraw.cxx +++ b/tree/treeplayer/src/TSelectorDraw.cxx @@ -35,8 +35,8 @@ A specialized TSelector for TTree::Draw. #include "TStyle.h" #include "TClass.h" #include "TColor.h" -#include "strlcpy.h" +#include const Int_t kCustomHistogram = BIT(17); @@ -119,7 +119,7 @@ void TSelectorDraw::Begin(TTree *tree) TString opt, abrt; char *hdefault = (char *)"htemp"; - char *varexp = nullptr; + std::string varexp; Int_t i, j, hkeep; opt = option; opt.ToLower(); @@ -213,9 +213,7 @@ void TSelectorDraw::Begin(TTree *tree) } // char *hname = (char*)strstr(varexp0,">>"); if (hname) { - hkeep = 1; - varexp = new char[i+1]; - varexp[0] = 0; //necessary if i=0 + hkeep = 1; bool hnameplus = false; while (*hname == ' ') hname++; if (*hname == '+') { @@ -231,7 +229,7 @@ void TSelectorDraw::Begin(TTree *tree) } if (i) { - strlcpy(varexp,varexp0,i+1); + varexp = std::string(varexp0, i); // everything before ">>" Int_t mustdelete = 0; SetBit(kCustomHistogram); @@ -378,9 +376,14 @@ void TSelectorDraw::Begin(TTree *tree) if (!fOldHistogram && oldObject && !oldObject->InheritsFrom(TH1::Class())) { abrt.Form("An object of type '%s' has the same name as the requested histo (%s)", oldObject->IsA()->GetName(), hname); Abort(abrt); - delete[] varexp; return; } + if (!fOldHistogram && hnameplus) { + Warning("TSelectorDraw", + "TTree::Draw was asked to fill the histogram '%s', but it was not found in the current directory." + "Did you forget to call histogram->SetDirectory(gDirectory) or similar?", + hname); + } if (fOldHistogram && !hnameplus) fOldHistogram->Reset(); // reset unless adding is wanted if (mustdelete) { @@ -401,15 +404,23 @@ void TSelectorDraw::Begin(TTree *tree) abrt.Form("An object of type '%s' has the same name as the requested event list (%s)", oldObject->IsA()->GetName(), hname); Abort(abrt); - delete[] varexp; return; } if (!enlist) { + if (hnameplus) { + Warning( + "TSelectorDraw", + "TTree::Draw was asked to append to TEntryList '%s', but it was not found in the current " + "directory." + "Did you forget to call entryList->SetDirectory(gDirectory) or similar? Creating a new list now.", + hname); + } if (optEnlistArray) { enlist = new TEntryListArray(hname, realSelection.GetTitle()); } else { enlist = new TEntryList(hname, realSelection.GetTitle()); } + enlist->SetDirectory(gDirectory); // TTree::Draw documentation promises it shows up in gDirectory } if (enlist) { if (!hnameplus) { @@ -421,6 +432,7 @@ void TSelectorDraw::Begin(TTree *tree) } else { inElist = new TEntryList(*enlist); } + inElist->SetDirectory(gDirectory); // TTree::Draw documentation promises it shows up in gDirectory fCleanElist = true; fTree->SetEntryList(inElist); } @@ -440,11 +452,19 @@ void TSelectorDraw::Begin(TTree *tree) abrt.Form("An object of type '%s' has the same name as the requested event list (%s)", oldObject->IsA()->GetName(), hname); Abort(abrt); - delete[] varexp; return; } if (!evlist) { + if (hnameplus) { + Warning( + "TSelectorDraw", + "TTree::Draw was asked to append to TEventList '%s', but it was not found in the current " + "directory." + "Did you forget to call eventList->SetDirectory(gDirectory) or similar? Creating a new list now.", + hname); + } evlist = new TEventList(hname, realSelection.GetTitle(), 1000, 0); + evlist->SetDirectory(gDirectory); // TTree::Draw documentation promises it shows up in gDirectory } if (evlist) { if (!hnameplus) { @@ -452,7 +472,6 @@ void TSelectorDraw::Begin(TTree *tree) // We have been asked to reset the input list!! // Let's set it aside for now ... Abort("Input and output lists are the same!"); - delete[] varexp; return; } evlist->Reset(); @@ -469,9 +488,7 @@ void TSelectorDraw::Begin(TTree *tree) } else { // if (hname) hname = hdefault; hkeep = 0; - const size_t varexpLen = strlen(varexp0) + 1; - varexp = new char[varexpLen]; - strlcpy(varexp, varexp0, varexpLen); + varexp = varexp0; if (gDirectory) { fOldHistogram = (TH1*)gDirectory->Get(hname); if (fOldHistogram) { fOldHistogram->Delete(); fOldHistogram = nullptr;} @@ -479,27 +496,24 @@ void TSelectorDraw::Begin(TTree *tree) } // Decode varexp and selection - if (!CompileVariables(varexp, realSelection.GetTitle())) { - abrt.Form("Variable compilation failed: {%s,%s}", varexp, realSelection.GetTitle()); + if (!CompileVariables(varexp.data(), realSelection.GetTitle())) { + abrt.Form("Variable compilation failed: {%s,%s}", varexp.data(), realSelection.GetTitle()); Abort(abrt); - delete[] varexp; return; } if (fDimension > 4 && !(optpara || optcandle || opt5d || opt.Contains("goff"))) { Abort("Too many variables. Use the option \"para\", \"gl5d\" or \"candle\" to display more than 4 variables."); - delete[] varexp; return; } if (fDimension < 2 && (optpara || optcandle)) { Abort("The options \"para\" and \"candle\" require at least 2 variables."); - delete[] varexp; return; } // In case fOldHistogram exists, check dimensionality Int_t nsel = strlen(selection); if (nsel > 1) { - htitle.Form("%s {%s}", varexp, selection); + htitle.Form("%s {%s}", varexp.data(), selection); } else { htitle = varexp; } @@ -535,7 +549,6 @@ void TSelectorDraw::Begin(TTree *tree) gROOT->MakeDefCanvas(); if (!gPad) { Abort("Creation of default canvas failed"); - delete[] varexp; return; } } @@ -915,7 +928,6 @@ void TSelectorDraw::Begin(TTree *tree) else if (opt5d) fAction = 8; else fAction = 6; } - if (varexp) delete[] varexp; for (i = 0; i < fValSize; ++i) fVarMultiple[i] = false; fSelectMultiple = false; diff --git a/tree/treeplayer/src/TTreePlayer.cxx b/tree/treeplayer/src/TTreePlayer.cxx index 4065337e4d956..6decc11d535fb 100644 --- a/tree/treeplayer/src/TTreePlayer.cxx +++ b/tree/treeplayer/src/TTreePlayer.cxx @@ -622,8 +622,6 @@ Long64_t TTreePlayer::GetEntriesToProcess(Long64_t firstentry, Long64_t nentries lastentry = fTree->GetEntriesFriend() - 1; nentries = lastentry - firstentry + 1; } - //TEventList *elist = fTree->GetEventList(); - //if (elist && elist->GetN() < nentries) nentries = elist->GetN(); TEntryList *elist = fTree->GetEntryList(); if (elist && elist->GetN() < nentries) nentries = elist->GetN(); return nentries; diff --git a/tree/treeplayer/src/TTreeTableInterface.cxx b/tree/treeplayer/src/TTreeTableInterface.cxx index 74ae4839c3191..dd946b96f9b03 100644 --- a/tree/treeplayer/src/TTreeTableInterface.cxx +++ b/tree/treeplayer/src/TTreeTableInterface.cxx @@ -200,6 +200,7 @@ void TTreeTableInterface::SyncFormulas() void TTreeTableInterface::InitEntries() { TEntryList *entrylist = new TEntryList(fTree); + entrylist->SetDirectory(fTree->GetDirectory()); UInt_t ui = 0; Int_t i = 0; diff --git a/tree/treeplayer/test/basic.cxx b/tree/treeplayer/test/basic.cxx index 1de23ff5eda91..9ea1713bd4992 100644 --- a/tree/treeplayer/test/basic.cxx +++ b/tree/treeplayer/test/basic.cxx @@ -1,3 +1,4 @@ +#include "TEventList.h" #include "TEntryListArray.h" #include "TChain.h" @@ -270,6 +271,7 @@ TEST(TTreeReaderBasic, EntryList) { auto tree = MakeTree(); EXPECT_EQ(9, tree->Draw(">>negZ","three.z<0", "entrylistarray")); TEntryListArray* selected = (TEntryListArray*)gDirectory->Get("negZ"); + ASSERT_NE(selected, nullptr); TTreeReader aReader(tree.get(), selected); EXPECT_EQ(9, aReader.GetEntries(false)); @@ -572,6 +574,43 @@ TEST(TTreeReaderBasic, ZeroEntriesTreeCheckValueStatus) } } +template +void TTreeDrawTest() +{ + constexpr auto optionString = std::is_same_v ? "entrylist" + : std::is_same_v ? "entrylistarray" + : ""; + auto tree = MakeTree(); + tree->Draw(">>elist", "z>2.", optionString); + auto eventList = gDirectory->Get("elist"); + ASSERT_NE(eventList, nullptr); + eventList->SetDirectory(nullptr); + delete eventList; + + auto elistUnreg = std::make_unique("elistUnreg", "Unregistered entry/event list"); + elistUnreg->SetDirectory(nullptr); + ROOT_EXPECT_WARNING_PARTIAL(tree->Draw(">>+elistUnreg", "z>2.", optionString), "TSelectorDraw", + "TTree::Draw was asked to append to"); + auto elistPost = gDirectory->Get("elistUnreg"); + EXPECT_NE(elistPost, nullptr); + EXPECT_NE(elistUnreg.get(), elistPost); + delete elistPost; +} + +TEST(TTreeDraw, TEventList) +{ + TTreeDrawTest(); +} + +TEST(TTreeDraw, TEntryList) +{ + TTreeDrawTest(); +} + +TEST(TTreeDraw, TEntryListArray) +{ + TTreeDrawTest(); +} #ifdef R__USE_IMT // Check the warning emitted to address ROOT-10972 diff --git a/tree/treeviewer/src/TSpider.cxx b/tree/treeviewer/src/TSpider.cxx index 4b8bd6b06c7c9..205939bb3bd3d 100644 --- a/tree/treeviewer/src/TSpider.cxx +++ b/tree/treeviewer/src/TSpider.cxx @@ -730,8 +730,6 @@ Long64_t TSpider::GetEntriesToProcess(Long64_t firstentry, Long64_t nentries) co lastentry = fTree->GetEntriesFriend() - 1; nentries = lastentry - firstentry + 1; } - //TEventList *elist = fTree->GetEventList(); - //if (elist && elist->GetN() < nentries) nentries = elist->GetN(); TEntryList *elist = fTree->GetEntryList(); if (elist && elist->GetN() < nentries) nentries = elist->GetN(); return nentries;