[RF] Improve the constructor matching in RooFactoryWSTool

So far, the a constructorr match was reported for
`RooWorkspace::factory` when the number of constructor args was larger
than the number of args passed to `factory`.

Obviously, this lead to false positives, and it should actually check if
the number of passed args (plus 2 for name and title) is somewhere
between the number of minimum and maximum arguments that the constructor
can take. In other works, instead of assuming there is an arbitrary
number of default arguments one should do the check with their exact
number.

A unit test to check that this works now is also implemented.

Closes root-project#7965.

Author: guitargeek

roofit/roofitcore/src/RooFactoryWSTool.cxx

@@ -241,7 +241,7 @@ namespace {
     return false;
   }
 
-  static pair<list<string>,unsigned int> ctorArgs(const char* classname, UInt_t nMinArg) {
+  static pair<list<string>,unsigned int> ctorArgs(const char* classname, std::size_t nPassedArgs) {
     // Utility function for RooFactoryWSTool. Return arguments of 'first' non-default, non-copy constructor of any RooAbsArg
     // derived class. Only constructors that start with two `const char*` arguments (for name and title) are considered
     // The returned object contains
@@ -269,7 +269,8 @@ namespace {
 
       // Skip default constructor
       int nargs = gInterpreter->MethodInfo_NArg(func);
-      if (nargs==0 || nargs==gInterpreter->MethodInfo_NDefaultArg(func)) {
+      int nDefaultArgs = gInterpreter->MethodInfo_NDefaultArg(func);
+      if (nargs == nDefaultArgs) {
         continue;
       }
 
@@ -287,12 +288,12 @@ namespace {
       }
       gInterpreter->MethodArgInfo_Delete(arg);
 
-      // Check that the number of required arguments is at least nMinArg
-      if (ret.size()<nMinArg) {
-        continue;
+      // If the number of passed args (plus 2 for name and title) is somewhere
+      // between the number of minimum and maximum arguments that the
+      // constructor can take, it's a match!
+      if(static_cast<int>(nPassedArgs) >= nargs - nDefaultArgs && static_cast<int>(nPassedArgs) <= nargs) {
+        break;
       }
-
-      break;
     }
     gInterpreter->MethodInfo_Delete(func);
     gInterpreter->ClassInfo_Delete(cls);

roofit/roofitcore/test/testWorkspace.cxx

@@ -22,6 +22,8 @@ using namespace RooStats;
 ///
 TEST(RooWorkspace, CloneModelConfig_ROOT_9777)
 {
+   constexpr bool verbose = false;
+
    const char* filename = "ROOT-9777.root";
 
    RooRealVar x("x", "x", 1, 0, 10);
@@ -56,17 +58,17 @@ TEST(RooWorkspace, CloneModelConfig_ROOT_9777)
       delete w;
    }
 
-   w2->Print();
+   if(verbose) w2->Print();
 
    ModelConfig *mc = dynamic_cast<ModelConfig*>(w2->genobj("ModelConfig"));
    ASSERT_TRUE(mc) << "ModelConfig not retrieved.";
    mc->Print();
 
    ASSERT_TRUE(mc->GetGlobalObservables()) << "GlobalObsevables in mc broken.";
-   mc->GetGlobalObservables()->Print();
+   if(verbose) mc->GetGlobalObservables()->Print();
 
    ASSERT_TRUE(mc->GetParametersOfInterest()) << "ParametersOfInterest in mc broken.";
-   mc->GetParametersOfInterest()->Print();
+   if(verbose) mc->GetParametersOfInterest()->Print();
 
    gSystem->Unlink(filename);
 }
@@ -234,3 +236,13 @@ TEST_F(TestRooWorkspaceWithGaussian, HashLookupInWorkspace) {
   EXPECT_FALSE(model_constrained_orig->dependsOn(*ws->var("mu2")));
   EXPECT_NE(ws->pdf("Gauss_editPdf_orig"), nullptr);
 }
+
+/// Covers an issue about a RooAddPdf constructor not properly picked up by
+/// RooFactoryWSTool.
+TEST(RooWorkspace, Issue_7965)
+{
+   RooWorkspace ws{"ws"};
+   ws.factory("RooAddPdf::addPdf({})");
+
+   ASSERT_NE(ws.pdf("addPdf"), nullptr);
+}