diff --git a/tree/dataframe/src/RDFInterfaceUtils.cxx b/tree/dataframe/src/RDFInterfaceUtils.cxx index 30a694b089a7f..f76e1070e13d7 100644 --- a/tree/dataframe/src/RDFInterfaceUtils.cxx +++ b/tree/dataframe/src/RDFInterfaceUtils.cxx @@ -171,10 +171,21 @@ ParsedExpression ParseRDFExpression(std::string_view expr, const ROOT::Internal: { // transform `#var` into `R_rdf_sizeof_var` TString preProcessedExpr(expr); - // match #varname at beginning of the sentence or after not-a-word, but exclude preprocessor directives like #ifdef + // regex explanation: + // # + // - matches the literal character # + // (?!(ifdef|ifndef|if|else|elif|endif|pragma|define|undef|include|line)) + // - negative lookahead (i.e. don't match these occurrences) to exclude preprocessor directives e.g. #ifdef + // ([a-zA-Z_][a-zA-Z0-9_]*) + // - match characters that make up the variable name after the # + // (?=(?:[^"]*"[^"]*")*[^"]*$) + // - positive lookahead (i.e. match these occurrences) checking that there is always an even number of quotes + // *after* the matched character sequence, using the non-capturing group (?:[^"]*"[^"]*"). We leverage the + // fact that for any character sequence to be within quotes there must be an odd number of quotes after the + // sequence and before the end of the line. TPRegexp colSizeReplacer( - "(^|\\W)#(?!(ifdef|ifndef|if|else|elif|endif|pragma|define|undef|include|line))([a-zA-Z_][a-zA-Z0-9_]*)"); - colSizeReplacer.Substitute(preProcessedExpr, "$1R_rdf_sizeof_$3", "g"); + R"REGEX(#(?!(ifdef|ifndef|if|else|elif|endif|pragma|define|undef|include|line))([a-zA-Z_][a-zA-Z0-9_]*)(?=(?:[^"]*"[^"]*")*[^"]*$))REGEX"); + colSizeReplacer.Substitute(preProcessedExpr, "R_rdf_sizeof_$2", "g"); ColumnNames_t usedCols; ColumnNames_t usedAliases; diff --git a/tree/dataframe/test/dataframe_interface.cxx b/tree/dataframe/test/dataframe_interface.cxx index 3411403ffcba5..5ee2235ab236c 100644 --- a/tree/dataframe/test/dataframe_interface.cxx +++ b/tree/dataframe/test/dataframe_interface.cxx @@ -845,6 +845,39 @@ TEST(RDataFrameInterface, StressShortSyntaxForCollectionSizes) EXPECT_EQ(c, 42ull); } +// Regression tests for https://github.com/root-project/root/issues/23790 +TEST(RDataFrameInterface, ShortSyntaxForCollectionSizesMultipleStringsAndVars) +{ + // We need to define first four target columns that have the longer name for collection size + auto df = ROOT::RDF::RNode(ROOT::RDataFrame(1)); + for (auto i = 1; i <= 4; i++) + df = df.Define("R_rdf_sizeof_var" + std::to_string(i), []() -> int { return 1; }); + std::string code{R"CODE( + auto value1 = #var1; // comment + #ifdef NONEXISTENTMACRO + // comment with "quoted string" + #endif + std::string simplestr("#simplestr"); // comment + std::string mystr("anything#hello"); // comment + auto value2 = #var2; // comment with "another string" + std::string anotherstr("We don't want #var3 and #var4 to be replaced"); + auto value5 = #var1 + #var2; std::string innerRawString{R"INNER( + This raw string contains #myvar which should + not be changed.)INNER"}; + return #var1 + #var2 + #var3 + #var4 + value1 + value2 + value5; + )CODE"}; + auto sum = df.Define("target_col", code).Sum("target_col"); + EXPECT_EQ(*sum, 8); +} + +// Regression test for https://github.com/root-project/root/issues/23790 +TEST(RDataFrameInterface, StringWithPotentialAmbiguityWithSortSyntaxForCollectionSizes) +{ + auto df = ROOT::RDataFrame(1).Define("col", "std::string(\"#a\")").Filter("col == \"#a\""); + EXPECT_EQ(df.Count().GetValue(), 1ull); + EXPECT_EQ(*df.Take("col")->begin(), "#a"); +} + TEST(RDataFrameInterface, MutableForeach) { int i = 0;