Skip to content

Commit 0c7c871

Browse files
jnthntatumcopybara-github
authored andcommitted
Update standard macro expanders to give a more stable numbering across compilers/compiler options.
Split benchmarks from parser tests. Re-add parser unit tests to cloud build. PiperOrigin-RevId: 721115170
1 parent 76b8c25 commit 0c7c871

5 files changed

Lines changed: 334 additions & 48 deletions

File tree

parser/BUILD

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,30 @@ cc_library(
171171
cc_test(
172172
name = "parser_test",
173173
srcs = ["parser_test.cc"],
174+
deps = [
175+
":macro",
176+
":options",
177+
":parser",
178+
":source_factory",
179+
"//base/ast_internal:ast_impl",
180+
"//common:constant",
181+
"//common:expr",
182+
"//common:source",
183+
"//internal:testing",
184+
"//testutil:expr_printer",
185+
"@com_google_absl//absl/algorithm:container",
186+
"@com_google_absl//absl/status",
187+
"@com_google_absl//absl/status:status_matchers",
188+
"@com_google_absl//absl/strings",
189+
"@com_google_absl//absl/strings:str_format",
190+
"@com_google_absl//absl/types:optional",
191+
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",
192+
],
193+
)
194+
195+
cc_test(
196+
name = "parser_benchmarks",
197+
srcs = ["parser_benchmarks.cc"],
174198
tags = ["benchmark"],
175199
deps = [
176200
":macro",
@@ -185,6 +209,7 @@ cc_test(
185209
"//internal:testing",
186210
"//testutil:expr_printer",
187211
"@com_google_absl//absl/algorithm:container",
212+
"@com_google_absl//absl/log:absl_check",
188213
"@com_google_absl//absl/status",
189214
"@com_google_absl//absl/status:status_matchers",
190215
"@com_google_absl//absl/strings",

parser/macro.cc

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,12 +164,15 @@ absl::optional<Expr> ExpandExistsOneMacro(MacroExprFactory& factory,
164164
}
165165
auto init = factory.NewIntConst(0);
166166
auto condition = factory.NewBoolConst(true);
167-
auto step =
168-
factory.NewCall(CelOperator::CONDITIONAL, std::move(args[1]),
169-
factory.NewCall(CelOperator::ADD, factory.NewAccuIdent(),
170-
factory.NewIntConst(1)),
171-
factory.NewAccuIdent());
172-
auto result = factory.NewCall(CelOperator::EQUALS, factory.NewAccuIdent(),
167+
auto accu_ident = factory.NewAccuIdent();
168+
auto const_1 = factory.NewIntConst(1);
169+
auto inc_step = factory.NewCall(CelOperator::ADD, std::move(accu_ident),
170+
std::move(const_1));
171+
172+
auto step = factory.NewCall(CelOperator::CONDITIONAL, std::move(args[1]),
173+
std::move(inc_step), factory.NewAccuIdent());
174+
accu_ident = factory.NewAccuIdent();
175+
auto result = factory.NewCall(CelOperator::EQUALS, std::move(accu_ident),
173176
factory.NewIntConst(1));
174177
return factory.NewComprehension(args[0].ident_expr().name(),
175178
std::move(target), factory.AccuVarName(),
@@ -200,9 +203,11 @@ absl::optional<Expr> ExpandMap2Macro(MacroExprFactory& factory, Expr& target,
200203
}
201204
auto init = factory.NewList();
202205
auto condition = factory.NewBoolConst(true);
203-
auto step = factory.NewCall(
204-
CelOperator::ADD, factory.NewAccuIdent(),
205-
factory.NewList(factory.NewListElement(std::move(args[1]))));
206+
auto accu_ref = factory.NewAccuIdent();
207+
auto accu_update =
208+
factory.NewList(factory.NewListElement(std::move(args[1])));
209+
auto step = factory.NewCall(CelOperator::ADD, std::move(accu_ref),
210+
std::move(accu_update));
206211
return factory.NewComprehension(args[0].ident_expr().name(),
207212
std::move(target), factory.AccuVarName(),
208213
std::move(init), std::move(condition),
@@ -231,9 +236,11 @@ absl::optional<Expr> ExpandMap3Macro(MacroExprFactory& factory, Expr& target,
231236
}
232237
auto init = factory.NewList();
233238
auto condition = factory.NewBoolConst(true);
234-
auto step = factory.NewCall(
235-
CelOperator::ADD, factory.NewAccuIdent(),
236-
factory.NewList(factory.NewListElement(std::move(args[2]))));
239+
auto accu_ref = factory.NewAccuIdent();
240+
auto accu_update =
241+
factory.NewList(factory.NewListElement(std::move(args[2])));
242+
auto step = factory.NewCall(CelOperator::ADD, std::move(accu_ref),
243+
std::move(accu_update));
237244
step = factory.NewCall(CelOperator::CONDITIONAL, std::move(args[1]),
238245
std::move(step), factory.NewAccuIdent());
239246
return factory.NewComprehension(args[0].ident_expr().name(),
@@ -266,9 +273,11 @@ absl::optional<Expr> ExpandFilterMacro(MacroExprFactory& factory, Expr& target,
266273

267274
auto init = factory.NewList();
268275
auto condition = factory.NewBoolConst(true);
269-
auto step = factory.NewCall(
270-
CelOperator::ADD, factory.NewAccuIdent(),
271-
factory.NewList(factory.NewListElement(std::move(args[0]))));
276+
auto accu_ref = factory.NewAccuIdent();
277+
auto accu_update =
278+
factory.NewList(factory.NewListElement(std::move(args[0])));
279+
auto step = factory.NewCall(CelOperator::ADD, std::move(accu_ref),
280+
std::move(accu_update));
272281
step = factory.NewCall(CelOperator::CONDITIONAL, std::move(args[1]),
273282
std::move(step), factory.NewAccuIdent());
274283
return factory.NewComprehension(std::move(name), std::move(target),

parser/parser.cc

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -685,16 +685,6 @@ class ParserVisitor final : public CelBaseVisitor,
685685
return GlobalCallOrMacroImpl(expr_id, function, std::move(arguments));
686686
}
687687

688-
template <typename... Args>
689-
Expr ReceiverCallOrMacro(int64_t expr_id, absl::string_view function,
690-
Expr target, Args&&... args) {
691-
std::vector<Expr> arguments;
692-
arguments.reserve(sizeof...(Args));
693-
(arguments.push_back(std::forward<Args>(args)), ...);
694-
return ReceiverCallOrMacroImpl(expr_id, function, std::move(target),
695-
std::move(arguments));
696-
}
697-
698688
Expr GlobalCallOrMacroImpl(int64_t expr_id, absl::string_view function,
699689
std::vector<Expr> args);
700690
Expr ReceiverCallOrMacroImpl(int64_t expr_id, absl::string_view function,

0 commit comments

Comments
 (0)