diff --git a/roofit/codegen/CMakeLists.txt b/roofit/codegen/CMakeLists.txt index 664dace90c5ce..e1fecef63dac2 100644 --- a/roofit/codegen/CMakeLists.txt +++ b/roofit/codegen/CMakeLists.txt @@ -34,3 +34,5 @@ if(NOT CMAKE_VERSION VERSION_LESS "3.23.0") inc/RooFit/CodegenImpl.h ) endif() + +ROOT_ADD_TEST_SUBDIRECTORY(test) diff --git a/roofit/codegen/src/CodegenImpl.cxx b/roofit/codegen/src/CodegenImpl.cxx index bc3bdc3556d3e..750f6912bc380 100644 --- a/roofit/codegen/src/CodegenImpl.cxx +++ b/roofit/codegen/src/CodegenImpl.cxx @@ -66,6 +66,7 @@ #include +#include #include namespace RooFit::Experimental { @@ -76,6 +77,9 @@ namespace { std::string doubleToString(double val) { std::stringstream ss; + // The formatting must not depend on the global locale: a comma decimal + // separator (e.g. from a German locale) would corrupt the generated C++. + ss.imbue(std::locale::classic()); ss << std::setprecision(std::numeric_limits::max_digits10) << val; return ss.str(); } diff --git a/roofit/codegen/test/CMakeLists.txt b/roofit/codegen/test/CMakeLists.txt new file mode 100644 index 0000000000000..6ec0ce7f477b1 --- /dev/null +++ b/roofit/codegen/test/CMakeLists.txt @@ -0,0 +1 @@ +ROOT_ADD_GTEST(testCodegen testCodegen.cxx LIBRARIES RooFitCore RooFit RooFitCodegen) diff --git a/roofit/codegen/test/testCodegen.cxx b/roofit/codegen/test/testCodegen.cxx new file mode 100644 index 0000000000000..e6c38407d7081 --- /dev/null +++ b/roofit/codegen/test/testCodegen.cxx @@ -0,0 +1,114 @@ +// Tests for the C++ code that the RooFit codegen backend generates. +// Author: Jonas Rembser, CERN 2026 + +#include + +#include +#include +#include +#include + +#include + +#include +#include +#include +#include + +namespace { + +/// Formats decimal points as ',' like a German locale does. Such a locale is +/// not installed on every test machine, so it is built from a custom facet +/// instead of requested by name. The resulting locale is unnamed, so making it +/// global does not also switch the C locale that std::strtod() uses. +struct CommaPunct : std::numpunct { + char do_decimal_point() const override { return ','; } +}; + +/// Generate the code for `arg` with `loc` as the global locale, restoring the +/// previous global locale even if code generation throws. +std::string codeUnderLocale(RooAbsArg &arg, std::locale const &loc) +{ + const std::locale old = std::locale::global(loc); + std::string code; + try { + RooFit::Experimental::CodegenContext ctx; + ctx.buildFunction(arg); + code = ctx.collectedCode(); + } catch (...) { + std::locale::global(old); + throw; + } + std::locale::global(old); + return code; +} + +/// Erase the global counter from the generated function name, which differs +/// between two code generations of the same model. +std::string normalized(std::string const &code) +{ + return std::regex_replace(code, std::regex{"roo_codegen_[0-9]+"}, "roo_codegen_N"); +} + +/// How a plain stream formats 0.5 under `loc`, to verify that the facet is +/// actually in effect (otherwise the tests below would pass vacuously). +std::string formatWithStream(double val, std::locale const &loc) +{ + const std::locale old = std::locale::global(loc); + std::stringstream ss; + ss << val; + std::locale::global(old); + return ss.str(); +} + +} // namespace + +// The generated code is C++ source, so its number formatting must not follow +// the global locale: under a comma-decimal locale the literals came out as +// "0,5", which does not compile, and inside a function call argument list the +// comma even turns one argument into two. +TEST(RooFitCodegen, ValueLiteralsAreLocaleIndependent) +{ + const std::locale comma{std::locale::classic(), new CommaPunct}; + ASSERT_EQ(formatWithStream(0.5, comma), "0,5"); + + RooRealVar x{"x", "x", 0.5, -10, 10}; + RooRealVar mean{"mean", "mean", 1.25}; + RooConstVar sigma{"sigma", "sigma", 0.75}; + RooGaussian gauss{"gauss", "gauss", x, mean, sigma}; + x.setConstant(true); + mean.setConstant(true); + + const std::string code = codeUnderLocale(gauss, comma); + + EXPECT_EQ(normalized(code), normalized(codeUnderLocale(gauss, std::locale::classic()))); + for (std::string const &literal : {"0.5", "1.25", "0.75"}) { + EXPECT_NE(code.find(literal), std::string::npos) << literal << " missing from:\n" << code; + } + for (std::string const &corrupted : {"0,5", "1,25", "0,75"}) { + EXPECT_EQ(code.find(corrupted), std::string::npos) << corrupted << " emitted in:\n" << code; + } +} + +// Same for the doubles that the codegen implementations pass to the generated +// function calls directly (here the observable range of RooChebychev), which +// are formatted by CodegenContext::buildArg() and not by codegen's +// doubleToString(). +TEST(RooFitCodegen, CallArgumentsAreLocaleIndependent) +{ + const std::locale comma{std::locale::classic(), new CommaPunct}; + + RooRealVar x{"x", "x", 0.125, -0.5, 2.25}; + RooRealVar a1{"a1", "a1", 0.375}; + RooChebychev cheby{"cheby", "cheby", x, a1}; + + const std::string code = codeUnderLocale(cheby, comma); + + EXPECT_EQ(normalized(code), normalized(codeUnderLocale(cheby, std::locale::classic()))); + for (std::string const &literal : {"-0.5", "2.25", "0.375"}) { + EXPECT_NE(code.find(literal), std::string::npos) << literal << " missing from:\n" << code; + } + for (std::string const &corrupted : {"0,5", "2,25", "0,375"}) { + EXPECT_EQ(code.find(corrupted), std::string::npos) << corrupted << " emitted in:\n" << code; + } +} diff --git a/roofit/roofitcore/inc/RooFit/CodegenContext.h b/roofit/roofitcore/inc/RooFit/CodegenContext.h index f61b3f08450bb..18e70482c5453 100644 --- a/roofit/roofitcore/inc/RooFit/CodegenContext.h +++ b/roofit/roofitcore/inc/RooFit/CodegenContext.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -139,6 +140,7 @@ class CodegenContext { std::string buildArg(T x) { std::stringstream ss; + ss.imbue(std::locale::classic()); // the generated code is C++, not locale-dependent text ss << std::setprecision(std::numeric_limits::max_digits10) << x; return ss.str(); } @@ -215,6 +217,7 @@ std::string CodegenContext::buildArgSpanImpl(std::span arr) unsigned int n = arr.size(); std::string arrName = getTmpVarName(); std::stringstream ss; + ss.imbue(std::locale::classic()); // the generated code is C++, not locale-dependent text ss << typeName() << " " << arrName << "[" << n << "] = {"; for (unsigned int i = 0; i < n; i++) { ss << " " << arr[i] << ","; diff --git a/roofit/roofitcore/src/RooFit/CodegenContext.cxx b/roofit/roofitcore/src/RooFit/CodegenContext.cxx index 8a1f10e7c958e..72338604602da 100644 --- a/roofit/roofitcore/src/RooFit/CodegenContext.cxx +++ b/roofit/roofitcore/src/RooFit/CodegenContext.cxx @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -285,6 +286,7 @@ std::string CodegenContext::buildArg(std::span arr) CodegenContext::ScopeRAII::ScopeRAII(RooAbsArg const *arg, CodegenContext &ctx) : _ctx(ctx), _arg(arg) { std::ostringstream os; + os.imbue(std::locale::classic()); // the generated code is C++, not locale-dependent text Option_t *opts = nullptr; arg->printStream(os, _arg->defaultPrintContents(opts), _arg->defaultPrintStyle(opts)); _fn = os.str();