diff --git a/compiler/optional_test.cc b/compiler/optional_test.cc index 699c69f76..f5091b9fc 100644 --- a/compiler/optional_test.cc +++ b/compiler/optional_test.cc @@ -111,11 +111,66 @@ optional.of( TestCase{ .expr = "optional.of('foo').optMap(x, x)", .expected_ast = R"( -_?_:_( +__comprehension__( + // Variable + #unused, + // Target + []~list(dyn), + // Accumulator + @target, + // Init optional.of( "foo"~string - )~optional_type(string)^optional_of.hasValue()~bool^optional_hasValue, + )~optional_type(string)^optional_of, + // LoopCondition + false~bool, + // LoopStep + @target~optional_type(string)^@target, + // Result + _?_:_( + @target~optional_type(string)^@target.hasValue()~bool^optional_hasValue, + optional.of( + __comprehension__( + // Variable + #unused, + // Target + []~list(dyn), + // Accumulator + x, + // Init + @target~optional_type(string)^@target.value()~string^optional_value, + // LoopCondition + false~bool, + // LoopStep + x~string^x, + // Result + x~string^x)~string + )~optional_type(string)^optional_of, + optional.none()~optional_type(string)^optional_none + )~optional_type(string)^conditional)~optional_type(string) +)", + }, + TestCase{ + .expr = "optional.of('foo').optFlatMap(x, optional.of(x))", + .expected_ast = R"( +__comprehension__( + // Variable + #unused, + // Target + []~list(dyn), + // Accumulator + @target, + // Init optional.of( + "foo"~string + )~optional_type(string)^optional_of, + // LoopCondition + false~bool, + // LoopStep + @target~optional_type(string)^@target, + // Result + _?_:_( + @target~optional_type(string)^@target.hasValue()~bool^optional_hasValue, __comprehension__( // Variable #unused, @@ -124,48 +179,17 @@ _?_:_( // Accumulator x, // Init - optional.of( - "foo"~string - )~optional_type(string)^optional_of.value()~string^optional_value, + @target~optional_type(string)^@target.value()~string^optional_value, // LoopCondition false~bool, // LoopStep x~string^x, // Result - x~string^x)~string - )~optional_type(string)^optional_of, - optional.none()~optional_type(string)^optional_none -)~optional_type(string)^conditional -)", - }, - TestCase{ - .expr = "optional.of('foo').optFlatMap(x, optional.of(x))", - .expected_ast = R"( -_?_:_( - optional.of( - "foo"~string - )~optional_type(string)^optional_of.hasValue()~bool^optional_hasValue, - __comprehension__( - // Variable - #unused, - // Target - []~list(dyn), - // Accumulator - x, - // Init - optional.of( - "foo"~string - )~optional_type(string)^optional_of.value()~string^optional_value, - // LoopCondition - false~bool, - // LoopStep - x~string^x, - // Result - optional.of( - x~string^x - )~optional_type(string)^optional_of)~optional_type(string), - optional.none()~optional_type(string)^optional_none -)~optional_type(string)^conditional + optional.of( + x~string^x + )~optional_type(string)^optional_of)~optional_type(string), + optional.none()~optional_type(string)^optional_none + )~optional_type(string)^conditional)~optional_type(string) )", }, TestCase{ diff --git a/parser/macro.cc b/parser/macro.cc index 815b07401..057787e8d 100644 --- a/parser/macro.cc +++ b/parser/macro.cc @@ -39,6 +39,8 @@ namespace cel { namespace { +constexpr absl::string_view kOptionalMapVar = "@target"; + using google::api::expr::common::CelOperator; bool IsSimpleIdentifier(const Expr& expr) { @@ -315,20 +317,50 @@ absl::optional ExpandOptMapMacro(MacroExprFactory& factory, Expr& target, } auto var_name = args[0].ident_expr().name(); - auto target_copy = factory.Copy(target); - std::vector call_args; - call_args.reserve(3); - call_args.push_back(factory.NewMemberCall("hasValue", std::move(target))); + if (target.has_ident_expr()) { + auto target_copy = factory.Copy(target); + std::vector call_args; + call_args.reserve(3); + call_args.push_back(factory.NewMemberCall("hasValue", std::move(target))); + auto iter_range = factory.NewList(); + auto accu_init = factory.NewMemberCall("value", std::move(target_copy)); + auto condition = factory.NewBoolConst(false); + auto fold = factory.NewComprehension( + "#unused", std::move(iter_range), std::move(var_name), + std::move(accu_init), std::move(condition), std::move(args[0]), + std::move(args[1])); + call_args.push_back(factory.NewCall("optional.of", std::move(fold))); + call_args.push_back(factory.NewCall("optional.none")); + return factory.NewCall(CelOperator::CONDITIONAL, std::move(call_args)); + } + + // If the target is complex, use an internal bind expression to avoid + // repeating it and blowing up the AST in the expansion + auto tmp = factory.NewIdent(kOptionalMapVar); + auto tmp_copy = factory.Copy(tmp); + auto iter_range = factory.NewList(); - auto accu_init = factory.NewMemberCall("value", std::move(target_copy)); + auto accu_init = factory.NewMemberCall("value", std::move(tmp_copy)); auto condition = factory.NewBoolConst(false); + auto loop_step = std::move(args[0]); auto fold = factory.NewComprehension( "#unused", std::move(iter_range), std::move(var_name), - std::move(accu_init), std::move(condition), std::move(args[0]), + std::move(accu_init), std::move(condition), std::move(loop_step), std::move(args[1])); + std::vector call_args; + call_args.reserve(3); + call_args.push_back(factory.NewMemberCall("hasValue", std::move(tmp))); call_args.push_back(factory.NewCall("optional.of", std::move(fold))); call_args.push_back(factory.NewCall("optional.none")); - return factory.NewCall(CelOperator::CONDITIONAL, std::move(call_args)); + auto result = factory.NewCall(CelOperator::CONDITIONAL, std::move(call_args)); + + iter_range = factory.NewList(); + accu_init = std::move(target); + condition = factory.NewBoolConst(false); + loop_step = factory.NewIdent(kOptionalMapVar); + return factory.NewComprehension( + "#unused", std::move(iter_range), kOptionalMapVar, std::move(accu_init), + std::move(condition), loop_step, std::move(result)); } Macro MakeOptMapMacro() { @@ -354,19 +386,47 @@ absl::optional ExpandOptFlatMapMacro(MacroExprFactory& factory, } auto var_name = args[0].ident_expr().name(); - auto target_copy = factory.Copy(target); - std::vector call_args; - call_args.reserve(3); - call_args.push_back(factory.NewMemberCall("hasValue", std::move(target))); + if (target.has_ident_expr()) { + auto target_copy = factory.Copy(target); + std::vector call_args; + call_args.reserve(3); + call_args.push_back(factory.NewMemberCall("hasValue", std::move(target))); + auto iter_range = factory.NewList(); + auto accu_init = factory.NewMemberCall("value", std::move(target_copy)); + auto condition = factory.NewBoolConst(false); + call_args.push_back(factory.NewComprehension( + "#unused", std::move(iter_range), std::move(var_name), + std::move(accu_init), std::move(condition), std::move(args[0]), + std::move(args[1]))); + call_args.push_back(factory.NewCall("optional.none")); + return factory.NewCall(CelOperator::CONDITIONAL, std::move(call_args)); + } + + auto tmp = factory.NewIdent(kOptionalMapVar); + auto tmp_copy = factory.Copy(tmp); + auto iter_range = factory.NewList(); - auto accu_init = factory.NewMemberCall("value", std::move(target_copy)); + auto accu_init = factory.NewMemberCall("value", std::move(tmp_copy)); auto condition = factory.NewBoolConst(false); - call_args.push_back(factory.NewComprehension( + auto loop_step = std::move(args[0]); + auto inner = factory.NewComprehension( "#unused", std::move(iter_range), std::move(var_name), - std::move(accu_init), std::move(condition), std::move(args[0]), - std::move(args[1]))); + std::move(accu_init), std::move(condition), std::move(loop_step), + std::move(args[1])); + std::vector call_args; + call_args.reserve(3); + call_args.push_back(factory.NewMemberCall("hasValue", std::move(tmp))); + call_args.push_back(std::move(inner)); call_args.push_back(factory.NewCall("optional.none")); - return factory.NewCall(CelOperator::CONDITIONAL, std::move(call_args)); + auto result = factory.NewCall(CelOperator::CONDITIONAL, std::move(call_args)); + + iter_range = factory.NewList(); + accu_init = std::move(target); + condition = factory.NewBoolConst(false); + loop_step = factory.NewIdent(kOptionalMapVar); + return factory.NewComprehension( + "#unused", std::move(iter_range), kOptionalMapVar, std::move(accu_init), + std::move(condition), loop_step, std::move(result)); } Macro MakeOptFlatMapMacro() {