Llvm16

README/ReleaseNotes/v632/index.md

@@ -39,6 +39,7 @@ The following people have contributed to this new version:
 
 ## Deprecation and Removal
 - The RooFit legacy iterators are deprecated and will be removed in ROOT 6.34 (see section "RooFit libraries")
+- Some memory-unsafe RooFit interfaces were removed
 
 ## Core Libraries
 
@@ -57,6 +58,81 @@ The following people have contributed to this new version:
 
 ## RooFit Libraries
 
+### Compile your code with memory safe interfaces
+
+If you define the `ROOFIT_MEMORY_SAFE_INTERFACES` preprocessor macro, the
+RooFit interface changes in a way such that memory leaks are avoided.
+
+The most prominent effect of this change is that many functions that used to
+return an owning pointer (e.g., a pointer to an object that you need to
+manually `delete`) are then returning a `std::unique_pt` for automatic memory
+management.
+
+For example this code would not compile anymore, because there is the rist that
+the caller forgets to `delete params`:
+```c++
+RooArgSet * params = pdf.getParameters(nullptr);
+```
+If you wrap such return values in a `std::unique_ptr`, then your code will
+compile both with and without memory safe interfaces:
+```c++
+std::unique_ptr<RooArgSet> params{pdf.getParameters(nullptr)};
+```
+
+Also some `virtual` RooFit functions like [RooAbsReal::createIntegral()](https://root.cern.ch/doc/master/classRooAbsReal.html#aff4be07dd6a131721daeeccf6359aea9)
+are returning a different type conditional on `ROOFIT_MEMORY_SAFE_INTERFACES`.
+If you are overriding such a function, you need to use the `RooFit::OwningPtr`
+return type, which is an alias for `std::unique_ptr` in memory-safe mode or an
+alias for a raw pointer otherwise.
+```c++
+RooFit::OwningPtr<RooAbsReal> RooAbsReal::createIntegral(...) const override
+{
+   std::unique_ptr<RooAbsReal> integral;
+   // Prepare a std::unique_ptr as the return value
+   ...
+   // Use the RooFit::makeOwningPtr<T>() helper to translate the
+   // std::unique_ptr to the actual return type (either std::unique_ptr<T> or T*).
+   return RooFit::makeOwningPtr<RooAbsReal>(std::move(integral));
+}
+```
+
+The biggest application of the memory-safe interfaces is to spot memory leaks
+in RooFit-based frameworks. If you make sure that your framework compiles both
+with and without `ROOFIT_MEMORY_SAFE_INTERFACES`, you can get rid of all memory
+leaks related to RooFit user error! After making the necessary changes, you can
+remove the marco definition again to keep backwards compatibility.
+
+Note that the memory-safe interfaces might become the default at some point, so
+doing this **backwards-compatible migration early** is strongly encouraged and
+appreciated.
+
+### Removal of some memory-unsafe interfaces
+
+* The final `bool takeOwnership` parameter of the **RooAddition** and
+  **RooStats::HistFactory::PiecewiseInterpolation** constructors was removed.
+  This is to avoid situations where ownership is not clear to the compiler.
+  Now, ownership of the input RooAbsArgs is never passed in the constructor. If
+  you want the pass input ownership to the created object, please use
+  `addOwnedComponents`. If you want to be extra safe, make sure the inputs are
+  in an owning collection and then `std::move` the collection, so that the
+  ownership is always clear.
+
+  Example:
+  ```c++
+  RooArgList sumSet;
+  sumSet.add(*(new RooRealVar("var1", "var1", 1.0)));
+  sumSet.add(*(new RooRealVar("var2", "var2", 3.0)));
+  RooAddition addition{"addition", "addition", sumSet, /*takeOwnership=*/true};
+  ```
+  should become:
+  ```c++
+  RooArgList sumSet;
+  sumSet.addOwned(std::make_unique<RooRealVar>("var1", "var1", 1.0));
+  sumSet.addOwned(std::make_unique<RooRealVar>("var2", "var2", 3.0));
+  RooAddition addition{"addition", "addition", sumSet};
+  addition.addOwnedComponents(std::move(sumSet));
+  ```
+
 ### Deprecation of legacy iterators
 
 The following methods related to the RooFit legacy iterators are deprecated and will be removed in ROOT 6.34.

bindings/pyroot/pythonizations/test/sofie_gnn.py

@@ -147,8 +147,7 @@ def test_a_parse_gnn(self):
         input_data.node_data = ROOT.TMVA.Experimental.AsRTensor(GraphData['nodes'])
         input_data.edge_data = ROOT.TMVA.Experimental.AsRTensor(GraphData['edges'])
         input_data.global_data = ROOT.TMVA.Experimental.AsRTensor(GraphData['globals'])
-        input_data.receivers = GraphData['receivers']
-        input_data.senders = GraphData['senders']
+        input_data.edge_index = ROOT.TMVA.Experimental.AsRTensor(np.stack((GraphData['receivers'],GraphData['senders'])))
 
         session = ROOT.TMVA_SOFIE_gnn_network.Session()
         session.infer(input_data)
@@ -192,8 +191,7 @@ def test_b_parse_graph_independent(self):
         input_data.node_data = ROOT.TMVA.Experimental.AsRTensor(GraphData['nodes'])
         input_data.edge_data = ROOT.TMVA.Experimental.AsRTensor(GraphData['edges'])
         input_data.global_data = ROOT.TMVA.Experimental.AsRTensor(GraphData['globals'])
-        input_data.receivers = GraphData['receivers']
-        input_data.senders = GraphData['senders']
+        input_data.edge_index = ROOT.TMVA.Experimental.AsRTensor(np.stack((GraphData['receivers'],GraphData['senders'])))
 
         session = ROOT.TMVA_SOFIE_graph_independent_network.Session()
         session.infer(input_data)
@@ -273,11 +271,8 @@ def test_c_lhcb_toy_inference(self):
         input_data.node_data = ROOT.TMVA.Experimental.AsRTensor(InputGraphData['nodes'])
         input_data.edge_data = ROOT.TMVA.Experimental.AsRTensor(InputGraphData['edges'])
         input_data.global_data = ROOT.TMVA.Experimental.AsRTensor(InputGraphData['globals'])
-        input_data.receivers = InputGraphData['receivers']
-        input_data.senders = InputGraphData['senders']
+        input_data.edge_index = ROOT.TMVA.Experimental.AsRTensor(np.stack((InputGraphData['receivers'],InputGraphData['senders'])))
 
-        print ("input receivers",input_data.receivers)
-        print ("input senders",input_data.senders)
 
         output_gn = ep_model(input_graphs, 2)
 

cmake/scripts/ROOTConfig.cmake.in

@@ -28,6 +28,11 @@
 #
 # ===========================================================================
 
+# Set any policies required by this module. Note that there is no check for
+# whether a given policy exists or not. If it is required, it is required...
+cmake_policy(PUSH)
+cmake_policy(SET CMP0074 NEW)
+
 #----------------------------------------------------------------------------
 # DEBUG : print out the variables passed via find_package arguments
 #
@@ -108,6 +113,9 @@ if(ROOT_minuit2_omp_FOUND)
   find_dependency(Threads)
 endif()
 if(ROOT_vdt_FOUND AND NOT TARGET VDT::VDT)
+  if(ROOT_builtin_vdt_FOUND)
+    set(Vdt_ROOT "${_thisdir}/.." CACHE PATH "Location of (the builtin) VDT")
+  endif()
   find_dependency(Vdt)
 endif()
 
@@ -166,3 +174,6 @@ foreach(_remaining ${ROOT_FIND_COMPONENTS})
     endif()
     unset(ROOT_FIND_REQUIRED_${_remaining})
 endforeach()
+
+# Clear out any policy settings made for this module.
+cmake_policy(POP)

core/clingutils/CMakeLists.txt

@@ -79,7 +79,7 @@ endforeach()
 set(CLANG_RESOURCE_DIR_STEM)
 if (builtin_clang)
   set(CLANG_RESOURCE_DIR_STEM ${CMAKE_BINARY_DIR}/interpreter/llvm-project/llvm/${CMAKE_CFG_INTDIR}/lib/clang)
-  set(CLANG_RESOURCE_DIR_VERSION ${LLVM_VERSION})
+  set(CLANG_RESOURCE_DIR_VERSION ${LLVM_VERSION_MAJOR})
 else ()
   set(CLANG_RESOURCE_DIR_STEM ${LLVM_LIBRARY_DIR}/clang)
   # A user can define a clang version to use, otherwise find it (but will error if more than one version is present)

core/clingutils/src/TClingUtils.cxx

@@ -2936,6 +2936,7 @@ clang::QualType ROOT::TMetaUtils::AddDefaultParameters(clang::QualType instanceT
       unsigned int dropDefault = normCtxt.GetConfig().DropDefaultArg(*Template);
 
       llvm::SmallVector<clang::TemplateArgument, 4> desArgs;
+      llvm::SmallVector<clang::TemplateArgument, 4> canonArgs;
       llvm::ArrayRef<clang::TemplateArgument> template_arguments = TST->template_arguments();
       unsigned int Idecl = 0, Edecl = TSTdecl->getTemplateArgs().size();
       unsigned int maxAddArg = TSTdecl->getTemplateArgs().size() - dropDefault;
@@ -2962,7 +2963,10 @@ clang::QualType ROOT::TMetaUtils::AddDefaultParameters(clang::QualType instanceT
                         desArgs.push_back(*I);
                         continue;
                      }
-                     clang::TemplateName templateNameWithNSS ( Ctx.getQualifiedTemplateName(nns, false, templateDecl) );
+                     clang::TemplateName UnderlyingTN(templateDecl);
+                     if (clang::UsingShadowDecl *USD = templateName.getAsUsingShadowDecl())
+                        UnderlyingTN = clang::TemplateName(USD);
+                     clang::TemplateName templateNameWithNSS ( Ctx.getQualifiedTemplateName(nns, false, UnderlyingTN) );
                      desArgs.push_back(clang::TemplateArgument(templateNameWithNSS));
                      mightHaveChanged = true;
                      continue;
@@ -3017,6 +3021,7 @@ clang::QualType ROOT::TMetaUtils::AddDefaultParameters(clang::QualType instanceT
                                                                                               RAngleLoc,
                                                                                               TTP,
                                                                                               desArgs,
+                                                                                              canonArgs,
                                                                                               HasDefaultArgs);
                // The substition can fail, in which case there would have been compilation
                // error printed on the screen.
@@ -3329,7 +3334,7 @@ std::string ROOT::TMetaUtils::GetFileName(const clang::Decl& decl,
 
    const FileEntry *headerFE = sourceManager.getFileEntryForID(headerFID);
    while (includeLoc.isValid() && sourceManager.isInSystemHeader(includeLoc)) {
-      const DirectoryLookup *foundDir = nullptr;
+      ConstSearchDirIterator *foundDir = nullptr;
       // use HeaderSearch on the basename, to make sure it takes a header from
       // the include path (e.g. not from /usr/include/bits/)
       assert(headerFE && "Couldn't find FileEntry from FID!");
@@ -3378,7 +3383,7 @@ std::string ROOT::TMetaUtils::GetFileName(const clang::Decl& decl,
    // points to the same file as the long version. If such a short version
    // exists it will be returned. If it doesn't the long version is returned.
    bool isAbsolute = llvm::sys::path::is_absolute(headerFileName);
-   llvm::Optional<clang::FileEntryRef> FELong;
+   clang::OptionalFileEntryRef FELong;
    // Find the longest available match.
    for (llvm::sys::path::const_iterator
            IDir = llvm::sys::path::begin(headerFileName),
@@ -3394,7 +3399,7 @@ std::string ROOT::TMetaUtils::GetFileName(const clang::Decl& decl,
       assert(trailingPart.data() + trailingPart.size()
              == headerFileName.data() + headerFileName.size()
              && "Mismatched partitioning of file name!");
-      const DirectoryLookup* FoundDir = nullptr;
+      ConstSearchDirIterator* FoundDir = nullptr;
       FELong = HdrSearch.LookupFile(trailingPart, SourceLocation(),
                                     true /*isAngled*/, nullptr/*FromDir*/, FoundDir,
                                     ArrayRef<std::pair<const FileEntry *, const DirectoryEntry *>>(),
@@ -3418,7 +3423,7 @@ std::string ROOT::TMetaUtils::GetFileName(const clang::Decl& decl,
       assert(trailingPart.data() + trailingPart.size()
              == headerFileName.data() + headerFileName.size()
              && "Mismatched partitioning of file name!");
-      const DirectoryLookup* FoundDir = nullptr;
+      ConstSearchDirIterator* FoundDir = nullptr;
       // Can we find it, and is it the same file as the long version?
       // (or are we back to the previously found spelling, which is fine, too)
       if (HdrSearch.LookupFile(trailingPart, SourceLocation(),
@@ -3613,12 +3618,14 @@ static bool areEqualTypes(const clang::TemplateArgument& tArg,
    {
       clang::Sema& S = interp.getCI()->getSema();
       cling::Interpreter::PushTransactionRAII clingRAII(const_cast<cling::Interpreter*>(&interp));
+      llvm::SmallVector<clang::TemplateArgument, 4> canonArgs;
       bool HasDefaultArgs;
       TemplateArgumentLoc defTArgLoc = S.SubstDefaultTemplateArgumentIfAvailable(Template,
                                                                                  TemplateLoc,
                                                                                  LAngleLoc,
                                                                                  ttpdPtr,
                                                                                  preceedingTArgs,
+                                                                                 canonArgs,
                                                                                  HasDefaultArgs);
       // The substition can fail, in which case there would have been compilation
       // error printed on the screen.
@@ -4494,7 +4501,7 @@ static bool hasSomeTypedefSomewhere(const clang::Type* T) {
       return false; // shrug...
     }
     bool VisitTypeOfType(const TypeOfType* TOT) {
-      return TOT->getUnderlyingType().getTypePtr();
+      return TOT->getUnmodifiedType().getTypePtr();
     }
     bool VisitElaboratedType(const ElaboratedType* ET) {
       NestedNameSpecifier* NNS = ET->getQualifier();
@@ -4658,7 +4665,7 @@ clang::QualType ROOT::TMetaUtils::ReSubstTemplateArg(clang::QualType input, cons
       // Make sure it got replaced from this template
       const clang::ClassTemplateDecl *replacedCtxt = nullptr;
 
-      const clang::DeclContext *replacedDeclCtxt = substType->getReplacedParameter()->getDecl()->getDeclContext();
+      const clang::DeclContext *replacedDeclCtxt = substType->getReplacedParameter()->getDeclContext();
       const clang::CXXRecordDecl *decl = llvm::dyn_cast<clang::CXXRecordDecl>(replacedDeclCtxt);
       unsigned int index = substType->getReplacedParameter()->getIndex();
       if (decl) {
@@ -4718,7 +4725,7 @@ clang::QualType ROOT::TMetaUtils::ReSubstTemplateArg(clang::QualType input, cons
 
       if ((replacedCtxt && replacedCtxt->getCanonicalDecl() == TSTdecl->getSpecializedTemplate()->getCanonicalDecl())
           || /* the following is likely just redundant */
-          substType->getReplacedParameter()->getDecl()
+          substType->getReplacedParameter()
           == TSTdecl->getSpecializedTemplate ()->getTemplateParameters()->getParam(index))
       {
          const auto &TAs = TST->template_arguments();
@@ -4753,7 +4760,8 @@ clang::QualType ROOT::TMetaUtils::ReSubstTemplateArg(clang::QualType input, cons
 
          clang::QualType SubTy = TA.getAsType();
          // Check if the type needs more desugaring and recurse.
-         if (llvm::isa<clang::SubstTemplateTypeParmType>(SubTy)
+         if (llvm::isa<clang::ElaboratedType>(SubTy)
+             || llvm::isa<clang::SubstTemplateTypeParmType>(SubTy)
              || llvm::isa<clang::TemplateSpecializationType>(SubTy)) {
             clang::QualType newSubTy = ReSubstTemplateArg(SubTy,instance);
             mightHaveChanged = SubTy != newSubTy;

core/dictgen/src/TModuleGenerator.cxx

@@ -115,7 +115,7 @@ TModuleGenerator::GetSourceFileKind(const char *filename) const
       // via the FileManager.
       clang::Preprocessor &PP = fCI->getPreprocessor();
       clang::HeaderSearch &HdrSearch = PP.getHeaderSearchInfo();
-      const clang::DirectoryLookup *CurDir = nullptr;
+      clang::ConstSearchDirIterator *CurDir = nullptr;
       auto hdrFileEntry
          =  HdrSearch.LookupFile(filename, clang::SourceLocation(),
                                  true /*isAngled*/, nullptr /*FromDir*/, CurDir,
@@ -577,7 +577,7 @@ bool TModuleGenerator::FindHeader(const std::string &hdrName, std::string &hdrFu
    }
    clang::Preprocessor &PP = fCI->getPreprocessor();
    clang::HeaderSearch &HdrSearch = PP.getHeaderSearchInfo();
-   const clang::DirectoryLookup *CurDir = nullptr;
+   clang::ConstSearchDirIterator *CurDir = nullptr;
    if (auto hdrFileEntry
          =  HdrSearch.LookupFile(hdrName, clang::SourceLocation(),
                                  true /*isAngled*/, nullptr /*FromDir*/, CurDir,

core/dictgen/src/rootcling_impl.cxx

@@ -3494,7 +3494,7 @@ class TRootClingCallbacks : public cling::InterpreterCallbacks {
 
    void InclusionDirective(clang::SourceLocation /*HashLoc*/, const clang::Token & /*IncludeTok*/,
                            llvm::StringRef FileName, bool IsAngled, clang::CharSourceRange /*FilenameRange*/,
-                           const clang::FileEntry * /*File*/, llvm::StringRef /*SearchPath*/,
+                           clang::OptionalFileEntryRef /*File*/, llvm::StringRef /*SearchPath*/,
                            llvm::StringRef /*RelativePath*/, const clang::Module * /*Imported*/,
                            clang::SrcMgr::CharacteristicKind /*FileType*/) override
    {
@@ -3536,7 +3536,8 @@ class TRootClingCallbacks : public cling::InterpreterCallbacks {
          Preprocessor& PP = m_Interpreter->getCI()->getPreprocessor();
          HeaderSearch& HS = PP.getHeaderSearchInfo();
          // FIXME: Reduce to Core.Rtypes.h.
-         Module* CoreModule = HS.lookupModule("Core", /*AllowSearch*/false);
+         Module* CoreModule = HS.lookupModule("Core", SourceLocation(),
+                                              /*AllowSearch*/false);
          assert(M && "Must have module Core");
          PP.makeModuleVisible(CoreModule, ImportLoc);
       }
@@ -3892,7 +3893,7 @@ static bool ModuleContainsHeaders(TModuleGenerator &modGen, clang::HeaderSearch
          continue;
 
       clang::ModuleMap::KnownHeader SuggestedModule;
-      const clang::DirectoryLookup *CurDir = nullptr;
+      clang::ConstSearchDirIterator *CurDir = nullptr;
       if (auto FE = headerSearch.LookupFile(
                header, clang::SourceLocation(),
                /*isAngled*/ false,