From 7d02c841d0dc7173aa8ea48a82ce737123a1abf3 Mon Sep 17 00:00:00 2001
From: Jonas Rembser <jonas.rembser@cern.ch>
Date: Wed, 15 Sep 2021 16:17:49 +0200
Subject: [PATCH] [RF] Avoid using forward-declared class as default template
 arguments

Avoid using forward-declared class as default template arguments in
`RooMinimizer` and `MinuitFcnGrad`.

This is the fix for the following test failures in the nightlies:

```
    projectroot.roottest.python.cling.roottest_python_cling_class
    projectroot.roottest.python.cling.roottest_python_cling_api
    projectroot.roottest.root.meta.tclass.regression.roottest_root_meta_tclass_regression_execNormalizationInfPy
    projectroot.roottest.python.cling.roottest_python_cling_cling
    projectroot.roottest.root.meta.enumPayloadManipulation.roottest_root_meta_enumPayloadManipulation_checkEnumFwdDecl
```

For sure we know that these failures got introduced by
https://github.com/root-project/root/pull/8700.

The failures that we see since [root-project/root#8700](https://github.com/root-project/root/pull/8700), here are the comments where the bot reported them first:
First Ubuntu 16 fail:
[root-project/root#8700 (comment)](https://github.com/root-project/root/pull/8700#issuecomment-916924242)

First Windows 10 fail:
[root-project/root#8700 (comment)](https://github.com/root-project/root/pull/8700#issuecomment-916992359)

I narrowed down the origin of this regression to a small part of the
diff of the full PR. The bad guy is some change in one of these files:

  * roofit/roofitcore/inc/RooMinimizer.h
  * roofit/roofitcore/test/CMakeLists.txt
  * roofit/roofitcore/test/TestStatistics/testLikelihoodSerial.cxx

What was fishy in `RooMinimizer.h` was the usage of a forward-declared
class as default template argument. The default template arguments are
commented out now, because these will only become relevant in later
developments by @egpbos.
---
 roofit/roofitcore/inc/RooMinimizer.h                 | 12 ++++++------
 roofit/roofitcore/inc/TestStatistics/MinuitFcnGrad.h |  6 +++---
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/roofit/roofitcore/inc/RooMinimizer.h b/roofit/roofitcore/inc/RooMinimizer.h
index 6d590a983d547..e614e62dbb296 100644
--- a/roofit/roofitcore/inc/RooMinimizer.h
+++ b/roofit/roofitcore/inc/RooMinimizer.h
@@ -48,7 +48,7 @@ class RooPlot ;
 namespace RooFit {
 namespace TestStatistics {
 class LikelihoodSerial;
-class LikelihoodGradientSerial;
+//class LikelihoodGradientSerial;
 }
 } // namespace RooFit
 
@@ -58,8 +58,8 @@ class RooMinimizer : public TObject {
 
   explicit RooMinimizer(RooAbsReal &function, FcnMode fcnMode = FcnMode::classic);
   static std::unique_ptr<RooMinimizer> create(RooAbsReal &function, FcnMode fcnMode = FcnMode::classic);
-  template <typename LikelihoodWrapperT = RooFit::TestStatistics::LikelihoodSerial,
-            typename LikelihoodGradientWrapperT = RooFit::TestStatistics::LikelihoodGradientSerial>
+  template <typename LikelihoodWrapperT /*= RooFit::TestStatistics::LikelihoodSerial*/,
+            typename LikelihoodGradientWrapperT /*= RooFit::TestStatistics::LikelihoodGradientSerial*/>
   static std::unique_ptr<RooMinimizer> create(std::shared_ptr<RooFit::TestStatistics::RooAbsL> likelihood);
 
   ~RooMinimizer() override;
@@ -136,10 +136,10 @@ class RooMinimizer : public TObject {
   bool fitFcn() const;
 
 private:
-  template <typename LikelihoodWrapperT = RooFit::TestStatistics::LikelihoodSerial, typename LikelihoodGradientWrapperT = RooFit::TestStatistics::LikelihoodGradientSerial>
+  template <typename LikelihoodWrapperT /*= RooFit::TestStatistics::LikelihoodSerial*/, typename LikelihoodGradientWrapperT /*= RooFit::TestStatistics::LikelihoodGradientSerial*/>
   RooMinimizer(std::shared_ptr<RooFit::TestStatistics::RooAbsL> likelihood,
-               LikelihoodWrapperT* /* used only for template deduction */ = static_cast<RooFit::TestStatistics::LikelihoodSerial*>(nullptr),
-               LikelihoodGradientWrapperT* /* used only for template deduction */ = static_cast<RooFit::TestStatistics::LikelihoodGradientSerial*>(nullptr));
+               LikelihoodWrapperT* /* used only for template deduction = static_cast<RooFit::TestStatistics::LikelihoodSerial*>(nullptr) */,
+               LikelihoodGradientWrapperT* /* used only for template deduction  = static_cast<RooFit::TestStatistics::LikelihoodGradientSerial*>(nullptr) */);
 
   Int_t _printLevel = 1;
   Int_t _status = -99;
diff --git a/roofit/roofitcore/inc/TestStatistics/MinuitFcnGrad.h b/roofit/roofitcore/inc/TestStatistics/MinuitFcnGrad.h
index 5dca9126adc2a..c0752f90d0cb8 100644
--- a/roofit/roofitcore/inc/TestStatistics/MinuitFcnGrad.h
+++ b/roofit/roofitcore/inc/TestStatistics/MinuitFcnGrad.h
@@ -37,7 +37,7 @@ namespace TestStatistics {
 
 // forward declaration
 class LikelihoodSerial;
-class LikelihoodGradientSerial;
+//class LikelihoodGradientSerial;
 
 /// For communication with wrappers, an instance of this struct must be shared between them and MinuitFcnGrad. It keeps
 /// track of what has been evaluated for the current parameter set provided by Minuit.
@@ -56,8 +56,8 @@ struct WrapperCalculationCleanFlags {
 class MinuitFcnGrad : public ROOT::Math::IMultiGradFunction, public RooAbsMinimizerFcn {
 public:
    // factory
-   template <typename LikelihoodWrapperT = RooFit::TestStatistics::LikelihoodSerial,
-             typename LikelihoodGradientWrapperT = RooFit::TestStatistics::LikelihoodGradientSerial>
+   template <typename LikelihoodWrapperT /*= RooFit::TestStatistics::LikelihoodSerial*/,
+             typename LikelihoodGradientWrapperT /*= RooFit::TestStatistics::LikelihoodGradientSerial*/>
    static MinuitFcnGrad *
    create(const std::shared_ptr<RooFit::TestStatistics::RooAbsL> &likelihood, RooMinimizer *context,
           std::vector<ROOT::Fit::ParameterSettings> &parameters, bool verbose = false);