From b95bae4bc5759678a25905924950592a0e463775 Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Fri, 20 Dec 2024 11:02:19 -0800 Subject: [PATCH] Default enable new accumulator variable. PiperOrigin-RevId: 708371506 --- compiler/compiler_factory_test.cc | 10 +- extensions/comprehensions_v2_macros.cc | 16 +-- parser/options.h | 2 +- parser/parser_test.cc | 149 +++++++++++++------------ testutil/expr_printer_test.cc | 13 ++- 5 files changed, 96 insertions(+), 94 deletions(-) diff --git a/compiler/compiler_factory_test.cc b/compiler/compiler_factory_test.cc index 85c2711b7..9d6c663b3 100644 --- a/compiler/compiler_factory_test.cc +++ b/compiler/compiler_factory_test.cc @@ -49,7 +49,7 @@ TEST(CompilerFactoryTest, Works) { NewCompilerBuilder(cel::internal::GetSharedTestingDescriptorPool())); ASSERT_THAT(builder->AddLibrary(StandardCheckerLibrary()), IsOk()); - + builder->GetParserBuilder().GetOptions().enable_hidden_accumulator_var = true; ASSERT_OK_AND_ASSIGN(auto compiler, std::move(*builder).Build()); ASSERT_OK_AND_ASSIGN( @@ -71,18 +71,18 @@ TEST(CompilerFactoryTest, Works) { "c"~string ]~list(string), // Accumulator - __result__, + @result, // Init false~bool, // LoopCondition @not_strictly_false( !_( - __result__~bool^__result__ + @result~bool^@result )~bool^logical_not )~bool^not_strictly_false, // LoopStep _||_( - __result__~bool^__result__, + @result~bool^@result, @in( x~string^x, [ @@ -93,7 +93,7 @@ TEST(CompilerFactoryTest, Works) { )~bool^in_list )~bool^logical_or, // Result - __result__~bool^__result__)~bool, + @result~bool^@result)~bool, _<_( 10~int, _-_( diff --git a/extensions/comprehensions_v2_macros.cc b/extensions/comprehensions_v2_macros.cc index 04793ad39..a89b206cf 100644 --- a/extensions/comprehensions_v2_macros.cc +++ b/extensions/comprehensions_v2_macros.cc @@ -72,7 +72,7 @@ absl::optional ExpandAllMacro2(MacroExprFactory& factory, Expr& target, auto result = factory.NewAccuIdent(); return factory.NewComprehension( args[0].ident_expr().name(), args[1].ident_expr().name(), - std::move(target), kAccumulatorVariableName, std::move(init), + std::move(target), factory.AccuVarName(), std::move(init), std::move(condition), std::move(step), std::move(result)); } @@ -119,7 +119,7 @@ absl::optional ExpandExistsMacro2(MacroExprFactory& factory, Expr& target, auto result = factory.NewAccuIdent(); return factory.NewComprehension( args[0].ident_expr().name(), args[1].ident_expr().name(), - std::move(target), kAccumulatorVariableName, std::move(init), + std::move(target), factory.AccuVarName(), std::move(init), std::move(condition), std::move(step), std::move(result)); } @@ -172,7 +172,7 @@ absl::optional ExpandExistsOneMacro2(MacroExprFactory& factory, factory.NewIntConst(1)); return factory.NewComprehension( args[0].ident_expr().name(), args[1].ident_expr().name(), - std::move(target), kAccumulatorVariableName, std::move(init), + std::move(target), factory.AccuVarName(), std::move(init), std::move(condition), std::move(step), std::move(result)); } @@ -220,7 +220,7 @@ absl::optional ExpandFilterMacro2(MacroExprFactory& factory, Expr& target, step = factory.NewCall(CelOperator::CONDITIONAL, std::move(args[2]), std::move(step), factory.NewAccuIdent()); return factory.NewComprehension( - name, name2, std::move(target), kAccumulatorVariableName, std::move(init), + name, name2, std::move(target), factory.AccuVarName(), std::move(init), std::move(condition), std::move(step), factory.NewAccuIdent()); } @@ -268,7 +268,7 @@ absl::optional ExpandTransformList3Macro(MacroExprFactory& factory, CelOperator::ADD, factory.NewAccuIdent(), factory.NewList(factory.NewListElement(std::move(args[2])))); return factory.NewComprehension(std::move(iter_var), std::move(iter_var2), - std::move(target), kAccumulatorVariableName, + std::move(target), factory.AccuVarName(), factory.NewList(), factory.NewBoolConst(true), std::move(step), factory.NewAccuIdent()); } @@ -319,7 +319,7 @@ absl::optional ExpandTransformList4Macro(MacroExprFactory& factory, step = factory.NewCall(CelOperator::CONDITIONAL, std::move(args[2]), std::move(step), factory.NewAccuIdent()); return factory.NewComprehension(std::move(iter_var), std::move(iter_var2), - std::move(target), kAccumulatorVariableName, + std::move(target), factory.AccuVarName(), factory.NewList(), factory.NewBoolConst(true), std::move(step), factory.NewAccuIdent()); } @@ -367,7 +367,7 @@ absl::optional ExpandTransformMap3Macro(MacroExprFactory& factory, auto step = factory.NewCall("cel.@mapInsert", factory.NewAccuIdent(), std::move(args[0]), std::move(args[2])); return factory.NewComprehension(std::move(iter_var), std::move(iter_var2), - std::move(target), kAccumulatorVariableName, + std::move(target), factory.AccuVarName(), factory.NewMap(), factory.NewBoolConst(true), std::move(step), factory.NewAccuIdent()); } @@ -417,7 +417,7 @@ absl::optional ExpandTransformMap4Macro(MacroExprFactory& factory, step = factory.NewCall(CelOperator::CONDITIONAL, std::move(args[2]), std::move(step), factory.NewAccuIdent()); return factory.NewComprehension(std::move(iter_var), std::move(iter_var2), - std::move(target), kAccumulatorVariableName, + std::move(target), factory.AccuVarName(), factory.NewMap(), factory.NewBoolConst(true), std::move(step), factory.NewAccuIdent()); } diff --git a/parser/options.h b/parser/options.h index 50f02a4bb..8e6d2ee3b 100644 --- a/parser/options.h +++ b/parser/options.h @@ -52,7 +52,7 @@ struct ParserOptions final { bool disable_standard_macros = false; // Enable hidden accumulator variable '@result' for builtin comprehensions. - bool enable_hidden_accumulator_var = false; + bool enable_hidden_accumulator_var = true; }; } // namespace cel diff --git a/parser/parser_test.cc b/parser/parser_test.cc index 1bf586bc8..1b5ff197a 100644 --- a/parser/parser_test.cc +++ b/parser/parser_test.cc @@ -458,7 +458,7 @@ std::vector test_cases = { " // Target\n" " m^#1:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " 0^#5:int64#,\n" " // LoopCondition\n" @@ -467,14 +467,14 @@ std::vector test_cases = { " _?_:_(\n" " f^#4:Expr.Ident#,\n" " _+_(\n" - " __result__^#7:Expr.Ident#,\n" + " @result^#7:Expr.Ident#,\n" " 1^#8:int64#\n" " )^#9:Expr.Call#,\n" - " __result__^#10:Expr.Ident#\n" + " @result^#10:Expr.Ident#\n" " )^#11:Expr.Call#,\n" " // Result\n" " _==_(\n" - " __result__^#12:Expr.Ident#,\n" + " @result^#12:Expr.Ident#,\n" " 1^#13:int64#\n" " )^#14:Expr.Call#)^#15:Expr.Comprehension#", "", "", "", @@ -489,20 +489,20 @@ std::vector test_cases = { " // Target\n" " m^#1:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " []^#5:Expr.CreateList#,\n" " // LoopCondition\n" " true^#6:bool#,\n" " // LoopStep\n" " _+_(\n" - " __result__^#7:Expr.Ident#,\n" + " @result^#7:Expr.Ident#,\n" " [\n" " f^#4:Expr.Ident#\n" " ]^#8:Expr.CreateList#\n" " )^#9:Expr.Call#,\n" " // Result\n" - " __result__^#10:Expr.Ident#)^#11:Expr.Comprehension#", + " @result^#10:Expr.Ident#)^#11:Expr.Comprehension#", "", "", "", "m^#1:Expr.Ident#.map(\n" " v^#3:Expr.Ident#,\n" @@ -515,7 +515,7 @@ std::vector test_cases = { " // Target\n" " m^#1:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " []^#6:Expr.CreateList#,\n" " // LoopCondition\n" @@ -524,15 +524,15 @@ std::vector test_cases = { " _?_:_(\n" " p^#4:Expr.Ident#,\n" " _+_(\n" - " __result__^#8:Expr.Ident#,\n" + " @result^#8:Expr.Ident#,\n" " [\n" " f^#5:Expr.Ident#\n" " ]^#9:Expr.CreateList#\n" " )^#10:Expr.Call#,\n" - " __result__^#11:Expr.Ident#\n" + " @result^#11:Expr.Ident#\n" " )^#12:Expr.Call#,\n" " // Result\n" - " __result__^#13:Expr.Ident#)^#14:Expr.Comprehension#", + " @result^#13:Expr.Ident#)^#14:Expr.Comprehension#", "", "", "", "m^#1:Expr.Ident#.map(\n" " v^#3:Expr.Ident#,\n" @@ -546,7 +546,7 @@ std::vector test_cases = { " // Target\n" " m^#1:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " []^#5:Expr.CreateList#,\n" " // LoopCondition\n" @@ -555,15 +555,15 @@ std::vector test_cases = { " _?_:_(\n" " p^#4:Expr.Ident#,\n" " _+_(\n" - " __result__^#7:Expr.Ident#,\n" + " @result^#7:Expr.Ident#,\n" " [\n" " v^#3:Expr.Ident#\n" " ]^#8:Expr.CreateList#\n" " )^#9:Expr.Call#,\n" - " __result__^#10:Expr.Ident#\n" + " @result^#10:Expr.Ident#\n" " )^#11:Expr.Call#,\n" " // Result\n" - " __result__^#12:Expr.Ident#)^#13:Expr.Comprehension#", + " @result^#12:Expr.Ident#)^#13:Expr.Comprehension#", "", "", "", "m^#1:Expr.Ident#.filter(\n" " v^#3:Expr.Ident#,\n" @@ -931,7 +931,7 @@ std::vector test_cases = { " // Target\n" " x^#1:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " []^#19:Expr.CreateList#,\n" " // LoopCondition\n" @@ -944,7 +944,7 @@ std::vector test_cases = { " // Target\n" " y^#4:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " []^#10:Expr.CreateList#,\n" " // LoopCondition\n" @@ -956,25 +956,25 @@ std::vector test_cases = { " 0^#9:int64#\n" " )^#8:Expr.Call#,\n" " _+_(\n" - " __result__^#12:Expr.Ident#,\n" + " @result^#12:Expr.Ident#,\n" " [\n" " z^#6:Expr.Ident#\n" " ]^#13:Expr.CreateList#\n" " )^#14:Expr.Call#,\n" - " __result__^#15:Expr.Ident#\n" + " @result^#15:Expr.Ident#\n" " )^#16:Expr.Call#,\n" " // Result\n" - " __result__^#17:Expr.Ident#)^#18:Expr.Comprehension#,\n" + " @result^#17:Expr.Ident#)^#18:Expr.Comprehension#,\n" " _+_(\n" - " __result__^#21:Expr.Ident#,\n" + " @result^#21:Expr.Ident#,\n" " [\n" " y^#3:Expr.Ident#\n" " ]^#22:Expr.CreateList#\n" " )^#23:Expr.Call#,\n" - " __result__^#24:Expr.Ident#\n" + " @result^#24:Expr.Ident#\n" " )^#25:Expr.Call#,\n" " // Result\n" - " __result__^#26:Expr.Ident#)^#27:Expr.Comprehension#" + " @result^#26:Expr.Ident#)^#27:Expr.Comprehension#" "", "", "", "", "x^#1:Expr.Ident#.filter(\n" @@ -995,7 +995,7 @@ std::vector test_cases = { " // Target\n" " a^#2:Expr.Ident#.b~test-only~^#4:Expr.Select#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " []^#8:Expr.CreateList#,\n" " // LoopCondition\n" @@ -1004,15 +1004,15 @@ std::vector test_cases = { " _?_:_(\n" " c^#7:Expr.Ident#,\n" " _+_(\n" - " __result__^#10:Expr.Ident#,\n" + " @result^#10:Expr.Ident#,\n" " [\n" " c^#6:Expr.Ident#\n" " ]^#11:Expr.CreateList#\n" " )^#12:Expr.Call#,\n" - " __result__^#13:Expr.Ident#\n" + " @result^#13:Expr.Ident#\n" " )^#14:Expr.Call#,\n" " // Result\n" - " __result__^#15:Expr.Ident#)^#16:Expr.Comprehension#", + " @result^#15:Expr.Ident#)^#16:Expr.Comprehension#", "", "", "", "^#4:has#.filter(\n" " c^#6:Expr.Ident#,\n" @@ -1028,7 +1028,7 @@ std::vector test_cases = { " // Target\n" " x^#1:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " []^#35:Expr.CreateList#,\n" " // LoopCondition\n" @@ -1042,55 +1042,55 @@ std::vector test_cases = { " // Target\n" " y^#4:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " false^#11:bool#,\n" " // LoopCondition\n" " @not_strictly_false(\n" " !_(\n" - " __result__^#12:Expr.Ident#\n" + " @result^#12:Expr.Ident#\n" " )^#13:Expr.Call#\n" " )^#14:Expr.Call#,\n" " // LoopStep\n" " _||_(\n" - " __result__^#15:Expr.Ident#,\n" + " @result^#15:Expr.Ident#,\n" " z^#8:Expr.Ident#.a~test-only~^#10:Expr.Select#\n" " )^#16:Expr.Call#,\n" " // Result\n" - " __result__^#17:Expr.Ident#)^#18:Expr.Comprehension#,\n" + " @result^#17:Expr.Ident#)^#18:Expr.Comprehension#,\n" " __comprehension__(\n" " // Variable\n" " z,\n" " // Target\n" " y^#19:Expr.Ident#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " false^#26:bool#,\n" " // LoopCondition\n" " @not_strictly_false(\n" " !_(\n" - " __result__^#27:Expr.Ident#\n" + " @result^#27:Expr.Ident#\n" " )^#28:Expr.Call#\n" " )^#29:Expr.Call#,\n" " // LoopStep\n" " _||_(\n" - " __result__^#30:Expr.Ident#,\n" + " @result^#30:Expr.Ident#,\n" " z^#23:Expr.Ident#.b~test-only~^#25:Expr.Select#\n" " )^#31:Expr.Call#,\n" " // Result\n" - " __result__^#32:Expr.Ident#)^#33:Expr.Comprehension#\n" + " @result^#32:Expr.Ident#)^#33:Expr.Comprehension#\n" " )^#34:Expr.Call#,\n" " _+_(\n" - " __result__^#37:Expr.Ident#,\n" + " @result^#37:Expr.Ident#,\n" " [\n" " y^#3:Expr.Ident#\n" " ]^#38:Expr.CreateList#\n" " )^#39:Expr.Call#,\n" - " __result__^#40:Expr.Ident#\n" + " @result^#40:Expr.Ident#\n" " )^#41:Expr.Call#,\n" " // Result\n" - " __result__^#42:Expr.Ident#)^#43:Expr.Comprehension#", + " @result^#42:Expr.Ident#)^#43:Expr.Comprehension#", "", "", "", "x^#1:Expr.Ident#.filter(\n" " y^#3:Expr.Ident#,\n" @@ -1121,22 +1121,22 @@ std::vector test_cases = { " // Target\n" " a^#2:Expr.Ident#.b~test-only~^#4:Expr.Select#.asList()^#5:Expr.Call#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " false^#9:bool#,\n" " // LoopCondition\n" " @not_strictly_false(\n" " !_(\n" - " __result__^#10:Expr.Ident#\n" + " @result^#10:Expr.Ident#\n" " )^#11:Expr.Call#\n" " )^#12:Expr.Call#,\n" " // LoopStep\n" " _||_(\n" - " __result__^#13:Expr.Ident#,\n" + " @result^#13:Expr.Ident#,\n" " c^#8:Expr.Ident#\n" " )^#14:Expr.Call#,\n" " // Result\n" - " __result__^#15:Expr.Ident#)^#16:Expr.Comprehension#", + " @result^#15:Expr.Ident#)^#16:Expr.Comprehension#", "", "", "", "^#4:has#.asList()^#5:Expr.Call#.exists(\n" " c^#7:Expr.Ident#,\n" @@ -1155,22 +1155,22 @@ std::vector test_cases = { " c^#7:Expr.Ident#.d~test-only~^#9:Expr.Select#\n" " ]^#1:Expr.CreateList#,\n" " // Accumulator\n" - " __result__,\n" + " @result,\n" " // Init\n" " false^#13:bool#,\n" " // LoopCondition\n" " @not_strictly_false(\n" " !_(\n" - " __result__^#14:Expr.Ident#\n" + " @result^#14:Expr.Ident#\n" " )^#15:Expr.Call#\n" " )^#16:Expr.Call#,\n" " // LoopStep\n" " _||_(\n" - " __result__^#17:Expr.Ident#,\n" + " @result^#17:Expr.Ident#,\n" " e^#12:Expr.Ident#\n" " )^#18:Expr.Call#,\n" " // Result\n" - " __result__^#19:Expr.Ident#)^#20:Expr.Comprehension#", + " @result^#19:Expr.Ident#)^#20:Expr.Comprehension#", "", "", "", "[\n" " ^#5:has#,\n" @@ -1401,6 +1401,7 @@ class ExpressionTest : public testing::TestWithParam {}; TEST_P(ExpressionTest, Parse) { const TestInfo& test_info = GetParam(); ParserOptions options; + options.enable_hidden_accumulator_var = true; if (!test_info.M.empty()) { options.add_macro_calls = true; } @@ -1549,25 +1550,25 @@ const std::vector& UpdatedAccuVarTestCases() { " // Target\n" " []^#1:Expr.CreateList#,\n" " // Accumulator\n" - " @result,\n" + " __result__,\n" " // Init\n" " false^#7:bool#,\n" " // LoopCondition\n" " @not_strictly_false(\n" " !_(\n" - " @result^#8:Expr.Ident#\n" + " __result__^#8:Expr.Ident#\n" " )^#9:Expr.Call#\n" " )^#10:Expr.Call#,\n" " // LoopStep\n" " _||_(\n" - " @result^#11:Expr.Ident#,\n" + " __result__^#11:Expr.Ident#,\n" " _>_(\n" " x^#4:Expr.Ident#,\n" " 0^#6:int64#\n" " )^#5:Expr.Call#\n" " )^#12:Expr.Call#,\n" " // Result\n" - " @result^#13:Expr.Ident#)^#14:Expr.Comprehension#"}, + " __result__^#13:Expr.Ident#)^#14:Expr.Comprehension#"}, {"[].exists_one(x, x > 0)", "__comprehension__(\n" " // Variable\n" @@ -1575,7 +1576,7 @@ const std::vector& UpdatedAccuVarTestCases() { " // Target\n" " []^#1:Expr.CreateList#,\n" " // Accumulator\n" - " @result,\n" + " __result__,\n" " // Init\n" " 0^#7:int64#,\n" " // LoopCondition\n" @@ -1587,14 +1588,14 @@ const std::vector& UpdatedAccuVarTestCases() { " 0^#6:int64#\n" " )^#5:Expr.Call#,\n" " _+_(\n" - " @result^#9:Expr.Ident#,\n" + " __result__^#9:Expr.Ident#,\n" " 1^#10:int64#\n" " )^#11:Expr.Call#,\n" - " @result^#12:Expr.Ident#\n" + " __result__^#12:Expr.Ident#\n" " )^#13:Expr.Call#,\n" " // Result\n" " _==_(\n" - " @result^#14:Expr.Ident#,\n" + " __result__^#14:Expr.Ident#,\n" " 1^#15:int64#\n" " )^#16:Expr.Call#)^#17:Expr.Comprehension#"}, {"[].all(x, x > 0)", @@ -1604,23 +1605,23 @@ const std::vector& UpdatedAccuVarTestCases() { " // Target\n" " []^#1:Expr.CreateList#,\n" " // Accumulator\n" - " @result,\n" + " __result__,\n" " // Init\n" " true^#7:bool#,\n" " // LoopCondition\n" " @not_strictly_false(\n" - " @result^#8:Expr.Ident#\n" + " __result__^#8:Expr.Ident#\n" " )^#9:Expr.Call#,\n" " // LoopStep\n" " _&&_(\n" - " @result^#10:Expr.Ident#,\n" + " __result__^#10:Expr.Ident#,\n" " _>_(\n" " x^#4:Expr.Ident#,\n" " 0^#6:int64#\n" " )^#5:Expr.Call#\n" " )^#11:Expr.Call#,\n" " // Result\n" - " @result^#12:Expr.Ident#)^#13:Expr.Comprehension#"}, + " __result__^#12:Expr.Ident#)^#13:Expr.Comprehension#"}, {"[].map(x, x + 1)", "__comprehension__(\n" " // Variable\n" @@ -1628,14 +1629,14 @@ const std::vector& UpdatedAccuVarTestCases() { " // Target\n" " []^#1:Expr.CreateList#,\n" " // Accumulator\n" - " @result,\n" + " __result__,\n" " // Init\n" " []^#7:Expr.CreateList#,\n" " // LoopCondition\n" " true^#8:bool#,\n" " // LoopStep\n" " _+_(\n" - " @result^#9:Expr.Ident#,\n" + " __result__^#9:Expr.Ident#,\n" " [\n" " _+_(\n" " x^#4:Expr.Ident#,\n" @@ -1644,7 +1645,7 @@ const std::vector& UpdatedAccuVarTestCases() { " ]^#10:Expr.CreateList#\n" " )^#11:Expr.Call#,\n" " // Result\n" - " @result^#12:Expr.Ident#)^#13:Expr.Comprehension#"}, + " __result__^#12:Expr.Ident#)^#13:Expr.Comprehension#"}, {"[].map(x, x > 0, x + 1)", "__comprehension__(\n" " // Variable\n" @@ -1652,7 +1653,7 @@ const std::vector& UpdatedAccuVarTestCases() { " // Target\n" " []^#1:Expr.CreateList#,\n" " // Accumulator\n" - " @result,\n" + " __result__,\n" " // Init\n" " []^#10:Expr.CreateList#,\n" " // LoopCondition\n" @@ -1664,7 +1665,7 @@ const std::vector& UpdatedAccuVarTestCases() { " 0^#6:int64#\n" " )^#5:Expr.Call#,\n" " _+_(\n" - " @result^#12:Expr.Ident#,\n" + " __result__^#12:Expr.Ident#,\n" " [\n" " _+_(\n" " x^#7:Expr.Ident#,\n" @@ -1672,10 +1673,10 @@ const std::vector& UpdatedAccuVarTestCases() { " )^#8:Expr.Call#\n" " ]^#13:Expr.CreateList#\n" " )^#14:Expr.Call#,\n" - " @result^#15:Expr.Ident#\n" + " __result__^#15:Expr.Ident#\n" " )^#16:Expr.Call#,\n" " // Result\n" - " @result^#17:Expr.Ident#)^#18:Expr.Comprehension#"}, + " __result__^#17:Expr.Ident#)^#18:Expr.Comprehension#"}, {"[].filter(x, x > 0)", "__comprehension__(\n" " // Variable\n" @@ -1683,7 +1684,7 @@ const std::vector& UpdatedAccuVarTestCases() { " // Target\n" " []^#1:Expr.CreateList#,\n" " // Accumulator\n" - " @result,\n" + " __result__,\n" " // Init\n" " []^#7:Expr.CreateList#,\n" " // LoopCondition\n" @@ -1695,15 +1696,15 @@ const std::vector& UpdatedAccuVarTestCases() { " 0^#6:int64#\n" " )^#5:Expr.Call#,\n" " _+_(\n" - " @result^#9:Expr.Ident#,\n" + " __result__^#9:Expr.Ident#,\n" " [\n" " x^#3:Expr.Ident#\n" " ]^#10:Expr.CreateList#\n" " )^#11:Expr.Call#,\n" - " @result^#12:Expr.Ident#\n" + " __result__^#12:Expr.Ident#\n" " )^#13:Expr.Call#,\n" " // Result\n" - " @result^#14:Expr.Ident#)^#15:Expr.Comprehension#"}, + " __result__^#14:Expr.Ident#)^#15:Expr.Comprehension#"}, // Maintain restriction on '__result__' variable name until the default is // changed everywhere. { @@ -1758,12 +1759,12 @@ const std::vector& UpdatedAccuVarTestCases() { return *kInstance; } -class UpdatedAccuVarTest : public testing::TestWithParam {}; +class UpdatedAccuVarDisabledTest : public testing::TestWithParam {}; -TEST_P(UpdatedAccuVarTest, Parse) { +TEST_P(UpdatedAccuVarDisabledTest, Parse) { const TestInfo& test_info = GetParam(); ParserOptions options; - options.enable_hidden_accumulator_var = true; + options.enable_hidden_accumulator_var = false; if (!test_info.M.empty()) { options.add_macro_calls = true; } @@ -1864,7 +1865,7 @@ std::string TestName(const testing::TestParamInfo& test_info) { INSTANTIATE_TEST_SUITE_P(CelParserTest, ExpressionTest, testing::ValuesIn(test_cases), TestName); -INSTANTIATE_TEST_SUITE_P(UpdatedAccuVarTest, UpdatedAccuVarTest, +INSTANTIATE_TEST_SUITE_P(UpdatedAccuVarTest, UpdatedAccuVarDisabledTest, testing::ValuesIn(UpdatedAccuVarTestCases()), TestName); diff --git a/testutil/expr_printer_test.cc b/testutil/expr_printer_test.cc index d15699d5a..9b1e7ca37 100644 --- a/testutil/expr_printer_test.cc +++ b/testutil/expr_printer_test.cc @@ -238,7 +238,7 @@ TEST(ExprPrinterTest, Comprehension) { Expr expr; expr.set_id(1); expr.mutable_comprehension_expr().set_iter_var("x"); - expr.mutable_comprehension_expr().set_accu_var("__result__"); + expr.mutable_comprehension_expr().set_accu_var("@result"); auto& range = expr.mutable_comprehension_expr().mutable_iter_range(); range.set_id(2); range.mutable_ident_expr().set_name("range"); @@ -263,7 +263,7 @@ TEST(ExprPrinterTest, Comprehension) { // Target range#2, // Accumulator - __result__, + @result, // Init accu_init#3, // LoopCondition @@ -277,6 +277,7 @@ TEST(ExprPrinterTest, Comprehension) { TEST(ExprPrinterTest, Proto) { ParserOptions options; options.enable_optional_syntax = true; + options.enable_hidden_accumulator_var = true; ASSERT_OK_AND_ASSIGN(auto parsed_expr, Parse(R"cel( "foo".startsWith("bar") || [1, ?2, 3].exists(x, x in {?"b": "foo"}) || @@ -306,18 +307,18 @@ TEST(ExprPrinterTest, Proto) { 3#7 ]#4, // Accumulator - __result__, + @result, // Init false#16, // LoopCondition @not_strictly_false( !_( - __result__#17 + @result#17 )#18 )#19, // LoopStep _||_( - __result__#20, + @result#20, @in( x#10, { @@ -326,7 +327,7 @@ TEST(ExprPrinterTest, Proto) { )#11 )#21, // Result - __result__#22)#23 + @result#22)#23 )#24, Foo{ byte_value:b"bytes"#27#26,