From 21136638a5543b832a67083bbca6b8e0cf14b6a3 Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Tue, 13 Oct 2026 15:34:27 -0700 Subject: [PATCH] Reject unexpected receiver targets in comprehension list append and map insert optimizations. PiperOrigin-RevId: 998934280 --- eval/compiler/flat_expr_builder.cc | 6 ++-- eval/compiler/flat_expr_builder_test.cc | 44 +++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/eval/compiler/flat_expr_builder.cc b/eval/compiler/flat_expr_builder.cc index 5677f2a98..660e34182 100644 --- a/eval/compiler/flat_expr_builder.cc +++ b/eval/compiler/flat_expr_builder.cc @@ -343,7 +343,8 @@ bool IsOptimizableListAppend(const cel::ComprehensionExpr* comprehension, call_expr = &(call_expr->args()[1].call_expr()); } - return call_expr->function() == cel::builtin::kAdd && + return !call_expr->has_target() && + call_expr->function() == cel::builtin::kAdd && call_expr->args().size() == 2 && call_expr->args()[0].has_ident_expr() && call_expr->args()[0].ident_expr().name() == accu_var && @@ -416,7 +417,8 @@ bool IsOptimizableMapInsert(const cel::ComprehensionExpr* comprehension, } call_expr = &(call_expr->args()[1].call_expr()); } - return call_expr->function() == "cel.@mapInsert" && + return !call_expr->has_target() && + call_expr->function() == "cel.@mapInsert" && (call_expr->args().size() == 2 || call_expr->args().size() == 3) && call_expr->args()[0].has_ident_expr() && call_expr->args()[0].ident_expr().name() == accu_var; diff --git a/eval/compiler/flat_expr_builder_test.cc b/eval/compiler/flat_expr_builder_test.cc index f9d302051..5ce27a603 100644 --- a/eval/compiler/flat_expr_builder_test.cc +++ b/eval/compiler/flat_expr_builder_test.cc @@ -3104,6 +3104,50 @@ TEST(FlatExprBuilderTest, EXPECT_THAT(result, test::IsCelBool(true)); } +TEST(FlatExprBuilderTest, ListAppendComprehensionWithTargetFailsPlanning) { + ParsedExpr parsed_expr; + ASSERT_TRUE(google::protobuf::TextFormat::ParseFromString( + R"pb( + expr { + comprehension_expr { + iter_var: "x" + iter_range { + list_expr { + elements { const_expr { int64_value: 1 } } + elements { const_expr { int64_value: 2 } } + elements { const_expr { int64_value: 3 } } + elements { const_expr { int64_value: 4 } } + } + } + accu_var: "__result__" + accu_init { list_expr {} } + loop_condition { const_expr { bool_value: true } } + loop_step { + call_expr { + target { const_expr { int64_value: 0 } } + function: "_+_" + args { ident_expr { name: "__result__" } } + args { + list_expr { elements { const_expr { int64_value: 1 } } } + } + } + } + result { ident_expr { name: "__result__" } } + } + } + )pb", + &parsed_expr)); + + cel::RuntimeOptions options; + options.enable_comprehension_list_append = true; + CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv(), options); + ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry()), IsOk()); + + EXPECT_THAT( + builder.CreateExpression(&parsed_expr.expr(), &parsed_expr.source_info()), + StatusIs(absl::StatusCode::kInvalidArgument)); +} + } // namespace } // namespace google::api::expr::runtime