Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions roofit/roofitcore/src/RooFormula.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@ RooFormula::RooFormula(const RooFormula& other, const char* name) :
#ifndef _MSC_VER
#if !defined(__GNUC__) || defined(__clang__) || (__GNUC__ > 4) || ( __GNUC__ == 4 && __GNUC_MINOR__ > 8)
#define ROOFORMULA_HAVE_STD_REGEX
#endif //GCC < 4.9 Check
#endif //_MSC_VER

#ifdef ROOFORMULA_HAVE_STD_REGEX
////////////////////////////////////////////////////////////////////////////////
/// Process given formula by replacing all ordinal and name references by
/// `x[i]`, where `i` matches the position of the argument in `_origList`.
Expand Down Expand Up @@ -208,6 +212,7 @@ std::string RooFormula::processFormula(std::string formula) const {
auto regex = std::string{"\\b"} + var.GetName();
regex = std::regex_replace(regex, std::regex("([\\[\\]\\{\\}])"), "\\$1"); // The name might contain [, ], {, or }.
regex += "\\b(?!\\[)"; // Veto '[' as next character. If the variable is called `x`, this might otherwise replace `x[0]`.
regex += "\\b(?!\\])"; // Veto ']' as next character. If the variable is called `0`, this might otherwise replace `x[0]`.
std::regex findParameterRegex(regex);

std::stringstream replacement;
Expand Down Expand Up @@ -270,8 +275,7 @@ std::string RooFormula::reconstructFormula(std::string internalRepr) const {

return internalRepr;
}
#endif //GCC < 4.9 Check
#endif //_MSC_VER
#endif //ROOFORMULA_HAVE_STD_REGEX



Expand Down Expand Up @@ -612,7 +616,7 @@ std::string RooFormula::processFormula(std::string formula) const {
const auto& var = _origList[i];
TString regex = "\\b";
regex += var.GetName();
regex += "\\b([^[]|$)"; //Negative lookahead. If the variable is called `x`, this might otherwise replace `x[0]`.
regex += "\\b([^\\[\\]]|$)"; //Negative lookahead. If the variable is called `x` or `0`, this might otherwise replace `x[0]`.
TPRegexp findParameterRegex(regex);

std::stringstream replacement;
Expand Down Expand Up @@ -687,4 +691,4 @@ std::string RooFormula::reconstructFormula(std::string internalRepr) const {

return internalReprT.Data();
}
#endif //GCC < 4.9 Check
#endif //ROOFORMULA_HAVE_STD_REGEX
17 changes: 17 additions & 0 deletions roofit/roofitcore/test/testRooFormula.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,20 @@ TEST(RooFormula, TestInvalidFormulae) {
EXPECT_FLOAT_EQ(form6->eval(nullptr), 1.337 - 1.);
delete form6;
}

// In case of named arguments, the RooFormula will replace the argument names
// with x[0] to x[n]. There are two things that can go wrong if RooFormula is
// not implemented right. First, if there is a variable named "x" it should
// only be substituted if the matching substring is not followed by "[", to not
// replace existing x[i]. Second, variables with integer names like "0" should
// only be substituted if the match is not followed by a "]", again to avoid
// replacing x[i]. This test checks that these cases are handled correctly.
TEST(RooFormula, TestDangerousVariableNames) {
RooRealVar dt("dt", "dt", -10, 10);
RooRealVar x("x", "x", 1.547);
RooRealVar zero("0", "0", 0);

// Create the formula, triggers an error if the formula doesn't compile
// correctly because the dangerous variable names haven't been treated right.
RooFormula formula("formula", "exp(-abs(@0)/@1)*cos(@0*@2)", {dt, x, zero});
}