Skip to content

Commit 9e73d93

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Deprecate option to switch to old accumulator var name in parser.
cel::ParserOption::enable_hidden_accumulator_var now has no effect and will be removed in a later change. The standard / extension macros should always use `@result` now. PiperOrigin-RevId: 908333116
1 parent 23c9913 commit 9e73d93

9 files changed

Lines changed: 72 additions & 347 deletions

File tree

‎common/expr.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ class MapExprEntry;
4545
class MapExpr;
4646
class ComprehensionExpr;
4747

48-
inline constexpr absl::string_view kAccumulatorVariableName = "__result__";
48+
inline constexpr absl::string_view kAccumulatorVariableName = "@result";
49+
inline constexpr absl::string_view kDeprecatedAccumulatorVariableName =
50+
"__result__";
4951

5052
bool operator==(const Expr& lhs, const Expr& rhs);
5153

‎common/expr_factory.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,6 @@ class ExprFactory {
357357
friend class ParserMacroExprFactory;
358358

359359
ExprFactory() : accu_var_(kAccumulatorVariableName) {}
360-
explicit ExprFactory(absl::string_view accu_var) : accu_var_(accu_var) {}
361360

362361
std::string accu_var_;
363362
};

‎extensions/comprehensions_v2_macros.cc‎

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,15 @@ absl::optional<Expr> ExpandAllMacro2(MacroExprFactory& factory, Expr& target,
5656
args[0],
5757
"all() second variable must be different from the first variable");
5858
}
59-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
59+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
6060
return factory.ReportErrorAt(
6161
args[0], absl::StrCat("all() first variable name cannot be ",
62-
kAccumulatorVariableName));
62+
kDeprecatedAccumulatorVariableName));
6363
}
64-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
64+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
6565
return factory.ReportErrorAt(
6666
args[1], absl::StrCat("all() second variable name cannot be ",
67-
kAccumulatorVariableName));
67+
kDeprecatedAccumulatorVariableName));
6868
}
6969
auto init = factory.NewBoolConst(true);
7070
auto condition =
@@ -102,15 +102,15 @@ absl::optional<Expr> ExpandExistsMacro2(MacroExprFactory& factory, Expr& target,
102102
args[0],
103103
"exists() second variable must be different from the first variable");
104104
}
105-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
105+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
106106
return factory.ReportErrorAt(
107107
args[0], absl::StrCat("exists() first variable name cannot be ",
108-
kAccumulatorVariableName));
108+
kDeprecatedAccumulatorVariableName));
109109
}
110-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
110+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
111111
return factory.ReportErrorAt(
112112
args[1], absl::StrCat("exists() second variable name cannot be ",
113-
kAccumulatorVariableName));
113+
kDeprecatedAccumulatorVariableName));
114114
}
115115
auto init = factory.NewBoolConst(false);
116116
auto condition = factory.NewCall(
@@ -153,15 +153,15 @@ absl::optional<Expr> ExpandExistsOneMacro2(MacroExprFactory& factory,
153153
"existsOne() second variable must be different "
154154
"from the first variable");
155155
}
156-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
156+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
157157
return factory.ReportErrorAt(
158158
args[0], absl::StrCat("existsOne() first variable name cannot be ",
159-
kAccumulatorVariableName));
159+
kDeprecatedAccumulatorVariableName));
160160
}
161-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
161+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
162162
return factory.ReportErrorAt(
163163
args[1], absl::StrCat("existsOne() second variable name cannot be ",
164-
kAccumulatorVariableName));
164+
kDeprecatedAccumulatorVariableName));
165165
}
166166
auto init = factory.NewIntConst(0);
167167
auto condition = factory.NewBoolConst(true);
@@ -205,15 +205,15 @@ absl::optional<Expr> ExpandTransformList3Macro(MacroExprFactory& factory,
205205
"transformList() second variable must be "
206206
"different from the first variable");
207207
}
208-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
208+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
209209
return factory.ReportErrorAt(
210210
args[0], absl::StrCat("transformList() first variable name cannot be ",
211-
kAccumulatorVariableName));
211+
kDeprecatedAccumulatorVariableName));
212212
}
213-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
213+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
214214
return factory.ReportErrorAt(
215215
args[1], absl::StrCat("transformList() second variable name cannot be ",
216-
kAccumulatorVariableName));
216+
kDeprecatedAccumulatorVariableName));
217217
}
218218
std::string iter_var = args[0].ident_expr().name();
219219
std::string iter_var2 = args[1].ident_expr().name();
@@ -254,15 +254,15 @@ absl::optional<Expr> ExpandTransformList4Macro(MacroExprFactory& factory,
254254
"transformList() second variable must be "
255255
"different from the first variable");
256256
}
257-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
257+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
258258
return factory.ReportErrorAt(
259259
args[0], absl::StrCat("transformList() first variable name cannot be ",
260-
kAccumulatorVariableName));
260+
kDeprecatedAccumulatorVariableName));
261261
}
262-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
262+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
263263
return factory.ReportErrorAt(
264264
args[1], absl::StrCat("transformList() second variable name cannot be ",
265-
kAccumulatorVariableName));
265+
kDeprecatedAccumulatorVariableName));
266266
}
267267
std::string iter_var = args[0].ident_expr().name();
268268
std::string iter_var2 = args[1].ident_expr().name();
@@ -305,15 +305,15 @@ absl::optional<Expr> ExpandTransformMap3Macro(MacroExprFactory& factory,
305305
"transformMap() second variable must be "
306306
"different from the first variable");
307307
}
308-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
308+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
309309
return factory.ReportErrorAt(
310310
args[0], absl::StrCat("transformMap() first variable name cannot be ",
311-
kAccumulatorVariableName));
311+
kDeprecatedAccumulatorVariableName));
312312
}
313-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
313+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
314314
return factory.ReportErrorAt(
315315
args[1], absl::StrCat("transformMap() second variable name cannot be ",
316-
kAccumulatorVariableName));
316+
kDeprecatedAccumulatorVariableName));
317317
}
318318
std::string iter_var = args[0].ident_expr().name();
319319
std::string iter_var2 = args[1].ident_expr().name();
@@ -353,15 +353,15 @@ absl::optional<Expr> ExpandTransformMap4Macro(MacroExprFactory& factory,
353353
"transformMap() second variable must be "
354354
"different from the first variable");
355355
}
356-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
356+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
357357
return factory.ReportErrorAt(
358358
args[0], absl::StrCat("transformMap() first variable name cannot be ",
359-
kAccumulatorVariableName));
359+
kDeprecatedAccumulatorVariableName));
360360
}
361-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
361+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
362362
return factory.ReportErrorAt(
363363
args[1], absl::StrCat("transformMap() second variable name cannot be ",
364-
kAccumulatorVariableName));
364+
kDeprecatedAccumulatorVariableName));
365365
}
366366
std::string iter_var = args[0].ident_expr().name();
367367
std::string iter_var2 = args[1].ident_expr().name();
@@ -403,17 +403,17 @@ absl::optional<Expr> ExpandTransformMapEntry3Macro(MacroExprFactory& factory,
403403
"transformMapEntry() second variable must be "
404404
"different from the first variable");
405405
}
406-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
406+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
407407
return factory.ReportErrorAt(
408408
args[0],
409409
absl::StrCat("transformMapEntry() first variable name cannot be ",
410-
kAccumulatorVariableName));
410+
kDeprecatedAccumulatorVariableName));
411411
}
412-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
412+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
413413
return factory.ReportErrorAt(
414414
args[1],
415415
absl::StrCat("transformMapEntry() second variable name cannot be ",
416-
kAccumulatorVariableName));
416+
kDeprecatedAccumulatorVariableName));
417417
}
418418
std::string iter_var = args[0].ident_expr().name();
419419
std::string iter_var2 = args[1].ident_expr().name();
@@ -453,17 +453,17 @@ absl::optional<Expr> ExpandTransformMapEntry4Macro(MacroExprFactory& factory,
453453
"transformMapEntry() second variable must be "
454454
"different from the first variable");
455455
}
456-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
456+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
457457
return factory.ReportErrorAt(
458458
args[0],
459459
absl::StrCat("transformMapEntry() first variable name cannot be ",
460-
kAccumulatorVariableName));
460+
kDeprecatedAccumulatorVariableName));
461461
}
462-
if (args[1].ident_expr().name() == kAccumulatorVariableName) {
462+
if (args[1].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
463463
return factory.ReportErrorAt(
464464
args[1],
465465
absl::StrCat("transformMapEntry() second variable name cannot be ",
466-
kAccumulatorVariableName));
466+
kDeprecatedAccumulatorVariableName));
467467
}
468468
std::string iter_var = args[0].ident_expr().name();
469469
std::string iter_var2 = args[1].ident_expr().name();

‎parser/macro.cc‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,10 @@ absl::optional<Expr> ExpandAllMacro(MacroExprFactory& factory, Expr& target,
9191
return factory.ReportErrorAt(
9292
args[0], "all() variable name must be a simple identifier");
9393
}
94-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
95-
return factory.ReportErrorAt(args[1],
96-
absl::StrCat("all() variable name cannot be ",
97-
kAccumulatorVariableName));
94+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
95+
return factory.ReportErrorAt(
96+
args[1], absl::StrCat("all() variable name cannot be ",
97+
kDeprecatedAccumulatorVariableName));
9898
}
9999
auto init = factory.NewBoolConst(true);
100100
auto condition =
@@ -123,10 +123,10 @@ absl::optional<Expr> ExpandExistsMacro(MacroExprFactory& factory, Expr& target,
123123
return factory.ReportErrorAt(
124124
args[0], "exists() variable name must be a simple identifier");
125125
}
126-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
126+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
127127
return factory.ReportErrorAt(
128128
args[1], absl::StrCat("exists() variable name cannot be ",
129-
kAccumulatorVariableName));
129+
kDeprecatedAccumulatorVariableName));
130130
}
131131
auto init = factory.NewBoolConst(false);
132132
auto condition = factory.NewCall(
@@ -157,10 +157,10 @@ absl::optional<Expr> ExpandExistsOneMacro(MacroExprFactory& factory,
157157
return factory.ReportErrorAt(
158158
args[0], "exists_one() variable name must be a simple identifier");
159159
}
160-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
160+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
161161
return factory.ReportErrorAt(
162162
args[1], absl::StrCat("exists_one() variable name cannot be ",
163-
kAccumulatorVariableName));
163+
kDeprecatedAccumulatorVariableName));
164164
}
165165
auto init = factory.NewIntConst(0);
166166
auto condition = factory.NewBoolConst(true);
@@ -196,10 +196,10 @@ absl::optional<Expr> ExpandMap2Macro(MacroExprFactory& factory, Expr& target,
196196
return factory.ReportErrorAt(
197197
args[0], "map() variable name must be a simple identifier");
198198
}
199-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
200-
return factory.ReportErrorAt(args[1],
201-
absl::StrCat("map() variable name cannot be ",
202-
kAccumulatorVariableName));
199+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
200+
return factory.ReportErrorAt(
201+
args[1], absl::StrCat("map() variable name cannot be ",
202+
kDeprecatedAccumulatorVariableName));
203203
}
204204
auto init = factory.NewList();
205205
auto condition = factory.NewBoolConst(true);
@@ -229,10 +229,10 @@ absl::optional<Expr> ExpandMap3Macro(MacroExprFactory& factory, Expr& target,
229229
return factory.ReportErrorAt(
230230
args[0], "map() variable name must be a simple identifier");
231231
}
232-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
233-
return factory.ReportErrorAt(args[1],
234-
absl::StrCat("map() variable name cannot be ",
235-
kAccumulatorVariableName));
232+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
233+
return factory.ReportErrorAt(
234+
args[1], absl::StrCat("map() variable name cannot be ",
235+
kDeprecatedAccumulatorVariableName));
236236
}
237237
auto init = factory.NewList();
238238
auto condition = factory.NewBoolConst(true);
@@ -264,10 +264,10 @@ absl::optional<Expr> ExpandFilterMacro(MacroExprFactory& factory, Expr& target,
264264
return factory.ReportErrorAt(
265265
args[0], "filter() variable name must be a simple identifier");
266266
}
267-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
267+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
268268
return factory.ReportErrorAt(
269269
args[1], absl::StrCat("filter() variable name cannot be ",
270-
kAccumulatorVariableName));
270+
kDeprecatedAccumulatorVariableName));
271271
}
272272
auto name = args[0].ident_expr().name();
273273

@@ -302,10 +302,10 @@ absl::optional<Expr> ExpandOptMapMacro(MacroExprFactory& factory, Expr& target,
302302
return factory.ReportErrorAt(
303303
args[0], "optMap() variable name must be a simple identifier");
304304
}
305-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
305+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
306306
return factory.ReportErrorAt(
307307
args[1], absl::StrCat("optMap() variable name cannot be ",
308-
kAccumulatorVariableName));
308+
kDeprecatedAccumulatorVariableName));
309309
}
310310
auto var_name = args[0].ident_expr().name();
311311

@@ -341,10 +341,10 @@ absl::optional<Expr> ExpandOptFlatMapMacro(MacroExprFactory& factory,
341341
return factory.ReportErrorAt(
342342
args[0], "optFlatMap() variable name must be a simple identifier");
343343
}
344-
if (args[0].ident_expr().name() == kAccumulatorVariableName) {
344+
if (args[0].ident_expr().name() == kDeprecatedAccumulatorVariableName) {
345345
return factory.ReportErrorAt(
346346
args[1], absl::StrCat("optFlatMap() variable name cannot be ",
347-
kAccumulatorVariableName));
347+
kDeprecatedAccumulatorVariableName));
348348
}
349349
auto var_name = args[0].ident_expr().name();
350350

‎parser/macro_expr_factory.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -319,8 +319,7 @@ class MacroExprFactory : protected ExprFactory {
319319
friend class ParserMacroExprFactory;
320320
friend class TestMacroExprFactory;
321321

322-
explicit MacroExprFactory(absl::string_view accu_var)
323-
: ExprFactory(accu_var) {}
322+
explicit MacroExprFactory() = default;
324323
};
325324

326325
} // namespace cel

‎parser/macro_expr_factory_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ namespace cel {
2727

2828
class TestMacroExprFactory final : public MacroExprFactory {
2929
public:
30-
TestMacroExprFactory() : MacroExprFactory(kAccumulatorVariableName) {}
30+
TestMacroExprFactory() = default;
3131

3232
ExprId id() const { return id_; }
3333

‎parser/options.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,9 @@ struct ParserOptions final {
5151
// Disable standard macros (has, all, exists, exists_one, filter, map).
5252
bool disable_standard_macros = false;
5353

54-
// Enable hidden accumulator variable '@result' for builtin comprehensions.
54+
// Deprecated: The builtin and extension macros now always use the new
55+
// accumulator variable name.
56+
// This option has no effect.
5557
bool enable_hidden_accumulator_var = true;
5658

5759
// Enables support for identifier quoting syntax:

‎parser/parser.cc‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -163,9 +163,8 @@ SourceRange SourceRangeFromParserRuleContext(
163163

164164
class ParserMacroExprFactory final : public MacroExprFactory {
165165
public:
166-
explicit ParserMacroExprFactory(const cel::Source& source,
167-
absl::string_view accu_var)
168-
: MacroExprFactory(accu_var), source_(source) {}
166+
explicit ParserMacroExprFactory(const cel::Source& source)
167+
: source_(source) {}
169168

170169
void BeginMacro(SourceRange macro_position) {
171170
macro_position_ = macro_position;
@@ -607,13 +606,12 @@ class ParserVisitor final : public CelBaseVisitor,
607606
public antlr4::BaseErrorListener {
608607
public:
609608
ParserVisitor(const cel::Source& source, int max_recursion_depth,
610-
absl::string_view accu_var,
611609
const cel::MacroRegistry& macro_registry,
612610
bool add_macro_calls = false,
613611
bool enable_optional_syntax = false,
614612
bool enable_quoted_identifiers = false)
615613
: source_(source),
616-
factory_(source_, accu_var),
614+
factory_(source_),
617615
macro_registry_(macro_registry),
618616
recursion_depth_(0),
619617
max_recursion_depth_(max_recursion_depth),
@@ -1654,14 +1652,9 @@ absl::StatusOr<ParseResult> ParseImpl(const cel::Source& source,
16541652
CommonTokenStream tokens(&lexer);
16551653
CelParser parser(&tokens);
16561654
ExprRecursionListener listener(options.max_recursion_depth);
1657-
absl::string_view accu_var = cel::kAccumulatorVariableName;
1658-
if (options.enable_hidden_accumulator_var) {
1659-
accu_var = cel::kHiddenAccumulatorVariableName;
1660-
}
1661-
ParserVisitor visitor(source, options.max_recursion_depth, accu_var,
1662-
registry, options.add_macro_calls,
1663-
options.enable_optional_syntax,
1664-
options.enable_quoted_identifiers);
1655+
ParserVisitor visitor(
1656+
source, options.max_recursion_depth, registry, options.add_macro_calls,
1657+
options.enable_optional_syntax, options.enable_quoted_identifiers);
16651658

16661659
lexer.removeErrorListeners();
16671660
parser.removeErrorListeners();

0 commit comments

Comments
 (0)